| Message ID | 517BC21F.4030002@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/27/2013 07:18 AM, Alex Elder wrote: > When a snapshot context update occurs, rbd_update_mapping_size() is > called to set the capacity of the disk to record the updated > size of the image in case it has changed. This patch, as well as the series of 4 and the series of 6 that I posted after it, are avaialble in the "review/wip-rbd-cleanup-3" branch of the ceph-client git respository. -Alex > There's a bug though. The mapping size is in units of *bytes*. The > code that updates the mapping size field is assigning a value that > has been scaled down to *sectors*. > > Fix that. Also, check to see if the size has actually changed, and > don't bother updating things (specifically, calling set_capacity()) > if it has not. > > This resolves: > http://tracker.ceph.com/issues/4833 > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > drivers/block/rbd.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index 5918e0b..37d9349 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -3034,15 +3034,17 @@ static void rbd_remove_all_snaps(struct > rbd_device *rbd_dev) > > static void rbd_update_mapping_size(struct rbd_device *rbd_dev) > { > - sector_t size; > - > if (rbd_dev->spec->snap_id != CEPH_NOSNAP) > return; > > - size = (sector_t) rbd_dev->header.image_size / SECTOR_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->mapping.size != rbd_dev->header.image_size) { > + sector_t size; > + > + rbd_dev->mapping.size = rbd_dev->header.image_size; > + size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; > + dout("setting size to %llu sectors", (unsigned long long)size); > + set_capacity(rbd_dev->disk, size); > + } > } > > /* > -- 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
On 04/27/2013 07:39 AM, Alex Elder wrote: > On 04/27/2013 07:18 AM, Alex Elder wrote: >> When a snapshot context update occurs, rbd_update_mapping_size() is >> called to set the capacity of the disk to record the updated >> size of the image in case it has changed. I have updated a few patches in this series, based on a problem or two I saw while working with some follow-on patches. Since they have not yet been reviewed, I'm no going to bother describing what has changed, I've just updated the "review/wip-rbd-cleanup-3" branch to contain the new code. -Alex > This patch, as well as the series of 4 and the series of 6 that > I posted after it, are avaialble in the "review/wip-rbd-cleanup-3" > branch of the ceph-client git respository. > > -Alex > > >> There's a bug though. The mapping size is in units of *bytes*. The >> code that updates the mapping size field is assigning a value that >> has been scaled down to *sectors*. >> >> Fix that. Also, check to see if the size has actually changed, and >> don't bother updating things (specifically, calling set_capacity()) >> if it has not. >> >> This resolves: >> http://tracker.ceph.com/issues/4833 >> >> Signed-off-by: Alex Elder <elder@inktank.com> >> --- >> drivers/block/rbd.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c >> index 5918e0b..37d9349 100644 >> --- a/drivers/block/rbd.c >> +++ b/drivers/block/rbd.c >> @@ -3034,15 +3034,17 @@ static void rbd_remove_all_snaps(struct >> rbd_device *rbd_dev) >> >> static void rbd_update_mapping_size(struct rbd_device *rbd_dev) >> { >> - sector_t size; >> - >> if (rbd_dev->spec->snap_id != CEPH_NOSNAP) >> return; >> >> - size = (sector_t) rbd_dev->header.image_size / SECTOR_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->mapping.size != rbd_dev->header.image_size) { >> + sector_t size; >> + >> + rbd_dev->mapping.size = rbd_dev->header.image_size; >> + size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; >> + dout("setting size to %llu sectors", (unsigned long long)size); >> + set_capacity(rbd_dev->disk, size); >> + } >> } >> >> /* >> > -- 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
On 04/29/2013 10:51 AM, Alex Elder wrote: > On 04/27/2013 07:39 AM, Alex Elder wrote: >> On 04/27/2013 07:18 AM, Alex Elder wrote: >>> When a snapshot context update occurs, rbd_update_mapping_size() is >>> called to set the capacity of the disk to record the updated >>> size of the image in case it has changed. This patch looks good. Reviewed-by: Josh Durgin <josh.durgin@inktank.com> >>> There's a bug though. The mapping size is in units of *bytes*. The >>> code that updates the mapping size field is assigning a value that >>> has been scaled down to *sectors*. >>> >>> Fix that. Also, check to see if the size has actually changed, and >>> don't bother updating things (specifically, calling set_capacity()) >>> if it has not. >>> >>> This resolves: >>> http://tracker.ceph.com/issues/4833 >>> >>> Signed-off-by: Alex Elder <elder@inktank.com> >>> --- >>> drivers/block/rbd.c | 14 ++++++++------ >>> 1 file changed, 8 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c >>> index 5918e0b..37d9349 100644 >>> --- a/drivers/block/rbd.c >>> +++ b/drivers/block/rbd.c >>> @@ -3034,15 +3034,17 @@ static void rbd_remove_all_snaps(struct >>> rbd_device *rbd_dev) >>> >>> static void rbd_update_mapping_size(struct rbd_device *rbd_dev) >>> { >>> - sector_t size; >>> - >>> if (rbd_dev->spec->snap_id != CEPH_NOSNAP) >>> return; >>> >>> - size = (sector_t) rbd_dev->header.image_size / SECTOR_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->mapping.size != rbd_dev->header.image_size) { >>> + sector_t size; >>> + >>> + rbd_dev->mapping.size = rbd_dev->header.image_size; >>> + size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; >>> + dout("setting size to %llu sectors", (unsigned long long)size); >>> + set_capacity(rbd_dev->disk, size); >>> + } >>> } >>> >>> /* >>> >> > -- 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 --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 5918e0b..37d9349 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -3034,15 +3034,17 @@ static void rbd_remove_all_snaps(struct rbd_device *rbd_dev) static void rbd_update_mapping_size(struct rbd_device *rbd_dev) { - sector_t size; - if (rbd_dev->spec->snap_id != CEPH_NOSNAP) return; - size = (sector_t) rbd_dev->header.image_size / SECTOR_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->mapping.size != rbd_dev->header.image_size) { + sector_t size; + + rbd_dev->mapping.size = rbd_dev->header.image_size; + size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; + dout("setting size to %llu sectors", (unsigned long long)size); + set_capacity(rbd_dev->disk, size); + } }
When a snapshot context update occurs, rbd_update_mapping_size() is called to set the capacity of the disk to record the updated size of the image in case it has changed. There's a bug though. The mapping size is in units of *bytes*. The code that updates the mapping size field is assigning a value that has been scaled down to *sectors*. Fix that. Also, check to see if the size has actually changed, and don't bother updating things (specifically, calling set_capacity()) if it has not. This resolves: http://tracker.ceph.com/issues/4833 Signed-off-by: Alex Elder <elder@inktank.com> --- drivers/block/rbd.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) /*