[Spice-devel] [PATCH spice-server] spice-char: Add some documentation to SpiceCharDeviceInterface

[Christophe de Dinechin]

> On 23 Mar 2018, at 17:20, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> Thank you for that. Looks good after two minor grammatical fixes.
>> 
>>> On 22 Mar 2018, at 11:12, Frediano Ziglio <fziglio at redhat.com> wrote:
>>> 
>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>> ---
>>> server/spice-char.h | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>> 
>>> diff --git a/server/spice-char.h b/server/spice-char.h
>>> index 1a8a031d..4d780eb7 100644
>>> --- a/server/spice-char.h
>>> +++ b/server/spice-char.h
>>> @@ -40,9 +40,30 @@ typedef enum {
>>> struct SpiceCharDeviceInterface {
>>>    SpiceBaseInterface base;
>>> 
>>> +    /* Set the state of the device.
>>> +     * connected should be 0 or 1.
>>> +     * Setting state to 0 cause the device to be disabled.
>> 
>> Maybe document uses cases for that function?
>> 
>> Why does the function return a void? Can’t fail? Can you always change the
>> state? (Does not look like an open/close to me)
>> 
> 
> Not sure but I suppose the function cannot fail.
> Is not a open/close, the open/close is more a guest aspect.
> I cannot find an easy analogy, we (spice server code) are kind of implementing
> a device (like a kernel module) and we basically are calling a kernel function
> to tell that this device is not available.
> The streaming device for instance uses it to enable/disable the guest device.
> 
>>> +     */
>>>    void (*state)(SpiceCharDeviceInstance *sin, int connected);
>>> +
>>> +    /* Write some bytes to the character device.
>>> +     * Returns bytes copied from buf or a value < 0 on errors.
>>> +     * Function can return a value < len, even 0.
>>> +     * errno is not determined after calling this function.
>>> +     * Function should be implemented as no-blocking.
>>> +     * A len < 0 cause indeterminate results.
>> 
>> cause -> causes
>> 
> 
> updated here and below.
> 
>> I suspect this is not an atomic operation, so you may have written some stuff
>> even if the result is < 0?
>> 
> 
> By atomic you mean "Function can return a value < len, even 0." or something related
> to threads?

The former. I was thinking of partial writes followed by an error, making the write non-atomic from the point of view of the caller.

In other words, say I write “HelloWorld”, and it fails at the W, I am wondering if there is a guarantee that write will return 5, and then that the retry will return < 0, or if it’s possible that it returns < 0 right away.

Probably the best way to know is to dig in the code ;-)

> Or maybe a comment that say that if you get an error after handling some bytes the
> function should return that amount of bytes not returning an error?
> No idea what we expect in this case, the only reason I can see this would
> happen is when the guest closes the device but I'm not sure what happens
> with current Qemu code and what we should expect. I personally would expect
> that we could receive last bytes sent by the guest.

> 
>> 
>>> +     */
>>>    int (*write)(SpiceCharDeviceInstance *sin, const uint8_t *buf, int
>>>    len);
>>> +
>>> +    /* Read some bytes from the character device.
>>> +     * Returns bytes copied into buf or a value < 0 on errors.
>>> +     * Function can return 0 if no data is available or len is 0.
>>> +     * errno is not determined after calling this function.
>>> +     * Function should be implemented as no-blocking.
>>> +     * A len < 0 cause indeterminate results.
>> 
>> cause -> causes
>> 
>>> +     */
>>>    int (*read)(SpiceCharDeviceInstance *sin, uint8_t *buf, int len);
>>> +
>>>    void (*event)(SpiceCharDeviceInstance *sin, uint8_t event);
>>>    spice_char_device_flags flags;
>>> };
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel