[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 05:30:45PM +0300, Michael S. Tsirkin wrote: > On Thu, May 24, 2018 at 06:59:36PM +0800, Tiwei Bie wrote: > > On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: > > > On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > > > > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > > > > 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> > > > > > > > > > > > > > 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. > > > > > > > > It's my fault to not notice this bug before. > > > > I'm very sorry. Thank you so much for finding > > > > the root cause! > > > > > > > > > > > > > > > > > > > --- > > > > > > > > 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 > > > > [...] > > > > > > > > qemu_chr_fe_set_handlers(>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. > > > > > > > > > >
Re: [virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 04:55:04PM +0300, Michael S. Tsirkin wrote: > On Thu, May 24, 2018 at 06:59:36PM +0800, Tiwei Bie wrote: > > On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: > > > On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > > > > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > > > > 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> > > > > > > > > > > > > > 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. > > > > > > > > It's my fault to not notice this bug before. > > > > I'm very sorry. Thank you so much for finding > > > > the root cause! > > > > > > > > > > > > > > > > > > > --- > > > > > > > > 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 > > > > [...] > > > > > > > > qemu_chr_fe_set_handlers(>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. > > > > > > > > > >
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 06:59:36PM +0800, Tiwei Bie wrote: > On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: > > On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > > > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > > > 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> > > > > > > > > > > > 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. > > > > > > It's my fault to not notice this bug before. > > > I'm very sorry. Thank you so much for finding > > > the root cause! > > > > > > > > > > > > > > > > --- > > > > > > > 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 > > > [...] > > > > > > > qemu_chr_fe_set_handlers(>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? > > > > > > Thank you so much for catching it and fixing > > > it! I'll keep your SoB there. Thank you so > > > much! I do appreciate it! > > > > > > You are right. This structure is freed multiple > > > times when multi-queue is enabled. > > > > After a deeper digging, I
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 06:59:36PM +0800, Tiwei Bie wrote: > On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: > > On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > > > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > > > 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> > > > > > > > > > > > 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. > > > > > > It's my fault to not notice this bug before. > > > I'm very sorry. Thank you so much for finding > > > the root cause! > > > > > > > > > > > > > > > > --- > > > > > > > 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 > > > [...] > > > > > > > qemu_chr_fe_set_handlers(>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? > > > > > > Thank you so much for catching it and fixing > > > it! I'll keep your SoB there. Thank you so > > > much! I do appreciate it! > > > > > > You are right. This structure is freed multiple > > > times when multi-queue is enabled. > > > > After a deeper digging, I
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: > On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > > 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> > > > > > > > > > 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. > > > > It's my fault to not notice this bug before. > > I'm very sorry. Thank you so much for finding > > the root cause! > > > > > > > > > > > > > --- > > > > > > 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 > > [...] > > > > > > qemu_chr_fe_set_handlers(>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? > > > > Thank you so much for catching it and fixing > > it! I'll keep your SoB there. Thank you so > > much! I do appreciate it! > > > > You are right. This structure is freed multiple > > times when multi-queue is enabled. > > After a deeper digging, I got your point now.. > It could be a use-after-free instead of a double > free.. As it's safe to deinit the char which is > shared by all queue pairs when cleanup the 1st > queue pair, it should be safe to free vhost-user > structure there too. One thing worried me is that, I can't reproduce
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 10:24:40AM +0800, Tiwei Bie wrote: [...] > > > + > > > qemu_purge_queued_packets(nc); > > > } > > > > > > @@ -341,7 +342,6 @@ err: > > > if (user) { > > > vhost_user_cleanup(user); > > > g_free(user); > > > -s->vhost_user = NULL; > > I don't get why cannot zero it in this case. Hmm.. Please just ignore above comment. Thanks for catching this bug! Best regards, Tiwei Bie - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Thu, May 24, 2018 at 07:21:01AM +0800, Tiwei Bie wrote: > On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > > 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> > > > > > > > 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. > > It's my fault to not notice this bug before. > I'm very sorry. Thank you so much for finding > the root cause! > > > > > > > > > > --- > > > > > 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 > [...] > > > > > qemu_chr_fe_set_handlers(>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? > > Thank you so much for catching it and fixing > it! I'll keep your SoB there. Thank you so > much! I do appreciate it! > > You are right. This structure is freed multiple > times when multi-queue is enabled. After a deeper digging, I got your point now.. It could be a use-after-free instead of a double free.. As it's safe to deinit the char which is shared by all queue pairs when cleanup the 1st queue pair, it should be safe to free vhost-user structure there too. > > I think it's safe to let the first queue pair > free the vhost-user structure, because it won't > be touched by other queue pairs during cleanup. > > Best regards, > Tiwei Bie > > > > > > Signed-off-by: Michael S. Tsirkin > > > > --- > > > > diff --git a/net/vhost-user.c b/net/vhost-user.c > > index 525a061..7549d25 100644 > > --- a/net/vhost-user.c
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
On Wed, May 23, 2018 at 06:43:29PM +0300, Michael S. Tsirkin wrote: > 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> > > > > > 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. It's my fault to not notice this bug before. I'm very sorry. Thank you so much for finding the root cause! > > > > > > > --- > > > > 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 [...] > > > > qemu_chr_fe_set_handlers(>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? Thank you so much for catching it and fixing it! I'll keep your SoB there. Thank you so much! I do appreciate it! You are right. This structure is freed multiple times when multi-queue is enabled. I think it's safe to let the first queue pair free the vhost-user structure, because it won't be touched by other queue pairs during cleanup. Best regards, Tiwei Bie > > Signed-off-by: Michael S. Tsirkin > > --- > > 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(>chr, true); > +if (s->vhost_user) { > +
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
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> > > > 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 = >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(); > > > @@ -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 = >chr; > > > +s->vhost_user = user; > > > + > > > qemu_chr_fe_set_handlers(>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 = >chardev; > > > +s->vhost_user = user; > > > + > > > virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, > > > sizeof(struct
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
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> > 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 = >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(); > > @@ -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 = >chr; > > +s->vhost_user = user; > > + > > qemu_chr_fe_set_handlers(>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 = >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(>dev, _ops); > > > > -ret = vhost_dev_init(>dev, >chardev, VHOST_BACKEND_TYPE_USER, 0); > > +ret = vhost_dev_init(>dev, s->vhost_user,
[virtio-dev] Re: [PATCH v3 2/6] vhost-user: introduce shared vhost-user state
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 BieUnfortunately 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 = >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(); > @@ -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 = >chr; > +s->vhost_user = user; > + > qemu_chr_fe_set_handlers(>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 = >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(>dev, _ops); > > -ret = vhost_dev_init(>dev, >chardev, VHOST_BACKEND_TYPE_USER, 0); > +ret = vhost_dev_init(>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); > +