>
> > 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 ;-)
>
I don't think we should document the actual implementation but more the contract,
is supposed to be an interface.
I think both returning a 0 <= value < len or < 0 should be fine in case of partial
read/write.
> > 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