Re: [PATCH 2/6] rbd: kill rbd_update_mapping_size()

[Josh Durgin <josh.durgin@inktank.com>]

Alex Elder <elder@inktank.com> wrote:

>Since rbd_update_mapping_size() is now a trivial wrapper, just open
>code it in its two callers.
>
>Signed-off-by: Alex Elder <elder@inktank.com>
>---
> drivers/block/rbd.c |   17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>index a0f1fe5..0a24cdf 100644
>--- a/drivers/block/rbd.c
>+++ b/drivers/block/rbd.c
>@@ -3118,15 +3118,6 @@ static int rbd_read_header(struct rbd_device
>*rbd_dev,
> 	return ret;
> }
>
>-static void rbd_update_mapping_size(struct rbd_device *rbd_dev)
>-{
>-	if (rbd_dev->spec->snap_id != CEPH_NOSNAP)
>-		return;
>-
>-	if (rbd_dev->mapping.size != rbd_dev->header.image_size)
>-		rbd_dev->mapping.size = rbd_dev->header.image_size;
>-}
>-
> /*
>* only read the first part of the ondisk header, without the snaps info
>  */
>@@ -3143,7 +3134,9 @@ static int rbd_dev_v1_refresh(struct rbd_device
>*rbd_dev)
>
> 	/* Update image size, and check for resize of mapped image */
> 	rbd_dev->header.image_size = h.image_size;
>-	rbd_update_mapping_size(rbd_dev);
>+	if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
>+		if (rbd_dev->mapping.size != rbd_dev->header.image_size)
>+			rbd_dev->mapping.size = rbd_dev->header.image_size;

Using && instead of two conditions would be a bit cleaner. Maybe you
have later patches that depend on this though. Open coding looks fine to
me in any case.

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

>
> 	/* rbd_dev->header.object_prefix shouldn't change */
> 	kfree(rbd_dev->header.snap_sizes);
>@@ -4074,7 +4067,9 @@ static int rbd_dev_v2_refresh(struct rbd_device
>*rbd_dev)
> 	ret = rbd_dev_v2_image_size(rbd_dev);
> 	if (ret)
> 		goto out;
>-	rbd_update_mapping_size(rbd_dev);
>+	if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
>+		if (rbd_dev->mapping.size != rbd_dev->header.image_size)
>+			rbd_dev->mapping.size = rbd_dev->header.image_size;
>
> 	ret = rbd_dev_v2_snap_context(rbd_dev);
> 	dout("rbd_dev_v2_snap_context returned %d\n", ret);