[PATCH 0/9] rbd: rearrange initialization sequence

[PATCH 0/9] rbd: rearrange initialization sequence

[Alex Elder]

This fifth series rearranges the order of a bunch of steps that
occur during initialization of a new rbd device structure.  It
consists almost entirely of simple movements of code, but it
significantly separates updating a device's snapshot list from
registering devices for newly-found snapshots (which will be
for the benefit of format 2 image initialization).

It is available as branch "wip-rbd-review-4" on the ceph-client git
repository, and is based on the branch "wip-rbd-review-3".

    https://github.com/ceph/ceph-client/tree/wip-rbd-review-4

					-Alex

[PATCH 1/9]  d58dd5e rbd: assign header name later
[PATCH 2/9]  c8fd9cf rbd: defer registering snapshot devices
[PATCH 3/9]  bf469fa rbd: call set_snap() before snap_devs_update()
[PATCH 4/9]  1ee626d rbd: read the header before registering device
[PATCH 5/9]  f05c12d rbd: defer setting device id
[PATCH 6/9]  8b8b8b7 rbd: call rbd_init_disk() sooner
[PATCH 7/9]  f1194c2 rbd: drop dev registration check for new snap
[PATCH 8/9]  345936d rbd: set initial capacity in rbd_init_disk()
[PATCH 9/9]  527bbdd rbd: set up watch before announcing disk

[PATCH 1/9] rbd: assign header name later

[Alex Elder]

Move the assignment of the header name for an rbd image a bit later,
outside rbd_add_parse_args() and into its caller.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 4dff92f..14034e3 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2475,15 +2475,6 @@ static char *rbd_add_parse_args(struct rbd_device
*rbd_dev,
 	if (!rbd_dev->image_name)
 		goto out_err;

-	/* Create the name of the header object */
-
-	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
-						+ sizeof (RBD_SUFFIX),
-					GFP_KERNEL);
-	if (!rbd_dev->header_name)
-		goto out_err;
-	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
-
 	/* Snapshot name is optional */
 	len = next_token(&buf);
 	if (!len) {
@@ -2501,8 +2492,6 @@ dout("    SNAP_NAME is <%s>, len is %zd\n",
snap_name, len);
 	return snap_name;

 out_err:
-	kfree(rbd_dev->header_name);
-	rbd_dev->header_name = NULL;
 	kfree(rbd_dev->image_name);
 	rbd_dev->image_name = NULL;
 	rbd_dev->image_name_len = 0;
@@ -2569,6 +2558,15 @@ static ssize_t rbd_add(struct bus_type *bus,
 		goto err_out_client;
 	rbd_dev->pool_id = rc;

+	/* Create the name of the header object */
+
+	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
+						+ sizeof (RBD_SUFFIX),
+					GFP_KERNEL);
+	if (!rbd_dev->header_name)
+		goto err_out_client;
+	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
+
 	/* register our block device */
 	rc = register_blkdev(0, rbd_dev->name);
 	if (rc < 0)
@@ -2630,11 +2628,11 @@ err_out_bus:
 err_out_blkdev:
 	unregister_blkdev(rbd_dev->major, rbd_dev->name);
 err_out_client:
+	kfree(rbd_dev->header_name);
 	rbd_put_client(rbd_dev);
 err_put_id:
 	if (rbd_dev->pool_name) {
 		kfree(rbd_dev->mapping.snap_name);
-		kfree(rbd_dev->header_name);
 		kfree(rbd_dev->image_name);
 		kfree(rbd_dev->pool_name);
 	}
-- 
1.7.9.5

[PATCH 2/9] rbd: defer registering snapshot devices

[Alex Elder]

When a new snapshot is found in an rbd device's updated snapshot
context, __rbd_add_snap_dev() is called to create and insert an
entry in the rbd devices list of snapshots.  In addition, a Linux
device is registered to represent the snapshot.

For version 2 rbd images, it will be undesirable to initialize the
device right away.  So in anticipation of that, this patch separates
the insertion of a snapshot entry in the snaps list from the
creation of devices for those snapshots.

To do this, create a new function rbd_dev_snaps_register() which
traverses the list of snapshots and calls rbd_register_snap_dev()
on any that have not yet been registered.

Rename rbd_dev_snap_devs_update() to be rbd_dev_snaps_update()
to better reflect that only the entry in the snaps list and not
the snapshot's device is affected by the function.

For now, call rbd_dev_snaps_register() immediately after each
call to rbd_dev_snaps_update().

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 14034e3..d03fb0f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -204,7 +204,9 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
 static LIST_HEAD(rbd_client_list);		/* clients */
 static DEFINE_SPINLOCK(rbd_client_list_lock);

-static int rbd_dev_snap_devs_update(struct rbd_device *rbd_dev);
+static int rbd_dev_snaps_update(struct rbd_device *rbd_dev);
+static int rbd_dev_snaps_register(struct rbd_device *rbd_dev);
+
 static void rbd_dev_release(struct device *dev);
 static ssize_t rbd_snap_add(struct device *dev,
 			    struct device_attribute *attr,
@@ -648,6 +650,7 @@ static int rbd_header_set_snap(struct rbd_device
*rbd_dev, char *snap_name)
 		rbd_dev->mapping.size = rbd_dev->header.image_size;
 		rbd_dev->mapping.snap_exists = false;
 		rbd_dev->mapping.read_only = rbd_dev->rbd_opts.read_only;
+		ret = 0;
 	} else {
 		ret = snap_by_name(rbd_dev, snap_name);
 		if (ret < 0)
@@ -656,8 +659,6 @@ static int rbd_header_set_snap(struct rbd_device
*rbd_dev, char *snap_name)
 		rbd_dev->mapping.read_only = true;
 	}
 	rbd_dev->mapping.snap_name = snap_name;
-
-	ret = 0;
 done:
 	return ret;
 }
@@ -1840,7 +1841,9 @@ static int __rbd_refresh_header(struct rbd_device
*rbd_dev, u64 *hver)
 	WARN_ON(strcmp(rbd_dev->header.object_prefix, h.object_prefix));
 	kfree(h.object_prefix);

-	ret = rbd_dev_snap_devs_update(rbd_dev);
+	ret = rbd_dev_snaps_update(rbd_dev);
+	if (!ret)
+		ret = rbd_dev_snaps_register(rbd_dev);

 	up_write(&rbd_dev->header_rwsem);

@@ -2085,10 +2088,16 @@ static struct device_type rbd_snap_device_type = {
 	.release	= rbd_snap_dev_release,
 };

+static bool rbd_snap_registered(struct rbd_snap *snap)
+{
+	return snap->dev.type == &rbd_snap_device_type;
+}
+
 static void __rbd_remove_snap_dev(struct rbd_snap *snap)
 {
 	list_del(&snap->node);
-	device_unregister(&snap->dev);
+	if (rbd_snap_registered(snap))
+		device_unregister(&snap->dev);
 }

 static int rbd_register_snap_dev(struct rbd_snap *snap,
@@ -2101,6 +2110,8 @@ static int rbd_register_snap_dev(struct rbd_snap
*snap,
 	dev->parent = parent;
 	dev->release = rbd_snap_dev_release;
 	dev_set_name(dev, "snap_%s", snap->name);
+	dout("%s: registering device for snapshot %s\n", __func__, snap->name);
+
 	ret = device_register(dev);

 	return ret;
@@ -2123,11 +2134,6 @@ static struct rbd_snap *__rbd_add_snap_dev(struct
rbd_device *rbd_dev,

 	snap->size = rbd_dev->header.snap_sizes[i];
 	snap->id = rbd_dev->header.snapc->snaps[i];
-	if (device_is_registered(&rbd_dev->dev)) {
-		ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
-		if (ret < 0)
-			goto err;
-	}

 	return snap;

@@ -2150,7 +2156,7 @@ err:
  * snapshot id, highest id first.  (Snapshots in the rbd_dev's list
  * are also maintained in that order.)
  */
-static int rbd_dev_snap_devs_update(struct rbd_device *rbd_dev)
+static int rbd_dev_snaps_update(struct rbd_device *rbd_dev)
 {
 	struct ceph_snap_context *snapc = rbd_dev->header.snapc;
 	const u32 snap_count = snapc->num_snaps;
@@ -2237,6 +2243,31 @@ static int rbd_dev_snap_devs_update(struct
rbd_device *rbd_dev)
 	return 0;
 }

+/*
+ * Scan the list of snapshots and register the devices for any that
+ * have not already been registered.
+ */
+static int rbd_dev_snaps_register(struct rbd_device *rbd_dev)
+{
+	struct rbd_snap *snap;
+	int ret = 0;
+
+	dout("%s called\n", __func__);
+	if (!device_is_registered(&rbd_dev->dev))
+		return 0;
+
+	list_for_each_entry(snap, &rbd_dev->snaps, node) {
+		if (!rbd_snap_registered(snap)) {
+			ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
+			if (ret < 0)
+				break;
+		}
+	}
+	dout("%s: returning %d\n", __func__, ret);
+
+	return ret;
+}
+
 static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
 {
 	struct device *dev;
@@ -2587,7 +2618,10 @@ static ssize_t rbd_add(struct bus_type *bus,
 	if (rc)
 		goto err_out_unlock;

-	rc = rbd_dev_snap_devs_update(rbd_dev);
+	rc = rbd_dev_snaps_update(rbd_dev);
+	if (rc)
+		goto err_out_unlock;
+	rc = rbd_dev_snaps_register(rbd_dev);
 	if (rc)
 		goto err_out_unlock;

-- 
1.7.9.5

[PATCH 3/9] rbd: call set_snap() before snap_devs_update()

[Alex Elder]

rbd_header_set_snap() is a simple initialization routine for an rbd
device's mapping.  It has to be called after the snapshot context
for the rbd_dev has been updated, but can be done before snapshot
devices have been registered.

Change the name to rbd_dev_set_mapping() to better reflect its
purpose, and call it a little sooner, before registering snapshot
devices.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d03fb0f..d60db25 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -640,7 +640,7 @@ static int snap_by_name(struct rbd_device *rbd_dev,
const char *snap_name)
 	return -ENOENT;
 }

-static int rbd_header_set_snap(struct rbd_device *rbd_dev, char *snap_name)
+static int rbd_dev_set_mapping(struct rbd_device *rbd_dev, char *snap_name)
 {
 	int ret;

@@ -2621,11 +2621,12 @@ static ssize_t rbd_add(struct bus_type *bus,
 	rc = rbd_dev_snaps_update(rbd_dev);
 	if (rc)
 		goto err_out_unlock;
-	rc = rbd_dev_snaps_register(rbd_dev);
+
+	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
 	if (rc)
 		goto err_out_unlock;

-	rc = rbd_header_set_snap(rbd_dev, snap_name);
+	rc = rbd_dev_snaps_register(rbd_dev);
 	if (rc)
 		goto err_out_unlock;

-- 
1.7.9.5

[PATCH 4/9] rbd: read the header before registering device

[Alex Elder]

Read the rbd header information and call rbd_dev_set_mapping()
earlier--before registering the block device or setting up the sysfs
entries for the image.  The sysfs entries provide users access to
some information that's only available after doing the rbd header
initialization, so this will make sure it's valid right away.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d60db25..89d8ed7 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2598,10 +2598,24 @@ static ssize_t rbd_add(struct bus_type *bus,
 		goto err_out_client;
 	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);

+	/* Get information about the image being mapped */
+
+	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
+	if (rc)
+		goto err_out_client;
+
+	rc = rbd_dev_snaps_update(rbd_dev);
+	if (rc)
+		goto err_out_header;
+
+	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
+	if (rc)
+		goto err_out_header;
+
 	/* register our block device */
 	rc = register_blkdev(0, rbd_dev->name);
 	if (rc < 0)
-		goto err_out_client;
+		goto err_out_header;
 	rbd_dev->major = rc;

 	rc = rbd_bus_add_dev(rbd_dev);
@@ -2613,19 +2627,6 @@ static ssize_t rbd_add(struct bus_type *bus,
 	 * of the sysfs code (initiated by rbd_bus_del_dev()).
 	 */

-	/* contact OSD, request size info about the object being mapped */
-	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
-	if (rc)
-		goto err_out_unlock;
-
-	rc = rbd_dev_snaps_update(rbd_dev);
-	if (rc)
-		goto err_out_unlock;
-
-	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
-	if (rc)
-		goto err_out_unlock;
-
 	rc = rbd_dev_snaps_register(rbd_dev);
 	if (rc)
 		goto err_out_unlock;
@@ -2662,6 +2663,8 @@ err_out_bus:

 err_out_blkdev:
 	unregister_blkdev(rbd_dev->major, rbd_dev->name);
+err_out_header:
+	rbd_header_free(&rbd_dev->header);
 err_out_client:
 	kfree(rbd_dev->header_name);
 	rbd_put_client(rbd_dev);
-- 
1.7.9.5

[PATCH 5/9] rbd: defer setting device id

[Alex Elder]

Hold off setting the device id and formatting the device name
in rbd_add() until just before it's needed.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 89d8ed7..53e5308 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2549,10 +2549,10 @@ static ssize_t rbd_add(struct bus_type *bus,

 	options = kmalloc(count, GFP_KERNEL);
 	if (!options)
-		goto err_nomem;
+		goto err_out_mem;
 	rbd_dev = kzalloc(sizeof(*rbd_dev), GFP_KERNEL);
 	if (!rbd_dev)
-		goto err_nomem;
+		goto err_out_mem;

 	/* static rbd_device initialization */
 	spin_lock_init(&rbd_dev->lock);
@@ -2562,25 +2562,17 @@ static ssize_t rbd_add(struct bus_type *bus,

 	down_write(&rbd_dev->header_rwsem);

-	/* generate unique id: find highest unique id, add one */
-	rbd_dev_id_get(rbd_dev);
-
-	/* Fill in the device name, now that we have its id. */
-	BUILD_BUG_ON(DEV_NAME_LEN
-			< sizeof (RBD_DRV_NAME) + MAX_INT_FORMAT_WIDTH);
-	sprintf(rbd_dev->name, "%s%d", RBD_DRV_NAME, rbd_dev->dev_id);
-
 	/* parse add command */
 	snap_name = rbd_add_parse_args(rbd_dev, buf,
 				&mon_addrs, &mon_addrs_size, options, count);
 	if (IS_ERR(snap_name)) {
 		rc = PTR_ERR(snap_name);
-		goto err_put_id;
+		goto err_out_mem;
 	}

 	rc = rbd_get_client(rbd_dev, mon_addrs, mon_addrs_size - 1, options);
 	if (rc < 0)
-		goto err_put_id;
+		goto err_out_args;

 	/* pick the pool */
 	osdc = &rbd_dev->rbd_client->client->osdc;
@@ -2612,10 +2604,19 @@ static ssize_t rbd_add(struct bus_type *bus,
 	if (rc)
 		goto err_out_header;

-	/* register our block device */
+	/* generate unique id: find highest unique id, add one */
+	rbd_dev_id_get(rbd_dev);
+
+	/* Fill in the device name, now that we have its id. */
+	BUILD_BUG_ON(DEV_NAME_LEN
+			< sizeof (RBD_DRV_NAME) + MAX_INT_FORMAT_WIDTH);
+	sprintf(rbd_dev->name, "%s%d", RBD_DRV_NAME, rbd_dev->dev_id);
+
+	/* Get our block major device number. */
+
 	rc = register_blkdev(0, rbd_dev->name);
 	if (rc < 0)
-		goto err_out_header;
+		goto err_out_id;
 	rbd_dev->major = rc;

 	rc = rbd_bus_add_dev(rbd_dev);
@@ -2663,20 +2664,19 @@ err_out_bus:

 err_out_blkdev:
 	unregister_blkdev(rbd_dev->major, rbd_dev->name);
+err_out_id:
+	rbd_dev_id_put(rbd_dev);
 err_out_header:
 	rbd_header_free(&rbd_dev->header);
 err_out_client:
 	kfree(rbd_dev->header_name);
 	rbd_put_client(rbd_dev);
-err_put_id:
-	if (rbd_dev->pool_name) {
-		kfree(rbd_dev->mapping.snap_name);
-		kfree(rbd_dev->image_name);
-		kfree(rbd_dev->pool_name);
-	}
-	rbd_dev_id_put(rbd_dev);
+err_out_args:
+	kfree(rbd_dev->mapping.snap_name);
+	kfree(rbd_dev->image_name);
+	kfree(rbd_dev->pool_name);
 	up_write(&rbd_dev->header_rwsem);
-err_nomem:
+err_out_mem:
 	kfree(rbd_dev);
 	kfree(options);

-- 
1.7.9.5

[PATCH 6/9] rbd: call rbd_init_disk() sooner

[Alex Elder]

Call rbd_init_disk() from rbd_add() as soon as we have the major
device number for the mapping.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 53e5308..8e82610 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2619,10 +2619,16 @@ static ssize_t rbd_add(struct bus_type *bus,
 		goto err_out_id;
 	rbd_dev->major = rc;

-	rc = rbd_bus_add_dev(rbd_dev);
+	/* Set up the blkdev mapping. */
+
+	rc = rbd_init_disk(rbd_dev);
 	if (rc)
 		goto err_out_blkdev;

+	rc = rbd_bus_add_dev(rbd_dev);
+	if (rc)
+		goto err_out_disk;
+
 	/*
 	 * At this point cleanup in the event of an error is the job
 	 * of the sysfs code (initiated by rbd_bus_del_dev()).
@@ -2634,12 +2640,6 @@ static ssize_t rbd_add(struct bus_type *bus,

 	up_write(&rbd_dev->header_rwsem);

-	/* Set up the blkdev mapping. */
-
-	rc = rbd_init_disk(rbd_dev);
-	if (rc)
-		goto err_out_bus;
-
 	/* Everything's ready.  Announce the disk to the world. */

 	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
@@ -2662,6 +2662,8 @@ err_out_bus:
 	kfree(options);
 	return rc;

+err_out_disk:
+	rbd_free_disk(rbd_dev);
 err_out_blkdev:
 	unregister_blkdev(rbd_dev->major, rbd_dev->name);
 err_out_id:
-- 
1.7.9.5

[PATCH 7/9] rbd: drop dev registration check for new snap

[Alex Elder]

By the time rbd_dev_snaps_register() gets called during rbd device
initialization, the main device will have already been registered.
Similarly, a header refresh will only occur for an rbd device whose
Linux device is registered.  There is therefore no need to verify
the main device is registered when registering a snapshot device.

For the time being, turn the check into a WARN_ON(), but it can
eventually just go away.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8e82610..e6af38f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2253,8 +2253,8 @@ static int rbd_dev_snaps_register(struct
rbd_device *rbd_dev)
 	int ret = 0;

 	dout("%s called\n", __func__);
-	if (!device_is_registered(&rbd_dev->dev))
-		return 0;
+	if (WARN_ON(!device_is_registered(&rbd_dev->dev)))
+		return -EIO;

 	list_for_each_entry(snap, &rbd_dev->snaps, node) {
 		if (!rbd_snap_registered(snap)) {
-- 
1.7.9.5

[PATCH 8/9] rbd: set initial capacity in rbd_init_disk()

[Alex Elder]

Move the setting of the initial capacity for an rbd image mapping
into rb_init_disk().

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index e6af38f..51e1d21 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1901,6 +1901,8 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)

 	rbd_dev->disk = disk;

+	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
+
 	return 0;
 out_disk:
 	put_disk(disk);
@@ -2642,7 +2644,6 @@ static ssize_t rbd_add(struct bus_type *bus,

 	/* Everything's ready.  Announce the disk to the world. */

-	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
 	add_disk(rbd_dev->disk);
 	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
 		(unsigned long long) rbd_dev->mapping.size);
-- 
1.7.9.5

[PATCH 9/9] rbd: set up watch before announcing disk

[Alex Elder]

We're ready to handle header object (refresh) events at the point we
call rbd_bus_add_dev().  Set up the watch request on the rbd image
header just after that, and after we've registered the devices for
the snapshots for the initial snapshot context.  Do this before
announce the disk as available for use.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 51e1d21..daa428e3 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2640,23 +2640,23 @@ static ssize_t rbd_add(struct bus_type *bus,
 	if (rc)
 		goto err_out_unlock;

+	rc = rbd_init_watch_dev(rbd_dev);
+	if (rc)
+		goto err_out_unlock;
+
 	up_write(&rbd_dev->header_rwsem);

 	/* Everything's ready.  Announce the disk to the world. */

 	add_disk(rbd_dev->disk);
+
 	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
 		(unsigned long long) rbd_dev->mapping.size);

-	rc = rbd_init_watch_dev(rbd_dev);
-	if (rc)
-		goto err_out_bus;
-
 	return count;

 err_out_unlock:
 	up_write(&rbd_dev->header_rwsem);
-err_out_bus:
 	/* this will also clean up rest of rbd_dev stuff */

 	rbd_bus_del_dev(rbd_dev);
-- 
1.7.9.5

Re: [PATCH 0/9] rbd: rearrange initialization sequence

[Alex Elder]

On 09/07/2012 10:39 AM, Alex Elder wrote:
> This fifth series rearranges the order of a bunch of steps that
> occur during initialization of a new rbd device structure.  It
> consists almost entirely of simple movements of code, but it
> significantly separates updating a device's snapshot list from
> registering devices for newly-found snapshots (which will be
> for the benefit of format 2 image initialization).
> 
> It is available as branch "wip-rbd-review-4" on the ceph-client git
> repository, and is based on the branch "wip-rbd-review-3".
> 
>     https://github.com/ceph/ceph-client/tree/wip-rbd-review-4

Whoops, forgot to update after copy/paste.  This should have read:


It is available as branch "wip-rbd-review-5" on the ceph-client git
repository, and is based on the branch "wip-rbd-review-4".

    https://github.com/ceph/ceph-client/tree/wip-rbd-review-5

> 
> 					-Alex
> 
> [PATCH 1/9]  d58dd5e rbd: assign header name later
> [PATCH 2/9]  c8fd9cf rbd: defer registering snapshot devices
> [PATCH 3/9]  bf469fa rbd: call set_snap() before snap_devs_update()
> [PATCH 4/9]  1ee626d rbd: read the header before registering device
> [PATCH 5/9]  f05c12d rbd: defer setting device id
> [PATCH 6/9]  8b8b8b7 rbd: call rbd_init_disk() sooner
> [PATCH 7/9]  f1194c2 rbd: drop dev registration check for new snap
> [PATCH 8/9]  345936d rbd: set initial capacity in rbd_init_disk()
> [PATCH 9/9]  527bbdd rbd: set up watch before announcing disk
> 

Re: [PATCH 1/9] rbd: assign header name later

[Josh Durgin]

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

On 09/07/2012 08:44 AM, Alex Elder wrote:
> Move the assignment of the header name for an rbd image a bit later,
> outside rbd_add_parse_args() and into its caller.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 4dff92f..14034e3 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2475,15 +2475,6 @@ static char *rbd_add_parse_args(struct rbd_device
> *rbd_dev,
>   	if (!rbd_dev->image_name)
>   		goto out_err;
>
> -	/* Create the name of the header object */
> -
> -	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
> -						+ sizeof (RBD_SUFFIX),
> -					GFP_KERNEL);
> -	if (!rbd_dev->header_name)
> -		goto out_err;
> -	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
> -
>   	/* Snapshot name is optional */
>   	len = next_token(&buf);
>   	if (!len) {
> @@ -2501,8 +2492,6 @@ dout("    SNAP_NAME is <%s>, len is %zd\n",
> snap_name, len);
>   	return snap_name;
>
>   out_err:
> -	kfree(rbd_dev->header_name);
> -	rbd_dev->header_name = NULL;
>   	kfree(rbd_dev->image_name);
>   	rbd_dev->image_name = NULL;
>   	rbd_dev->image_name_len = 0;
> @@ -2569,6 +2558,15 @@ static ssize_t rbd_add(struct bus_type *bus,
>   		goto err_out_client;
>   	rbd_dev->pool_id = rc;
>
> +	/* Create the name of the header object */
> +
> +	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
> +						+ sizeof (RBD_SUFFIX),
> +					GFP_KERNEL);
> +	if (!rbd_dev->header_name)
> +		goto err_out_client;
> +	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
> +
>   	/* register our block device */
>   	rc = register_blkdev(0, rbd_dev->name);
>   	if (rc < 0)
> @@ -2630,11 +2628,11 @@ err_out_bus:
>   err_out_blkdev:
>   	unregister_blkdev(rbd_dev->major, rbd_dev->name);
>   err_out_client:
> +	kfree(rbd_dev->header_name);
>   	rbd_put_client(rbd_dev);
>   err_put_id:
>   	if (rbd_dev->pool_name) {
>   		kfree(rbd_dev->mapping.snap_name);
> -		kfree(rbd_dev->header_name);
>   		kfree(rbd_dev->image_name);
>   		kfree(rbd_dev->pool_name);
>   	}
>

Re: [PATCH 2/9] rbd: defer registering snapshot devices

[Josh Durgin]

On 09/07/2012 08:44 AM, Alex Elder wrote:
> When a new snapshot is found in an rbd device's updated snapshot
> context, __rbd_add_snap_dev() is called to create and insert an
> entry in the rbd devices list of snapshots.  In addition, a Linux
> device is registered to represent the snapshot.
>
> For version 2 rbd images, it will be undesirable to initialize the
> device right away.  So in anticipation of that, this patch separates
> the insertion of a snapshot entry in the snaps list from the
> creation of devices for those snapshots.
>
> To do this, create a new function rbd_dev_snaps_register() which
> traverses the list of snapshots and calls rbd_register_snap_dev()
> on any that have not yet been registered.
>
> Rename rbd_dev_snap_devs_update() to be rbd_dev_snaps_update()
> to better reflect that only the entry in the snaps list and not
> the snapshot's device is affected by the function.
>
> For now, call rbd_dev_snaps_register() immediately after each
> call to rbd_dev_snaps_update().
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   58
> ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 14034e3..d03fb0f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -204,7 +204,9 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
>   static LIST_HEAD(rbd_client_list);		/* clients */
>   static DEFINE_SPINLOCK(rbd_client_list_lock);
>
> -static int rbd_dev_snap_devs_update(struct rbd_device *rbd_dev);
> +static int rbd_dev_snaps_update(struct rbd_device *rbd_dev);
> +static int rbd_dev_snaps_register(struct rbd_device *rbd_dev);
> +
>   static void rbd_dev_release(struct device *dev);
>   static ssize_t rbd_snap_add(struct device *dev,
>   			    struct device_attribute *attr,
> @@ -648,6 +650,7 @@ static int rbd_header_set_snap(struct rbd_device
> *rbd_dev, char *snap_name)
>   		rbd_dev->mapping.size = rbd_dev->header.image_size;
>   		rbd_dev->mapping.snap_exists = false;
>   		rbd_dev->mapping.read_only = rbd_dev->rbd_opts.read_only;
> +		ret = 0;
>   	} else {
>   		ret = snap_by_name(rbd_dev, snap_name);
>   		if (ret < 0)
> @@ -656,8 +659,6 @@ static int rbd_header_set_snap(struct rbd_device
> *rbd_dev, char *snap_name)
>   		rbd_dev->mapping.read_only = true;
>   	}
>   	rbd_dev->mapping.snap_name = snap_name;
> -
> -	ret = 0;
>   done:
>   	return ret;
>   }

This hunk seems to be a separate clean up.

> @@ -1840,7 +1841,9 @@ static int __rbd_refresh_header(struct rbd_device
> *rbd_dev, u64 *hver)
>   	WARN_ON(strcmp(rbd_dev->header.object_prefix, h.object_prefix));
>   	kfree(h.object_prefix);
>
> -	ret = rbd_dev_snap_devs_update(rbd_dev);
> +	ret = rbd_dev_snaps_update(rbd_dev);
> +	if (!ret)
> +		ret = rbd_dev_snaps_register(rbd_dev);
>
>   	up_write(&rbd_dev->header_rwsem);
>
> @@ -2085,10 +2088,16 @@ static struct device_type rbd_snap_device_type = {
>   	.release	= rbd_snap_dev_release,
>   };
>
> +static bool rbd_snap_registered(struct rbd_snap *snap)
> +{
> +	return snap->dev.type == &rbd_snap_device_type;
> +}
> +
>   static void __rbd_remove_snap_dev(struct rbd_snap *snap)
>   {
>   	list_del(&snap->node);
> -	device_unregister(&snap->dev);
> +	if (rbd_snap_registered(snap))

Could this be device_is_registered(snap->dev)?

> +		device_unregister(&snap->dev);
>   }
>
>   static int rbd_register_snap_dev(struct rbd_snap *snap,
> @@ -2101,6 +2110,8 @@ static int rbd_register_snap_dev(struct rbd_snap
> *snap,
>   	dev->parent = parent;
>   	dev->release = rbd_snap_dev_release;
>   	dev_set_name(dev, "snap_%s", snap->name);
> +	dout("%s: registering device for snapshot %s\n", __func__, snap->name);
> +
>   	ret = device_register(dev);
>
>   	return ret;
> @@ -2123,11 +2134,6 @@ static struct rbd_snap *__rbd_add_snap_dev(struct
> rbd_device *rbd_dev,
>
>   	snap->size = rbd_dev->header.snap_sizes[i];
>   	snap->id = rbd_dev->header.snapc->snaps[i];
> -	if (device_is_registered(&rbd_dev->dev)) {
> -		ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
> -		if (ret < 0)
> -			goto err;
> -	}
>
>   	return snap;
>
> @@ -2150,7 +2156,7 @@ err:
>    * snapshot id, highest id first.  (Snapshots in the rbd_dev's list
>    * are also maintained in that order.)
>    */
> -static int rbd_dev_snap_devs_update(struct rbd_device *rbd_dev)
> +static int rbd_dev_snaps_update(struct rbd_device *rbd_dev)
>   {
>   	struct ceph_snap_context *snapc = rbd_dev->header.snapc;
>   	const u32 snap_count = snapc->num_snaps;
> @@ -2237,6 +2243,31 @@ static int rbd_dev_snap_devs_update(struct
> rbd_device *rbd_dev)
>   	return 0;
>   }
>
> +/*
> + * Scan the list of snapshots and register the devices for any that
> + * have not already been registered.
> + */
> +static int rbd_dev_snaps_register(struct rbd_device *rbd_dev)
> +{
> +	struct rbd_snap *snap;
> +	int ret = 0;
> +
> +	dout("%s called\n", __func__);
> +	if (!device_is_registered(&rbd_dev->dev))
> +		return 0;
> +
> +	list_for_each_entry(snap, &rbd_dev->snaps, node) {
> +		if (!rbd_snap_registered(snap)) {
> +			ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
> +			if (ret < 0)
> +				break;
> +		}
> +	}
> +	dout("%s: returning %d\n", __func__, ret);
> +
> +	return ret;
> +}
> +
>   static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
>   {
>   	struct device *dev;
> @@ -2587,7 +2618,10 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	if (rc)
>   		goto err_out_unlock;
>
> -	rc = rbd_dev_snap_devs_update(rbd_dev);
> +	rc = rbd_dev_snaps_update(rbd_dev);
> +	if (rc)
> +		goto err_out_unlock;
> +	rc = rbd_dev_snaps_register(rbd_dev);
>   	if (rc)
>   		goto err_out_unlock;
>

Re: [PATCH 3/9] rbd: call set_snap() before snap_devs_update()

[Josh Durgin]

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

On 09/07/2012 08:44 AM, Alex Elder wrote:
> rbd_header_set_snap() is a simple initialization routine for an rbd
> device's mapping.  It has to be called after the snapshot context
> for the rbd_dev has been updated, but can be done before snapshot
> devices have been registered.
>
> Change the name to rbd_dev_set_mapping() to better reflect its
> purpose, and call it a little sooner, before registering snapshot
> devices.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |    7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d03fb0f..d60db25 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -640,7 +640,7 @@ static int snap_by_name(struct rbd_device *rbd_dev,
> const char *snap_name)
>   	return -ENOENT;
>   }
>
> -static int rbd_header_set_snap(struct rbd_device *rbd_dev, char *snap_name)
> +static int rbd_dev_set_mapping(struct rbd_device *rbd_dev, char *snap_name)
>   {
>   	int ret;
>
> @@ -2621,11 +2621,12 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	rc = rbd_dev_snaps_update(rbd_dev);
>   	if (rc)
>   		goto err_out_unlock;
> -	rc = rbd_dev_snaps_register(rbd_dev);
> +
> +	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
>   	if (rc)
>   		goto err_out_unlock;
>
> -	rc = rbd_header_set_snap(rbd_dev, snap_name);
> +	rc = rbd_dev_snaps_register(rbd_dev);
>   	if (rc)
>   		goto err_out_unlock;
>

Re: [PATCH 4/9] rbd: read the header before registering device

[Josh Durgin]

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

On 09/07/2012 08:44 AM, Alex Elder wrote:
> Read the rbd header information and call rbd_dev_set_mapping()
> earlier--before registering the block device or setting up the sysfs
> entries for the image.  The sysfs entries provide users access to
> some information that's only available after doing the rbd header
> initialization, so this will make sure it's valid right away.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   31 +++++++++++++++++--------------
>   1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d60db25..89d8ed7 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2598,10 +2598,24 @@ static ssize_t rbd_add(struct bus_type *bus,
>   		goto err_out_client;
>   	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
>
> +	/* Get information about the image being mapped */
> +
> +	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
> +	if (rc)
> +		goto err_out_client;
> +
> +	rc = rbd_dev_snaps_update(rbd_dev);
> +	if (rc)
> +		goto err_out_header;
> +
> +	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
> +	if (rc)
> +		goto err_out_header;
> +
>   	/* register our block device */
>   	rc = register_blkdev(0, rbd_dev->name);
>   	if (rc < 0)
> -		goto err_out_client;
> +		goto err_out_header;
>   	rbd_dev->major = rc;
>
>   	rc = rbd_bus_add_dev(rbd_dev);
> @@ -2613,19 +2627,6 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	 * of the sysfs code (initiated by rbd_bus_del_dev()).
>   	 */
>
> -	/* contact OSD, request size info about the object being mapped */
> -	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
> -	if (rc)
> -		goto err_out_unlock;
> -
> -	rc = rbd_dev_snaps_update(rbd_dev);
> -	if (rc)
> -		goto err_out_unlock;
> -
> -	rc = rbd_dev_set_mapping(rbd_dev, snap_name);
> -	if (rc)
> -		goto err_out_unlock;
> -
>   	rc = rbd_dev_snaps_register(rbd_dev);
>   	if (rc)
>   		goto err_out_unlock;
> @@ -2662,6 +2663,8 @@ err_out_bus:
>
>   err_out_blkdev:
>   	unregister_blkdev(rbd_dev->major, rbd_dev->name);
> +err_out_header:
> +	rbd_header_free(&rbd_dev->header);
>   err_out_client:
>   	kfree(rbd_dev->header_name);
>   	rbd_put_client(rbd_dev);
>

Re: [PATCH 5/9] rbd: defer setting device id

[Josh Durgin]

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

On 09/07/2012 08:45 AM, Alex Elder wrote:
> Hold off setting the device id and formatting the device name
> in rbd_add() until just before it's needed.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   44 ++++++++++++++++++++++----------------------
>   1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 89d8ed7..53e5308 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2549,10 +2549,10 @@ static ssize_t rbd_add(struct bus_type *bus,
>
>   	options = kmalloc(count, GFP_KERNEL);
>   	if (!options)
> -		goto err_nomem;
> +		goto err_out_mem;
>   	rbd_dev = kzalloc(sizeof(*rbd_dev), GFP_KERNEL);
>   	if (!rbd_dev)
> -		goto err_nomem;
> +		goto err_out_mem;
>
>   	/* static rbd_device initialization */
>   	spin_lock_init(&rbd_dev->lock);
> @@ -2562,25 +2562,17 @@ static ssize_t rbd_add(struct bus_type *bus,
>
>   	down_write(&rbd_dev->header_rwsem);
>
> -	/* generate unique id: find highest unique id, add one */
> -	rbd_dev_id_get(rbd_dev);
> -
> -	/* Fill in the device name, now that we have its id. */
> -	BUILD_BUG_ON(DEV_NAME_LEN
> -			< sizeof (RBD_DRV_NAME) + MAX_INT_FORMAT_WIDTH);
> -	sprintf(rbd_dev->name, "%s%d", RBD_DRV_NAME, rbd_dev->dev_id);
> -
>   	/* parse add command */
>   	snap_name = rbd_add_parse_args(rbd_dev, buf,
>   				&mon_addrs, &mon_addrs_size, options, count);
>   	if (IS_ERR(snap_name)) {
>   		rc = PTR_ERR(snap_name);
> -		goto err_put_id;
> +		goto err_out_mem;
>   	}
>
>   	rc = rbd_get_client(rbd_dev, mon_addrs, mon_addrs_size - 1, options);
>   	if (rc < 0)
> -		goto err_put_id;
> +		goto err_out_args;
>
>   	/* pick the pool */
>   	osdc = &rbd_dev->rbd_client->client->osdc;
> @@ -2612,10 +2604,19 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	if (rc)
>   		goto err_out_header;
>
> -	/* register our block device */
> +	/* generate unique id: find highest unique id, add one */
> +	rbd_dev_id_get(rbd_dev);
> +
> +	/* Fill in the device name, now that we have its id. */
> +	BUILD_BUG_ON(DEV_NAME_LEN
> +			< sizeof (RBD_DRV_NAME) + MAX_INT_FORMAT_WIDTH);
> +	sprintf(rbd_dev->name, "%s%d", RBD_DRV_NAME, rbd_dev->dev_id);
> +
> +	/* Get our block major device number. */
> +
>   	rc = register_blkdev(0, rbd_dev->name);
>   	if (rc < 0)
> -		goto err_out_header;
> +		goto err_out_id;
>   	rbd_dev->major = rc;
>
>   	rc = rbd_bus_add_dev(rbd_dev);
> @@ -2663,20 +2664,19 @@ err_out_bus:
>
>   err_out_blkdev:
>   	unregister_blkdev(rbd_dev->major, rbd_dev->name);
> +err_out_id:
> +	rbd_dev_id_put(rbd_dev);
>   err_out_header:
>   	rbd_header_free(&rbd_dev->header);
>   err_out_client:
>   	kfree(rbd_dev->header_name);
>   	rbd_put_client(rbd_dev);
> -err_put_id:
> -	if (rbd_dev->pool_name) {
> -		kfree(rbd_dev->mapping.snap_name);
> -		kfree(rbd_dev->image_name);
> -		kfree(rbd_dev->pool_name);
> -	}
> -	rbd_dev_id_put(rbd_dev);
> +err_out_args:
> +	kfree(rbd_dev->mapping.snap_name);
> +	kfree(rbd_dev->image_name);
> +	kfree(rbd_dev->pool_name);
>   	up_write(&rbd_dev->header_rwsem);
> -err_nomem:
> +err_out_mem:
>   	kfree(rbd_dev);
>   	kfree(options);
>

Re: [PATCH 6/9] rbd: call rbd_init_disk() sooner

[Josh Durgin]

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

On 09/07/2012 08:45 AM, Alex Elder wrote:
> Call rbd_init_disk() from rbd_add() as soon as we have the major
> device number for the mapping.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 53e5308..8e82610 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2619,10 +2619,16 @@ static ssize_t rbd_add(struct bus_type *bus,
>   		goto err_out_id;
>   	rbd_dev->major = rc;
>
> -	rc = rbd_bus_add_dev(rbd_dev);
> +	/* Set up the blkdev mapping. */
> +
> +	rc = rbd_init_disk(rbd_dev);
>   	if (rc)
>   		goto err_out_blkdev;
>
> +	rc = rbd_bus_add_dev(rbd_dev);
> +	if (rc)
> +		goto err_out_disk;
> +
>   	/*
>   	 * At this point cleanup in the event of an error is the job
>   	 * of the sysfs code (initiated by rbd_bus_del_dev()).
> @@ -2634,12 +2640,6 @@ static ssize_t rbd_add(struct bus_type *bus,
>
>   	up_write(&rbd_dev->header_rwsem);
>
> -	/* Set up the blkdev mapping. */
> -
> -	rc = rbd_init_disk(rbd_dev);
> -	if (rc)
> -		goto err_out_bus;
> -
>   	/* Everything's ready.  Announce the disk to the world. */
>
>   	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
> @@ -2662,6 +2662,8 @@ err_out_bus:
>   	kfree(options);
>   	return rc;
>
> +err_out_disk:
> +	rbd_free_disk(rbd_dev);
>   err_out_blkdev:
>   	unregister_blkdev(rbd_dev->major, rbd_dev->name);
>   err_out_id:
>

Re: [PATCH 7/9] rbd: drop dev registration check for new snap

[Josh Durgin]

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

On 09/07/2012 08:45 AM, Alex Elder wrote:
> By the time rbd_dev_snaps_register() gets called during rbd device
> initialization, the main device will have already been registered.
> Similarly, a header refresh will only occur for an rbd device whose
> Linux device is registered.  There is therefore no need to verify
> the main device is registered when registering a snapshot device.
>
> For the time being, turn the check into a WARN_ON(), but it can
> eventually just go away.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8e82610..e6af38f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2253,8 +2253,8 @@ static int rbd_dev_snaps_register(struct
> rbd_device *rbd_dev)
>   	int ret = 0;
>
>   	dout("%s called\n", __func__);
> -	if (!device_is_registered(&rbd_dev->dev))
> -		return 0;
> +	if (WARN_ON(!device_is_registered(&rbd_dev->dev)))
> +		return -EIO;
>
>   	list_for_each_entry(snap, &rbd_dev->snaps, node) {
>   		if (!rbd_snap_registered(snap)) {
>

Re: [PATCH 8/9] rbd: set initial capacity in rbd_init_disk()

[Josh Durgin]

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

On 09/07/2012 08:45 AM, Alex Elder wrote:
> Move the setting of the initial capacity for an rbd image mapping
> into rb_init_disk().
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index e6af38f..51e1d21 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1901,6 +1901,8 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
>
>   	rbd_dev->disk = disk;
>
> +	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
> +
>   	return 0;
>   out_disk:
>   	put_disk(disk);
> @@ -2642,7 +2644,6 @@ static ssize_t rbd_add(struct bus_type *bus,
>
>   	/* Everything's ready.  Announce the disk to the world. */
>
> -	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
>   	add_disk(rbd_dev->disk);
>   	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
>   		(unsigned long long) rbd_dev->mapping.size);
>

Re: [PATCH 9/9] rbd: set up watch before announcing disk

[Josh Durgin]

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

On 09/07/2012 08:45 AM, Alex Elder wrote:
> We're ready to handle header object (refresh) events at the point we
> call rbd_bus_add_dev().  Set up the watch request on the rbd image
> header just after that, and after we've registered the devices for
> the snapshots for the initial snapshot context.  Do this before
> announce the disk as available for use.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 51e1d21..daa428e3 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2640,23 +2640,23 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	if (rc)
>   		goto err_out_unlock;
>
> +	rc = rbd_init_watch_dev(rbd_dev);
> +	if (rc)
> +		goto err_out_unlock;
> +
>   	up_write(&rbd_dev->header_rwsem);
>
>   	/* Everything's ready.  Announce the disk to the world. */
>
>   	add_disk(rbd_dev->disk);
> +
>   	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
>   		(unsigned long long) rbd_dev->mapping.size);
>
> -	rc = rbd_init_watch_dev(rbd_dev);
> -	if (rc)
> -		goto err_out_bus;
> -
>   	return count;
>
>   err_out_unlock:
>   	up_write(&rbd_dev->header_rwsem);
> -err_out_bus:
>   	/* this will also clean up rest of rbd_dev stuff */
>
>   	rbd_bus_del_dev(rbd_dev);
>