[PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Peter Jones]

This makes it possible to use DM_DEV_RENAME to add a uuid to a device so
long as one has not been previously set either with DM_DEV_CREATE or
with DM_DEV_RENAME.
---
 drivers/md/dm-ioctl.c    |  113 ++++++++++++++++++++++++++++++++-------------
 include/linux/dm-ioctl.h |    5 ++
 2 files changed, 85 insertions(+), 33 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 3e39193..bd76846 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -298,7 +298,7 @@ retry:
 static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 					    const char *new)
 {
-	char *new_name, *old_name;
+	char *new_data, *old_data;
 	struct hash_cell *hc;
 	struct dm_table *table;
 	struct mapped_device *md;
@@ -306,8 +306,8 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * duplicate new.
 	 */
-	new_name = kstrdup(new, GFP_KERNEL);
-	if (!new_name)
+	new_data = kstrdup(new, GFP_KERNEL);
+	if (!new_data)
 		return ERR_PTR(-ENOMEM);
 
 	down_write(&_hash_lock);
@@ -315,14 +315,28 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * Is new free ?
 	 */
-	hc = __get_name_cell(new);
-	if (hc) {
-		DMWARN("asked to rename to an already-existing name %s -> %s",
-		       param->name, new);
-		dm_put(hc->md);
-		up_write(&_hash_lock);
-		kfree(new_name);
-		return ERR_PTR(-EBUSY);
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		hc = __get_uuid_cell(new);
+		if (hc) {
+			DMWARN("asked to change uuid to an already-existing uuid"
+			       " %s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
+	} else {
+		hc = __get_name_cell(new);
+		if (hc) {
+			DMWARN("asked to rename to an already-existing name "
+			       "%s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
 	}
 
 	/*
@@ -333,19 +347,42 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		DMWARN("asked to rename a non-existent device %s -> %s",
 		       param->name, new);
 		up_write(&_hash_lock);
-		kfree(new_name);
+		kfree(new_data);
 		return ERR_PTR(-ENXIO);
 	}
 
-	/*
-	 * rename and move the name cell.
-	 */
-	list_del(&hc->name_list);
-	old_name = hc->name;
-	mutex_lock(&dm_hash_cells_mutex);
-	hc->name = new_name;
-	mutex_unlock(&dm_hash_cells_mutex);
-	list_add(&hc->name_list, _name_buckets + hash_str(new_name));
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		/*
+		 * Does this device already have a uuid?
+		 */
+		if (hc->uuid) {
+			DMWARN("asked to change uuid of device with uuid "
+			       "already set %s %s -> %s",
+			       param->name, hc->uuid, param->uuid);
+			up_write(&_hash_lock);
+			kfree(new_uuid);
+			return ERR_PTR(-EINVAL);
+		}
+		/*
+		 * reuuid and move the uuid cell.
+		 */
+		list_del(&hc->uuid_list);
+		old_data = hc->uuid;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->uuid = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->uuid_list, _uuid_buckets + hash_str(new_data));
+	} else {
+		/*
+		 * rename and move the name cell.
+		 */
+		list_del(&hc->name_list);
+		old_data = hc->name;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->name = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->name_list, _name_buckets + hash_str(new_data));
+	}
 
 	/*
 	 * Wake up any dm event waiters.
@@ -361,7 +398,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 
 	md = hc->md;
 	up_write(&_hash_lock);
-	kfree(old_name);
+	kfree(old_data);
 
 	return md;
 }
@@ -774,21 +811,31 @@ static int invalid_str(char *str, void *end)
 static int dev_rename(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
-	char *new_name = (char *) param + param->data_start;
+	char *new_data = (char *) param + param->data_start;
 	struct mapped_device *md;
 
-	if (new_name < param->data ||
-	    invalid_str(new_name, (void *) param + param_size) ||
-	    strlen(new_name) > DM_NAME_LEN - 1) {
-		DMWARN("Invalid new logical volume name supplied.");
-		return -EINVAL;
-	}
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		struct hash_cell *hc;
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_UUID_LEN - 1) {
+			DMWARN("Invalid new logical volume uuid supplied.");
+			return -EINVAL;
+		}
+	} else {
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_NAME_LEN - 1) {
+			DMWARN("Invalid new logical volume name supplied.");
+			return -EINVAL;
+		}
 
-	r = check_name(new_name);
-	if (r)
-		return r;
+		r = check_name(new_data);
+		if (r)
+			return r;
+	}
 
-	md = dm_hash_rename(param, new_name);
+	md = dm_hash_rename(param, new_data);
 	if (IS_ERR(md))
 		return PTR_ERR(md);
 
diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h
index 49eab36..8f1a610 100644
--- a/include/linux/dm-ioctl.h
+++ b/include/linux/dm-ioctl.h
@@ -322,4 +322,9 @@ enum {
  */
 #define DM_UEVENT_GENERATED_FLAG	(1 << 13) /* Out */
 
+/*
+ * If set, rename operates on uuid, not name.
+ */
+#define DM_NEW_UUID_FLAG        (1 << 14) /* In */
+
 #endif				/* _LINUX_DM_IOCTL_H */
-- 
1.7.2.2

[PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Peter Jones]

This makes it possible to use DM_DEV_RENAME to add a uuid to a device so
long as one has not been previously set either with DM_DEV_CREATE or
with DM_DEV_RENAME.

Also bump the minor number to 19.
---
 drivers/md/dm-ioctl.c    |  113 ++++++++++++++++++++++++++++++++-------------
 include/linux/dm-ioctl.h |    9 +++-
 2 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 3e39193..bd76846 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -298,7 +298,7 @@ retry:
 static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 					    const char *new)
 {
-	char *new_name, *old_name;
+	char *new_data, *old_data;
 	struct hash_cell *hc;
 	struct dm_table *table;
 	struct mapped_device *md;
@@ -306,8 +306,8 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * duplicate new.
 	 */
-	new_name = kstrdup(new, GFP_KERNEL);
-	if (!new_name)
+	new_data = kstrdup(new, GFP_KERNEL);
+	if (!new_data)
 		return ERR_PTR(-ENOMEM);
 
 	down_write(&_hash_lock);
@@ -315,14 +315,28 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * Is new free ?
 	 */
-	hc = __get_name_cell(new);
-	if (hc) {
-		DMWARN("asked to rename to an already-existing name %s -> %s",
-		       param->name, new);
-		dm_put(hc->md);
-		up_write(&_hash_lock);
-		kfree(new_name);
-		return ERR_PTR(-EBUSY);
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		hc = __get_uuid_cell(new);
+		if (hc) {
+			DMWARN("asked to change uuid to an already-existing uuid"
+			       " %s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
+	} else {
+		hc = __get_name_cell(new);
+		if (hc) {
+			DMWARN("asked to rename to an already-existing name "
+			       "%s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
 	}
 
 	/*
@@ -333,19 +347,42 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		DMWARN("asked to rename a non-existent device %s -> %s",
 		       param->name, new);
 		up_write(&_hash_lock);
-		kfree(new_name);
+		kfree(new_data);
 		return ERR_PTR(-ENXIO);
 	}
 
-	/*
-	 * rename and move the name cell.
-	 */
-	list_del(&hc->name_list);
-	old_name = hc->name;
-	mutex_lock(&dm_hash_cells_mutex);
-	hc->name = new_name;
-	mutex_unlock(&dm_hash_cells_mutex);
-	list_add(&hc->name_list, _name_buckets + hash_str(new_name));
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		/*
+		 * Does this device already have a uuid?
+		 */
+		if (hc->uuid) {
+			DMWARN("asked to change uuid of device with uuid "
+			       "already set %s %s -> %s",
+			       param->name, hc->uuid, param->uuid);
+			up_write(&_hash_lock);
+			kfree(new_uuid);
+			return ERR_PTR(-EINVAL);
+		}
+		/*
+		 * reuuid and move the uuid cell.
+		 */
+		list_del(&hc->uuid_list);
+		old_data = hc->uuid;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->uuid = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->uuid_list, _uuid_buckets + hash_str(new_data));
+	} else {
+		/*
+		 * rename and move the name cell.
+		 */
+		list_del(&hc->name_list);
+		old_data = hc->name;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->name = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->name_list, _name_buckets + hash_str(new_data));
+	}
 
 	/*
 	 * Wake up any dm event waiters.
@@ -361,7 +398,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 
 	md = hc->md;
 	up_write(&_hash_lock);
-	kfree(old_name);
+	kfree(old_data);
 
 	return md;
 }
@@ -774,21 +811,31 @@ static int invalid_str(char *str, void *end)
 static int dev_rename(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
-	char *new_name = (char *) param + param->data_start;
+	char *new_data = (char *) param + param->data_start;
 	struct mapped_device *md;
 
-	if (new_name < param->data ||
-	    invalid_str(new_name, (void *) param + param_size) ||
-	    strlen(new_name) > DM_NAME_LEN - 1) {
-		DMWARN("Invalid new logical volume name supplied.");
-		return -EINVAL;
-	}
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		struct hash_cell *hc;
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_UUID_LEN - 1) {
+			DMWARN("Invalid new logical volume uuid supplied.");
+			return -EINVAL;
+		}
+	} else {
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_NAME_LEN - 1) {
+			DMWARN("Invalid new logical volume name supplied.");
+			return -EINVAL;
+		}
 
-	r = check_name(new_name);
-	if (r)
-		return r;
+		r = check_name(new_data);
+		if (r)
+			return r;
+	}
 
-	md = dm_hash_rename(param, new_name);
+	md = dm_hash_rename(param, new_data);
 	if (IS_ERR(md))
 		return PTR_ERR(md);
 
diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h
index 49eab36..e369460 100644
--- a/include/linux/dm-ioctl.h
+++ b/include/linux/dm-ioctl.h
@@ -267,9 +267,9 @@ enum {
 #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
 
 #define DM_VERSION_MAJOR	4
-#define DM_VERSION_MINOR	18
+#define DM_VERSION_MINOR	19
 #define DM_VERSION_PATCHLEVEL	0
-#define DM_VERSION_EXTRA	"-ioctl (2010-06-29)"
+#define DM_VERSION_EXTRA	"-ioctl (2010-10-07)"
 
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
@@ -322,4 +322,9 @@ enum {
  */
 #define DM_UEVENT_GENERATED_FLAG	(1 << 13) /* Out */
 
+/*
+ * If set, rename operates on uuid, not name.
+ */
+#define DM_NEW_UUID_FLAG        (1 << 14) /* In */
+
 #endif				/* _LINUX_DM_IOCTL_H */
-- 
1.7.2.2

Re: [PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Jonathan Brassow]

Minor nits below

  brassow

On Oct 7, 2010, at 2:28 PM, Peter Jones wrote:

> This makes it possible to use DM_DEV_RENAME to add a uuid to a  
> device so
> long as one has not been previously set either with DM_DEV_CREATE or
> with DM_DEV_RENAME.

Seems like a sensible thing.  Is there a reason this is coming up now?

>
> Also bump the minor number to 19.
> ---
> drivers/md/dm-ioctl.c    |  113 +++++++++++++++++++++++++++++++ 
> +-------------
> include/linux/dm-ioctl.h |    9 +++-
> 2 files changed, 87 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 3e39193..bd76846 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -298,7 +298,7 @@ retry:
> static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
>

<snip>

> 	/*
> @@ -333,19 +347,42 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
> 		DMWARN("asked to rename a non-existent device %s -> %s",
> 		       param->name, new);
> 		up_write(&_hash_lock);
> -		kfree(new_name);
> +		kfree(new_data);
> 		return ERR_PTR(-ENXIO);
> 	}
>
> -	/*
> -	 * rename and move the name cell.
> -	 */
> -	list_del(&hc->name_list);
> -	old_name = hc->name;
> -	mutex_lock(&dm_hash_cells_mutex);
> -	hc->name = new_name;
> -	mutex_unlock(&dm_hash_cells_mutex);
> -	list_add(&hc->name_list, _name_buckets + hash_str(new_name));
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		/*
> +		 * Does this device already have a uuid?
> +		 */
> +		if (hc->uuid) {
> +			DMWARN("asked to change uuid of device with uuid "
> +			       "already set %s %s -> %s",
> +			       param->name, hc->uuid, param->uuid);

Message doesn't imply this is illegal... Perhaps "Unable to change...  
set"?

> +			up_write(&_hash_lock);
> +			kfree(new_uuid);

Will patch compile?  s/new_uuid/new_data/

> +			return ERR_PTR(-EINVAL);
> +		}
> +		/*
> +		 * reuuid and move the uuid cell.
> +		 */
> +		list_del(&hc->uuid_list);
> +		old_data = hc->uuid;
> +		mutex_lock(&dm_hash_cells_mutex);
> +		hc->uuid = new_data;
> +		mutex_unlock(&dm_hash_cells_mutex);
> +		list_add(&hc->uuid_list, _uuid_buckets + hash_str(new_data));
> +	} else {
> +		/*
> +		 * rename and move the name cell.
> +		 */
> +		list_del(&hc->name_list);
> +		old_data = hc->name;
> +		mutex_lock(&dm_hash_cells_mutex);
> +		hc->name = new_data;
> +		mutex_unlock(&dm_hash_cells_mutex);
> +		list_add(&hc->name_list, _name_buckets + hash_str(new_data));
> +	}
>
> 	/*
> 	 * Wake up any dm event waiters.
> @@ -361,7 +398,7 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
>
> 	md = hc->md;
> 	up_write(&_hash_lock);
> -	kfree(old_name);
> +	kfree(old_data);
>
> 	return md;
> }
> @@ -774,21 +811,31 @@ static int invalid_str(char *str, void *end)
> static int dev_rename(struct dm_ioctl *param, size_t param_size)
> {
> 	int r;
> -	char *new_name = (char *) param + param->data_start;
> +	char *new_data = (char *) param + param->data_start;
> 	struct mapped_device *md;
>
> -	if (new_name < param->data ||
> -	    invalid_str(new_name, (void *) param + param_size) ||
> -	    strlen(new_name) > DM_NAME_LEN - 1) {
> -		DMWARN("Invalid new logical volume name supplied.");
> -		return -EINVAL;
> -	}
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		struct hash_cell *hc;

Is this newly declared variable needed?

> +		if (new_data < param->data ||
> +		    invalid_str(new_data, (void *) param + param_size) ||
> +		    strlen(new_data) > DM_UUID_LEN - 1) {
> +			DMWARN("Invalid new logical volume uuid supplied.");

I wonder why we were using "logical volume" instead of "device" like  
everywhere else... can we change it now?

> +			return -EINVAL;
> +		}
> +	} else {
> +		if (new_data < param->data ||
> +		    invalid_str(new_data, (void *) param + param_size) ||
> +		    strlen(new_data) > DM_NAME_LEN - 1) {
> +			DMWARN("Invalid new logical volume name supplied.");

s/logical volume/device/

> +			return -EINVAL;
> +		}
>
> -	r = check_name(new_name);
> -	if (r)
> -		return r;
> +		r = check_name(new_data);
> +		if (r)
> +			return r;
> +	}
>
> -	md = dm_hash_rename(param, new_name);
> +	md = dm_hash_rename(param, new_data);
> 	if (IS_ERR(md))
> 		return PTR_ERR(md);
>
> diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h
> index 49eab36..e369460 100644
> --- a/include/linux/dm-ioctl.h
> +++ b/include/linux/dm-ioctl.h
> @@ -267,9 +267,9 @@ enum {
> #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD,  
> struct dm_ioctl)
>
> #define DM_VERSION_MAJOR	4
> -#define DM_VERSION_MINOR	18
> +#define DM_VERSION_MINOR	19
> #define DM_VERSION_PATCHLEVEL	0
> -#define DM_VERSION_EXTRA	"-ioctl (2010-06-29)"
> +#define DM_VERSION_EXTRA	"-ioctl (2010-10-07)"
>
> /* Status bits */
> #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
> @@ -322,4 +322,9 @@ enum {
>  */
> #define DM_UEVENT_GENERATED_FLAG	(1 << 13) /* Out */
>
> +/*
> + * If set, rename operates on uuid, not name.
> + */
> +#define DM_NEW_UUID_FLAG        (1 << 14) /* In */
> +
> #endif				/* _LINUX_DM_IOCTL_H */
> -- 
> 1.7.2.2
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Peter Jones]

This makes it possible to use DM_DEV_RENAME to add a uuid to a device so
long as one has not been previously set either with DM_DEV_CREATE or
with DM_DEV_RENAME. This is proposed as a fix to rhbz#584328 .

Also bump the minor number to 19.

Includes changes suggested by jbrassow.
---
 drivers/md/dm-ioctl.c    |  112 ++++++++++++++++++++++++++++++++-------------
 include/linux/dm-ioctl.h |    9 +++-
 2 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 3e39193..450dfe3 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -298,7 +298,7 @@ retry:
 static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 					    const char *new)
 {
-	char *new_name, *old_name;
+	char *new_data, *old_data;
 	struct hash_cell *hc;
 	struct dm_table *table;
 	struct mapped_device *md;
@@ -306,8 +306,8 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * duplicate new.
 	 */
-	new_name = kstrdup(new, GFP_KERNEL);
-	if (!new_name)
+	new_data = kstrdup(new, GFP_KERNEL);
+	if (!new_data)
 		return ERR_PTR(-ENOMEM);
 
 	down_write(&_hash_lock);
@@ -315,14 +315,28 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	/*
 	 * Is new free ?
 	 */
-	hc = __get_name_cell(new);
-	if (hc) {
-		DMWARN("asked to rename to an already-existing name %s -> %s",
-		       param->name, new);
-		dm_put(hc->md);
-		up_write(&_hash_lock);
-		kfree(new_name);
-		return ERR_PTR(-EBUSY);
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		hc = __get_uuid_cell(new);
+		if (hc) {
+			DMWARN("Unable to change uuid on device with an "
+			       "already-existing uuid %s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
+	} else {
+		hc = __get_name_cell(new);
+		if (hc) {
+			DMWARN("asked to rename to an already-existing name "
+			       "%s -> %s",
+			       param->name, new);
+			dm_put(hc->md);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EBUSY);
+		}
 	}
 
 	/*
@@ -333,19 +347,42 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		DMWARN("asked to rename a non-existent device %s -> %s",
 		       param->name, new);
 		up_write(&_hash_lock);
-		kfree(new_name);
+		kfree(new_data);
 		return ERR_PTR(-ENXIO);
 	}
 
-	/*
-	 * rename and move the name cell.
-	 */
-	list_del(&hc->name_list);
-	old_name = hc->name;
-	mutex_lock(&dm_hash_cells_mutex);
-	hc->name = new_name;
-	mutex_unlock(&dm_hash_cells_mutex);
-	list_add(&hc->name_list, _name_buckets + hash_str(new_name));
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		/*
+		 * Does this device already have a uuid?
+		 */
+		if (hc->uuid) {
+			DMWARN("asked to change uuid of device with uuid "
+			       "already set %s %s -> %s",
+			       param->name, hc->uuid, param->uuid);
+			up_write(&_hash_lock);
+			kfree(new_data);
+			return ERR_PTR(-EINVAL);
+		}
+		/*
+		 * reuuid and move the uuid cell.
+		 */
+		list_del(&hc->uuid_list);
+		old_data = hc->uuid;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->uuid = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->uuid_list, _uuid_buckets + hash_str(new_data));
+	} else {
+		/*
+		 * rename and move the name cell.
+		 */
+		list_del(&hc->name_list);
+		old_data = hc->name;
+		mutex_lock(&dm_hash_cells_mutex);
+		hc->name = new_data;
+		mutex_unlock(&dm_hash_cells_mutex);
+		list_add(&hc->name_list, _name_buckets + hash_str(new_data));
+	}
 
 	/*
 	 * Wake up any dm event waiters.
@@ -361,7 +398,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 
 	md = hc->md;
 	up_write(&_hash_lock);
-	kfree(old_name);
+	kfree(old_data);
 
 	return md;
 }
@@ -774,21 +811,30 @@ static int invalid_str(char *str, void *end)
 static int dev_rename(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
-	char *new_name = (char *) param + param->data_start;
+	char *new_data = (char *) param + param->data_start;
 	struct mapped_device *md;
 
-	if (new_name < param->data ||
-	    invalid_str(new_name, (void *) param + param_size) ||
-	    strlen(new_name) > DM_NAME_LEN - 1) {
-		DMWARN("Invalid new logical volume name supplied.");
-		return -EINVAL;
-	}
+	if (param->flags & DM_NEW_UUID_FLAG) {
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_UUID_LEN - 1) {
+			DMWARN("Invalid new device uuid supplied.");
+			return -EINVAL;
+		}
+	} else {
+		if (new_data < param->data ||
+		    invalid_str(new_data, (void *) param + param_size) ||
+		    strlen(new_data) > DM_NAME_LEN - 1) {
+			DMWARN("Invalid new device name supplied.");
+			return -EINVAL;
+		}
 
-	r = check_name(new_name);
-	if (r)
-		return r;
+		r = check_name(new_data);
+		if (r)
+			return r;
+	}
 
-	md = dm_hash_rename(param, new_name);
+	md = dm_hash_rename(param, new_data);
 	if (IS_ERR(md))
 		return PTR_ERR(md);
 
diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h
index 49eab36..e369460 100644
--- a/include/linux/dm-ioctl.h
+++ b/include/linux/dm-ioctl.h
@@ -267,9 +267,9 @@ enum {
 #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
 
 #define DM_VERSION_MAJOR	4
-#define DM_VERSION_MINOR	18
+#define DM_VERSION_MINOR	19
 #define DM_VERSION_PATCHLEVEL	0
-#define DM_VERSION_EXTRA	"-ioctl (2010-06-29)"
+#define DM_VERSION_EXTRA	"-ioctl (2010-10-07)"
 
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
@@ -322,4 +322,9 @@ enum {
  */
 #define DM_UEVENT_GENERATED_FLAG	(1 << 13) /* Out */
 
+/*
+ * If set, rename operates on uuid, not name.
+ */
+#define DM_NEW_UUID_FLAG        (1 << 14) /* In */
+
 #endif				/* _LINUX_DM_IOCTL_H */
-- 
1.7.2.2

Re: [PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Jonathan Brassow]

[-- Attachment #1.1: Type: text/plain, Size: 6280 bytes --]


On Oct 8, 2010, at 12:53 PM, Peter Jones wrote:

> This makes it possible to use DM_DEV_RENAME to add a uuid to a  
> device so
> long as one has not been previously set either with DM_DEV_CREATE or
> with DM_DEV_RENAME. This is proposed as a fix to rhbz#584328 .
>
> Also bump the minor number to 19.
>
> Includes changes suggested by jbrassow.
> ---
> drivers/md/dm-ioctl.c    |  112 +++++++++++++++++++++++++++++++ 
> +-------------
> include/linux/dm-ioctl.h |    9 +++-
> 2 files changed, 86 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 3e39193..450dfe3 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -298,7 +298,7 @@ retry:
> static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
> 					    const char *new)
> {
> -	char *new_name, *old_name;
> +	char *new_data, *old_data;
> 	struct hash_cell *hc;
> 	struct dm_table *table;
> 	struct mapped_device *md;
> @@ -306,8 +306,8 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
> 	/*
> 	 * duplicate new.
> 	 */
> -	new_name = kstrdup(new, GFP_KERNEL);
> -	if (!new_name)
> +	new_data = kstrdup(new, GFP_KERNEL);
> +	if (!new_data)
> 		return ERR_PTR(-ENOMEM);
>
> 	down_write(&_hash_lock);
> @@ -315,14 +315,28 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
> 	/*
> 	 * Is new free ?
> 	 */
> -	hc = __get_name_cell(new);
> -	if (hc) {
> -		DMWARN("asked to rename to an already-existing name %s -> %s",
> -		       param->name, new);
> -		dm_put(hc->md);
> -		up_write(&_hash_lock);
> -		kfree(new_name);
> -		return ERR_PTR(-EBUSY);
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		hc = __get_uuid_cell(new);
> +		if (hc) {
> +			DMWARN("Unable to change uuid on device with an "
> +			       "already-existing uuid %s -> %s",
> +			       param->name, new);
> +			dm_put(hc->md);
> +			up_write(&_hash_lock);
> +			kfree(new_data);
> +			return ERR_PTR(-EBUSY);
> +		}
> +	} else {
> +		hc = __get_name_cell(new);
> +		if (hc) {
> +			DMWARN("asked to rename to an already-existing name "
> +			       "%s -> %s",
> +			       param->name, new);
> +			dm_put(hc->md);
> +			up_write(&_hash_lock);
> +			kfree(new_data);
> +			return ERR_PTR(-EBUSY);
> +		}
> 	}

>
> 	/*
> @@ -333,19 +347,42 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
> 		DMWARN("asked to rename a non-existent device %s -> %s",
> 		       param->name, new);
> 		up_write(&_hash_lock);
> -		kfree(new_name);
> +		kfree(new_data);
> 		return ERR_PTR(-ENXIO);
> 	}
>
> -	/*
> -	 * rename and move the name cell.
> -	 */
> -	list_del(&hc->name_list);
> -	old_name = hc->name;
> -	mutex_lock(&dm_hash_cells_mutex);
> -	hc->name = new_name;
> -	mutex_unlock(&dm_hash_cells_mutex);
> -	list_add(&hc->name_list, _name_buckets + hash_str(new_name));
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		/*
> +		 * Does this device already have a uuid?
> +		 */
> +		if (hc->uuid) {
> +			DMWARN("asked to change uuid of device with uuid "
> +			       "already set %s %s -> %s",
> +			       param->name, hc->uuid, param->uuid);
> +			up_write(&_hash_lock);
> +			kfree(new_data);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		/*
> +		 * reuuid and move the uuid cell.
> +		 */
> +		list_del(&hc->uuid_list);
> +		old_data = hc->uuid;
> +		mutex_lock(&dm_hash_cells_mutex);
> +		hc->uuid = new_data;
> +		mutex_unlock(&dm_hash_cells_mutex);
> +		list_add(&hc->uuid_list, _uuid_buckets + hash_str(new_data));
> +	} else {
> +		/*
> +		 * rename and move the name cell.
> +		 */
> +		list_del(&hc->name_list);
> +		old_data = hc->name;
> +		mutex_lock(&dm_hash_cells_mutex);
> +		hc->name = new_data;
> +		mutex_unlock(&dm_hash_cells_mutex);
> +		list_add(&hc->name_list, _name_buckets + hash_str(new_data));
> +	}
>
> 	/*
> 	 * Wake up any dm event waiters.
> @@ -361,7 +398,7 @@ static struct mapped_device  
> *dm_hash_rename(struct dm_ioctl *param,
>
> 	md = hc->md;
> 	up_write(&_hash_lock);
> -	kfree(old_name);
> +	kfree(old_data);
>
> 	return md;
> }
> @@ -774,21 +811,30 @@ static int invalid_str(char *str, void *end)
> static int dev_rename(struct dm_ioctl *param, size_t param_size)
> {
> 	int r;
> -	char *new_name = (char *) param + param->data_start;
> +	char *new_data = (char *) param + param->data_start;
> 	struct mapped_device *md;
>
> -	if (new_name < param->data ||
> -	    invalid_str(new_name, (void *) param + param_size) ||
> -	    strlen(new_name) > DM_NAME_LEN - 1) {
> -		DMWARN("Invalid new logical volume name supplied.");
> -		return -EINVAL;
> -	}
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		if (new_data < param->data ||
> +		    invalid_str(new_data, (void *) param + param_size) ||
> +		    strlen(new_data) > DM_UUID_LEN - 1) {
> +			DMWARN("Invalid new device uuid supplied.");
> +			return -EINVAL;
> +		}
> +	} else {
> +		if (new_data < param->data ||
> +		    invalid_str(new_data, (void *) param + param_size) ||
> +		    strlen(new_data) > DM_NAME_LEN - 1) {
> +			DMWARN("Invalid new device name supplied.");
> +			return -EINVAL;
> +		}
>
> -	r = check_name(new_name);
> -	if (r)
> -		return r;
> +		r = check_name(new_data);
> +		if (r)
> +			return r;
> +	}
>
> -	md = dm_hash_rename(param, new_name);
> +	md = dm_hash_rename(param, new_data);
> 	if (IS_ERR(md))
> 		return PTR_ERR(md);
>
> diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h
> index 49eab36..e369460 100644
> --- a/include/linux/dm-ioctl.h
> +++ b/include/linux/dm-ioctl.h
> @@ -267,9 +267,9 @@ enum {
> #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD,  
> struct dm_ioctl)
>
> #define DM_VERSION_MAJOR	4
> -#define DM_VERSION_MINOR	18
> +#define DM_VERSION_MINOR	19
> #define DM_VERSION_PATCHLEVEL	0
> -#define DM_VERSION_EXTRA	"-ioctl (2010-06-29)"
> +#define DM_VERSION_EXTRA	"-ioctl (2010-10-07)"
>
> /* Status bits */
> #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
> @@ -322,4 +322,9 @@ enum {
>  */
> #define DM_UEVENT_GENERATED_FLAG	(1 << 13) /* Out */
>
> +/*
> + * If set, rename operates on uuid, not name.
> + */
> +#define DM_NEW_UUID_FLAG        (1 << 14) /* In */
> +
> #endif				/* _LINUX_DM_IOCTL_H */
> -- 
> 1.7.2.2
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


[-- Attachment #1.2: Type: text/html, Size: 23350 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Mike Snitzer]

On Fri, Oct 08 2010 at  1:53pm -0400,
Peter Jones <pjones@redhat.com> wrote:

> This makes it possible to use DM_DEV_RENAME to add a uuid to a device so
> long as one has not been previously set either with DM_DEV_CREATE or
> with DM_DEV_RENAME. This is proposed as a fix to rhbz#584328 .
> 
> Also bump the minor number to 19.

We still need a better patch header.  I'll look at the BZ and draft one.

But here is a follow-on patch that I think we should fold ontop of this
latest patch.

I basically just cleaned up dm_hash_rename a bit.  Added "Unable to"
consistency to DMWARN messages.  And eliminated a bit of duplicate code
in the first hunk of dm_hash_rename.

Peter, please advise.

Thanks,
Mike

---
 drivers/md/dm-ioctl.c |   44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

Index: linux-2.6/drivers/md/dm-ioctl.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-ioctl.c
+++ linux-2.6/drivers/md/dm-ioctl.c
@@ -315,28 +315,24 @@ static struct mapped_device *dm_hash_ren
 	/*
 	 * Is new free ?
 	 */
-	if (param->flags & DM_NEW_UUID_FLAG) {
+	if (param->flags & DM_NEW_UUID_FLAG)
 		hc = __get_uuid_cell(new);
-		if (hc) {
-			DMWARN("Unable to change uuid on device with an "
-			       "already-existing uuid %s -> %s",
-			       param->name, new);
-			dm_put(hc->md);
-			up_write(&_hash_lock);
-			kfree(new_data);
-			return ERR_PTR(-EBUSY);
-		}
-	} else {
+	else
 		hc = __get_name_cell(new);
-		if (hc) {
-			DMWARN("asked to rename to an already-existing name "
-			       "%s -> %s",
+
+	if (hc) {
+		if (param->flags & DM_NEW_UUID_FLAG)
+			DMWARN("Unable to change uuid on device %s to an "
+			       "already-existing uuid %s",
 			       param->name, new);
-			dm_put(hc->md);
-			up_write(&_hash_lock);
-			kfree(new_data);
-			return ERR_PTR(-EBUSY);
-		}
+		else
+			DMWARN("Unable to rename device %s to an "
+			       "already-existing name %s",
+			       param->name, new);
+		dm_put(hc->md);
+		up_write(&_hash_lock);
+		kfree(new_data);
+		return ERR_PTR(-EBUSY);
 	}
 
 	/*
@@ -344,7 +340,7 @@ static struct mapped_device *dm_hash_ren
 	 */
 	hc = __get_name_cell(param->name);
 	if (!hc) {
-		DMWARN("asked to rename a non-existent device %s -> %s",
+		DMWARN("Unable to rename a non-existent device %s to %s",
 		       param->name, new);
 		up_write(&_hash_lock);
 		kfree(new_data);
@@ -356,15 +352,15 @@ static struct mapped_device *dm_hash_ren
 		 * Does this device already have a uuid?
 		 */
 		if (hc->uuid) {
-			DMWARN("asked to change uuid of device with uuid "
-			       "already set %s %s -> %s",
-			       param->name, hc->uuid, param->uuid);
+			DMWARN("Unable to change uuid of device %s because "
+			       "its uuid is already set to %s",
+			       param->name, hc->uuid);
 			up_write(&_hash_lock);
 			kfree(new_data);
 			return ERR_PTR(-EINVAL);
 		}
 		/*
-		 * reuuid and move the uuid cell.
+		 * change uuid and move the uuid cell.
 		 */
 		list_del(&hc->uuid_list);
 		old_data = hc->uuid;

Re: [PATCH] Make it possible to set a uuid if one was not set during DM_DEV_CREATE.

[Alasdair G Kergon]

Peter Rajnoha tested this this morning and 'dmsetup remove' hung.

On Fri, Oct 08, 2010 at 01:53:43PM -0400, Peter Jones wrote:
> +	if (param->flags & DM_NEW_UUID_FLAG) {
> +		/*
> +		 * Does this device already have a uuid?
> +		 */
> +		if (hc->uuid) {
> +			DMWARN("asked to change uuid of device with uuid "
> +			       "already set %s %s -> %s",
> +			       param->name, hc->uuid, param->uuid);

It's missing dm_put(hd->md) here.

> +			up_write(&_hash_lock);
> +			kfree(new_data);
> +			return ERR_PTR(-EINVAL);

Alasdair