* [PATCH 1/4] rbd: define rbd_update_size()
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
@ 2012-10-09 21:00 ` Alex Elder
2012-10-09 23:27 ` Josh Durgin
2012-10-09 21:00 ` [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh() Alex Elder
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2012-10-09 21:00 UTC (permalink / raw)
To: ceph-devel
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);
+ 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 */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] rbd: define rbd_update_size()
2012-10-09 21:00 ` [PATCH 1/4] rbd: define rbd_update_size() Alex Elder
@ 2012-10-09 23:27 ` Josh Durgin
2012-10-09 23:46 ` Alex Elder
2012-10-10 1:14 ` [PATCH 1/4, v2] rbd: define rbd_update_mapping_size() Alex Elder
0 siblings, 2 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-09 23:27 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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 */
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] rbd: define rbd_update_size()
2012-10-09 23:27 ` Josh Durgin
@ 2012-10-09 23:46 ` Alex Elder
2012-10-10 1:14 ` [PATCH 1/4, v2] rbd: define rbd_update_mapping_size() Alex Elder
1 sibling, 0 replies; 16+ messages in thread
From: Alex Elder @ 2012-10-09 23:46 UTC (permalink / raw)
To: Josh Durgin; +Cc: ceph-devel
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 */
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/4, v2] rbd: define rbd_update_mapping_size()
2012-10-09 23:27 ` Josh Durgin
2012-10-09 23:46 ` Alex Elder
@ 2012-10-10 1:14 ` Alex Elder
2012-10-10 1:17 ` Josh Durgin
1 sibling, 1 reply; 16+ messages in thread
From: Alex Elder @ 2012-10-10 1:14 UTC (permalink / raw)
To: Josh Durgin, ceph-devel
Encapsulate the code that handles updating the size of a mapping
after an rbd image has been refreshed. 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>
---
Changed since v1:
- renamed renamed the function to include "mapping"
- now call the function unconditionally--whether or not
the size has changed, and only update the mapping if
the mapping is to the base image.
drivers/block/rbd.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index bb3d9be..b64125d 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1716,6 +1716,19 @@ static void __rbd_remove_all_snaps(struct
rbd_device *rbd_dev)
__rbd_remove_snap_dev(snap);
}
+static void rbd_update_mapping_size(struct rbd_device *rbd_dev)
+{
+ sector_t size;
+
+ if (rbd_dev->mapping.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);
+}
+
/*
* only read the first part of the ondisk header, without the snaps info
*/
@@ -1730,17 +1743,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);
- }
- }
+ /* Update image size, and check for resize of mapped image */
+ rbd_dev->header.image_size = h.image_size;
+ rbd_update_mapping_size(rbd_dev);
/* rbd_dev->header.object_prefix shouldn't change */
kfree(rbd_dev->header.snap_sizes);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/4, v2] rbd: define rbd_update_mapping_size()
2012-10-10 1:14 ` [PATCH 1/4, v2] rbd: define rbd_update_mapping_size() Alex Elder
@ 2012-10-10 1:17 ` Josh Durgin
0 siblings, 0 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-10 1:17 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/09/2012 06:14 PM, Alex Elder wrote:
> Encapsulate the code that handles updating the size of a mapping
> after an rbd image has been refreshed. 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>
> ---
> Changed since v1:
> - renamed renamed the function to include "mapping"
> - now call the function unconditionally--whether or not
> the size has changed, and only update the mapping if
> the mapping is to the base image.
>
> drivers/block/rbd.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index bb3d9be..b64125d 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1716,6 +1716,19 @@ static void __rbd_remove_all_snaps(struct
> rbd_device *rbd_dev)
> __rbd_remove_snap_dev(snap);
> }
>
> +static void rbd_update_mapping_size(struct rbd_device *rbd_dev)
> +{
> + sector_t size;
> +
> + if (rbd_dev->mapping.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);
> +}
> +
> /*
> * only read the first part of the ondisk header, without the snaps info
> */
> @@ -1730,17 +1743,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);
> - }
> - }
> + /* Update image size, and check for resize of mapped image */
> + rbd_dev->header.image_size = h.image_size;
> + rbd_update_mapping_size(rbd_dev);
>
> /* rbd_dev->header.object_prefix shouldn't change */
> kfree(rbd_dev->header.snap_sizes);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh()
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
2012-10-09 21:00 ` [PATCH 1/4] rbd: define rbd_update_size() Alex Elder
@ 2012-10-09 21:00 ` Alex Elder
2012-10-09 23:30 ` Josh Durgin
2012-10-09 21:01 ` [PATCH 3/4] rbd: implement feature checks Alex Elder
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2012-10-09 21:00 UTC (permalink / raw)
To: ceph-devel
Define a new function rbd_dev_v2_refresh() to update/refresh the
snapshot context for a format version 2 rbd image. This function
will update anything that is not fixed for the life of an rbd
image--at the moment this is mainly the snapshot context and (for
a base mapping) the size.
Update rbd_refresh_header() so it selects which function to use
based on the image format.
Rename __rbd_refresh_header() to be rbd_dev_v1_refresh()
to be consistent with the naming of its version 2 counterpart.
Similarly rename rbd_refresh_header() to be rbd_dev_refresh().
Unrelated--we use rbd_image_format_valid() here. Delete the other
use of it, which was primarily put in place to ensure that function
was referenced at the time it was defined.
Signed-off-by: Alex Elder <elder@inktank.com>
---
Changes in v2:
- also fetch size when refereshing image
- drop "snapc" from refresh function names
drivers/block/rbd.c | 55
+++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d36e6d7..23f3beb 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -268,7 +268,8 @@ static void rbd_put_dev(struct rbd_device *rbd_dev)
put_device(&rbd_dev->dev);
}
-static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver);
+static int rbd_dev_refresh(struct rbd_device *rbd_dev, u64 *hver);
+static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver);
static int rbd_open(struct block_device *bdev, fmode_t mode)
{
@@ -1304,7 +1305,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
dout("rbd_watch_cb %s notify_id=%llu opcode=%u\n",
rbd_dev->header_name, (unsigned long long) notify_id,
(unsigned int) opcode);
- rc = rbd_refresh_header(rbd_dev, &hver);
+ rc = rbd_dev_refresh(rbd_dev, &hver);
if (rc)
pr_warning(RBD_DRV_NAME "%d got notification but failed to "
" update snaps: %d\n", rbd_dev->major, rc);
@@ -1729,7 +1730,7 @@ static void rbd_update_size(struct rbd_device
*rbd_dev)
/*
* only read the first part of the ondisk header, without the snaps info
*/
-static int __rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
+static int rbd_dev_v1_refresh(struct rbd_device *rbd_dev, u64 *hver)
{
int ret;
struct rbd_image_header h;
@@ -1772,12 +1773,16 @@ static int __rbd_refresh_header(struct
rbd_device *rbd_dev, u64 *hver)
return ret;
}
-static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
+static int rbd_dev_refresh(struct rbd_device *rbd_dev, u64 *hver)
{
int ret;
+ rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
- ret = __rbd_refresh_header(rbd_dev, hver);
+ if (rbd_dev->image_format == 1)
+ ret = rbd_dev_v1_refresh(rbd_dev, hver);
+ else
+ ret = rbd_dev_v2_refresh(rbd_dev, hver);
mutex_unlock(&ctl_mutex);
return ret;
@@ -1937,7 +1942,7 @@ static ssize_t rbd_image_refresh(struct device *dev,
struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
int ret;
- ret = rbd_refresh_header(rbd_dev, NULL);
+ ret = rbd_dev_refresh(rbd_dev, NULL);
return ret < 0 ? ret : size;
}
@@ -2401,6 +2406,41 @@ static char *rbd_dev_snap_info(struct rbd_device
*rbd_dev, u32 which,
return ERR_PTR(-EINVAL);
}
+static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver)
+{
+ int ret;
+ __u8 obj_order;
+ u64 image_size;
+
+ down_write(&rbd_dev->header_rwsem);
+
+ /* Grab old values first, to see if they change */
+
+ obj_order = rbd_dev->header.obj_order,
+ image_size = rbd_dev->header.image_size;
+ ret = rbd_dev_v2_image_size(rbd_dev);
+ if (ret)
+ goto out;
+ rbd_assert(rbd_dev->header.obj_order == obj_order);
+ if (image_size != rbd_dev->header.image_size)
+ rbd_update_size(rbd_dev);
+
+ ret = rbd_dev_v2_snap_context(rbd_dev, hver);
+ dout("rbd_dev_v2_snap_context returned %d\n", ret);
+ if (ret)
+ goto out;
+ ret = rbd_dev_snaps_update(rbd_dev);
+ dout("rbd_dev_snaps_update returned %d\n", ret);
+ if (ret)
+ goto out;
+ ret = rbd_dev_snaps_register(rbd_dev);
+ dout("rbd_dev_snaps_register returned %d\n", ret);
+out:
+ up_write(&rbd_dev->header_rwsem);
+
+ return 0;
+}
+
/*
* Scan the rbd device's current snapshot list and compare it to the
* newly-received snapshot context. Remove any existing snapshots
@@ -2563,7 +2603,7 @@ static int rbd_init_watch_dev(struct rbd_device
*rbd_dev)
do {
ret = rbd_req_sync_watch(rbd_dev);
if (ret == -ERANGE) {
- rc = rbd_refresh_header(rbd_dev, NULL);
+ rc = rbd_dev_refresh(rbd_dev, NULL);
if (rc < 0)
return rc;
}
@@ -3044,7 +3084,6 @@ static ssize_t rbd_add(struct bus_type *bus,
rc = rbd_dev_probe(rbd_dev);
if (rc < 0)
goto err_out_client;
- rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
/* no need to lock here, as rbd_dev is not registered yet */
rc = rbd_dev_snaps_update(rbd_dev);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh()
2012-10-09 21:00 ` [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh() Alex Elder
@ 2012-10-09 23:30 ` Josh Durgin
0 siblings, 0 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-09 23:30 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/09/2012 02:00 PM, Alex Elder wrote:
> Define a new function rbd_dev_v2_refresh() to update/refresh the
> snapshot context for a format version 2 rbd image. This function
> will update anything that is not fixed for the life of an rbd
> image--at the moment this is mainly the snapshot context and (for
> a base mapping) the size.
>
> Update rbd_refresh_header() so it selects which function to use
> based on the image format.
>
> Rename __rbd_refresh_header() to be rbd_dev_v1_refresh()
> to be consistent with the naming of its version 2 counterpart.
> Similarly rename rbd_refresh_header() to be rbd_dev_refresh().
>
> Unrelated--we use rbd_image_format_valid() here. Delete the other
> use of it, which was primarily put in place to ensure that function
> was referenced at the time it was defined.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> Changes in v2:
> - also fetch size when refereshing image
> - drop "snapc" from refresh function names
> drivers/block/rbd.c | 55
> +++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d36e6d7..23f3beb 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -268,7 +268,8 @@ static void rbd_put_dev(struct rbd_device *rbd_dev)
> put_device(&rbd_dev->dev);
> }
>
> -static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver);
> +static int rbd_dev_refresh(struct rbd_device *rbd_dev, u64 *hver);
> +static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver);
>
> static int rbd_open(struct block_device *bdev, fmode_t mode)
> {
> @@ -1304,7 +1305,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
> u8 opcode, void *data)
> dout("rbd_watch_cb %s notify_id=%llu opcode=%u\n",
> rbd_dev->header_name, (unsigned long long) notify_id,
> (unsigned int) opcode);
> - rc = rbd_refresh_header(rbd_dev, &hver);
> + rc = rbd_dev_refresh(rbd_dev, &hver);
> if (rc)
> pr_warning(RBD_DRV_NAME "%d got notification but failed to "
> " update snaps: %d\n", rbd_dev->major, rc);
> @@ -1729,7 +1730,7 @@ static void rbd_update_size(struct rbd_device
> *rbd_dev)
> /*
> * only read the first part of the ondisk header, without the snaps info
> */
> -static int __rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
> +static int rbd_dev_v1_refresh(struct rbd_device *rbd_dev, u64 *hver)
> {
> int ret;
> struct rbd_image_header h;
> @@ -1772,12 +1773,16 @@ static int __rbd_refresh_header(struct
> rbd_device *rbd_dev, u64 *hver)
> return ret;
> }
>
> -static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
> +static int rbd_dev_refresh(struct rbd_device *rbd_dev, u64 *hver)
> {
> int ret;
>
> + rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
> mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> - ret = __rbd_refresh_header(rbd_dev, hver);
> + if (rbd_dev->image_format == 1)
> + ret = rbd_dev_v1_refresh(rbd_dev, hver);
> + else
> + ret = rbd_dev_v2_refresh(rbd_dev, hver);
> mutex_unlock(&ctl_mutex);
>
> return ret;
> @@ -1937,7 +1942,7 @@ static ssize_t rbd_image_refresh(struct device *dev,
> struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
> int ret;
>
> - ret = rbd_refresh_header(rbd_dev, NULL);
> + ret = rbd_dev_refresh(rbd_dev, NULL);
>
> return ret < 0 ? ret : size;
> }
> @@ -2401,6 +2406,41 @@ static char *rbd_dev_snap_info(struct rbd_device
> *rbd_dev, u32 which,
> return ERR_PTR(-EINVAL);
> }
>
> +static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver)
> +{
> + int ret;
> + __u8 obj_order;
> + u64 image_size;
> +
> + down_write(&rbd_dev->header_rwsem);
> +
> + /* Grab old values first, to see if they change */
> +
> + obj_order = rbd_dev->header.obj_order,
> + image_size = rbd_dev->header.image_size;
> + ret = rbd_dev_v2_image_size(rbd_dev);
> + if (ret)
> + goto out;
> + rbd_assert(rbd_dev->header.obj_order == obj_order);
> + if (image_size != rbd_dev->header.image_size)
> + rbd_update_size(rbd_dev);
> +
> + ret = rbd_dev_v2_snap_context(rbd_dev, hver);
> + dout("rbd_dev_v2_snap_context returned %d\n", ret);
> + if (ret)
> + goto out;
> + ret = rbd_dev_snaps_update(rbd_dev);
> + dout("rbd_dev_snaps_update returned %d\n", ret);
> + if (ret)
> + goto out;
> + ret = rbd_dev_snaps_register(rbd_dev);
> + dout("rbd_dev_snaps_register returned %d\n", ret);
> +out:
> + up_write(&rbd_dev->header_rwsem);
> +
> + return 0;
> +}
> +
> /*
> * Scan the rbd device's current snapshot list and compare it to the
> * newly-received snapshot context. Remove any existing snapshots
> @@ -2563,7 +2603,7 @@ static int rbd_init_watch_dev(struct rbd_device
> *rbd_dev)
> do {
> ret = rbd_req_sync_watch(rbd_dev);
> if (ret == -ERANGE) {
> - rc = rbd_refresh_header(rbd_dev, NULL);
> + rc = rbd_dev_refresh(rbd_dev, NULL);
> if (rc < 0)
> return rc;
> }
> @@ -3044,7 +3084,6 @@ static ssize_t rbd_add(struct bus_type *bus,
> rc = rbd_dev_probe(rbd_dev);
> if (rc < 0)
> goto err_out_client;
> - rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
>
> /* no need to lock here, as rbd_dev is not registered yet */
> rc = rbd_dev_snaps_update(rbd_dev);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] rbd: implement feature checks
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
2012-10-09 21:00 ` [PATCH 1/4] rbd: define rbd_update_size() Alex Elder
2012-10-09 21:00 ` [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh() Alex Elder
@ 2012-10-09 21:01 ` Alex Elder
2012-10-09 23:31 ` Josh Durgin
2012-10-09 21:01 ` [PATCH 4/4] rbd: activate v2 image support Alex Elder
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2012-10-09 21:01 UTC (permalink / raw)
To: ceph-devel
Version 2 images have two sets of feature bit fields. The first
indicates features possibly used by the image. The second indicates
features that the client *must* support in order to use the image.
When an image (or snapshot) is first examined, we need to make sure
that the local implementation supports the image's required
features. If not, fail the probe for the image.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 23f3beb..86ab032 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -70,6 +70,14 @@
#define RBD_IMAGE_ID_LEN_MAX 64
#define RBD_OBJ_PREFIX_LEN_MAX 64
+/* Feature bits */
+
+#define RBD_FEATURE_LAYERING 1
+
+/* Features supported by this (client software) implementation. */
+
+#define RBD_FEATURES_ALL (0)
+
/*
* An RBD device name will be "rbd#", where the "rbd" comes from
* RBD_DRV_NAME above, and # is a unique integer identifier.
@@ -2225,6 +2233,7 @@ static int _rbd_dev_v2_snap_features(struct
rbd_device *rbd_dev, u64 snap_id,
__le64 features;
__le64 incompat;
} features_buf = { 0 };
+ u64 incompat;
int ret;
ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
@@ -2235,6 +2244,11 @@ static int _rbd_dev_v2_snap_features(struct
rbd_device *rbd_dev, u64 snap_id,
dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
if (ret < 0)
return ret;
+
+ incompat = le64_to_cpu(features_buf.incompat);
+ if (incompat & ~RBD_FEATURES_ALL)
+ return -ENOTSUPP;
+
*snap_features = le64_to_cpu(features_buf.features);
dout(" snap_id 0x%016llx features = 0x%016llx incompat = 0x%016llx\n",
@@ -2976,7 +2990,7 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
if (ret < 0)
goto out_err;
- /* Get the features for the image */
+ /* Get the and check features for the image */
ret = rbd_dev_v2_features(rbd_dev);
if (ret < 0)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] rbd: implement feature checks
2012-10-09 21:01 ` [PATCH 3/4] rbd: implement feature checks Alex Elder
@ 2012-10-09 23:31 ` Josh Durgin
0 siblings, 0 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-09 23:31 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/09/2012 02:01 PM, Alex Elder wrote:
> Version 2 images have two sets of feature bit fields. The first
> indicates features possibly used by the image. The second indicates
> features that the client *must* support in order to use the image.
>
> When an image (or snapshot) is first examined, we need to make sure
> that the local implementation supports the image's required
> features. If not, fail the probe for the image.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 23f3beb..86ab032 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -70,6 +70,14 @@
> #define RBD_IMAGE_ID_LEN_MAX 64
> #define RBD_OBJ_PREFIX_LEN_MAX 64
>
> +/* Feature bits */
> +
> +#define RBD_FEATURE_LAYERING 1
> +
> +/* Features supported by this (client software) implementation. */
> +
> +#define RBD_FEATURES_ALL (0)
> +
> /*
> * An RBD device name will be "rbd#", where the "rbd" comes from
> * RBD_DRV_NAME above, and # is a unique integer identifier.
> @@ -2225,6 +2233,7 @@ static int _rbd_dev_v2_snap_features(struct
> rbd_device *rbd_dev, u64 snap_id,
> __le64 features;
> __le64 incompat;
> } features_buf = { 0 };
> + u64 incompat;
> int ret;
>
> ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> @@ -2235,6 +2244,11 @@ static int _rbd_dev_v2_snap_features(struct
> rbd_device *rbd_dev, u64 snap_id,
> dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> if (ret < 0)
> return ret;
> +
> + incompat = le64_to_cpu(features_buf.incompat);
> + if (incompat & ~RBD_FEATURES_ALL)
> + return -ENOTSUPP;
> +
> *snap_features = le64_to_cpu(features_buf.features);
>
> dout(" snap_id 0x%016llx features = 0x%016llx incompat =
> 0x%016llx\n",
> @@ -2976,7 +2990,7 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
> if (ret < 0)
> goto out_err;
>
> - /* Get the features for the image */
> + /* Get the and check features for the image */
>
> ret = rbd_dev_v2_features(rbd_dev);
> if (ret < 0)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] rbd: activate v2 image support
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
` (2 preceding siblings ...)
2012-10-09 21:01 ` [PATCH 3/4] rbd: implement feature checks Alex Elder
@ 2012-10-09 21:01 ` Alex Elder
2012-10-09 23:32 ` Josh Durgin
2012-10-10 15:55 ` [PATCH 0/4] rbd: finish up basic format 2 support Cláudio Martins
2013-08-15 11:42 ` rbd: format 2 support in rbd.ko ? Kasper Dieter
5 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2012-10-09 21:01 UTC (permalink / raw)
To: ceph-devel
Now that v2 images support is fully implemented, have
rbd_dev_v2_probe() return 0 to indicate a successful probe.
(Note that an image that implements layering will fail
the probe early because of the feature chekc.)
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 86ab032..6052fff 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3013,7 +3013,7 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
dout("discovered version 2 image, header name is %s\n",
rbd_dev->header_name);
- return -ENOTSUPP;
+ return 0;
out_err:
kfree(rbd_dev->header_name);
rbd_dev->header_name = NULL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] rbd: activate v2 image support
2012-10-09 21:01 ` [PATCH 4/4] rbd: activate v2 image support Alex Elder
@ 2012-10-09 23:32 ` Josh Durgin
0 siblings, 0 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-09 23:32 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/09/2012 02:01 PM, Alex Elder wrote:
> Now that v2 images support is fully implemented, have
> rbd_dev_v2_probe() return 0 to indicate a successful probe.
>
> (Note that an image that implements layering will fail
> the probe early because of the feature chekc.)
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 86ab032..6052fff 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3013,7 +3013,7 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
> dout("discovered version 2 image, header name is %s\n",
> rbd_dev->header_name);
>
> - return -ENOTSUPP;
> + return 0;
> out_err:
> kfree(rbd_dev->header_name);
> rbd_dev->header_name = NULL;
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] rbd: finish up basic format 2 support
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
` (3 preceding siblings ...)
2012-10-09 21:01 ` [PATCH 4/4] rbd: activate v2 image support Alex Elder
@ 2012-10-10 15:55 ` Cláudio Martins
2012-10-10 16:29 ` Josh Durgin
2013-08-15 11:42 ` rbd: format 2 support in rbd.ko ? Kasper Dieter
5 siblings, 1 reply; 16+ messages in thread
From: Cláudio Martins @ 2012-10-10 15:55 UTC (permalink / raw)
To: ceph-devel; +Cc: elder
On Tue, 09 Oct 2012 13:57:09 -0700 Alex Elder <elder@inktank.com> wrote:
> This series includes updates for two patches posted previously.
>
> -Alex
Greetings,
We're gearing up to test v0.52 (specifically the RBD stuff) on our
cluster. After reading this series of posts about rbd format 2 patches
I began wondering if we should start testing these patches as well or
not. To put it simply, what I'd like to know is:
Is it enough to use the 3.6 vanilla kernel client to take full
advantage of the rbd changes in v0.52 (i.e. new RBD cloning features)?
Do we have any benefits from applying any of these patches on top of
v3.6 and using "format 2", assuming that we stick to v0.52 on the
server, or is this strictly v0.53 and beyond stuff?
I apologize if this is a dumb question, but by looking at the v0.52
changelog, at doc/rbd/* and the list, it doesn't seem clear how this
fits with v0.52.
Thanks in advance
Best regards
Cláudio
--
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 0/4] rbd: finish up basic format 2 support
2012-10-10 15:55 ` [PATCH 0/4] rbd: finish up basic format 2 support Cláudio Martins
@ 2012-10-10 16:29 ` Josh Durgin
0 siblings, 0 replies; 16+ messages in thread
From: Josh Durgin @ 2012-10-10 16:29 UTC (permalink / raw)
To: Cláudio Martins; +Cc: ceph-devel, elder
On 10/10/2012 08:55 AM, Cláudio Martins wrote:
>
> On Tue, 09 Oct 2012 13:57:09 -0700 Alex Elder <elder@inktank.com> wrote:
>> This series includes updates for two patches posted previously.
>>
>> -Alex
>
> Greetings,
>
> We're gearing up to test v0.52 (specifically the RBD stuff) on our
> cluster. After reading this series of posts about rbd format 2 patches
> I began wondering if we should start testing these patches as well or
> not. To put it simply, what I'd like to know is:
>
> Is it enough to use the 3.6 vanilla kernel client to take full
> advantage of the rbd changes in v0.52 (i.e. new RBD cloning features)?
>
> Do we have any benefits from applying any of these patches on top of
> v3.6 and using "format 2", assuming that we stick to v0.52 on the
> server, or is this strictly v0.53 and beyond stuff?
>
>
> I apologize if this is a dumb question, but by looking at the v0.52
> changelog, at doc/rbd/* and the list, it doesn't seem clear how this
> fits with v0.52.
>
> Thanks in advance
>
> Best regards
>
> Cláudio
These patches support using format 2, to make adding new features
easy, but this is not very useful to you yet. They don't yet support
any new features (like cloning) - that's the next step, but it will
take a bunch more work.
To use rbd cloning, you'll need to access rbd through userspace (e.g.
with qemu and librbd) for now.
Josh
--
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* rbd: format 2 support in rbd.ko ?
2012-10-09 20:57 [PATCH 0/4] rbd: finish up basic format 2 support Alex Elder
` (4 preceding siblings ...)
2012-10-10 15:55 ` [PATCH 0/4] rbd: finish up basic format 2 support Cláudio Martins
@ 2013-08-15 11:42 ` Kasper Dieter
2013-08-15 12:30 ` Damien Churchill
5 siblings, 1 reply; 16+ messages in thread
From: Kasper Dieter @ 2013-08-15 11:42 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
When will 'format 2' support be available in rbd.ko ?
-Dieter
# rbd info ssd2-v2-1T-4m -p SSD-r2
rbd image 'ssd2-v2-1T-4m':
size 1000 GB in 256000 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.13032ae8944a
format: 2
features: layering
# rbd map -p SSD-r2 ssd2-v2-1T-4m
rbd: add failed: (6) No such device or address
# uname -a
Linux rx37-2.primary.ceph-poc.fsc.net 3.8.13 #11 SMP Thu Aug 1 09:27:48 CEST 2013 x86_64 x86_64 x86_64 GNU/Linux
On Tue, Oct 09, 2012 at 10:57:09PM +0200, Alex Elder wrote:
> This series includes updates for two patches posted previously.
>
> -Alex
>
> [PATCH 1/4] rbd: define rbd_update_size()
> This is a simple addition so a bit of code used by both
> format 1 and 2 can be common rather than duplicated.
> [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh()
> Updated to fetch the image size in addition to the snapshot
> context when refreshing. The names of some functions no
> longer contain "snapc" to reflect this.
> [PATCH 3/4] rbd: implement feature checks
> This ensures that images requiring features not supported
> by the local rbd client software are rejected.
> [PATCH 4/4] rbd: activate v2 image support
> Same as before, and trivial.
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread