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

[Guoqing Jiang <guoqing.jiang@xxxxxxxxx>]

On 6/22/22 6:57 PM, Jinpu Wang wrote:
> On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >   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