Re: [PATCH 1/4] rbd: define rbd_update_size()

[Alex Elder <elder@inktank.com>]

On 10/09/2012 04:27 PM, Josh Durgin wrote:
> On 10/09/2012 02:00 PM, Alex Elder wrote:
>> Encapsulate the code that handles the case where an image's size has
>> been found to have changed.  This is done in anticipation of the
>> next patch, which will make this common code for format 1 and 2
>> images.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>>   drivers/block/rbd.c |   22 +++++++++++++---------
>>   1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index bb3d9be..d36e6d7 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -1716,6 +1716,16 @@ static void __rbd_remove_all_snaps(struct
>> rbd_device *rbd_dev)
>>           __rbd_remove_snap_dev(snap);
>>   }
>>
>> +static void rbd_update_size(struct rbd_device *rbd_dev)
>> +{
>> +    sector_t size = (sector_t) rbd_dev->header.image_size / SECTOR_SIZE;
>> +
>> +    rbd_assert(rbd_dev->mapping.snap_id == CEPH_NOSNAP);
>
> I might be confused since it's been a while since I looked at these
> refactorings, but I think this should still be a conditional block
> instead of an assert.
>
> Even if we've only mapped a snapshot, the non-snapshot version could
> still be resized.

This function now only gets called if we find that the size
has changed as a result of a refresh.  The assertion is to
verify that won't happen for a snapshot.

Since I'm returning a value here I may be able to be a little
less hardcore and return -EIO instead.

					-Alex

>> +    dout("setting size to %llu sectors", (unsigned long long) size);
>> +    rbd_dev->mapping.size = (u64) size;
>> +    set_capacity(rbd_dev->disk, size);
>> +}
>> +
>>   /*
>>    * only read the first part of the ondisk header, without the snaps
>> info
>>    */
>> @@ -1731,15 +1741,9 @@ static int __rbd_refresh_header(struct rbd_device
>> *rbd_dev, u64 *hver)
>>       down_write(&rbd_dev->header_rwsem);
>>
>>       /* resized? */
>> -    if (rbd_dev->mapping.snap_id == CEPH_NOSNAP) {
>> -        sector_t size = (sector_t) h.image_size / SECTOR_SIZE;
>> -
>> -        if (size != (sector_t) rbd_dev->mapping.size) {
>> -            dout("setting size to %llu sectors",
>> -                (unsigned long long) size);
>> -            rbd_dev->mapping.size = (u64) size;
>> -            set_capacity(rbd_dev->disk, size);
>> -        }
>> +    if (rbd_dev->header.image_size != h.image_size) {
>> +        rbd_dev->header.image_size = h.image_size;
>> +        rbd_update_size(rbd_dev);
>>       }
>>
>>       /* rbd_dev->header.object_prefix shouldn't change */
>