diff mbox

[1/4] rbd: rearrange rbd_header_from_disk()

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

Commit Message

Alex Elder Aug. 6, 2012, 6:17 p.m. UTC 
This just moves code around for the most part.  It was pulled out as
a separate patch to avoid cluttering up some upcoming patches which
are more substantive.  The point is basically to group everything
related to initializing the snapshot context together.

The only functional change is that rbd_header_from_disk() now
ensures the (in-core) header it is passed is zero-filled.  This
allows a simpler error handling path in rbd_header_from_disk().

Signed-off-by: Alex Elder <elder@inktank.com>
---
  drivers/block/rbd.c |   41 ++++++++++++++++++++++-------------------
  1 file changed, 22 insertions(+), 19 deletions(-)

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

Comments

Josh Durgin Aug. 8, 2012, 12:29 a.m. UTC | #1 
On 08/06/2012 11:17 AM, Alex Elder wrote:
> This just moves code around for the most part.  It was pulled out as
> a separate patch to avoid cluttering up some upcoming patches which
> are more substantive.  The point is basically to group everything
> related to initializing the snapshot context together.
>
> The only functional change is that rbd_header_from_disk() now
> ensures the (in-core) header it is passed is zero-filled.  This
> allows a simpler error handling path in rbd_header_from_disk().
>
> Signed-off-by: Alex Elder <elder@inktank.com>

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

> ---
>   drivers/block/rbd.c |   41 ++++++++++++++++++++++-------------------
>   1 file changed, 22 insertions(+), 19 deletions(-)
>
> Index: b/drivers/block/rbd.c
> ===================================================================
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -507,11 +507,14 @@ static int rbd_header_from_disk(struct r
>       if (snap_count > size / sizeof (header->snapc->snaps[0]))
>           return -EINVAL;
>
> -    size = sizeof (struct ceph_snap_context);
> -    size += snap_count * sizeof (header->snapc->snaps[0]);
> -    header->snapc = kmalloc(size, GFP_KERNEL);
> -    if (!header->snapc)
> +    memset(header, 0, sizeof (*header));
> +
> +    size = sizeof (ondisk->block_name) + 1;
> +    header->object_prefix = kmalloc(size, GFP_KERNEL);
> +    if (!header->object_prefix)
>           return -ENOMEM;
> +    memcpy(header->object_prefix, ondisk->block_name, size - 1);
> +    header->object_prefix[size - 1] = '\0';
>
>       if (snap_count) {
>           header->snap_names_len = le64_to_cpu(ondisk->snap_names_len);
> @@ -519,11 +522,12 @@ static int rbd_header_from_disk(struct r
>           header->snap_names = kmalloc(header->snap_names_len,
>                            GFP_KERNEL);
>           if (!header->snap_names)
> -            goto err_snapc;
> +            goto out_err;
> +
>           size = snap_count * sizeof (*header->snap_sizes);
>           header->snap_sizes = kmalloc(size, GFP_KERNEL);
>           if (!header->snap_sizes)
> -            goto err_names;
> +            goto out_err;
>       } else {
>           WARN_ON(ondisk->snap_names_len);
>           header->snap_names_len = 0;
> @@ -531,22 +535,23 @@ static int rbd_header_from_disk(struct r
>           header->snap_sizes = NULL;
>       }
>
> -    size = sizeof (ondisk->block_name) + 1;
> -    header->object_prefix = kmalloc(size, GFP_KERNEL);
> -    if (!header->object_prefix)
> -        goto err_sizes;
> -    memcpy(header->object_prefix, ondisk->block_name, size - 1);
> -    header->object_prefix[size - 1] = '\0';
> -
>       header->image_size = le64_to_cpu(ondisk->image_size);
>       header->obj_order = ondisk->options.order;
>       header->crypt_type = ondisk->options.crypt_type;
>       header->comp_type = ondisk->options.comp_type;
> +    header->total_snaps = snap_count;
> +
> +    /* Set up the snapshot context */
> +
> +    size = sizeof (struct ceph_snap_context);
> +    size += snap_count * sizeof (header->snapc->snaps[0]);
> +    header->snapc = kzalloc(size, GFP_KERNEL);
> +    if (!header->snapc)
> +        goto out_err;
>
>       atomic_set(&header->snapc->nref, 1);
>       header->snapc->seq = le64_to_cpu(ondisk->snap_seq);
>       header->snapc->num_snaps = snap_count;
> -    header->total_snaps = snap_count;
>
>       if (snap_count && allocated_snaps == snap_count) {
>           int i;
> @@ -565,16 +570,14 @@ static int rbd_header_from_disk(struct r
>
>       return 0;
>
> -err_sizes:
> +out_err:
>       kfree(header->snap_sizes);
>       header->snap_sizes = NULL;
> -err_names:
>       kfree(header->snap_names);
>       header->snap_names = NULL;
>       header->snap_names_len = 0;
> -err_snapc:
> -    kfree(header->snapc);
> -    header->snapc = NULL;
> +    kfree(header->object_prefix);
> +    header->object_prefix = NULL;
>
>       return -ENOMEM;
>   }

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

Index: b/drivers/block/rbd.c
===================================================================
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -507,11 +507,14 @@  static int rbd_header_from_disk(struct r
  	if (snap_count > size / sizeof (header->snapc->snaps[0]))
  		return -EINVAL;

-	size = sizeof (struct ceph_snap_context);
-	size += snap_count * sizeof (header->snapc->snaps[0]);
-	header->snapc = kmalloc(size, GFP_KERNEL);
-	if (!header->snapc)
+	memset(header, 0, sizeof (*header));
+
+	size = sizeof (ondisk->block_name) + 1;
+	header->object_prefix = kmalloc(size, GFP_KERNEL);
+	if (!header->object_prefix)
  		return -ENOMEM;
+	memcpy(header->object_prefix, ondisk->block_name, size - 1);
+	header->object_prefix[size - 1] = '\0';

  	if (snap_count) {
  		header->snap_names_len = le64_to_cpu(ondisk->snap_names_len);
@@ -519,11 +522,12 @@  static int rbd_header_from_disk(struct r
  		header->snap_names = kmalloc(header->snap_names_len,
  					     GFP_KERNEL);
  		if (!header->snap_names)
-			goto err_snapc;
+			goto out_err;
+
  		size = snap_count * sizeof (*header->snap_sizes);
  		header->snap_sizes = kmalloc(size, GFP_KERNEL);
  		if (!header->snap_sizes)
-			goto err_names;
+			goto out_err;
  	} else {
  		WARN_ON(ondisk->snap_names_len);
  		header->snap_names_len = 0;
@@ -531,22 +535,23 @@  static int rbd_header_from_disk(struct r
  		header->snap_sizes = NULL;
  	}

-	size = sizeof (ondisk->block_name) + 1;
-	header->object_prefix = kmalloc(size, GFP_KERNEL);
-	if (!header->object_prefix)
-		goto err_sizes;
-	memcpy(header->object_prefix, ondisk->block_name, size - 1);
-	header->object_prefix[size - 1] = '\0';
-
  	header->image_size = le64_to_cpu(ondisk->image_size);
  	header->obj_order = ondisk->options.order;
  	header->crypt_type = ondisk->options.crypt_type;
  	header->comp_type = ondisk->options.comp_type;
+	header->total_snaps = snap_count;
+
+	/* Set up the snapshot context */
+
+	size = sizeof (struct ceph_snap_context);
+	size += snap_count * sizeof (header->snapc->snaps[0]);
+	header->snapc = kzalloc(size, GFP_KERNEL);
+	if (!header->snapc)
+		goto out_err;

  	atomic_set(&header->snapc->nref, 1);
  	header->snapc->seq = le64_to_cpu(ondisk->snap_seq);
  	header->snapc->num_snaps = snap_count;
-	header->total_snaps = snap_count;

  	if (snap_count && allocated_snaps == snap_count) {
  		int i;
@@ -565,16 +570,14 @@  static int rbd_header_from_disk(struct r

  	return 0;

-err_sizes:
+out_err:
  	kfree(header->snap_sizes);
  	header->snap_sizes = NULL;
-err_names:
  	kfree(header->snap_names);
  	header->snap_names = NULL;
  	header->snap_names_len = 0;
-err_snapc:
-	kfree(header->snapc);
-	header->snapc = NULL;
+	kfree(header->object_prefix);
+	header->object_prefix = NULL;

  	return -ENOMEM;
  }