diff mbox

[2/6] rbd: kill rbd_update_mapping_size()

Message ID 51885AE4.1030209@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Elder May 7, 2013, 1:37 a.m. UTC
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(-)

  */
@@ -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;

 	/* 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);

Comments

Josh Durgin May 7, 2013, 2:56 p.m. UTC | #1
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);

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Elder May 7, 2013, 3:41 p.m. UTC | #2
On 05/07/2013 09:56 AM, Josh Durgin wrote:
> 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.

Honestly, that's the way I had it, but I looked at both
and liked this better, I think because the lines ended up
long or something.

I'll take another look before I commit it.

					-Alex


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

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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