On Thu, 22 Sep 2016 07:38:29 +0000 "Gonglei (Arei)" <arei.gong...@huawei.com> wrote:
> Hi Greg, > > > > > -----Original Message----- > > From: Greg Kurz [mailto:gr...@kaod.org] > > Sent: Thursday, September 22, 2016 3:22 PM > > To: Gonglei (Arei) > > Cc: qemu-devel@nongnu.org; Kevin Wolf; Michael S. Tsirkin; Jason Wang; Max > > Reitz; Aneesh Kumar K.V; Stefan Hajnoczi; Cornelia Huck; Paolo Bonzini > > Subject: Re: [Qemu-devel] [PATCH 0/7] virtio: avoid inappropriate QEMU > > termination > > > > On Thu, 22 Sep 2016 06:55:43 +0000 > > "Gonglei (Arei)" <arei.gong...@huawei.com> wrote: > > > > > > -----Original Message----- > > > > From: Greg Kurz [mailto:gr...@kaod.org] > > > > Sent: Thursday, September 22, 2016 2:43 PM > > > > To: Gonglei (Arei) > > > > Cc: qemu-devel@nongnu.org; Kevin Wolf; Michael S. Tsirkin; Jason Wang; > > Max > > > > Reitz; Aneesh Kumar K.V; Stefan Hajnoczi; Cornelia Huck; Paolo Bonzini > > > > Subject: Re: [Qemu-devel] [PATCH 0/7] virtio: avoid inappropriate QEMU > > > > termination > > > > > > > > On Thu, 22 Sep 2016 09:19:49 +0800 > > > > Gonglei <arei.gong...@huawei.com> wrote: > > > > > > > > > On 2016/9/21 21:13, Greg Kurz wrote: > > > > > > This series is a follow up to Stefan's work to eradicate most calls > > > > > > to > > > > > > exit() we currently have in the virtio code. > > > > > > > > > > > > It addresses all exit() call sites in the blk, net and scsi device > > > > > > code, > > > > > > where the error is about a missing or malformed in/out header sent > > > > > > by > > > > > > the guest. They are converted to use virtio_error() and stop any > > processing, > > > > > > instead of exiting. > > > > > > > > > > > Actually if you just stop procesing when encounter a missing in/out > > > > > header > > > > but > > > > > send a interrupt to the guest, the guest maybe be stuck. > > > > virtio_net_handle_ctrl() > > > > > > > > The virtio_error() function sets the device status to > > > > DEVICE_NEEDS_RESET > > and > > > > does send a device configuration change interrupt to the guest, so it > > > > can > > take > > > > appropriate action (i.e. reset the device). > > > > > > I saw virtio_error() only handle the virtio 1.0 device, the legacy virtio > device may > still stuck, am I right? > The DEVICE_NEEDS_RESET bit was introduced by the virtio 1.0 specification. http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-100001 Yes, the legacy device will stay stuck since the broken flag is set. Cheers. -- Greg > +void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, > ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + error_vreport(fmt, ap); > + va_end(ap); > + > + vdev->broken = true; > + > + if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + virtio_set_status(vdev, vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET); > + virtio_notify_config(vdev); > + } > +} > > > Regards, > -Gonglei > > > > That's appropriate. Where is realization of virtio_error() ? > > > I'm sure I missed something. > > > > > > > Sorry for that... Michael already "lectured" me about not providing these > > details. He is right indeed :) > > > > > > This is work in progress by Stefan. The latest version of the patchset (v5) > > was > > posted yesterday: > > > > <1474473146-19337-1-git-send-email-stefa...@redhat.com> > > > > The virtio_error() function itself is in patch 2/9: > > > > <1474473146-19337-3-git-send-email-stefa...@redhat.com> > > > > Cheers. > > > > -- > > Greg > > > > > Regards, > > > -Gonglei > > > > > > > Maybe I should have mentioned that in the changelog... > > > > > > > > Cheers. > > > > > > > > -- > > > > Greg > > > > > > > > > is an example, the guest frontend driver infinite loop to wait the > > interrupt's > > > > coming. > > > > > The guest can't work anymore though you don't exit the Qemu process . > > > > > > > > > > Regards, > > > > > -Gonglei > > > > > > > > > > > The remaining call sites are related to a host misconfiguration or a > > > > > > migration stream issue. > > > > > > > > > > > > The 9P code currently calls assert() instead of exit(), but it also > > > > > > about > > > > > > malformed or missing headers, so it gets converted the same way. > > > > > > > > > > > > Next work will be to check all assert() call sites in the device > > > > > > code, in > > > > > > case some of them actually refer to a bug in the guest, and should > > > > > > be > > > > > > converted to use virtio_error() as well. > > > > > > > > > > > > --- > > > > > > > > > > > > Greg Kurz (7): > > > > > > virtio-9p: handle handle_9p_output() error > > > > > > virtio-blk: handle virtio_blk_handle_request() errors > > > > > > virtio-net: handle virtio_net_handle_ctrl() error > > > > > > virtio-net: handle virtio_net_receive() errors > > > > > > virtio-net: handle virtio_net_flush_tx() errors > > > > > > virtio-scsi: convert virtio_scsi_bad_req() to use > > > > > > virtio_error() > > > > > > virtio-scsi: handle virtio_scsi_set_config() error > > > > > > > > > > > > > > > > > > hw/9pfs/virtio-9p-device.c | 14 ++++++++++-- > > > > > > hw/block/virtio-blk.c | 27 +++++++++++++++-------- > > > > > > hw/net/virtio-net.c | 51 > > > > +++++++++++++++++++++++++------------------- > > > > > > hw/scsi/virtio-scsi.c | 21 ++++++++++-------- > > > > > > 4 files changed, 70 insertions(+), 43 deletions(-) > > > > > > > > > > > > -- > > > > > > Greg > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
