[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/2] vhost-user-blk: use different event handlers on initi
From: |
Raphael Norwitz |
Subject: |
Re: [PATCH v2 1/2] vhost-user-blk: use different event handlers on initialization |
Date: |
Wed, 24 Mar 2021 18:25:07 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Couple commit message NITs but otherwise I'm happy with this.
Reviewed-by: Raphael Norwitz <raphael.norwitz@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-plotnikov@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
>