1. Patchew
  2. QEMU
  3. [PATCH v4 0/3] vhost-user-blk: live resize additional APIs
  4. View patch

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

Vladimir Sementsov-Ogievskiy posted 3 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v4 2/3] vhost-user-blk: split vhost_user_blk_sync_config()
Posted by Vladimir Sementsov-Ogievskiy 1 year, 9 months ago 
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);
+    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;
 }
 
-- 
2.34.1
Re: [PATCH v4 2/3] vhost-user-blk: split vhost_user_blk_sync_config()
Posted by Markus Armbruster 1 year, 9 months ago 
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;
>  }
Re: [PATCH v4 2/3] vhost-user-blk: split vhost_user_blk_sync_config()
Posted by Vladimir Sementsov-Ogievskiy 1 year, 9 months ago 
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
Re: [PATCH v4 2/3] vhost-user-blk: split vhost_user_blk_sync_config()
Posted by Markus Armbruster 1 year, 9 months ago 
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;
>>>  }

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.