On 21.10.25 21:06, Raphael Norwitz wrote:
ACK on your point. One more question about setting s->connected = false.
On Tue, Oct 21, 2025 at 12:29 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
On 21.10.25 02:35, Raphael Norwitz wrote:
On Thu, Oct 16, 2025 at 7:48 AM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
We'll need to postpone further connecting/reconnecting logic to the
later point to support backend-transfer migration for vhost-user-blk.
For now, move first call to vhost_user_blk_init() to _realize() (this
call will not be postponed). To support this, we also have to move
re-initialization to vhost_user_blk_realize_connect_loop().
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
hw/block/vhost-user-blk.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 36e32229ad..af4a97b8e4 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -464,14 +464,12 @@ static int vhost_user_blk_realize_connect(VHostUserBlk
*s, Error **errp)
DeviceState *dev = DEVICE(s);
int ret;
- s->connected = false;
-
ret = qemu_chr_fe_wait_connected(&s->chardev, errp);
if (ret < 0) {
return ret;
}
- ret = vhost_user_blk_init(dev, true, errp);
+ ret = vhost_user_blk_connect(dev, errp);
if (ret < 0) {
qemu_chr_fe_disconnect(&s->chardev);
return ret;
@@ -501,7 +499,16 @@ static int
vhost_user_blk_realize_connect_loop(VHostUserBlk *s, Error **errp)
error_prepend(errp, "Reconnecting after error: ");
error_report_err(*errp);
*errp = NULL;
+
Having removed setting s->connected = false from
vhost_user_blk_realize_connect() we will now only set s->connected =
false here in the if (*errp) {} error path. Shouldn't we also set
s->connected = false outside the error path here or in
vhost_user_blk_device_realize()?
On success path we are calling vhost_user_blk_realize_connect() for the first
time, so s->connected was never set to true at this time, it must be false
(due to zero-initialization of the structure). We can add an assertion.
+ s->connected = false;
+
+ ret = vhost_user_blk_init(dev, false, errp);
+ if (ret < 0) {
+ /* No reason to retry initialization */
+ return ret;
+ }
}
+
ret = vhost_user_blk_realize_connect(s, errp);
} while (ret < 0 && retries--);
@@ -566,6 +573,10 @@ static void vhost_user_blk_device_realize(DeviceState
*dev, Error **errp)
s->inflight = g_new0(struct vhost_inflight, 1);
s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
Why call vhost_user_blk_init() here if we call it in
host_user_blk_realize_connect_loop()?
To be able to postpone the whole realize-connect-loop to the later
point (not in realize) in further commits.
So this first init will stay in realize, for early initialization of the device.
Makes sense - I missed that vhost_user_blk_init() is only called in
the error path.
+ if (vhost_user_blk_init(dev, false, errp) < 0) {
+ goto fail;
+ }
+
if (vhost_user_blk_realize_connect_loop(s, errp) < 0) {
goto fail;
}
--
2.48.1
--
Best regards,
Vladimir
--
Best regards,
Vladimir
