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

[Frediano Ziglio]

> 
> 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?
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