[PATCH 0/5] rbd: kill create_snap sysfs entry

[PATCH 0/5] rbd: kill create_snap sysfs entry

[Alex Elder]

The point of this sixth series is as the subject indicates--to
get rid of the /sys/bus/rbd/create_snap entry.  There is no need
to provide this functionality via the kernel client, it is done
quite well using the user space "rbd" command.

Simply removing the functionality would leave rbd_req_sync_exec()
with no other references though, and we'll be making quite a bit
of use of that function in supporting format 2 rbd images.

So this series inserts a patch that adds another function we will
want that will make use of rbd_req_sync_exec(). And that patch
requires the other three as prerequisites as well--supporting
data returned from requests made via rbd_req_sync_exec(), then
using the function to perform new "image_id" retrieval operation
on a special image id file used for version 2 images.

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

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

					-Alex

[PATCH 1/5]  1ab7ae5 rbd: pass flags to rbd_req_sync_exec()
[PATCH 2/5]  44a187e rbd: support data returned from OSD methods
[PATCH 3/5]  f1e1bff rbd: define some new format constants
[PATCH 4/5]  574d0d3 rbd: define rbd_dev_image_id()
[PATCH 5/5]  48080a0 rbd: kill create_snap sysfs entry

[PATCH 1/5] rbd: pass flags to rbd_req_sync_exec()

[Alex Elder]

In order to allow both read requests and write requests to be
initiated using rbd_req_sync_exec(), add an OSD flags value
which can be passed down to rbd_req_sync_op().  Rename the "data"
and "len" parameters to be more clear that they represent data
that is outbound.

At this point, this function is still only used (and only works) for
write requests.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index daa428e3..4e84f99 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1437,23 +1437,33 @@ fail:
 }

 /*
- * Request sync osd read
+ * Synchronous osd object method call
  */
 static int rbd_req_sync_exec(struct rbd_device *rbd_dev,
 			     const char *object_name,
 			     const char *class_name,
 			     const char *method_name,
-			     const char *data,
-			     int len,
+			     const char *outbound,
+			     size_t outbound_size,
+			     int flags,
 			     u64 *ver)
 {
 	struct ceph_osd_req_op *ops;
 	int class_name_len = strlen(class_name);
 	int method_name_len = strlen(method_name);
+	int payload_size;
 	int ret;

-	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_CALL,
-				    class_name_len + method_name_len + len);
+	/*
+	 * Any input parameters required by the method we're calling
+	 * will be sent along with the class and method names as
+	 * part of the message payload.  That data and its size are
+	 * supplied via the indata and indata_len fields (named from
+	 * the perspective of the server side) in the OSD request
+	 * operation.
+	 */
+	payload_size = class_name_len + method_name_len + outbound_size;
+	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_CALL, payload_size);
 	if (!ops)
 		return -ENOMEM;

@@ -1462,13 +1472,12 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
 	ops[0].cls.method_name = method_name;
 	ops[0].cls.method_len = (__u8) method_name_len;
 	ops[0].cls.argc = 0;
-	ops[0].cls.indata = data;
-	ops[0].cls.indata_len = len;
+	ops[0].cls.indata = outbound;
+	ops[0].cls.indata_len = outbound_size;

 	ret = rbd_req_sync_op(rbd_dev, NULL,
 			       CEPH_NOSNAP,
-			       CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
-			       ops,
+			       flags, ops,
 			       object_name, 0, 0, NULL, NULL, ver);

 	rbd_destroy_ops(ops);
@@ -1780,7 +1789,9 @@ static int rbd_header_add_snap(struct rbd_device
*rbd_dev,

 	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
 				"rbd", "snap_add",
-				data, p - data, NULL);
+				data, (size_t) (p - data),
+				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
+				NULL);

 	kfree(data);

-- 
1.7.9.5

[PATCH 2/5] rbd: support data returned from OSD methods

[Alex Elder]

An OSD object method call can be made using rbd_req_sync_exec().
Until now this has only been used for creating a new RBD snapshot,
and that has only required sending data out, not receiving anything
back from the OSD.

We will now need to get data back from an OSD on a method call, so
add parameters to rbd_req_sync_exec() that allow a buffer into which
returned data should be placed to be specified, along with its size.

Previously, rbd_req_sync_exec() passed a null pointer and zero
size to rbd_req_sync_op(); change this so the new inbound buffer
information is provided instead.

Rename the "buf" and "len" parameters in rbd_req_sync_op() to
make it more obvious they are describing inbound data.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 4e84f99..ba68566 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1098,8 +1098,8 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
 			   int flags,
 			   struct ceph_osd_req_op *ops,
 			   const char *object_name,
-			   u64 ofs, u64 len,
-			   char *buf,
+			   u64 ofs, u64 inbound_size,
+			   char *inbound,
 			   struct ceph_osd_request **linger_req,
 			   u64 *ver)
 {
@@ -1109,13 +1109,13 @@ static int rbd_req_sync_op(struct rbd_device
*rbd_dev,

 	rbd_assert(ops != NULL);

-	num_pages = calc_pages_for(ofs , len);
+	num_pages = calc_pages_for(ofs, inbound_size);
 	pages = ceph_alloc_page_vector(num_pages, GFP_KERNEL);
 	if (IS_ERR(pages))
 		return PTR_ERR(pages);

 	ret = rbd_do_request(NULL, rbd_dev, snapc, snapid,
-			  object_name, ofs, len, NULL,
+			  object_name, ofs, inbound_size, NULL,
 			  pages, num_pages,
 			  flags,
 			  ops,
@@ -1125,8 +1125,8 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
 	if (ret < 0)
 		goto done;

-	if ((flags & CEPH_OSD_FLAG_READ) && buf)
-		ret = ceph_copy_from_page_vector(pages, buf, ofs, ret);
+	if ((flags & CEPH_OSD_FLAG_READ) && inbound)
+		ret = ceph_copy_from_page_vector(pages, inbound, ofs, ret);

 done:
 	ceph_release_page_vector(pages, num_pages);
@@ -1445,6 +1445,8 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
 			     const char *method_name,
 			     const char *outbound,
 			     size_t outbound_size,
+			     char *inbound,
+			     size_t inbound_size,
 			     int flags,
 			     u64 *ver)
 {
@@ -1478,7 +1480,8 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
 	ret = rbd_req_sync_op(rbd_dev, NULL,
 			       CEPH_NOSNAP,
 			       flags, ops,
-			       object_name, 0, 0, NULL, NULL, ver);
+			       object_name, 0, inbound_size, inbound,
+			       NULL, ver);

 	rbd_destroy_ops(ops);

@@ -1789,7 +1792,7 @@ static int rbd_header_add_snap(struct rbd_device
*rbd_dev,

 	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
 				"rbd", "snap_add",
-				data, (size_t) (p - data),
+				data, (size_t) (p - data), NULL, 0,
 				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
 				NULL);

-- 
1.7.9.5

[PATCH 3/5] rbd: define some new format constants

[Alex Elder]

Define constant symbols related to the rbd format 2 object names.
This begins to bring this version of the "rbd_types.h" header
more in line with the current user-space version of that file.
Complete reconciliation of differences will be done at some
point later, as a separate task.

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

diff --git a/drivers/block/rbd_types.h b/drivers/block/rbd_types.h
index d9d8a77..cbe77fa 100644
--- a/drivers/block/rbd_types.h
+++ b/drivers/block/rbd_types.h
@@ -15,15 +15,30 @@

 #include <linux/types.h>

+/* For format version 2, rbd image 'foo' consists of objects
+ *   rbd_id.foo		- id of image
+ *   rbd_header.<id>	- image metadata
+ *   rbd_data.<id>.0000000000000000
+ *   rbd_data.<id>.0000000000000001
+ *   ...		- data
+ * Clients do not access header data directly in rbd format 2.
+ */
+
+#define RBD_HEADER_PREFIX      "rbd_header."
+#define RBD_DATA_PREFIX        "rbd_data."
+#define RBD_ID_PREFIX          "rbd_id."
+
 /*
- * rbd image 'foo' consists of objects
- *   foo.rbd      - image metadata
- *   foo.00000000
- *   foo.00000001
- *   ...          - data
+ * For format version 1, rbd image 'foo' consists of objects
+ *   foo.rbd		- image metadata
+ *   rb.<idhi>.<idlo>.00000000
+ *   rb.<idhi>.<idlo>.00000001
+ *   ...		- data
+ * There is no notion of a persistent image id in rbd format 1.
  */

 #define RBD_SUFFIX		".rbd"
+
 #define RBD_DIRECTORY           "rbd_directory"
 #define RBD_INFO                "rbd_info"

-- 
1.7.9.5

[PATCH 4/5] rbd: define rbd_dev_image_id()

[Alex Elder]

New format 2 rbd images are permanently identified by a unique image
id.  Each rbd image also has a name, but the name can be changed.
A format 2 rbd image will have an object--whose name is based on the
image name--which maps an image's name to its image id.

Create a new function rbd_dev_image_id() that checks for the
existence of the image id object, and if it's found, records the
image id in the rbd_device structure.

Create a new rbd device attribute (/sys/bus/rbd/<num>/image_id) that
makes this information available.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 Documentation/ABI/testing/sysfs-bus-rbd |    5 ++
 drivers/block/rbd.c                     |   99
+++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-rbd
b/Documentation/ABI/testing/sysfs-bus-rbd
index 3c17b62..7cbbe34 100644
--- a/Documentation/ABI/testing/sysfs-bus-rbd
+++ b/Documentation/ABI/testing/sysfs-bus-rbd
@@ -33,6 +33,11 @@ name

 	The name of the rbd image.

+image_id
+
+	The unique id for the rbd image.  (For rbd image format 1
+	this is empty.)
+
 pool

 	The name of the storage pool where this rbd image resides.
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index ba68566..5a3132e 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -66,6 +66,8 @@

 #define RBD_SNAP_HEAD_NAME	"-"

+#define	RBD_IMAGE_ID_LEN_MAX	64
+
 /*
  * An RBD device name will be "rbd#", where the "rbd" comes from
  * RBD_DRV_NAME above, and # is a unique integer identifier.
@@ -173,6 +175,8 @@ struct rbd_device {
 	spinlock_t		lock;		/* queue lock */

 	struct rbd_image_header	header;
+	char			*image_id;
+	size_t			image_id_len;
 	char			*image_name;
 	size_t			image_name_len;
 	char			*header_name;
@@ -1987,6 +1991,14 @@ static ssize_t rbd_name_show(struct device *dev,
 	return sprintf(buf, "%s\n", rbd_dev->image_name);
 }

+static ssize_t rbd_image_id_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
+
+	return sprintf(buf, "%s\n", rbd_dev->image_id);
+}
+
 static ssize_t rbd_snap_show(struct device *dev,
 			     struct device_attribute *attr,
 			     char *buf)
@@ -2015,6 +2027,7 @@ static DEVICE_ATTR(client_id, S_IRUGO,
rbd_client_id_show, NULL);
 static DEVICE_ATTR(pool, S_IRUGO, rbd_pool_show, NULL);
 static DEVICE_ATTR(pool_id, S_IRUGO, rbd_pool_id_show, NULL);
 static DEVICE_ATTR(name, S_IRUGO, rbd_name_show, NULL);
+static DEVICE_ATTR(image_id, S_IRUGO, rbd_image_id_show, NULL);
 static DEVICE_ATTR(refresh, S_IWUSR, NULL, rbd_image_refresh);
 static DEVICE_ATTR(current_snap, S_IRUGO, rbd_snap_show, NULL);
 static DEVICE_ATTR(create_snap, S_IWUSR, NULL, rbd_snap_add);
@@ -2026,6 +2039,7 @@ static struct attribute *rbd_attrs[] = {
 	&dev_attr_pool.attr,
 	&dev_attr_pool_id.attr,
 	&dev_attr_name.attr,
+	&dev_attr_image_id.attr,
 	&dev_attr_current_snap.attr,
 	&dev_attr_refresh.attr,
 	&dev_attr_create_snap.attr,
@@ -2548,6 +2562,75 @@ out_err:
 	return err_ptr;
 }

+/*
+ * An rbd format 2 image has a unique identifier, distinct from the
+ * name given to it by the user.  Internally, that identifier is
+ * what's used to specify the names of objects related to the image.
+ *
+ * A special "rbd id" object is used to map an rbd image name to its
+ * id.  If that object doesn't exist, then there is no v2 rbd image
+ * with the supplied name.
+ *
+ * This function will record the given rbd_dev's image_id field if
+ * it can be determined, and in that case will return 0.  If any
+ * errors occur a negative errno will be returned and the rbd_dev's
+ * image_id field will be unchanged (and should be NULL).
+ */
+static int rbd_dev_image_id(struct rbd_device *rbd_dev)
+{
+	int ret;
+	size_t size;
+	char *object_name;
+	void *response;
+	void *p;
+
+	/*
+	 * First, see if the format 2 image id file exists, and if
+	 * so, get the image's persistent id from it.
+	 */
+	size = sizeof (RBD_ID_PREFIX) + rbd_dev->image_name_len;
+	object_name = kmalloc(size, GFP_NOIO);
+	if (!object_name)
+		return -ENOMEM;
+	sprintf(object_name, "%s%s", RBD_ID_PREFIX, rbd_dev->image_name);
+	dout("rbd id object name is %s\n", object_name);
+
+	/* Response will be an encoded string, which includes a length */
+
+	size = sizeof (__le32) + RBD_IMAGE_ID_LEN_MAX;
+	response = kzalloc(size, GFP_NOIO);
+	if (!response) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = rbd_req_sync_exec(rbd_dev, object_name,
+				"rbd", "get_id",
+				NULL, 0,
+				response, RBD_IMAGE_ID_LEN_MAX,
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		goto out;
+	ret = 0;	/* rbd_req_sync_exec() can return positive */
+
+	p = response;
+	rbd_dev->image_id = ceph_extract_encoded_string(&p,
+						p + RBD_IMAGE_ID_LEN_MAX,
+						&rbd_dev->image_id_len,
+						GFP_NOIO);
+	if (IS_ERR(rbd_dev->image_id)) {
+		ret = PTR_ERR(rbd_dev->image_id);
+		rbd_dev->image_id = NULL;
+	} else
+		dout("image_id is %s\n", rbd_dev->image_id);
+out:
+	kfree(response);
+	kfree(object_name);
+
+	return ret;
+}
+
 static ssize_t rbd_add(struct bus_type *bus,
 		       const char *buf,
 		       size_t count)
@@ -2597,6 +2680,20 @@ static ssize_t rbd_add(struct bus_type *bus,
 		goto err_out_client;
 	rbd_dev->pool_id = rc;

+	rc = rbd_dev_image_id(rbd_dev);
+	if (rc == -ENOENT) {
+		/* Version 1 images have no id; empty string is used */
+		rbd_dev->image_id = kstrdup("", GFP_KERNEL);
+		if (!rbd_dev->image_id) {
+			rc = -ENOMEM;
+			goto err_out_client;
+		}
+		rbd_dev->image_id_len = 0;
+	} else {
+	        /* Not actually supporting format 2 yet */
+		goto err_out_client;
+	}
+
 	/* Create the name of the header object */

 	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
@@ -2688,6 +2785,7 @@ err_out_header:
 err_out_client:
 	kfree(rbd_dev->header_name);
 	rbd_put_client(rbd_dev);
+	kfree(rbd_dev->image_id);
 err_out_args:
 	kfree(rbd_dev->mapping.snap_name);
 	kfree(rbd_dev->image_name);
@@ -2744,6 +2842,7 @@ static void rbd_dev_release(struct device *dev)

 	/* done with the id, and with the rbd_dev */
 	kfree(rbd_dev->mapping.snap_name);
+	kfree(rbd_dev->image_id);
 	kfree(rbd_dev->header_name);
 	kfree(rbd_dev->pool_name);
 	kfree(rbd_dev->image_name);
-- 
1.7.9.5

[PATCH 5/5] rbd: kill create_snap sysfs entry

[Alex Elder]

Josh proposed the following change, and I don't think I could
explain it any better than he did:

    From: Josh Durgin <josh.durgin@inktank.com>
    Date: Tue, 24 Jul 2012 14:22:11 -0700
    To: ceph-devel <ceph-devel@vger.kernel.org>
    Message-ID: <500F1203.9050605@inktank.com>

    Right now the kernel still has one piece of rbd management
    duplicated from the rbd command line tool: snapshot creation.
    There's nothing special about snapshot creation that makes it
    advantageous to do from the kernel, so I'd like to remove the
    create_snap sysfs interface.  That is,
	/sys/bus/rbd/devices/<id>/create_snap
    would be removed.

    Does anyone rely on the sysfs interface for creating rbd
    snapshots?  If so, how hard would it be to replace with:

	rbd snap create pool/image@snap

    Is there any benefit to the sysfs interface that I'm missing?

    Josh

This patch implements this proposal, removing the code that
implements the "snap_create" sysfs interface for rbd images.
As a result, quite a lot of other supporting code goes away.

Suggested-by: Josh Durgin <josh.durgin@inktank.com>
Signed-off-by: Alex Elder <elder@inktank.com>
---
 Documentation/ABI/testing/sysfs-bus-rbd |    6 --
 drivers/block/rbd.c                     |  158
-------------------------------
 2 files changed, 164 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-rbd
b/Documentation/ABI/testing/sysfs-bus-rbd
index 7cbbe34..6fe4224 100644
--- a/Documentation/ABI/testing/sysfs-bus-rbd
+++ b/Documentation/ABI/testing/sysfs-bus-rbd
@@ -62,12 +62,6 @@ current_snap

 	The current snapshot for which the device is mapped.

-create_snap
-
-	Create a snapshot:
-
-	 $ echo <snap-name> > /sys/bus/rbd/devices/<dev-id>/snap_create
-
 snap_*

 	A directory per each snapshot
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 5a3132e..d73edb1 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -212,10 +212,6 @@ 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,
-			    const char *buf,
-			    size_t count);
 static void __rbd_remove_snap_dev(struct rbd_snap *snap);

 static ssize_t rbd_add(struct bus_type *bus, const char *buf,
@@ -1375,71 +1371,6 @@ static int rbd_req_sync_unwatch(struct rbd_device
*rbd_dev)
 	return ret;
 }

-struct rbd_notify_info {
-	struct rbd_device *rbd_dev;
-};
-
-static void rbd_notify_cb(u64 ver, u64 notify_id, u8 opcode, void *data)
-{
-	struct rbd_device *rbd_dev = (struct rbd_device *)data;
-	if (!rbd_dev)
-		return;
-
-	dout("rbd_notify_cb %s notify_id=%llu opcode=%u\n",
-			rbd_dev->header_name, (unsigned long long) notify_id,
-			(unsigned int) opcode);
-}
-
-/*
- * Request sync osd notify
- */
-static int rbd_req_sync_notify(struct rbd_device *rbd_dev)
-{
-	struct ceph_osd_req_op *ops;
-	struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
-	struct ceph_osd_event *event;
-	struct rbd_notify_info info;
-	int payload_len = sizeof(u32) + sizeof(u32);
-	int ret;
-
-	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_NOTIFY, payload_len);
-	if (!ops)
-		return -ENOMEM;
-
-	info.rbd_dev = rbd_dev;
-
-	ret = ceph_osdc_create_event(osdc, rbd_notify_cb, 1,
-				     (void *)&info, &event);
-	if (ret < 0)
-		goto fail;
-
-	ops[0].watch.ver = 1;
-	ops[0].watch.flag = 1;
-	ops[0].watch.cookie = event->cookie;
-	ops[0].watch.prot_ver = RADOS_NOTIFY_VER;
-	ops[0].watch.timeout = 12;
-
-	ret = rbd_req_sync_op(rbd_dev, NULL,
-			       CEPH_NOSNAP,
-			       CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
-			       ops,
-			       rbd_dev->header_name,
-			       0, 0, NULL, NULL, NULL);
-	if (ret < 0)
-		goto fail_event;
-
-	ret = ceph_osdc_wait_event(event, CEPH_OSD_TIMEOUT_DEFAULT);
-	dout("ceph_osdc_wait_event returned %d\n", ret);
-	rbd_destroy_ops(ops);
-	return 0;
-
-fail_event:
-	ceph_osdc_cancel_event(event);
-fail:
-	rbd_destroy_ops(ops);
-	return ret;
-}
-
 /*
  * Synchronous osd object method call
  */
@@ -1761,52 +1692,6 @@ static int rbd_read_header(struct rbd_device
*rbd_dev,
 	return ret;
 }

-/*
- * create a snapshot
- */
-static int rbd_header_add_snap(struct rbd_device *rbd_dev,
-			       const char *snap_name,
-			       gfp_t gfp_flags)
-{
-	int name_len = strlen(snap_name);
-	u64 new_snapid;
-	int ret;
-	void *data, *p, *e;
-	struct ceph_mon_client *monc;
-
-	/* we should create a snapshot only if we're pointing at the head */
-	if (rbd_dev->mapping.snap_id != CEPH_NOSNAP)
-		return -EINVAL;
-
-	monc = &rbd_dev->rbd_client->client->monc;
-	ret = ceph_monc_create_snapid(monc, rbd_dev->pool_id, &new_snapid);
-	dout("created snapid=%llu\n", (unsigned long long) new_snapid);
-	if (ret < 0)
-		return ret;
-
-	data = kmalloc(name_len + 16, gfp_flags);
-	if (!data)
-		return -ENOMEM;
-
-	p = data;
-	e = data + name_len + 16;
-
-	ceph_encode_string_safe(&p, e, snap_name, name_len, bad);
-	ceph_encode_64_safe(&p, e, new_snapid, bad);
-
-	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
-				"rbd", "snap_add",
-				data, (size_t) (p - data), NULL, 0,
-				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
-				NULL);
-
-	kfree(data);
-
-	return ret < 0 ? ret : 0;
-bad:
-	return -ERANGE;
-}
-
 static void __rbd_remove_all_snaps(struct rbd_device *rbd_dev)
 {
 	struct rbd_snap *snap;
@@ -2030,7 +1915,6 @@ static DEVICE_ATTR(name, S_IRUGO, rbd_name_show,
NULL);
 static DEVICE_ATTR(image_id, S_IRUGO, rbd_image_id_show, NULL);
 static DEVICE_ATTR(refresh, S_IWUSR, NULL, rbd_image_refresh);
 static DEVICE_ATTR(current_snap, S_IRUGO, rbd_snap_show, NULL);
-static DEVICE_ATTR(create_snap, S_IWUSR, NULL, rbd_snap_add);

 static struct attribute *rbd_attrs[] = {
 	&dev_attr_size.attr,
@@ -2042,7 +1926,6 @@ static struct attribute *rbd_attrs[] = {
 	&dev_attr_image_id.attr,
 	&dev_attr_current_snap.attr,
 	&dev_attr_refresh.attr,
-	&dev_attr_create_snap.attr,
 	NULL
 };

@@ -2888,47 +2771,6 @@ done:
 	return ret;
 }

-static ssize_t rbd_snap_add(struct device *dev,
-			    struct device_attribute *attr,
-			    const char *buf,
-			    size_t count)
-{
-	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
-	int ret;
-	char *name = kmalloc(count + 1, GFP_KERNEL);
-	if (!name)
-		return -ENOMEM;
-
-	snprintf(name, count, "%s", buf);
-
-	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
-
-	ret = rbd_header_add_snap(rbd_dev,
-				  name, GFP_KERNEL);
-	if (ret < 0)
-		goto err_unlock;
-
-	ret = __rbd_refresh_header(rbd_dev, NULL);
-	if (ret < 0)
-		goto err_unlock;
-
-	/* shouldn't hold ctl_mutex when notifying.. notify might
-	   trigger a watch callback that would need to get that mutex */
-	mutex_unlock(&ctl_mutex);
-
-	/* make a best effort, don't error if failed */
-	rbd_req_sync_notify(rbd_dev);
-
-	ret = count;
-	kfree(name);
-	return ret;
-
-err_unlock:
-	mutex_unlock(&ctl_mutex);
-	kfree(name);
-	return ret;
-}
-
 /*
  * create control files in sysfs
  * /sys/bus/rbd/...
-- 
1.7.9.5

Re: [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec()

[Josh Durgin]

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

On 09/07/2012 11:19 AM, Alex Elder wrote:
> In order to allow both read requests and write requests to be
> initiated using rbd_req_sync_exec(), add an OSD flags value
> which can be passed down to rbd_req_sync_op().  Rename the "data"
> and "len" parameters to be more clear that they represent data
> that is outbound.
>
> At this point, this function is still only used (and only works) for
> write requests.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   31 +++++++++++++++++++++----------
>   1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index daa428e3..4e84f99 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1437,23 +1437,33 @@ fail:
>   }
>
>   /*
> - * Request sync osd read
> + * Synchronous osd object method call
>    */
>   static int rbd_req_sync_exec(struct rbd_device *rbd_dev,
>   			     const char *object_name,
>   			     const char *class_name,
>   			     const char *method_name,
> -			     const char *data,
> -			     int len,
> +			     const char *outbound,
> +			     size_t outbound_size,
> +			     int flags,
>   			     u64 *ver)
>   {
>   	struct ceph_osd_req_op *ops;
>   	int class_name_len = strlen(class_name);
>   	int method_name_len = strlen(method_name);
> +	int payload_size;
>   	int ret;
>
> -	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_CALL,
> -				    class_name_len + method_name_len + len);
> +	/*
> +	 * Any input parameters required by the method we're calling
> +	 * will be sent along with the class and method names as
> +	 * part of the message payload.  That data and its size are
> +	 * supplied via the indata and indata_len fields (named from
> +	 * the perspective of the server side) in the OSD request
> +	 * operation.
> +	 */
> +	payload_size = class_name_len + method_name_len + outbound_size;
> +	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_CALL, payload_size);
>   	if (!ops)
>   		return -ENOMEM;
>
> @@ -1462,13 +1472,12 @@ static int rbd_req_sync_exec(struct rbd_device
> *rbd_dev,
>   	ops[0].cls.method_name = method_name;
>   	ops[0].cls.method_len = (__u8) method_name_len;
>   	ops[0].cls.argc = 0;
> -	ops[0].cls.indata = data;
> -	ops[0].cls.indata_len = len;
> +	ops[0].cls.indata = outbound;
> +	ops[0].cls.indata_len = outbound_size;
>
>   	ret = rbd_req_sync_op(rbd_dev, NULL,
>   			       CEPH_NOSNAP,
> -			       CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
> -			       ops,
> +			       flags, ops,
>   			       object_name, 0, 0, NULL, NULL, ver);
>
>   	rbd_destroy_ops(ops);
> @@ -1780,7 +1789,9 @@ static int rbd_header_add_snap(struct rbd_device
> *rbd_dev,
>
>   	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
>   				"rbd", "snap_add",
> -				data, p - data, NULL);
> +				data, (size_t) (p - data),
> +				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
> +				NULL);
>
>   	kfree(data);
>

Re: [PATCH 2/5] rbd: support data returned from OSD methods

[Josh Durgin]

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

On 09/07/2012 11:19 AM, Alex Elder wrote:
> An OSD object method call can be made using rbd_req_sync_exec().
> Until now this has only been used for creating a new RBD snapshot,
> and that has only required sending data out, not receiving anything
> back from the OSD.
>
> We will now need to get data back from an OSD on a method call, so
> add parameters to rbd_req_sync_exec() that allow a buffer into which
> returned data should be placed to be specified, along with its size.
>
> Previously, rbd_req_sync_exec() passed a null pointer and zero
> size to rbd_req_sync_op(); change this so the new inbound buffer
> information is provided instead.
>
> Rename the "buf" and "len" parameters in rbd_req_sync_op() to
> make it more obvious they are describing inbound data.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 4e84f99..ba68566 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1098,8 +1098,8 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
>   			   int flags,
>   			   struct ceph_osd_req_op *ops,
>   			   const char *object_name,
> -			   u64 ofs, u64 len,
> -			   char *buf,
> +			   u64 ofs, u64 inbound_size,
> +			   char *inbound,
>   			   struct ceph_osd_request **linger_req,
>   			   u64 *ver)
>   {
> @@ -1109,13 +1109,13 @@ static int rbd_req_sync_op(struct rbd_device
> *rbd_dev,
>
>   	rbd_assert(ops != NULL);
>
> -	num_pages = calc_pages_for(ofs , len);
> +	num_pages = calc_pages_for(ofs, inbound_size);
>   	pages = ceph_alloc_page_vector(num_pages, GFP_KERNEL);
>   	if (IS_ERR(pages))
>   		return PTR_ERR(pages);
>
>   	ret = rbd_do_request(NULL, rbd_dev, snapc, snapid,
> -			  object_name, ofs, len, NULL,
> +			  object_name, ofs, inbound_size, NULL,
>   			  pages, num_pages,
>   			  flags,
>   			  ops,
> @@ -1125,8 +1125,8 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
>   	if (ret < 0)
>   		goto done;
>
> -	if ((flags & CEPH_OSD_FLAG_READ) && buf)
> -		ret = ceph_copy_from_page_vector(pages, buf, ofs, ret);
> +	if ((flags & CEPH_OSD_FLAG_READ) && inbound)
> +		ret = ceph_copy_from_page_vector(pages, inbound, ofs, ret);
>
>   done:
>   	ceph_release_page_vector(pages, num_pages);
> @@ -1445,6 +1445,8 @@ static int rbd_req_sync_exec(struct rbd_device
> *rbd_dev,
>   			     const char *method_name,
>   			     const char *outbound,
>   			     size_t outbound_size,
> +			     char *inbound,
> +			     size_t inbound_size,
>   			     int flags,
>   			     u64 *ver)
>   {
> @@ -1478,7 +1480,8 @@ static int rbd_req_sync_exec(struct rbd_device
> *rbd_dev,
>   	ret = rbd_req_sync_op(rbd_dev, NULL,
>   			       CEPH_NOSNAP,
>   			       flags, ops,
> -			       object_name, 0, 0, NULL, NULL, ver);
> +			       object_name, 0, inbound_size, inbound,
> +			       NULL, ver);
>
>   	rbd_destroy_ops(ops);
>
> @@ -1789,7 +1792,7 @@ static int rbd_header_add_snap(struct rbd_device
> *rbd_dev,
>
>   	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
>   				"rbd", "snap_add",
> -				data, (size_t) (p - data),
> +				data, (size_t) (p - data), NULL, 0,
>   				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
>   				NULL);
>

Re: [PATCH 3/5] rbd: define some new format constants

[Josh Durgin]

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

On 09/07/2012 11:19 AM, Alex Elder wrote:
> Define constant symbols related to the rbd format 2 object names.
> This begins to bring this version of the "rbd_types.h" header
> more in line with the current user-space version of that file.
> Complete reconciliation of differences will be done at some
> point later, as a separate task.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd_types.h |   25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd_types.h b/drivers/block/rbd_types.h
> index d9d8a77..cbe77fa 100644
> --- a/drivers/block/rbd_types.h
> +++ b/drivers/block/rbd_types.h
> @@ -15,15 +15,30 @@
>
>   #include <linux/types.h>
>
> +/* For format version 2, rbd image 'foo' consists of objects
> + *   rbd_id.foo		- id of image
> + *   rbd_header.<id>	- image metadata
> + *   rbd_data.<id>.0000000000000000
> + *   rbd_data.<id>.0000000000000001
> + *   ...		- data
> + * Clients do not access header data directly in rbd format 2.
> + */
> +
> +#define RBD_HEADER_PREFIX      "rbd_header."
> +#define RBD_DATA_PREFIX        "rbd_data."
> +#define RBD_ID_PREFIX          "rbd_id."
> +
>   /*
> - * rbd image 'foo' consists of objects
> - *   foo.rbd      - image metadata
> - *   foo.00000000
> - *   foo.00000001
> - *   ...          - data
> + * For format version 1, rbd image 'foo' consists of objects
> + *   foo.rbd		- image metadata
> + *   rb.<idhi>.<idlo>.00000000
> + *   rb.<idhi>.<idlo>.00000001
> + *   ...		- data
> + * There is no notion of a persistent image id in rbd format 1.
>    */
>
>   #define RBD_SUFFIX		".rbd"
> +
>   #define RBD_DIRECTORY           "rbd_directory"
>   #define RBD_INFO                "rbd_info"
>

Re: [PATCH 4/5] rbd: define rbd_dev_image_id()

[Josh Durgin]

On 09/07/2012 11:19 AM, Alex Elder wrote:
> New format 2 rbd images are permanently identified by a unique image
> id.  Each rbd image also has a name, but the name can be changed.
> A format 2 rbd image will have an object--whose name is based on the
> image name--which maps an image's name to its image id.
>
> Create a new function rbd_dev_image_id() that checks for the
> existence of the image id object, and if it's found, records the
> image id in the rbd_device structure.
>
> Create a new rbd device attribute (/sys/bus/rbd/<num>/image_id) that
> makes this information available.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   Documentation/ABI/testing/sysfs-bus-rbd |    5 ++
>   drivers/block/rbd.c                     |   99
> +++++++++++++++++++++++++++++++
>   2 files changed, 104 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-rbd
> b/Documentation/ABI/testing/sysfs-bus-rbd
> index 3c17b62..7cbbe34 100644
> --- a/Documentation/ABI/testing/sysfs-bus-rbd
> +++ b/Documentation/ABI/testing/sysfs-bus-rbd
> @@ -33,6 +33,11 @@ name
>
>   	The name of the rbd image.
>
> +image_id
> +
> +	The unique id for the rbd image.  (For rbd image format 1
> +	this is empty.)
> +
>   pool
>
>   	The name of the storage pool where this rbd image resides.
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index ba68566..5a3132e 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -66,6 +66,8 @@
>
>   #define RBD_SNAP_HEAD_NAME	"-"
>
> +#define	RBD_IMAGE_ID_LEN_MAX	64
> +
>   /*
>    * An RBD device name will be "rbd#", where the "rbd" comes from
>    * RBD_DRV_NAME above, and # is a unique integer identifier.
> @@ -173,6 +175,8 @@ struct rbd_device {
>   	spinlock_t		lock;		/* queue lock */
>
>   	struct rbd_image_header	header;
> +	char			*image_id;
> +	size_t			image_id_len;
>   	char			*image_name;
>   	size_t			image_name_len;
>   	char			*header_name;
> @@ -1987,6 +1991,14 @@ static ssize_t rbd_name_show(struct device *dev,
>   	return sprintf(buf, "%s\n", rbd_dev->image_name);
>   }
>
> +static ssize_t rbd_image_id_show(struct device *dev,
> +			     struct device_attribute *attr, char *buf)
> +{
> +	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
> +
> +	return sprintf(buf, "%s\n", rbd_dev->image_id);
> +}
> +
>   static ssize_t rbd_snap_show(struct device *dev,
>   			     struct device_attribute *attr,
>   			     char *buf)
> @@ -2015,6 +2027,7 @@ static DEVICE_ATTR(client_id, S_IRUGO,
> rbd_client_id_show, NULL);
>   static DEVICE_ATTR(pool, S_IRUGO, rbd_pool_show, NULL);
>   static DEVICE_ATTR(pool_id, S_IRUGO, rbd_pool_id_show, NULL);
>   static DEVICE_ATTR(name, S_IRUGO, rbd_name_show, NULL);
> +static DEVICE_ATTR(image_id, S_IRUGO, rbd_image_id_show, NULL);
>   static DEVICE_ATTR(refresh, S_IWUSR, NULL, rbd_image_refresh);
>   static DEVICE_ATTR(current_snap, S_IRUGO, rbd_snap_show, NULL);
>   static DEVICE_ATTR(create_snap, S_IWUSR, NULL, rbd_snap_add);
> @@ -2026,6 +2039,7 @@ static struct attribute *rbd_attrs[] = {
>   	&dev_attr_pool.attr,
>   	&dev_attr_pool_id.attr,
>   	&dev_attr_name.attr,
> +	&dev_attr_image_id.attr,
>   	&dev_attr_current_snap.attr,
>   	&dev_attr_refresh.attr,
>   	&dev_attr_create_snap.attr,
> @@ -2548,6 +2562,75 @@ out_err:
>   	return err_ptr;
>   }
>
> +/*
> + * An rbd format 2 image has a unique identifier, distinct from the
> + * name given to it by the user.  Internally, that identifier is
> + * what's used to specify the names of objects related to the image.
> + *
> + * A special "rbd id" object is used to map an rbd image name to its
> + * id.  If that object doesn't exist, then there is no v2 rbd image
> + * with the supplied name.
> + *
> + * This function will record the given rbd_dev's image_id field if
> + * it can be determined, and in that case will return 0.  If any
> + * errors occur a negative errno will be returned and the rbd_dev's
> + * image_id field will be unchanged (and should be NULL).
> + */
> +static int rbd_dev_image_id(struct rbd_device *rbd_dev)
> +{
> +	int ret;
> +	size_t size;
> +	char *object_name;
> +	void *response;
> +	void *p;
> +
> +	/*
> +	 * First, see if the format 2 image id file exists, and if
> +	 * so, get the image's persistent id from it.
> +	 */
> +	size = sizeof (RBD_ID_PREFIX) + rbd_dev->image_name_len;
> +	object_name = kmalloc(size, GFP_NOIO);
> +	if (!object_name)
> +		return -ENOMEM;
> +	sprintf(object_name, "%s%s", RBD_ID_PREFIX, rbd_dev->image_name);
> +	dout("rbd id object name is %s\n", object_name);
> +
> +	/* Response will be an encoded string, which includes a length */
> +
> +	size = sizeof (__le32) + RBD_IMAGE_ID_LEN_MAX;
> +	response = kzalloc(size, GFP_NOIO);
> +	if (!response) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = rbd_req_sync_exec(rbd_dev, object_name,
> +				"rbd", "get_id",
> +				NULL, 0,
> +				response, RBD_IMAGE_ID_LEN_MAX,
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		goto out;
> +	ret = 0;	/* rbd_req_sync_exec() can return positive */

The get_id class method will not return positive.

> +
> +	p = response;
> +	rbd_dev->image_id = ceph_extract_encoded_string(&p,
> +						p + RBD_IMAGE_ID_LEN_MAX,
> +						&rbd_dev->image_id_len,
> +						GFP_NOIO);
> +	if (IS_ERR(rbd_dev->image_id)) {
> +		ret = PTR_ERR(rbd_dev->image_id);
> +		rbd_dev->image_id = NULL;

It would be clearer that this can't stay NULL if it were initialized
here instead of in the caller.

> +	} else
> +		dout("image_id is %s\n", rbd_dev->image_id);

If the first branch got braces, the second one should too.

> +out:
> +	kfree(response);
> +	kfree(object_name);
> +
> +	return ret;
> +}
> +
>   static ssize_t rbd_add(struct bus_type *bus,
>   		       const char *buf,
>   		       size_t count)
> @@ -2597,6 +2680,20 @@ static ssize_t rbd_add(struct bus_type *bus,
>   		goto err_out_client;
>   	rbd_dev->pool_id = rc;
>
> +	rc = rbd_dev_image_id(rbd_dev);
> +	if (rc == -ENOENT) {

It's more than just -ENOENT. If the osd doesn't have a get_id method
for example, it will be -EINVAL. It's probably best to try format 1
if any error occurred, rather than trying to come up with an exhaustive
list that won't change in the future.

> +		/* Version 1 images have no id; empty string is used */
> +		rbd_dev->image_id = kstrdup("", GFP_KERNEL);
> +		if (!rbd_dev->image_id) {
> +			rc = -ENOMEM;
> +			goto err_out_client;
> +		}
> +		rbd_dev->image_id_len = 0;
> +	} else {
> +	        /* Not actually supporting format 2 yet */
> +		goto err_out_client;
> +	}
> +
>   	/* Create the name of the header object */
>
>   	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
> @@ -2688,6 +2785,7 @@ err_out_header:
>   err_out_client:
>   	kfree(rbd_dev->header_name);
>   	rbd_put_client(rbd_dev);
> +	kfree(rbd_dev->image_id);
>   err_out_args:
>   	kfree(rbd_dev->mapping.snap_name);
>   	kfree(rbd_dev->image_name);
> @@ -2744,6 +2842,7 @@ static void rbd_dev_release(struct device *dev)
>
>   	/* done with the id, and with the rbd_dev */
>   	kfree(rbd_dev->mapping.snap_name);
> +	kfree(rbd_dev->image_id);
>   	kfree(rbd_dev->header_name);
>   	kfree(rbd_dev->pool_name);
>   	kfree(rbd_dev->image_name);
>

Re: [PATCH 5/5] rbd: kill create_snap sysfs entry

[Josh Durgin]

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

On 09/07/2012 11:19 AM, Alex Elder wrote:
> Josh proposed the following change, and I don't think I could
> explain it any better than he did:
>
>      From: Josh Durgin <josh.durgin@inktank.com>
>      Date: Tue, 24 Jul 2012 14:22:11 -0700
>      To: ceph-devel <ceph-devel@vger.kernel.org>
>      Message-ID: <500F1203.9050605@inktank.com>
>
>      Right now the kernel still has one piece of rbd management
>      duplicated from the rbd command line tool: snapshot creation.
>      There's nothing special about snapshot creation that makes it
>      advantageous to do from the kernel, so I'd like to remove the
>      create_snap sysfs interface.  That is,
> 	/sys/bus/rbd/devices/<id>/create_snap
>      would be removed.
>
>      Does anyone rely on the sysfs interface for creating rbd
>      snapshots?  If so, how hard would it be to replace with:
>
> 	rbd snap create pool/image@snap
>
>      Is there any benefit to the sysfs interface that I'm missing?
>
>      Josh
>
> This patch implements this proposal, removing the code that
> implements the "snap_create" sysfs interface for rbd images.
> As a result, quite a lot of other supporting code goes away.
>
> Suggested-by: Josh Durgin <josh.durgin@inktank.com>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   Documentation/ABI/testing/sysfs-bus-rbd |    6 --
>   drivers/block/rbd.c                     |  158
> -------------------------------
>   2 files changed, 164 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-rbd
> b/Documentation/ABI/testing/sysfs-bus-rbd
> index 7cbbe34..6fe4224 100644
> --- a/Documentation/ABI/testing/sysfs-bus-rbd
> +++ b/Documentation/ABI/testing/sysfs-bus-rbd
> @@ -62,12 +62,6 @@ current_snap
>
>   	The current snapshot for which the device is mapped.
>
> -create_snap
> -
> -	Create a snapshot:
> -
> -	 $ echo <snap-name> > /sys/bus/rbd/devices/<dev-id>/snap_create
> -
>   snap_*
>
>   	A directory per each snapshot
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 5a3132e..d73edb1 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -212,10 +212,6 @@ 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,
> -			    const char *buf,
> -			    size_t count);
>   static void __rbd_remove_snap_dev(struct rbd_snap *snap);
>
>   static ssize_t rbd_add(struct bus_type *bus, const char *buf,
> @@ -1375,71 +1371,6 @@ static int rbd_req_sync_unwatch(struct rbd_device
> *rbd_dev)
>   	return ret;
>   }
>
> -struct rbd_notify_info {
> -	struct rbd_device *rbd_dev;
> -};
> -
> -static void rbd_notify_cb(u64 ver, u64 notify_id, u8 opcode, void *data)
> -{
> -	struct rbd_device *rbd_dev = (struct rbd_device *)data;
> -	if (!rbd_dev)
> -		return;
> -
> -	dout("rbd_notify_cb %s notify_id=%llu opcode=%u\n",
> -			rbd_dev->header_name, (unsigned long long) notify_id,
> -			(unsigned int) opcode);
> -}
> -
> -/*
> - * Request sync osd notify
> - */
> -static int rbd_req_sync_notify(struct rbd_device *rbd_dev)
> -{
> -	struct ceph_osd_req_op *ops;
> -	struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> -	struct ceph_osd_event *event;
> -	struct rbd_notify_info info;
> -	int payload_len = sizeof(u32) + sizeof(u32);
> -	int ret;
> -
> -	ops = rbd_create_rw_ops(1, CEPH_OSD_OP_NOTIFY, payload_len);
> -	if (!ops)
> -		return -ENOMEM;
> -
> -	info.rbd_dev = rbd_dev;
> -
> -	ret = ceph_osdc_create_event(osdc, rbd_notify_cb, 1,
> -				     (void *)&info, &event);
> -	if (ret < 0)
> -		goto fail;
> -
> -	ops[0].watch.ver = 1;
> -	ops[0].watch.flag = 1;
> -	ops[0].watch.cookie = event->cookie;
> -	ops[0].watch.prot_ver = RADOS_NOTIFY_VER;
> -	ops[0].watch.timeout = 12;
> -
> -	ret = rbd_req_sync_op(rbd_dev, NULL,
> -			       CEPH_NOSNAP,
> -			       CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
> -			       ops,
> -			       rbd_dev->header_name,
> -			       0, 0, NULL, NULL, NULL);
> -	if (ret < 0)
> -		goto fail_event;
> -
> -	ret = ceph_osdc_wait_event(event, CEPH_OSD_TIMEOUT_DEFAULT);
> -	dout("ceph_osdc_wait_event returned %d\n", ret);
> -	rbd_destroy_ops(ops);
> -	return 0;
> -
> -fail_event:
> -	ceph_osdc_cancel_event(event);
> -fail:
> -	rbd_destroy_ops(ops);
> -	return ret;
> -}
> -
>   /*
>    * Synchronous osd object method call
>    */
> @@ -1761,52 +1692,6 @@ static int rbd_read_header(struct rbd_device
> *rbd_dev,
>   	return ret;
>   }
>
> -/*
> - * create a snapshot
> - */
> -static int rbd_header_add_snap(struct rbd_device *rbd_dev,
> -			       const char *snap_name,
> -			       gfp_t gfp_flags)
> -{
> -	int name_len = strlen(snap_name);
> -	u64 new_snapid;
> -	int ret;
> -	void *data, *p, *e;
> -	struct ceph_mon_client *monc;
> -
> -	/* we should create a snapshot only if we're pointing at the head */
> -	if (rbd_dev->mapping.snap_id != CEPH_NOSNAP)
> -		return -EINVAL;
> -
> -	monc = &rbd_dev->rbd_client->client->monc;
> -	ret = ceph_monc_create_snapid(monc, rbd_dev->pool_id, &new_snapid);
> -	dout("created snapid=%llu\n", (unsigned long long) new_snapid);
> -	if (ret < 0)
> -		return ret;
> -
> -	data = kmalloc(name_len + 16, gfp_flags);
> -	if (!data)
> -		return -ENOMEM;
> -
> -	p = data;
> -	e = data + name_len + 16;
> -
> -	ceph_encode_string_safe(&p, e, snap_name, name_len, bad);
> -	ceph_encode_64_safe(&p, e, new_snapid, bad);
> -
> -	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> -				"rbd", "snap_add",
> -				data, (size_t) (p - data), NULL, 0,
> -				CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
> -				NULL);
> -
> -	kfree(data);
> -
> -	return ret < 0 ? ret : 0;
> -bad:
> -	return -ERANGE;
> -}
> -
>   static void __rbd_remove_all_snaps(struct rbd_device *rbd_dev)
>   {
>   	struct rbd_snap *snap;
> @@ -2030,7 +1915,6 @@ static DEVICE_ATTR(name, S_IRUGO, rbd_name_show,
> NULL);
>   static DEVICE_ATTR(image_id, S_IRUGO, rbd_image_id_show, NULL);
>   static DEVICE_ATTR(refresh, S_IWUSR, NULL, rbd_image_refresh);
>   static DEVICE_ATTR(current_snap, S_IRUGO, rbd_snap_show, NULL);
> -static DEVICE_ATTR(create_snap, S_IWUSR, NULL, rbd_snap_add);
>
>   static struct attribute *rbd_attrs[] = {
>   	&dev_attr_size.attr,
> @@ -2042,7 +1926,6 @@ static struct attribute *rbd_attrs[] = {
>   	&dev_attr_image_id.attr,
>   	&dev_attr_current_snap.attr,
>   	&dev_attr_refresh.attr,
> -	&dev_attr_create_snap.attr,
>   	NULL
>   };
>
> @@ -2888,47 +2771,6 @@ done:
>   	return ret;
>   }
>
> -static ssize_t rbd_snap_add(struct device *dev,
> -			    struct device_attribute *attr,
> -			    const char *buf,
> -			    size_t count)
> -{
> -	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
> -	int ret;
> -	char *name = kmalloc(count + 1, GFP_KERNEL);
> -	if (!name)
> -		return -ENOMEM;
> -
> -	snprintf(name, count, "%s", buf);
> -
> -	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> -
> -	ret = rbd_header_add_snap(rbd_dev,
> -				  name, GFP_KERNEL);
> -	if (ret < 0)
> -		goto err_unlock;
> -
> -	ret = __rbd_refresh_header(rbd_dev, NULL);
> -	if (ret < 0)
> -		goto err_unlock;
> -
> -	/* shouldn't hold ctl_mutex when notifying.. notify might
> -	   trigger a watch callback that would need to get that mutex */
> -	mutex_unlock(&ctl_mutex);
> -
> -	/* make a best effort, don't error if failed */
> -	rbd_req_sync_notify(rbd_dev);
> -
> -	ret = count;
> -	kfree(name);
> -	return ret;
> -
> -err_unlock:
> -	mutex_unlock(&ctl_mutex);
> -	kfree(name);
> -	return ret;
> -}
> -
>   /*
>    * create control files in sysfs
>    * /sys/bus/rbd/...
>