| Message ID | 20231006202045.1161543-2-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-user-blk: live resize additional APIs | expand |
I don’t understand the “valid for resize only” comment. Looks like this is zero day behavior and I can’t tell why it was added. Does anyone know? With that, reasoning and code looks good: Acked-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > On Oct 6, 2023, at 4:20 PM, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > Let's not care about what was changed and update the whole config, > reasons: > > 1. config->geometry should be updated together with capacity, so we fix > a bug. > > 2. Vhost-user protocol doesn't say anything about config change > limitation. Silent ignore of changes doesn't seem to be correct. > > 3. vhost-user-vsock reads the whole config > > 4. on realize we don't do any checks on retrieved config, so no reason > to care here > > Also, let's notify guest unconditionally: > > 1. So does vhost-user-vsock > > 2. We are going to reuse the functionality in new cases when we do want > to notify the guest unconditionally. So, no reason to create extra > branches in the logic. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hw/block/vhost-user-blk.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index eecf3f7a81..1ee05b46ee 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -93,7 +93,6 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) > static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) > { > int ret; > - struct virtio_blk_config blkcfg; > VirtIODevice *vdev = dev->vdev; > VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); > Error *local_err = NULL; > @@ -102,19 +101,15 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) > return 0; > } > > - ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg, > + ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, > vdev->config_len, &local_err); > if (ret < 0) { > error_report_err(local_err); > return ret; > } > > - /* valid for resize only */ > - if (blkcfg.capacity != s->blkcfg.capacity) { > - s->blkcfg.capacity = blkcfg.capacity; > - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); > - virtio_notify_config(dev->vdev); > - } > + memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); > + virtio_notify_config(dev->vdev); > > return 0; > } > -- > 2.34.1 >
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index eecf3f7a81..1ee05b46ee 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -93,7 +93,6 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) { int ret; - struct virtio_blk_config blkcfg; VirtIODevice *vdev = dev->vdev; VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); Error *local_err = NULL; @@ -102,19 +101,15 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) return 0; } - ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg, + ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, vdev->config_len, &local_err); if (ret < 0) { error_report_err(local_err); return ret; } - /* valid for resize only */ - if (blkcfg.capacity != s->blkcfg.capacity) { - s->blkcfg.capacity = blkcfg.capacity; - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); - virtio_notify_config(dev->vdev); - } + memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); + virtio_notify_config(dev->vdev); return 0; }
Let's not care about what was changed and update the whole config, reasons: 1. config->geometry should be updated together with capacity, so we fix a bug. 2. Vhost-user protocol doesn't say anything about config change limitation. Silent ignore of changes doesn't seem to be correct. 3. vhost-user-vsock reads the whole config 4. on realize we don't do any checks on retrieved config, so no reason to care here Also, let's notify guest unconditionally: 1. So does vhost-user-vsock 2. We are going to reuse the functionality in new cases when we do want to notify the guest unconditionally. So, no reason to create extra branches in the logic. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- hw/block/vhost-user-blk.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)