Message ID | 20240429101623.1992943-3-vsementsov@yandex-team.ru |
---|---|
State | New |
Headers | show |
Series | [v4,1/3] qdev-monitor: add option to report GenericError from find_device_state | expand |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > Split vhost_user_blk_sync_config() out from > vhost_user_blk_handle_config_change(), to be reused in the following > commit. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hw/block/vhost-user-blk.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 9e6bbc6950..091d2c6acf 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -88,27 +88,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) > s->blkcfg.wce = blkcfg->wce; > } > > +static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp) > +{ > + int ret; > + VirtIODevice *vdev = VIRTIO_DEVICE(dev); Note for later: all this function does with paramter @dev is cast it to VirtIODevice *. > + VHostUserBlk *s = VHOST_USER_BLK(vdev); > + > + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, > + vdev->config_len, errp); > + if (ret < 0) { > + return ret; > + } > + > + memcpy(vdev->config, &s->blkcfg, vdev->config_len); > + virtio_notify_config(vdev); > + > + return 0; > +} > + > static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) > { > int ret; > - VirtIODevice *vdev = dev->vdev; > - VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); > Error *local_err = NULL; > > if (!dev->started) { > return 0; > } > > - ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, > - vdev->config_len, &local_err); > + ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err); dev->vdev is a VirtIODevice *. You cast it to DeviceState * for vhost_user_blk_sync_config(), which casts it right back. Could you simply pass it as is instead? > if (ret < 0) { > error_report_err(local_err); > return ret; > } > > - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); > - virtio_notify_config(dev->vdev); > - > return 0; > }
On 30.04.24 11:15, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > >> Split vhost_user_blk_sync_config() out from >> vhost_user_blk_handle_config_change(), to be reused in the following >> commit. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> hw/block/vhost-user-blk.c | 26 +++++++++++++++++++------- >> 1 file changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c >> index 9e6bbc6950..091d2c6acf 100644 >> --- a/hw/block/vhost-user-blk.c >> +++ b/hw/block/vhost-user-blk.c >> @@ -88,27 +88,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) >> s->blkcfg.wce = blkcfg->wce; >> } >> >> +static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp) >> +{ >> + int ret; >> + VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > Note for later: all this function does with paramter @dev is cast it to > VirtIODevice *. > >> + VHostUserBlk *s = VHOST_USER_BLK(vdev); >> + >> + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, >> + vdev->config_len, errp); >> + if (ret < 0) { >> + return ret; >> + } >> + >> + memcpy(vdev->config, &s->blkcfg, vdev->config_len); >> + virtio_notify_config(vdev); >> + >> + return 0; >> +} >> + >> static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) >> { >> int ret; >> - VirtIODevice *vdev = dev->vdev; >> - VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); >> Error *local_err = NULL; >> >> if (!dev->started) { >> return 0; >> } >> >> - ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, >> - vdev->config_len, &local_err); >> + ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err); > > dev->vdev is a VirtIODevice *. You cast it to DeviceState * for > vhost_user_blk_sync_config(), which casts it right back. > > Could you simply pass it as is instead? vhost_user_blk_sync_config() is generic handler, which will be used as ->sync_config() in the following commit, so it's good and convenient for it to have DeviceState* argument. > >> if (ret < 0) { >> error_report_err(local_err); >> return ret; >> } >> >> - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); >> - virtio_notify_config(dev->vdev); >> - >> return 0; >> } >
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > On 30.04.24 11:15, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: >> >>> Split vhost_user_blk_sync_config() out from >>> vhost_user_blk_handle_config_change(), to be reused in the following >>> commit. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >>> --- >>> hw/block/vhost-user-blk.c | 26 +++++++++++++++++++------- >>> 1 file changed, 19 insertions(+), 7 deletions(-) >>> >>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c >>> index 9e6bbc6950..091d2c6acf 100644 >>> --- a/hw/block/vhost-user-blk.c >>> +++ b/hw/block/vhost-user-blk.c >>> @@ -88,27 +88,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) >>> s->blkcfg.wce = blkcfg->wce; >>> } >>> +static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp) >>> +{ >>> + int ret; >>> + VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >> Note for later: all this function does with paramter @dev is cast it to >> VirtIODevice *. >> >>> + VHostUserBlk *s = VHOST_USER_BLK(vdev); >>> + >>> + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, >>> + vdev->config_len, errp); >>> + if (ret < 0) { >>> + return ret; >>> + } >>> + >>> + memcpy(vdev->config, &s->blkcfg, vdev->config_len); >>> + virtio_notify_config(vdev); >>> + >>> + return 0; >>> +} >>> + >>> static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) >>> { >>> int ret; >>> - VirtIODevice *vdev = dev->vdev; >>> - VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); >>> Error *local_err = NULL; >>> >>> if (!dev->started) { >>> return 0; >>> } >>> >>> - ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, >>> - vdev->config_len, &local_err); >>> + ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err); >> >> dev->vdev is a VirtIODevice *. You cast it to DeviceState * for >> vhost_user_blk_sync_config(), which casts it right back. >> Could you simply pass it as is instead? > > vhost_user_blk_sync_config() is generic handler, which will be used as ->sync_config() in the following commit, so it's good and convenient for it to have DeviceState* argument. Ah, that's what I missed. >>> if (ret < 0) { >>> error_report_err(local_err); >>> return ret; >>> } >>> >>> - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); >>> - virtio_notify_config(dev->vdev); >>> - >>> return 0; >>> }
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 9e6bbc6950..091d2c6acf 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -88,27 +88,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config) s->blkcfg.wce = blkcfg->wce; } +static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp) +{ + int ret; + VirtIODevice *vdev = VIRTIO_DEVICE(dev); + VHostUserBlk *s = VHOST_USER_BLK(vdev); + + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, + vdev->config_len, errp); + if (ret < 0) { + return ret; + } + + memcpy(vdev->config, &s->blkcfg, vdev->config_len); + virtio_notify_config(vdev); + + return 0; +} + static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) { int ret; - VirtIODevice *vdev = dev->vdev; - VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); Error *local_err = NULL; if (!dev->started) { return 0; } - ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg, - vdev->config_len, &local_err); + ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err); if (ret < 0) { error_report_err(local_err); return ret; } - memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len); - virtio_notify_config(dev->vdev); - return 0; }
Split vhost_user_blk_sync_config() out from vhost_user_blk_handle_config_change(), to be reused in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- hw/block/vhost-user-blk.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)