diff mbox

[4/4] rbd: kill rbd_device->rbd_opts

Message ID 508579A5.7070008@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Elder Oct. 22, 2012, 4:51 p.m. UTC
The rbd_device structure has an embedded rbd_options structure.
Such a structure is needed to work with the generic ceph argument
parsing code, but there's no need to keep it around once argument
parsing is done.

Use a local variable to hold the rbd options used in parsing in
rbd_get_client(), and just transfer its content (it's just a
read_only flag) into the field in the rbd_mapping sub-structure
that requires that information.

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

 		/* using an existing client */
@@ -685,7 +690,6 @@ static int rbd_dev_set_mapping(struct rbd_device
*rbd_dev, char *snap_name)
 		rbd_dev->mapping.size = rbd_dev->header.image_size;
 		rbd_dev->mapping.features = rbd_dev->header.features;
 		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);

Comments

Josh Durgin Oct. 24, 2012, 10:30 p.m. UTC | #1
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 10/22/2012 09:51 AM, Alex Elder wrote:
> The rbd_device structure has an embedded rbd_options structure.
> Such a structure is needed to work with the generic ceph argument
> parsing code, but there's no need to keep it around once argument
> parsing is done.
>
> Use a local variable to hold the rbd options used in parsing in
> rbd_get_client(), and just transfer its content (it's just a
> read_only flag) into the field in the rbd_mapping sub-structure
> that requires that information.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 4ccce4d..3c131ab 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -184,7 +184,6 @@ struct rbd_device {
>   	struct gendisk		*disk;		/* blkdev's gendisk and rq */
>
>   	u32			image_format;	/* Either 1 or 2 */
> -	struct rbd_options	rbd_opts;
>   	struct rbd_client	*rbd_client;
>
>   	char			name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */
> @@ -456,18 +455,24 @@ static int parse_rbd_opts_token(char *c, void
> *private)
>   static int rbd_get_client(struct rbd_device *rbd_dev, const char *mon_addr,
>   				size_t mon_addr_len, char *options)
>   {
> -	struct rbd_options *rbd_opts = &rbd_dev->rbd_opts;
> +	struct rbd_options rbd_opts;
>   	struct ceph_options *ceph_opts;
>   	struct rbd_client *rbdc;
>
> -	rbd_opts->read_only = RBD_READ_ONLY_DEFAULT;
> +	/* Initialize all rbd options to the defaults */
> +
> +	rbd_opts.read_only = RBD_READ_ONLY_DEFAULT;
>
>   	ceph_opts = ceph_parse_options(options, mon_addr,
>   					mon_addr + mon_addr_len,
> -					parse_rbd_opts_token, rbd_opts);
> +					parse_rbd_opts_token, &rbd_opts);
>   	if (IS_ERR(ceph_opts))
>   		return PTR_ERR(ceph_opts);
>
> +	/* Record the parsed rbd options */
> +
> +	rbd_dev->mapping.read_only = rbd_opts.read_only;
> +
>   	rbdc = rbd_client_find(ceph_opts);
>   	if (rbdc) {
>   		/* using an existing client */
> @@ -685,7 +690,6 @@ static int rbd_dev_set_mapping(struct rbd_device
> *rbd_dev, char *snap_name)
>   		rbd_dev->mapping.size = rbd_dev->header.image_size;
>   		rbd_dev->mapping.features = rbd_dev->header.features;
>   		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);
>

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Mick Oct. 24, 2012, 10:37 p.m. UTC | #2
Reviewed-by: Dan Mick <dan.mick@inktank.com>

On 10/22/2012 09:51 AM, Alex Elder wrote:
> The rbd_device structure has an embedded rbd_options structure.
> Such a structure is needed to work with the generic ceph argument
> parsing code, but there's no need to keep it around once argument
> parsing is done.
>
> Use a local variable to hold the rbd options used in parsing in
> rbd_get_client(), and just transfer its content (it's just a
> read_only flag) into the field in the rbd_mapping sub-structure
> that requires that information.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 4ccce4d..3c131ab 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -184,7 +184,6 @@ struct rbd_device {
>   	struct gendisk		*disk;		/* blkdev's gendisk and rq */
>
>   	u32			image_format;	/* Either 1 or 2 */
> -	struct rbd_options	rbd_opts;
>   	struct rbd_client	*rbd_client;
>
>   	char			name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */
> @@ -456,18 +455,24 @@ static int parse_rbd_opts_token(char *c, void
> *private)
>   static int rbd_get_client(struct rbd_device *rbd_dev, const char *mon_addr,
>   				size_t mon_addr_len, char *options)
>   {
> -	struct rbd_options *rbd_opts = &rbd_dev->rbd_opts;
> +	struct rbd_options rbd_opts;
>   	struct ceph_options *ceph_opts;
>   	struct rbd_client *rbdc;
>
> -	rbd_opts->read_only = RBD_READ_ONLY_DEFAULT;
> +	/* Initialize all rbd options to the defaults */
> +
> +	rbd_opts.read_only = RBD_READ_ONLY_DEFAULT;
>
>   	ceph_opts = ceph_parse_options(options, mon_addr,
>   					mon_addr + mon_addr_len,
> -					parse_rbd_opts_token, rbd_opts);
> +					parse_rbd_opts_token, &rbd_opts);
>   	if (IS_ERR(ceph_opts))
>   		return PTR_ERR(ceph_opts);
>
> +	/* Record the parsed rbd options */
> +
> +	rbd_dev->mapping.read_only = rbd_opts.read_only;
> +
>   	rbdc = rbd_client_find(ceph_opts);
>   	if (rbdc) {
>   		/* using an existing client */
> @@ -685,7 +690,6 @@ static int rbd_dev_set_mapping(struct rbd_device
> *rbd_dev, char *snap_name)
>   		rbd_dev->mapping.size = rbd_dev->header.image_size;
>   		rbd_dev->mapping.features = rbd_dev->header.features;
>   		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);
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 4ccce4d..3c131ab 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -184,7 +184,6 @@  struct rbd_device {
 	struct gendisk		*disk;		/* blkdev's gendisk and rq */

 	u32			image_format;	/* Either 1 or 2 */
-	struct rbd_options	rbd_opts;
 	struct rbd_client	*rbd_client;

 	char			name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */
@@ -456,18 +455,24 @@  static int parse_rbd_opts_token(char *c, void
*private)
 static int rbd_get_client(struct rbd_device *rbd_dev, const char *mon_addr,
 				size_t mon_addr_len, char *options)
 {
-	struct rbd_options *rbd_opts = &rbd_dev->rbd_opts;
+	struct rbd_options rbd_opts;
 	struct ceph_options *ceph_opts;
 	struct rbd_client *rbdc;

-	rbd_opts->read_only = RBD_READ_ONLY_DEFAULT;
+	/* Initialize all rbd options to the defaults */
+
+	rbd_opts.read_only = RBD_READ_ONLY_DEFAULT;

 	ceph_opts = ceph_parse_options(options, mon_addr,
 					mon_addr + mon_addr_len,
-					parse_rbd_opts_token, rbd_opts);
+					parse_rbd_opts_token, &rbd_opts);
 	if (IS_ERR(ceph_opts))
 		return PTR_ERR(ceph_opts);

+	/* Record the parsed rbd options */
+
+	rbd_dev->mapping.read_only = rbd_opts.read_only;
+
 	rbdc = rbd_client_find(ceph_opts);
 	if (rbdc) {