On Wed, May 23, 2018 at 06:36:05PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 23, 2018 at 04:44:51PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Apr 12, 2018 at 11:12:28PM +0800, Tiwei Bie wrote:
> > > When multi queue is enabled e.g. for a virtio-net device,
> > > each queue pair will have a vhost_dev, and the only thing
> > > shared between vhost devs currently is the chardev. This
> > > patch introduces a vhost-user state structure which will
> > > be shared by all vhost devs of the same virtio device.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei....@intel.com>
> >
> > Unfortunately this patch seems to cause crashes.
> > To reproduce, simply run
> > make check-qtest-x86_64
> >
> > Sorry that it took me a while to find - it triggers 90% of runs but not
> > 100% which complicates bisection somewhat.
> >
> > > ---
> > > backends/cryptodev-vhost-user.c | 20 ++++++++++++++++++-
> > > hw/block/vhost-user-blk.c | 22 +++++++++++++++++++-
> > > hw/scsi/vhost-user-scsi.c | 20 ++++++++++++++++++-
> > > hw/virtio/Makefile.objs | 2 +-
> > > hw/virtio/vhost-stub.c | 10 ++++++++++
> > > hw/virtio/vhost-user.c | 31 +++++++++++++++++++---------
> > > include/hw/virtio/vhost-user-blk.h | 2 ++
> > > include/hw/virtio/vhost-user-scsi.h | 2 ++
> > > include/hw/virtio/vhost-user.h | 20 +++++++++++++++++++
> > > net/vhost-user.c | 40
> > > ++++++++++++++++++++++++++++++-------
> > > 10 files changed, 149 insertions(+), 20 deletions(-)
> > > create mode 100644 include/hw/virtio/vhost-user.h
> > >
> > > diff --git a/backends/cryptodev-vhost-user.c
> > > b/backends/cryptodev-vhost-user.c
> > > index 862d4f2580..d52daccfcd 100644
> > > --- a/backends/cryptodev-vhost-user.c
> > > +++ b/backends/cryptodev-vhost-user.c
> > > @@ -26,6 +26,7 @@
> > > #include "qapi/error.h"
> > > #include "qapi/qmp/qerror.h"
> > > #include "qemu/error-report.h"
> > > +#include "hw/virtio/vhost-user.h"
> > > #include "standard-headers/linux/virtio_crypto.h"
> > > #include "sysemu/cryptodev-vhost.h"
> > > #include "chardev/char-fe.h"
> > > @@ -46,6 +47,7 @@
> > > typedef struct CryptoDevBackendVhostUser {
> > > CryptoDevBackend parent_obj;
> > >
> > > + VhostUserState *vhost_user;
> > > CharBackend chr;
> > > char *chr_name;
> > > bool opened;
> > > @@ -102,7 +104,7 @@ cryptodev_vhost_user_start(int queues,
> > > continue;
> > > }
> > >
> > > - options.opaque = &s->chr;
> > > + options.opaque = s->vhost_user;
> > > options.backend_type = VHOST_BACKEND_TYPE_USER;
> > > options.cc = b->conf.peers.ccs[i];
> > > s->vhost_crypto[i] = cryptodev_vhost_init(&options);
> > > @@ -185,6 +187,7 @@ static void cryptodev_vhost_user_init(
> > > size_t i;
> > > Error *local_err = NULL;
> > > Chardev *chr;
> > > + VhostUserState *user;
> > > CryptoDevBackendClient *cc;
> > > CryptoDevBackendVhostUser *s =
> > > CRYPTODEV_BACKEND_VHOST_USER(backend);
> > > @@ -215,6 +218,15 @@ static void cryptodev_vhost_user_init(
> > > }
> > > }
> > >
> > > + user = vhost_user_init();
> > > + if (!user) {
> > > + error_setg(errp, "Failed to init vhost_user");
> > > + return;
> > > + }
> > > +
> > > + user->chr = &s->chr;
> > > + s->vhost_user = user;
> > > +
> > > qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
> > > cryptodev_vhost_user_event, NULL, s, NULL, true);
> > >
> > > @@ -299,6 +311,12 @@ static void cryptodev_vhost_user_cleanup(
> > > backend->conf.peers.ccs[i] = NULL;
> > > }
> > > }
> > > +
> > > + if (s->vhost_user) {
> > > + vhost_user_cleanup(s->vhost_user);
> > > + g_free(s->vhost_user);
> > > + s->vhost_user = NULL;
> > > + }
> > > }
> > >
> > > static void cryptodev_vhost_user_set_chardev(Object *obj,
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index 262baca432..4021d71c31 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -229,6 +229,7 @@ static void vhost_user_blk_device_realize(DeviceState
> > > *dev, Error **errp)
> > > {
> > > VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > > VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > > + VhostUserState *user;
> > > int i, ret;
> > >
> > > if (!s->chardev.chr) {
> > > @@ -246,6 +247,15 @@ static void
> > > vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> > > return;
> > > }
> > >
> > > + user = vhost_user_init();
> > > + if (!user) {
> > > + error_setg(errp, "vhost-user-blk: failed to init vhost_user");
> > > + return;
> > > + }
> > > +
> > > + user->chr = &s->chardev;
> > > + s->vhost_user = user;
> > > +
> > > virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> > > sizeof(struct virtio_blk_config));
> > >
> > > @@ -261,7 +271,7 @@ static void vhost_user_blk_device_realize(DeviceState
> > > *dev, Error **errp)
> > >
> > > vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > >
> > > - ret = vhost_dev_init(&s->dev, &s->chardev, VHOST_BACKEND_TYPE_USER,
> > > 0);
> > > + ret = vhost_dev_init(&s->dev, s->vhost_user,
> > > VHOST_BACKEND_TYPE_USER, 0);
> > > if (ret < 0) {
> > > error_setg(errp, "vhost-user-blk: vhost initialization failed:
> > > %s",
> > > strerror(-ret));
> > > @@ -286,6 +296,10 @@ vhost_err:
> > > virtio_err:
> > > g_free(s->dev.vqs);
> > > virtio_cleanup(vdev);
> > > +
> > > + vhost_user_cleanup(user);
> > > + g_free(user);
> > > + s->vhost_user = NULL;
> > > }
> > >
> > > static void vhost_user_blk_device_unrealize(DeviceState *dev, Error
> > > **errp)
> > > @@ -297,6 +311,12 @@ static void
> > > vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp)
> > > vhost_dev_cleanup(&s->dev);
> > > g_free(s->dev.vqs);
> > > virtio_cleanup(vdev);
> > > +
> > > + if (s->vhost_user) {
> > > + vhost_user_cleanup(s->vhost_user);
> > > + g_free(s->vhost_user);
> > > + s->vhost_user = NULL;
> > > + }
> > > }
> > >
> > > static void vhost_user_blk_instance_init(Object *obj)
> > > diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> > > index 9389ed48e0..9355cfdf07 100644
> > > --- a/hw/scsi/vhost-user-scsi.c
> > > +++ b/hw/scsi/vhost-user-scsi.c
> > > @@ -69,6 +69,7 @@ static void vhost_user_scsi_realize(DeviceState *dev,
> > > Error **errp)
> > > VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
> > > VHostUserSCSI *s = VHOST_USER_SCSI(dev);
> > > VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
> > > + VhostUserState *user;
> > > Error *err = NULL;
> > > int ret;
> > >
> > > @@ -85,19 +86,30 @@ static void vhost_user_scsi_realize(DeviceState *dev,
> > > Error **errp)
> > > return;
> > > }
> > >
> > > + user = vhost_user_init();
> > > + if (!user) {
> > > + error_setg(errp, "vhost-user-scsi: failed to init vhost_user");
> > > + return;
> > > + }
> > > + user->chr = &vs->conf.chardev;
> > > +
> > > vsc->dev.nvqs = 2 + vs->conf.num_queues;
> > > vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs);
> > > vsc->dev.vq_index = 0;
> > > vsc->dev.backend_features = 0;
> > >
> > > - ret = vhost_dev_init(&vsc->dev, (void *)&vs->conf.chardev,
> > > + ret = vhost_dev_init(&vsc->dev, user,
> > > VHOST_BACKEND_TYPE_USER, 0);
> > > if (ret < 0) {
> > > error_setg(errp, "vhost-user-scsi: vhost initialization failed:
> > > %s",
> > > strerror(-ret));
> > > + vhost_user_cleanup(user);
> > > + g_free(user);
> > > return;
> > > }
> > >
> > > + s->vhost_user = user;
> > > +
> > > /* Channel and lun both are 0 for bootable vhost-user-scsi disk */
> > > vsc->channel = 0;
> > > vsc->lun = 0;
> > > @@ -117,6 +129,12 @@ static void vhost_user_scsi_unrealize(DeviceState
> > > *dev, Error **errp)
> > > g_free(vsc->dev.vqs);
> > >
> > > virtio_scsi_common_unrealize(dev, errp);
> > > +
> > > + if (s->vhost_user) {
> > > + vhost_user_cleanup(s->vhost_user);
> > > + g_free(s->vhost_user);
> > > + s->vhost_user = NULL;
> > > + }
> > > }
> > >
> > > static uint64_t vhost_user_scsi_get_features(VirtIODevice *vdev,
> > > diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> > > index 765d363c1f..030969e28c 100644
> > > --- a/hw/virtio/Makefile.objs
> > > +++ b/hw/virtio/Makefile.objs
> > > @@ -11,5 +11,5 @@ obj-y += virtio-crypto.o
> > > obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> > > endif
> > >
> > > -common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o
> > > +common-obj-$(call lnot,$(call land,$(CONFIG_VIRTIO),$(CONFIG_LINUX))) +=
> > > vhost-stub.o
> > > common-obj-$(CONFIG_ALL) += vhost-stub.o
> > > diff --git a/hw/virtio/vhost-stub.c b/hw/virtio/vhost-stub.c
> > > index 2d76cdebdc..049089b5e2 100644
> > > --- a/hw/virtio/vhost-stub.c
> > > +++ b/hw/virtio/vhost-stub.c
> > > @@ -1,7 +1,17 @@
> > > #include "qemu/osdep.h"
> > > #include "hw/virtio/vhost.h"
> > > +#include "hw/virtio/vhost-user.h"
> > >
> > > bool vhost_has_free_slot(void)
> > > {
> > > return true;
> > > }
> > > +
> > > +VhostUserState *vhost_user_init(void)
> > > +{
> > > + return NULL;
> > > +}
> > > +
> > > +void vhost_user_cleanup(VhostUserState *user)
> > > +{
> > > +}
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > index 38da8692bb..91edd95453 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -11,6 +11,7 @@
> > > #include "qemu/osdep.h"
> > > #include "qapi/error.h"
> > > #include "hw/virtio/vhost.h"
> > > +#include "hw/virtio/vhost-user.h"
> > > #include "hw/virtio/vhost-backend.h"
> > > #include "hw/virtio/virtio-net.h"
> > > #include "chardev/char-fe.h"
> > > @@ -173,7 +174,8 @@ static VhostUserMsg m __attribute__ ((unused));
> > >
> > > struct vhost_user {
> > > struct vhost_dev *dev;
> > > - CharBackend *chr;
> > > + /* Shared between vhost devs of the same virtio device */
> > > + VhostUserState *user;
> > > int slave_fd;
> > > NotifierWithReturn postcopy_notifier;
> > > struct PostCopyFD postcopy_fd;
> > > @@ -199,7 +201,7 @@ static bool ioeventfd_enabled(void)
> > > static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
> > > {
> > > struct vhost_user *u = dev->opaque;
> > > - CharBackend *chr = u->chr;
> > > + CharBackend *chr = u->user->chr;
> > > uint8_t *p = (uint8_t *) msg;
> > > int r, size = VHOST_USER_HDR_SIZE;
> > >
> > > @@ -285,7 +287,7 @@ static int vhost_user_write(struct vhost_dev *dev,
> > > VhostUserMsg *msg,
> > > int *fds, int fd_num)
> > > {
> > > struct vhost_user *u = dev->opaque;
> > > - CharBackend *chr = u->chr;
> > > + CharBackend *chr = u->user->chr;
> > > int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
> > >
> > > /*
> > > @@ -1044,7 +1046,7 @@ static int vhost_user_postcopy_waker(struct
> > > PostCopyFD *pcfd, RAMBlock *rb,
> > > static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error
> > > **errp)
> > > {
> > > struct vhost_user *u = dev->opaque;
> > > - CharBackend *chr = u->chr;
> > > + CharBackend *chr = u->user->chr;
> > > int ufd;
> > > VhostUserMsg msg = {
> > > .hdr.request = VHOST_USER_POSTCOPY_ADVISE,
> > > @@ -1182,7 +1184,7 @@ static int
> > > vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
> > > return 0;
> > > }
> > >
> > > -static int vhost_user_init(struct vhost_dev *dev, void *opaque)
> > > +static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
> > > {
> > > uint64_t features, protocol_features;
> > > struct vhost_user *u;
> > > @@ -1191,7 +1193,7 @@ static int vhost_user_init(struct vhost_dev *dev,
> > > void *opaque)
> > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> > >
> > > u = g_new0(struct vhost_user, 1);
> > > - u->chr = opaque;
> > > + u->user = opaque;
> > > u->slave_fd = -1;
> > > u->dev = dev;
> > > dev->opaque = u;
> > > @@ -1267,7 +1269,7 @@ static int vhost_user_init(struct vhost_dev *dev,
> > > void *opaque)
> > > return 0;
> > > }
> > >
> > > -static int vhost_user_cleanup(struct vhost_dev *dev)
> > > +static int vhost_user_backend_cleanup(struct vhost_dev *dev)
> > > {
> > > struct vhost_user *u;
> > >
> > > @@ -1581,10 +1583,21 @@ vhost_user_crypto_close_session(struct vhost_dev
> > > *dev, uint64_t session_id)
> > > return 0;
> > > }
> > >
> > > +VhostUserState *vhost_user_init(void)
> > > +{
> > > + VhostUserState *user = g_new0(struct VhostUserState, 1);
> > > +
> > > + return user;
> > > +}
> > > +
> > > +void vhost_user_cleanup(VhostUserState *user)
> > > +{
> > > +}
> > > +
> > > const VhostOps user_ops = {
> > > .backend_type = VHOST_BACKEND_TYPE_USER,
> > > - .vhost_backend_init = vhost_user_init,
> > > - .vhost_backend_cleanup = vhost_user_cleanup,
> > > + .vhost_backend_init = vhost_user_backend_init,
> > > + .vhost_backend_cleanup = vhost_user_backend_cleanup,
> > > .vhost_backend_memslots_limit = vhost_user_memslots_limit,
> > > .vhost_set_log_base = vhost_user_set_log_base,
> > > .vhost_set_mem_table = vhost_user_set_mem_table,
> > > diff --git a/include/hw/virtio/vhost-user-blk.h
> > > b/include/hw/virtio/vhost-user-blk.h
> > > index 5804cc904a..f1258ae545 100644
> > > --- a/include/hw/virtio/vhost-user-blk.h
> > > +++ b/include/hw/virtio/vhost-user-blk.h
> > > @@ -21,6 +21,7 @@
> > > #include "hw/block/block.h"
> > > #include "chardev/char-fe.h"
> > > #include "hw/virtio/vhost.h"
> > > +#include "hw/virtio/vhost-user.h"
> > >
> > > #define TYPE_VHOST_USER_BLK "vhost-user-blk"
> > > #define VHOST_USER_BLK(obj) \
> > > @@ -36,6 +37,7 @@ typedef struct VHostUserBlk {
> > > uint32_t config_wce;
> > > uint32_t config_ro;
> > > struct vhost_dev dev;
> > > + VhostUserState *vhost_user;
> > > } VHostUserBlk;
> > >
> > > #endif
> > > diff --git a/include/hw/virtio/vhost-user-scsi.h
> > > b/include/hw/virtio/vhost-user-scsi.h
> > > index 01861f78d0..3ec34ae867 100644
> > > --- a/include/hw/virtio/vhost-user-scsi.h
> > > +++ b/include/hw/virtio/vhost-user-scsi.h
> > > @@ -21,6 +21,7 @@
> > > #include "hw/qdev.h"
> > > #include "hw/virtio/virtio-scsi.h"
> > > #include "hw/virtio/vhost.h"
> > > +#include "hw/virtio/vhost-user.h"
> > > #include "hw/virtio/vhost-scsi-common.h"
> > >
> > > #define TYPE_VHOST_USER_SCSI "vhost-user-scsi"
> > > @@ -30,6 +31,7 @@
> > > typedef struct VHostUserSCSI {
> > > VHostSCSICommon parent_obj;
> > > uint64_t host_features;
> > > + VhostUserState *vhost_user;
> > > } VHostUserSCSI;
> > >
> > > #endif /* VHOST_USER_SCSI_H */
> > > diff --git a/include/hw/virtio/vhost-user.h
> > > b/include/hw/virtio/vhost-user.h
> > > new file mode 100644
> > > index 0000000000..eb8bc0d90d
> > > --- /dev/null
> > > +++ b/include/hw/virtio/vhost-user.h
> > > @@ -0,0 +1,20 @@
> > > +/*
> > > + * Copyright (c) 2017-2018 Intel Corporation
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2.
> > > + * See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#ifndef HW_VIRTIO_VHOST_USER_H
> > > +#define HW_VIRTIO_VHOST_USER_H
> > > +
> > > +#include "chardev/char-fe.h"
> > > +
> > > +typedef struct VhostUserState {
> > > + CharBackend *chr;
> > > +} VhostUserState;
> > > +
> > > +VhostUserState *vhost_user_init(void);
> > > +void vhost_user_cleanup(VhostUserState *user);
> > > +
> > > +#endif
> > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > index fa28aad12d..525a061acf 100644
> > > --- a/net/vhost-user.c
> > > +++ b/net/vhost-user.c
> > > @@ -12,6 +12,7 @@
> > > #include "clients.h"
> > > #include "net/vhost_net.h"
> > > #include "net/vhost-user.h"
> > > +#include "hw/virtio/vhost-user.h"
> > > #include "chardev/char-fe.h"
> > > #include "qapi/error.h"
> > > #include "qapi/qapi-commands-net.h"
> > > @@ -23,6 +24,7 @@
> > > typedef struct NetVhostUserState {
> > > NetClientState nc;
> > > CharBackend chr; /* only queue index 0 */
> > > + VhostUserState *vhost_user;
> > > VHostNetState *vhost_net;
> > > guint watch;
> > > uint64_t acked_features;
> > > @@ -64,7 +66,8 @@ static void vhost_user_stop(int queues, NetClientState
> > > *ncs[])
> > > }
> > > }
> > >
> > > -static int vhost_user_start(int queues, NetClientState *ncs[],
> > > CharBackend *be)
> > > +static int vhost_user_start(int queues, NetClientState *ncs[],
> > > + VhostUserState *be)
> > > {
> > > VhostNetOptions options;
> > > struct vhost_net *net = NULL;
> > > @@ -144,7 +147,7 @@ static ssize_t vhost_user_receive(NetClientState *nc,
> > > const uint8_t *buf,
> > > return size;
> > > }
> > >
> > > -static void vhost_user_cleanup(NetClientState *nc)
> > > +static void net_vhost_user_cleanup(NetClientState *nc)
> > > {
> > > NetVhostUserState *s = DO_UPCAST(NetVhostUserState, nc, nc);
> > >
> > > @@ -153,6 +156,11 @@ static void vhost_user_cleanup(NetClientState *nc)
> > > g_free(s->vhost_net);
> > > s->vhost_net = NULL;
> > > }
> > > + if (s->vhost_user) {
> > > + vhost_user_cleanup(s->vhost_user);
> > > + g_free(s->vhost_user);
> > > + s->vhost_user = NULL;
> > > + }
> > > if (nc->queue_index == 0) {
> > > if (s->watch) {
> > > g_source_remove(s->watch);
> > > @@ -182,7 +190,7 @@ static NetClientInfo net_vhost_user_info = {
> > > .type = NET_CLIENT_DRIVER_VHOST_USER,
> > > .size = sizeof(NetVhostUserState),
> > > .receive = vhost_user_receive,
> > > - .cleanup = vhost_user_cleanup,
> > > + .cleanup = net_vhost_user_cleanup,
> > > .has_vnet_hdr = vhost_user_has_vnet_hdr,
> > > .has_ufo = vhost_user_has_ufo,
> > > };
> > > @@ -244,7 +252,7 @@ static void net_vhost_user_event(void *opaque, int
> > > event)
> > > trace_vhost_user_event(chr->label, event);
> > > switch (event) {
> > > case CHR_EVENT_OPENED:
> > > - if (vhost_user_start(queues, ncs, &s->chr) < 0) {
> > > + if (vhost_user_start(queues, ncs, s->vhost_user) < 0) {
> > > qemu_chr_fe_disconnect(&s->chr);
> > > return;
> > > }
> > > @@ -283,12 +291,19 @@ static int net_vhost_user_init(NetClientState
> > > *peer, const char *device,
> > > {
> > > Error *err = NULL;
> > > NetClientState *nc, *nc0 = NULL;
> > > + VhostUserState *user = NULL;
> > > NetVhostUserState *s;
> > > int i;
> > >
> > > assert(name);
> > > assert(queues > 0);
> > >
> > > + user = vhost_user_init();
> > > + if (!user) {
> > > + error_report("failed to init vhost_user");
> > > + goto err;
> > > + }
> > > +
> > > for (i = 0; i < queues; i++) {
> > > nc = qemu_new_net_client(&net_vhost_user_info, peer, device,
> > > name);
> > > snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to
> > > %s",
> > > @@ -299,17 +314,19 @@ static int net_vhost_user_init(NetClientState
> > > *peer, const char *device,
> > > s = DO_UPCAST(NetVhostUserState, nc, nc);
> > > if (!qemu_chr_fe_init(&s->chr, chr, &err)) {
> > > error_report_err(err);
> > > - return -1;
> > > + goto err;
> > > }
> > > + user->chr = &s->chr;
> > > }
> > > -
> > > + s = DO_UPCAST(NetVhostUserState, nc, nc);
> > > + s->vhost_user = user;
> > > }
> > >
> > > s = DO_UPCAST(NetVhostUserState, nc, nc0);
> > > do {
> > > if (qemu_chr_fe_wait_connected(&s->chr, &err) < 0) {
> > > error_report_err(err);
> > > - return -1;
> > > + goto err;
> > > }
> > > qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
> > > net_vhost_user_event, NULL, nc0->name,
> > > NULL,
> > > @@ -319,6 +336,15 @@ static int net_vhost_user_init(NetClientState *peer,
> > > const char *device,
> > > assert(s->vhost_net);
> > >
> > > return 0;
> > > +
> > > +err:
> > > + if (user) {
> > > + vhost_user_cleanup(user);
> > > + g_free(user);
> > > + s->vhost_user = NULL;
> > > + }
> > > +
> > > + return -1;
> > > }
> > >
> > > static Chardev *net_vhost_claim_chardev(
> > > --
> > > 2.11.0
>
> So far I figured out that commenting the free of
> the structure removes the crash, so we seem to
> be dealing with a use-after free here.
> I suspect that in a MQ situation, one queue gets
> closed and attempts to free the structure
> while others still use it.
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 525a061..6a1573b 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -157,8 +157,8 @@ static void net_vhost_user_cleanup(NetClientState *nc)
> s->vhost_net = NULL;
> }
> if (s->vhost_user) {
> - vhost_user_cleanup(s->vhost_user);
> - g_free(s->vhost_user);
> + //vhost_user_cleanup(s->vhost_user);
> + //g_free(s->vhost_user);
> s->vhost_user = NULL;
> }
> if (nc->queue_index == 0) {
> @@ -339,8 +339,8 @@ static int net_vhost_user_init(NetClientState *peer,
> const char *device,
>
> err:
> if (user) {
> - vhost_user_cleanup(user);
> - g_free(user);
> + //vhost_user_cleanup(user);
> + //g_free(user);
> s->vhost_user = NULL;
> }
>
So the following at least gets rid of the crashes.
I am not sure it does not leak memory though,
and not sure there aren't any configurations where
the 1st queue gets cleaned up first.
Thoughts?
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
---
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 525a061..7549d25 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -156,19 +156,20 @@ static void net_vhost_user_cleanup(NetClientState *nc)
g_free(s->vhost_net);
s->vhost_net = NULL;
}
- if (s->vhost_user) {
- vhost_user_cleanup(s->vhost_user);
- g_free(s->vhost_user);
- s->vhost_user = NULL;
- }
if (nc->queue_index == 0) {
if (s->watch) {
g_source_remove(s->watch);
s->watch = 0;
}
qemu_chr_fe_deinit(&s->chr, true);
+ if (s->vhost_user) {
+ vhost_user_cleanup(s->vhost_user);
+ g_free(s->vhost_user);
+ }
}
+ s->vhost_user = NULL;
+
qemu_purge_queued_packets(nc);
}
@@ -341,7 +342,6 @@ err:
if (user) {
vhost_user_cleanup(user);
g_free(user);
- s->vhost_user = NULL;
}
return -1;
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
