Hi On Fri, Aug 3, 2018 at 7:36 PM, Marc-André Lureau <marcandre.lur...@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.lur...@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