* [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec()
2012-09-07 18:16 [PATCH 0/5] rbd: kill create_snap sysfs entry Alex Elder
@ 2012-09-07 18:19 ` Alex Elder
2012-09-11 15:14 ` Josh Durgin
2012-09-07 18:19 ` [PATCH 2/5] rbd: support data returned from OSD methods Alex Elder
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2012-09-07 18:19 UTC (permalink / raw)
To: ceph-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec()
2012-09-07 18:19 ` [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec() Alex Elder
@ 2012-09-11 15:14 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2012-09-11 15:14 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] rbd: support data returned from OSD methods
2012-09-07 18:16 [PATCH 0/5] rbd: kill create_snap sysfs entry Alex Elder
2012-09-07 18:19 ` [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec() Alex Elder
@ 2012-09-07 18:19 ` Alex Elder
2012-09-11 15:16 ` Josh Durgin
2012-09-07 18:19 ` [PATCH 3/5] rbd: define some new format constants Alex Elder
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2012-09-07 18:19 UTC (permalink / raw)
To: ceph-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] rbd: support data returned from OSD methods
2012-09-07 18:19 ` [PATCH 2/5] rbd: support data returned from OSD methods Alex Elder
@ 2012-09-11 15:16 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2012-09-11 15:16 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] rbd: define some new format constants
2012-09-07 18:16 [PATCH 0/5] rbd: kill create_snap sysfs entry Alex Elder
2012-09-07 18:19 ` [PATCH 1/5] rbd: pass flags to rbd_req_sync_exec() Alex Elder
2012-09-07 18:19 ` [PATCH 2/5] rbd: support data returned from OSD methods Alex Elder
@ 2012-09-07 18:19 ` Alex Elder
2012-09-11 15:18 ` Josh Durgin
2012-09-07 18:19 ` [PATCH 4/5] rbd: define rbd_dev_image_id() Alex Elder
2012-09-07 18:19 ` [PATCH 5/5] rbd: kill create_snap sysfs entry Alex Elder
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2012-09-07 18:19 UTC (permalink / raw)
To: ceph-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] rbd: define some new format constants
2012-09-07 18:19 ` [PATCH 3/5] rbd: define some new format constants Alex Elder
@ 2012-09-11 15:18 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2012-09-11 15:18 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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"
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] rbd: define rbd_dev_image_id()
2012-09-07 18:16 [PATCH 0/5] rbd: kill create_snap sysfs entry Alex Elder
` (2 preceding siblings ...)
2012-09-07 18:19 ` [PATCH 3/5] rbd: define some new format constants Alex Elder
@ 2012-09-07 18:19 ` Alex Elder
2012-09-11 15:50 ` Josh Durgin
2012-09-07 18:19 ` [PATCH 5/5] rbd: kill create_snap sysfs entry Alex Elder
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2012-09-07 18:19 UTC (permalink / raw)
To: ceph-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/5] rbd: define rbd_dev_image_id()
2012-09-07 18:19 ` [PATCH 4/5] rbd: define rbd_dev_image_id() Alex Elder
@ 2012-09-11 15:50 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2012-09-11 15:50 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] rbd: kill create_snap sysfs entry
2012-09-07 18:16 [PATCH 0/5] rbd: kill create_snap sysfs entry Alex Elder
` (3 preceding siblings ...)
2012-09-07 18:19 ` [PATCH 4/5] rbd: define rbd_dev_image_id() Alex Elder
@ 2012-09-07 18:19 ` Alex Elder
2012-09-11 20:53 ` Josh Durgin
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2012-09-07 18:19 UTC (permalink / raw)
To: ceph-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] rbd: kill create_snap sysfs entry
2012-09-07 18:19 ` [PATCH 5/5] rbd: kill create_snap sysfs entry Alex Elder
@ 2012-09-11 20:53 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2012-09-11 20:53 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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/...
>
^ permalink raw reply [flat|nested] 11+ messages in thread