> On 23 Mar 2018, at 17:20, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
>>
>> Thank you for that. Looks good after two minor grammatical fixes.
>>
>>> On 22 Mar 2018, at 11:12, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>>
>>> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel