diff mbox

[1/4] rbd: define rbd_update_size()

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

Commit Message

Alex Elder Oct. 9, 2012, 9 p.m. UTC
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(-)

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

Comments

Josh Durgin Oct. 9, 2012, 11:27 p.m. UTC | #1
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.

> +    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 */

--
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 Oct. 9, 2012, 11:46 p.m. UTC | #2
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 */
>

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