Hi
On Fri, Aug 3, 2018 at 7:36 PM, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> Most chardev backend handle write() as discarded data if underlying
> system is disconnected. For unknown historical reasons, the Spice
> backend has "reliable" write. It will wait until the client end is
> reconnected to accept further write().
>
> Let's review Spice chardev usage and handling of a disconnected
> client:
>
> * spice vdagent
> The agent will reopen the virtio port on disconnect.
>
> * usb redirection
> A disconnect creates a device disconnection.
>
> * smartcard emulation
> Data is discarded in passthru_apdu_from_guest()
Doing more testing, I realize this creates is a regression on Spice
smartcard. Sadly, the Spice smartcard channel isn't actually based on
spicevmc (as the chardev name may imply), and doesn't set the
connected sif->state(). I am sending a patch to spice-devel to fix
this. I'll update this patch to set the chardev opened at "smartcard"
spicevmc creation time when we have an old spice server.
>
> * spice webdavd
> The daemon will restart the service, and reopen the virtio port.
>
> * spice ports (serial console, qemu monitor..)
> Depends on the associated device or usage.
>
> - 16550A serial does nothing special, and may block guest on write
>
> - QMP/HMP monitor have some CLOSED event handling, but want to
> flush the write, which will finish when a new client connects.
>
> For all these use cases, it is better to discard pending write when
> the client is disconnected, and expect the device/agent to behave
> correctly on CHR_EVENT_CLOSED (to stop reading and writing from
> chardev).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/spice.c | 6 ++++++
> chardev/trace-events | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/chardev/spice.c b/chardev/spice.c
> index fe06034d7f..6ad95ffe62 100644
> --- a/chardev/spice.c
> +++ b/chardev/spice.c
> @@ -212,6 +212,12 @@ static int spice_chr_write(Chardev *chr, const uint8_t *buf, int len)
> int read_bytes;
>
> assert(s->datalen == 0);
> +
> + if (!chr->be_open) {
> + trace_spice_chr_discard_write(len);
> + return len;
> + }
> +
> s->datapos = buf;
> s->datalen = len;
> spice_server_char_device_wakeup(&s->sin);
> diff --git a/chardev/trace-events b/chardev/trace-events
> index d0e5f3bbc1..b8a7596344 100644
> --- a/chardev/trace-events
> +++ b/chardev/trace-events
> @@ -10,6 +10,7 @@ wct_cmd_other(const char *cmd) "%s"
> wct_speed(int speed) "%d"
>
> # chardev/spice.c
> +spice_chr_discard_write(int len) "spice chr write discarded %d"
> spice_vmc_write(ssize_t out, int len) "spice wrote %zd of requested %d"
> spice_vmc_read(int bytes, int len) "spice read %d of requested %d"
> spice_vmc_register_interface(void *scd) "spice vmc registered interface %p"
> --
> 2.18.0.547.g1d89318c48
>
>
--
Marc-André Lureau