Couple commit message NITs but otherwise I'm happy with this. Reviewed-by: Raphael Norwitz <raphael.norw...@nutanix.com>
On Wed, Mar 24, 2021 at 12:38:28PM +0300, Denis Plotnikov wrote: > It is useful to use different connect/disconnect event handlers > on device initialization and operation as seen from the further > commit fixing a bug on device initialization. > > The patch refactor the code to make use of them: we don't rely any s/The/This and s/refactor/refactors > more on the VM state for choosing how to cleanup the device, instead > we explicitly use the proper event handler dependping on whether s/dependping/depending > the device has been initialized. > > Signed-off-by: Denis Plotnikov <den-plotni...@yandex-team.ru> > --- > hw/block/vhost-user-blk.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index b870a50e6b20..1af95ec6aae7 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -362,7 +362,18 @@ static void vhost_user_blk_disconnect(DeviceState *dev) > vhost_dev_cleanup(&s->dev); > } > > -static void vhost_user_blk_event(void *opaque, QEMUChrEvent event); > +static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, > + bool realized); > + > +static void vhost_user_blk_event_realize(void *opaque, QEMUChrEvent event) > +{ > + vhost_user_blk_event(opaque, event, false); > +} > + > +static void vhost_user_blk_event_oper(void *opaque, QEMUChrEvent event) > +{ > + vhost_user_blk_event(opaque, event, true); > +} > > static void vhost_user_blk_chr_closed_bh(void *opaque) > { > @@ -371,11 +382,12 @@ static void vhost_user_blk_chr_closed_bh(void *opaque) > VHostUserBlk *s = VHOST_USER_BLK(vdev); > > vhost_user_blk_disconnect(dev); > - qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event, > - NULL, opaque, NULL, true); > + qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, > + vhost_user_blk_event_oper, NULL, opaque, NULL, true); > } > > -static void vhost_user_blk_event(void *opaque, QEMUChrEvent event) > +static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, > + bool realized) > { > DeviceState *dev = opaque; > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > @@ -406,7 +418,7 @@ static void vhost_user_blk_event(void *opaque, > QEMUChrEvent event) > * TODO: maybe it is a good idea to make the same fix > * for other vhost-user devices. > */ > - if (runstate_is_running()) { > + if (realized) { > AioContext *ctx = qemu_get_current_aio_context(); > > qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, NULL, NULL, > @@ -473,8 +485,9 @@ static void vhost_user_blk_device_realize(DeviceState > *dev, Error **errp) > s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues); > s->connected = false; > > - qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event, > - NULL, (void *)dev, NULL, true); > + qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, > + vhost_user_blk_event_realize, NULL, (void *)dev, > + NULL, true); > > reconnect: > if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) { > @@ -494,6 +507,10 @@ reconnect: > goto reconnect; > } > > + /* we're fully initialized, now we can operate, so change the handler */ > + qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, > + vhost_user_blk_event_oper, NULL, (void *)dev, > + NULL, true); > return; > > virtio_err: > -- > 2.25.1 >