On Sun, Feb 18, 2018 at 06:58:10PM +0000, Frediano Ziglio wrote:
> To communicate the error and avoiding to have to read any data the
> guest want to write disable the device.
I am having troubles parsing this. Is this missing a comma before
"disable"?
"To communicate the error and avoiding to have to read any data the
guest want to write, disable the device.". If so, I'd reformulate it,
"Once the device is an error state, we don't want the guest to keep
reading/writing to it, especially as this could put the device in an
inconsistent state. This commit closes the device when an error occurs
to prevent further unintended use of the device by the guest."
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/stream-device.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/server/stream-device.c b/server/stream-device.c
> index f22946fb..688a4e07 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -94,6 +94,7 @@ stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin)
> while (sif->read(sin, buf, sizeof(buf)) > 0) {
> continue;
> }
> + sif->state(sin, 0);
> return false;
> }
>
> @@ -560,6 +561,19 @@ reset_channels(StreamDevice *dev)
> }
> }
>
> +static void
> +char_device_enable(RedCharDevice *char_dev)
> +{
> + SpiceCharDeviceInstance *sin = NULL;
> + g_object_get(char_dev, "sin", &sin, NULL);
> + spice_assert(sin != NULL);
> +
> + SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> + if (sif->state) {
> + sif->state(sin, 1);
> + }
> +}
> +
> static void
> stream_device_port_event(RedCharDevice *char_dev, uint8_t event)
> {
> @@ -580,6 +594,12 @@ stream_device_port_event(RedCharDevice *char_dev, uint8_t event)
> dev->flow_stopped = false;
> red_char_device_reset(char_dev);
> reset_channels(dev);
> +
> + // enable the device again. We try to enable it even on close as
> + // this could prevent a possible race condition where we open the
> + // device from the guest and it take some time to enable causing
> + // temporary writing issues.
I am not sure I understand the race from that comment :(
Patch looks good otherwise,
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
> + char_device_enable(char_dev);
> }
>
> static void
> --
> 2.14.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel