Re: [PATCH v4 2/3] vhost-user-blk: split vhost_user_blk_sync_config()

[Vladimir Sementsov-Ogievskiy]

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;
> >   }

--
Best regards,
Vladimir