From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoWF2-0000vy-I4 for qemu-devel@nongnu.org; Tue, 20 Oct 2015 08:42:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZoWEx-0002NG-Dt for qemu-devel@nongnu.org; Tue, 20 Oct 2015 08:42:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52683) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoWEx-0002N2-6s for qemu-devel@nongnu.org; Tue, 20 Oct 2015 08:42:07 -0400 Date: Tue, 20 Oct 2015 15:42:03 +0300 From: "Michael S. Tsirkin" Message-ID: <20151020154040-mutt-send-email-mst@redhat.com> References: <20151020091640.25419.99985.stgit@bahia.huguette.org> <20151020091700.25419.56847.stgit@bahia.huguette.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151020091700.25419.56847.stgit@bahia.huguette.org> Subject: Re: [Qemu-devel] [PATCH v3 3/5] virtio-9p: block hot-unplug when device is active List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: Cornelia Huck , aneesh.kumar@linux.vnet.ibm.com, qemu-devel@nongnu.org, Andreas =?iso-8859-1?Q?F=E4rber?= , Alexander Graf On Tue, Oct 20, 2015 at 11:17:00AM +0200, Greg Kurz wrote: > Hot-unplug of an active virtio-9p device is not currently supported. Until > we have that, let's block hot-unplugging when the 9p share is mounted in > the guest. > > This patch implements a hot-unplug blocker feature like we already have for > migration. Both virtio-9p-pci and virtio-9p-ccw were adapted accordingly. > > Signed-off-by: Greg Kurz I'm not sure that's right. This isn't what we do for other devices. Why doesn't unplug request cause filesystem to be unmounted? Isn't this just a guest bug? And if yes, why do we need a blocker in qemu? > --- > hw/9pfs/virtio-9p.c | 14 ++++++++++++++ > hw/9pfs/virtio-9p.h | 2 ++ > hw/s390x/virtio-ccw.c | 8 ++++++++ > hw/virtio/virtio-pci.c | 8 ++++++++ > 4 files changed, 32 insertions(+) > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index f972731f5a8d..bbf39ed33983 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -345,6 +345,7 @@ static int put_fid(V9fsPDU *pdu, V9fsFidState *fidp) > migrate_del_blocker(pdu->s->migration_blocker); > error_free(pdu->s->migration_blocker); > pdu->s->migration_blocker = NULL; > + pdu->s->unplug_is_blocked = false; > } > } > return free_fid(pdu, fidp); > @@ -991,6 +992,7 @@ static void v9fs_attach(void *opaque) > "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'", > s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag); > migrate_add_blocker(s->migration_blocker); > + s->unplug_is_blocked = true; > } > out: > put_fid(pdu, fidp); > @@ -3288,6 +3290,18 @@ void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) > free_pdu(s, pdu); > } > > +bool v9fs_unplug_is_blocked(V9fsState *s, Error **errp) > +{ > + if (s->unplug_is_blocked) { > + error_setg(errp, > + "Unplug is disabled when VirtFS export path '%s' is mounted" > + " in the guest using mount_tag '%s'", > + s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag); > + return false; > + } > + return true; > +} > + > static void __attribute__((__constructor__)) virtio_9p_set_fd_limit(void) > { > struct rlimit rlim; > diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h > index 2e7d48857083..8d4cbed2a5c4 100644 > --- a/hw/9pfs/virtio-9p.h > +++ b/hw/9pfs/virtio-9p.h > @@ -218,6 +218,7 @@ typedef struct V9fsState > CoRwlock rename_lock; > int32_t root_fid; > Error *migration_blocker; > + bool unplug_is_blocked; > V9fsConf fsconf; > } V9fsState; > > @@ -381,6 +382,7 @@ extern void v9fs_path_free(V9fsPath *path); > extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs); > extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath, > const char *name, V9fsPath *path); > +extern bool v9fs_unplug_is_blocked(V9fsState *s, Error **errp); > > #define pdu_marshal(pdu, offset, fmt, args...) \ > v9fs_marshal(pdu->elem.in_sg, pdu->elem.in_num, offset, 1, fmt, ##args) > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index fb103b78ac28..5c13072ec96e 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -1924,6 +1924,13 @@ static void virtio_ccw_9p_realize(VirtioCcwDevice *ccw_dev, Error **errp) > } > } > > +static bool virtio_ccw_9p_unplug_is_blocked(DeviceState *dev, Error **errp) > +{ > + V9fsCCWState *v9fs_ccw_dev = VIRTIO_9P_CCW(dev); > + > + return v9fs_unplug_is_blocked(&v9fs_ccw_dev->vdev, errp); > +} > + > static void virtio_ccw_9p_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -1931,6 +1938,7 @@ static void virtio_ccw_9p_class_init(ObjectClass *klass, void *data) > > k->exit = virtio_ccw_exit; > k->realize = virtio_ccw_9p_realize; > + dc->unplug_is_blocked = virtio_ccw_9p_unplug_is_blocked; > dc->reset = virtio_ccw_reset; > dc->props = virtio_ccw_9p_properties; > set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index e5c406d1d255..bf0d516528ee 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1015,6 +1015,13 @@ static void virtio_9p_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > } > > +static bool virtio_9p_pci_unplug_is_blocked(DeviceState *dev, Error **errp) > +{ > + V9fsPCIState *v9fs_pci_dev = VIRTIO_9P_PCI(dev); > + > + return v9fs_unplug_is_blocked(&v9fs_pci_dev->vdev, errp); > +} > + > static Property virtio_9p_pci_properties[] = { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > @@ -1035,6 +1042,7 @@ static void virtio_9p_pci_class_init(ObjectClass *klass, void *data) > pcidev_k->class_id = 0x2; > set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > dc->props = virtio_9p_pci_properties; > + dc->unplug_is_blocked = virtio_9p_pci_unplug_is_blocked; > } > > static void virtio_9p_pci_instance_init(Object *obj)