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

[Alex Elder <elder@inktank.com>]

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