* [PATCH v2 1/2] Revert "vhost-user: fix lost reconnect" [not found] <20240513071052.129581-1-fengli@smartx.com> @ 2024-05-13 7:10 ` Li Feng 2024-05-13 7:15 ` Michael S. Tsirkin 2024-05-13 7:10 ` [PATCH v2 2/2] vhost-user: fix lost reconnect again Li Feng 1 sibling, 1 reply; 3+ messages in thread From: Li Feng @ 2024-05-13 7:10 UTC (permalink / raw) To: Michael S. Tsirkin, Raphael Norwitz, Kevin Wolf, Hanna Reitz, Paolo Bonzini, Fam Zheng, Alex Bennée, open list:Block layer core, open list:All patches CC here Cc: Yajun Wu, Stefano Garzarella This reverts commit f02a4b8e6431598612466f76aac64ab492849abf. Signed-off-by: Li Feng <fengli@smartx.com> --- hw/block/vhost-user-blk.c | 2 +- hw/scsi/vhost-user-scsi.c | 3 +-- hw/virtio/vhost-user-base.c | 2 +- hw/virtio/vhost-user.c | 10 ++-------- include/hw/virtio/vhost-user.h | 3 +-- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 9e6bbc6950..41d1ac3a5a 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -384,7 +384,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event) case CHR_EVENT_CLOSED: /* defer close until later to avoid circular close */ vhost_user_async_close(dev, &s->chardev, &s->dev, - vhost_user_blk_disconnect, vhost_user_blk_event); + vhost_user_blk_disconnect); break; case CHR_EVENT_BREAK: case CHR_EVENT_MUX_IN: diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index a63b1f4948..48a59e020e 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -214,8 +214,7 @@ static void vhost_user_scsi_event(void *opaque, QEMUChrEvent event) case CHR_EVENT_CLOSED: /* defer close until later to avoid circular close */ vhost_user_async_close(dev, &vs->conf.chardev, &vsc->dev, - vhost_user_scsi_disconnect, - vhost_user_scsi_event); + vhost_user_scsi_disconnect); break; case CHR_EVENT_BREAK: case CHR_EVENT_MUX_IN: diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c index a83167191e..4b54255682 100644 --- a/hw/virtio/vhost-user-base.c +++ b/hw/virtio/vhost-user-base.c @@ -254,7 +254,7 @@ static void vub_event(void *opaque, QEMUChrEvent event) case CHR_EVENT_CLOSED: /* defer close until later to avoid circular close */ vhost_user_async_close(dev, &vub->chardev, &vub->vhost_dev, - vub_disconnect, vub_event); + vub_disconnect); break; case CHR_EVENT_BREAK: case CHR_EVENT_MUX_IN: diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index cdf9af4a4b..c929097e87 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -2776,7 +2776,6 @@ typedef struct { DeviceState *dev; CharBackend *cd; struct vhost_dev *vhost; - IOEventHandler *event_cb; } VhostAsyncCallback; static void vhost_user_async_close_bh(void *opaque) @@ -2791,10 +2790,7 @@ static void vhost_user_async_close_bh(void *opaque) */ if (vhost->vdev) { data->cb(data->dev); - } else if (data->event_cb) { - qemu_chr_fe_set_handlers(data->cd, NULL, NULL, data->event_cb, - NULL, data->dev, NULL, true); - } + } g_free(data); } @@ -2806,8 +2802,7 @@ static void vhost_user_async_close_bh(void *opaque) */ void vhost_user_async_close(DeviceState *d, CharBackend *chardev, struct vhost_dev *vhost, - vu_async_close_fn cb, - IOEventHandler *event_cb) + vu_async_close_fn cb) { if (!runstate_check(RUN_STATE_SHUTDOWN)) { /* @@ -2823,7 +2818,6 @@ void vhost_user_async_close(DeviceState *d, data->dev = d; data->cd = chardev; data->vhost = vhost; - data->event_cb = event_cb; /* Disable any further notifications on the chardev */ qemu_chr_fe_set_handlers(chardev, diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index d7c09ffd34..324cd8663a 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -108,7 +108,6 @@ typedef void (*vu_async_close_fn)(DeviceState *cb); void vhost_user_async_close(DeviceState *d, CharBackend *chardev, struct vhost_dev *vhost, - vu_async_close_fn cb, - IOEventHandler *event_cb); + vu_async_close_fn cb); #endif -- 2.45.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/2] Revert "vhost-user: fix lost reconnect" 2024-05-13 7:10 ` [PATCH v2 1/2] Revert "vhost-user: fix lost reconnect" Li Feng @ 2024-05-13 7:15 ` Michael S. Tsirkin 0 siblings, 0 replies; 3+ messages in thread From: Michael S. Tsirkin @ 2024-05-13 7:15 UTC (permalink / raw) To: Li Feng Cc: Raphael Norwitz, Kevin Wolf, Hanna Reitz, Paolo Bonzini, Fam Zheng, Alex Bennée, open list:Block layer core, open list:All patches CC here, Yajun Wu, Stefano Garzarella On Mon, May 13, 2024 at 03:10:47PM +0800, Li Feng wrote: > This reverts commit f02a4b8e6431598612466f76aac64ab492849abf. > include subject of reverted commit and motivation for the revert pls. > Signed-off-by: Li Feng <fengli@smartx.com> > --- > hw/block/vhost-user-blk.c | 2 +- > hw/scsi/vhost-user-scsi.c | 3 +-- > hw/virtio/vhost-user-base.c | 2 +- > hw/virtio/vhost-user.c | 10 ++-------- > include/hw/virtio/vhost-user.h | 3 +-- > 5 files changed, 6 insertions(+), 14 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 9e6bbc6950..41d1ac3a5a 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -384,7 +384,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event) > case CHR_EVENT_CLOSED: > /* defer close until later to avoid circular close */ > vhost_user_async_close(dev, &s->chardev, &s->dev, > - vhost_user_blk_disconnect, vhost_user_blk_event); > + vhost_user_blk_disconnect); > break; > case CHR_EVENT_BREAK: > case CHR_EVENT_MUX_IN: > diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c > index a63b1f4948..48a59e020e 100644 > --- a/hw/scsi/vhost-user-scsi.c > +++ b/hw/scsi/vhost-user-scsi.c > @@ -214,8 +214,7 @@ static void vhost_user_scsi_event(void *opaque, QEMUChrEvent event) > case CHR_EVENT_CLOSED: > /* defer close until later to avoid circular close */ > vhost_user_async_close(dev, &vs->conf.chardev, &vsc->dev, > - vhost_user_scsi_disconnect, > - vhost_user_scsi_event); > + vhost_user_scsi_disconnect); > break; > case CHR_EVENT_BREAK: > case CHR_EVENT_MUX_IN: > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c > index a83167191e..4b54255682 100644 > --- a/hw/virtio/vhost-user-base.c > +++ b/hw/virtio/vhost-user-base.c > @@ -254,7 +254,7 @@ static void vub_event(void *opaque, QEMUChrEvent event) > case CHR_EVENT_CLOSED: > /* defer close until later to avoid circular close */ > vhost_user_async_close(dev, &vub->chardev, &vub->vhost_dev, > - vub_disconnect, vub_event); > + vub_disconnect); > break; > case CHR_EVENT_BREAK: > case CHR_EVENT_MUX_IN: > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index cdf9af4a4b..c929097e87 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -2776,7 +2776,6 @@ typedef struct { > DeviceState *dev; > CharBackend *cd; > struct vhost_dev *vhost; > - IOEventHandler *event_cb; > } VhostAsyncCallback; > > static void vhost_user_async_close_bh(void *opaque) > @@ -2791,10 +2790,7 @@ static void vhost_user_async_close_bh(void *opaque) > */ > if (vhost->vdev) { > data->cb(data->dev); > - } else if (data->event_cb) { > - qemu_chr_fe_set_handlers(data->cd, NULL, NULL, data->event_cb, > - NULL, data->dev, NULL, true); > - } > + } > > g_free(data); > } > @@ -2806,8 +2802,7 @@ static void vhost_user_async_close_bh(void *opaque) > */ > void vhost_user_async_close(DeviceState *d, > CharBackend *chardev, struct vhost_dev *vhost, > - vu_async_close_fn cb, > - IOEventHandler *event_cb) > + vu_async_close_fn cb) > { > if (!runstate_check(RUN_STATE_SHUTDOWN)) { > /* > @@ -2823,7 +2818,6 @@ void vhost_user_async_close(DeviceState *d, > data->dev = d; > data->cd = chardev; > data->vhost = vhost; > - data->event_cb = event_cb; > > /* Disable any further notifications on the chardev */ > qemu_chr_fe_set_handlers(chardev, > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > index d7c09ffd34..324cd8663a 100644 > --- a/include/hw/virtio/vhost-user.h > +++ b/include/hw/virtio/vhost-user.h > @@ -108,7 +108,6 @@ typedef void (*vu_async_close_fn)(DeviceState *cb); > > void vhost_user_async_close(DeviceState *d, > CharBackend *chardev, struct vhost_dev *vhost, > - vu_async_close_fn cb, > - IOEventHandler *event_cb); > + vu_async_close_fn cb); > > #endif > -- > 2.45.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 2/2] vhost-user: fix lost reconnect again [not found] <20240513071052.129581-1-fengli@smartx.com> 2024-05-13 7:10 ` [PATCH v2 1/2] Revert "vhost-user: fix lost reconnect" Li Feng @ 2024-05-13 7:10 ` Li Feng 1 sibling, 0 replies; 3+ messages in thread From: Li Feng @ 2024-05-13 7:10 UTC (permalink / raw) To: Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz, Paolo Bonzini, Fam Zheng, Alex Bennée, open list:Block layer core, open list:All patches CC here Cc: Yajun Wu, Stefano Garzarella When the vhost-user is reconnecting to the backend, and if the vhost-user fails at the get_features in vhost_dev_init(), then the reconnect will fail and it will not be retriggered forever. The reason is: When the vhost-user fail at get_features, the vhost_dev_cleanup will be called immediately. vhost_dev_cleanup calls 'memset(hdev, 0, sizeof(struct vhost_dev))'. The reconnect path is: vhost_user_blk_event vhost_user_async_close(.. vhost_user_blk_disconnect ..) qemu_chr_fe_set_handlers <----- clear the notifier callback schedule vhost_user_async_close_bh The vhost->vdev is null, so the vhost_user_blk_disconnect will not be called, then the event fd callback will not be reinstalled. With this patch, the vhost_user_blk_disconnect will call the vhost_dev_cleanup() again, it's safe. In addition, the CLOSE event may occur in a scenario where connected is false. At this time, the event handler will be cleared. We need to ensure that the event handler can remain installed. All vhost-user devices have this issue, including vhost-user-blk/scsi. Fixes: 71e076a07d ("hw/virtio: generalise CHR_EVENT_CLOSED handling") Signed-off-by: Li Feng <fengli@smartx.com> --- hw/block/vhost-user-blk.c | 3 ++- hw/scsi/vhost-user-scsi.c | 3 ++- hw/virtio/vhost-user-base.c | 3 ++- hw/virtio/vhost-user.c | 10 +--------- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 41d1ac3a5a..c6842ced48 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -353,7 +353,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev) VHostUserBlk *s = VHOST_USER_BLK(vdev); if (!s->connected) { - return; + goto done; } s->connected = false; @@ -361,6 +361,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev) vhost_dev_cleanup(&s->dev); +done: /* Re-instate the event handler for new connections */ qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event, NULL, dev, NULL, true); diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 48a59e020e..b49a11d23b 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -181,7 +181,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev) VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); if (!s->connected) { - return; + goto done; } s->connected = false; @@ -189,6 +189,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev) vhost_dev_cleanup(&vsc->dev); +done: /* Re-instate the event handler for new connections */ qemu_chr_fe_set_handlers(&vs->conf.chardev, NULL, NULL, vhost_user_scsi_event, NULL, dev, NULL, true); diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c index 4b54255682..11e72b1e3b 100644 --- a/hw/virtio/vhost-user-base.c +++ b/hw/virtio/vhost-user-base.c @@ -225,13 +225,14 @@ static void vub_disconnect(DeviceState *dev) VHostUserBase *vub = VHOST_USER_BASE(vdev); if (!vub->connected) { - return; + goto done; } vub->connected = false; vub_stop(vdev); vhost_dev_cleanup(&vub->vhost_dev); +done: /* Re-instate the event handler for new connections */ qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index c929097e87..c407ea8939 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -2781,16 +2781,8 @@ typedef struct { static void vhost_user_async_close_bh(void *opaque) { VhostAsyncCallback *data = opaque; - struct vhost_dev *vhost = data->vhost; - /* - * If the vhost_dev has been cleared in the meantime there is - * nothing left to do as some other path has completed the - * cleanup. - */ - if (vhost->vdev) { - data->cb(data->dev); - } + data->cb(data->dev); g_free(data); } -- 2.45.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-13 7:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240513071052.129581-1-fengli@smartx.com>
2024-05-13 7:10 ` [PATCH v2 1/2] Revert "vhost-user: fix lost reconnect" Li Feng
2024-05-13 7:15 ` Michael S. Tsirkin
2024-05-13 7:10 ` [PATCH v2 2/2] vhost-user: fix lost reconnect again Li Feng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.