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 <[email protected]> > On Oct 6, 2023, at 4:20 PM, Vladimir Sementsov-Ogievskiy > <[email protected]> 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 <[email protected]> > --- > 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 >
