Re: [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity

[Guoqing Jiang <guoqing.jiang@linux.dev>]

On 6/22/22 6:57 PM, Jinpu Wang wrote:
> On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> 1. process_msg_open_rsp checks if capacity changed or not before call
>> rnbd_clt_change_capacity while the checking also make sense for
>> rnbd_clt_resize_dev_store, let's move the checking into the function.
>>
>> 2. change the parameter type to 'sector_t' then we don't need to cast
>> it from rnbd_clt_resize_dev_store, and update rnbd_clt_resize_disk too.
>>
>> 3. no need to checking the return value, make it return void.
>>
> better to split this into 3 patches.

Will do.

>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>>   drivers/block/rnbd/rnbd-clt-sysfs.c |  2 +-
>>   drivers/block/rnbd/rnbd-clt.c       | 24 ++++++++++++------------
>>   drivers/block/rnbd/rnbd-clt.h       |  2 +-
>>   3 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
>> index 2be5d87a3ca6..e7c7d9a68168 100644
>> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
>> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
>> @@ -376,7 +376,7 @@ static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj,
>>          if (ret)
>>                  return ret;
>>
>> -       ret = rnbd_clt_resize_disk(dev, (size_t)sectors);
>> +       ret = rnbd_clt_resize_disk(dev, sectors);
>>          if (ret)
>>                  return ret;
>>
>> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
>> index 2c63cd5ac09d..6c6c4ba3d41d 100644
>> --- a/drivers/block/rnbd/rnbd-clt.c
>> +++ b/drivers/block/rnbd/rnbd-clt.c
>> @@ -68,13 +68,18 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
>>          return refcount_inc_not_zero(&dev->refcount);
>>   }
>>
>> -static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
>> -                                   size_t new_nsectors)
>> +static void rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
>> +                                    sector_t new_nsectors)
>>   {
>> -       rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
>> +       if (get_capacity(dev->gd) != new_nsectors)
>> +               return;
> This change is broken, it leads to resize no longer work, should be "=="

Yes, thanks for catch it.

Thanks,
Guoqing