[PATCH 0/4] rbd: finish up basic format 2 support

[PATCH 0/4] rbd: finish up basic format 2 support

[Alex Elder]

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.

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

[Alex Elder]

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

[PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh()

[Alex Elder]

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

[PATCH 3/4] rbd: implement feature checks

[Alex Elder]

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

[PATCH 4/4] rbd: activate v2 image support

[Alex Elder]

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

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

[Josh Durgin]

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

Re: [PATCH 2/4, v2] rbd: define rbd_dev_v2_refresh()

[Josh Durgin]

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

Re: [PATCH 3/4] rbd: implement feature checks

[Josh Durgin]

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)

Re: [PATCH 4/4] rbd: activate v2 image support

[Josh Durgin]

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;

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

[Alex Elder]

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

[PATCH 1/4, v2] rbd: define rbd_update_mapping_size()

[Alex Elder]

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

Re: [PATCH 1/4, v2] rbd: define rbd_update_mapping_size()

[Josh Durgin]

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

Re: [PATCH 0/4] rbd: finish up basic format 2 support

[Cláudio Martins]

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

Re: [PATCH 0/4] rbd: finish up basic format 2 support

[Josh Durgin]

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

rbd: format 2 support in rbd.ko ?

[Kasper Dieter]

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

Re: rbd: format 2 support in rbd.ko ?

[Damien Churchill]

On 15 August 2013 12:42, Kasper Dieter <dieter.kasper@ts.fujitsu.com> wrote:
> When will 'format 2' support be available in rbd.ko ?
>

Format 2 is supported in kernel 3.10 or newer.