Linux Device Mapper development
 help / color / mirror / Atom feed
* [PATCH] kpartx: support device names with spaces
@ 2016-01-07 18:31 Mauricio Faria de Oliveira
  2016-01-20  7:15 ` Christophe Varoqui
  0 siblings, 1 reply; 3+ messages in thread
From: Mauricio Faria de Oliveira @ 2016-01-07 18:31 UTC (permalink / raw)
  To: dm-devel; +Cc: christophe.varoqui

Device names with spaces fail to be added to device-mapper tables
because spaces are field separators.  In order to support it, use
device major:minor numbers, instead.  While still there, make the
verbose output of container- and contained-slices consistent.

This is useful on some scenarios with arbitrary loop device names,
and non-user_friendly_names multipath WWIDs with spaces (eg, QEMU
emulated hard drives, and some IBM devices) on some distros.

Test-case:

	# img="/tmp/my.img"
	# spacename="/dev/my space name"

	# truncate --size=1G "$img"
	# lodev=$(sudo losetup --find --show "$img")
	# ln -sf "$lodev" "$spacename"

	# ls -l $lodev "$spacename"
	brw-rw---- 1 root disk 7, 0 Jan  7 10:36 /dev/loop0
	lrwxrwxrwx 1 root root   10 Jan  7 10:36 /dev/my space name -> /dev/loop0

	# fdisk $lodev
	<...>

	# fdisk -l $lodev
	<...>
	      Device Boot      Start         End      Blocks   Id  System
	/dev/loop0p1            2048      264191      131072   83  Linux
	/dev/loop0p2          264192      788479      262144   83  Linux
	/dev/loop0p3          788480     2097151      654336    5  Extended
	/dev/loop0p5          790528      856063       32768   83  Linux
	/dev/loop0p6          858112      989183       65536   83  Linux
	/dev/loop0p7          991232     1515519      262144   83  Linux

Without patch:

	# kpartx -av "$spacename"
	device-mapper: reload ioctl on my\x20space\x20name1 failed: Invalid argument
	create/reload failed on my space name1
	add map my space name1 (0:0): 0 262144 linear /dev/my space name 2048

	device-mapper: reload ioctl on my\x20space\x20name2 failed: Invalid argument
	create/reload failed on my space name2
	add map my space name2 (0:0): 0 524288 linear /dev/my space name 264192

	device-mapper: reload ioctl on my\x20space\x20name3 failed: Invalid argument
	create/reload failed on my space name3
	add map my space name3 (0:0): 0 2 linear /dev/my space name 788480

	device-mapper: reload ioctl on my\x20space\x20name5 failed: Invalid argument
	add map my space name5 : 0 65536 linear /dev/my space name 790528

	device-mapper: reload ioctl on my\x20space\x20name6 failed: Invalid argument
	add map my space name6 : 0 131072 linear /dev/my space name 858112

	device-mapper: reload ioctl on my\x20space\x20name7 failed: Invalid argument
	add map my space name7 : 0 524288 linear /dev/my space name 991232

With patch:

	# kpartx -av "$spacename"
	add map my space name1 (252:6): 0 262144 linear 7:0 2048
	add map my space name2 (252:7): 0 524288 linear 7:0 264192
	add map my space name3 (252:8): 0 2 linear 7:0 788480
	add map my space name5 (252:9): 0 65536 linear 7:0 790528
	add map my space name6 (252:10): 0 131072 linear 7:0 858112
	add map my space name7 (252:11): 0 524288 linear 7:0 991232

	# dmsetup table | sort
	my space name1: 0 262144 linear 7:0 2048
	my space name2: 0 524288 linear 7:0 264192
	my space name3: 0 2 linear 7:0 788480
	my space name5: 0 65536 linear 7:0 790528
	my space name6: 0 131072 linear 7:0 858112
	my space name7: 0 524288 linear 7:0 991232

	# ls -l /dev/mapper/
	total 0
	crw------- 1 root root  10, 236 Jan  7 12:00 control
	brw-rw---- 1 root disk 252,   6 Jan  7 15:50 my\x20space\x20name1
	brw-rw---- 1 root disk 252,   7 Jan  7 15:50 my\x20space\x20name2
	brw-rw---- 1 root disk 252,   8 Jan  7 15:50 my\x20space\x20name3
	brw-rw---- 1 root disk 252,   9 Jan  7 15:50 my\x20space\x20name5
	brw-rw---- 1 root disk 252,  10 Jan  7 15:50 my\x20space\x20name6
	brw-rw---- 1 root disk 252,  11 Jan  7 15:50 my\x20space\x20name7

Thanks: Scott Moser (pointers to the error and test-case).

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 kpartx/kpartx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index a9d4c98..d5eeea9 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -477,8 +477,8 @@ main(int argc, char **argv){
 				}
 				strip_slash(partname);
 
-				if (safe_sprintf(params, "%s %" PRIu64 ,
-						 device, slices[j].start)) {
+				if (safe_sprintf(params, "%d:%d %" PRIu64 ,
+						 MAJOR(buf.st_rdev), MINOR(buf.st_rdev), slices[j].start)) {
 					fprintf(stderr, "params too small\n");
 					exit(1);
 				}
@@ -540,8 +540,8 @@ main(int argc, char **argv){
 					}
 					strip_slash(partname);
 
-					if (safe_sprintf(params, "%s %" PRIu64,
-							 device,
+					if (safe_sprintf(params, "%d:%d %" PRIu64,
+							 MAJOR(buf.st_rdev), MINOR(buf.st_rdev),
 							 slices[j].start)) {
 						fprintf(stderr, "params too small\n");
 						exit(1);
@@ -563,8 +563,8 @@ main(int argc, char **argv){
 						&slices[j].minor);
 
 					if (verbose)
-						printf("add map %s : 0 %" PRIu64 " %s %s\n",
-						       partname, slices[j].size,
+						printf("add map %s (%d:%d): 0 %" PRIu64 " %s %s\n",
+						       partname, slices[j].major, slices[j].minor, slices[j].size,
 						       DM_TARGET, params);
 					c--;
 				}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] kpartx: support device names with spaces
  2016-01-07 18:31 [PATCH] kpartx: support device names with spaces Mauricio Faria de Oliveira
@ 2016-01-20  7:15 ` Christophe Varoqui
  2016-01-20 13:57   ` Mauricio Faria de Oliveira
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe Varoqui @ 2016-01-20  7:15 UTC (permalink / raw)
  To: Mauricio Faria de Oliveira; +Cc: device-mapper development


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

Hello,

shouldn't you either define the MAJOR and MINOR macros, or include the
appropriate kernel header ? As is the build emits :

kpartx.c:481:8: warning: implicit declaration of function ‘MAJOR’
[-Wimplicit-function-declaration]
        MAJOR(buf.st_rdev), MINOR(buf.st_rdev), slices[j].start)) {
...

And the linking emits :

kpartx.o: In function `main':
/home/cvaroqui/multipath-tools/kpartx/kpartx.c:480: undefined reference to
`MINOR'
/home/cvaroqui/multipath-tools/kpartx/kpartx.c:480: undefined reference to
`MAJOR'
...

Regards,
Christophe Varoqui
OpenSVC

On Thu, Jan 7, 2016 at 7:31 PM, Mauricio Faria de Oliveira <
mauricfo@linux.vnet.ibm.com> wrote:

> Device names with spaces fail to be added to device-mapper tables
> because spaces are field separators.  In order to support it, use
> device major:minor numbers, instead.  While still there, make the
> verbose output of container- and contained-slices consistent.
>
> This is useful on some scenarios with arbitrary loop device names,
> and non-user_friendly_names multipath WWIDs with spaces (eg, QEMU
> emulated hard drives, and some IBM devices) on some distros.
>
> Test-case:
>
>         # img="/tmp/my.img"
>         # spacename="/dev/my space name"
>
>         # truncate --size=1G "$img"
>         # lodev=$(sudo losetup --find --show "$img")
>         # ln -sf "$lodev" "$spacename"
>
>         # ls -l $lodev "$spacename"
>         brw-rw---- 1 root disk 7, 0 Jan  7 10:36 /dev/loop0
>         lrwxrwxrwx 1 root root   10 Jan  7 10:36 /dev/my space name ->
> /dev/loop0
>
>         # fdisk $lodev
>         <...>
>
>         # fdisk -l $lodev
>         <...>
>               Device Boot      Start         End      Blocks   Id  System
>         /dev/loop0p1            2048      264191      131072   83  Linux
>         /dev/loop0p2          264192      788479      262144   83  Linux
>         /dev/loop0p3          788480     2097151      654336    5  Extended
>         /dev/loop0p5          790528      856063       32768   83  Linux
>         /dev/loop0p6          858112      989183       65536   83  Linux
>         /dev/loop0p7          991232     1515519      262144   83  Linux
>
> Without patch:
>
>         # kpartx -av "$spacename"
>         device-mapper: reload ioctl on my\x20space\x20name1 failed:
> Invalid argument
>         create/reload failed on my space name1
>         add map my space name1 (0:0): 0 262144 linear /dev/my space name
> 2048
>
>         device-mapper: reload ioctl on my\x20space\x20name2 failed:
> Invalid argument
>         create/reload failed on my space name2
>         add map my space name2 (0:0): 0 524288 linear /dev/my space name
> 264192
>
>         device-mapper: reload ioctl on my\x20space\x20name3 failed:
> Invalid argument
>         create/reload failed on my space name3
>         add map my space name3 (0:0): 0 2 linear /dev/my space name 788480
>
>         device-mapper: reload ioctl on my\x20space\x20name5 failed:
> Invalid argument
>         add map my space name5 : 0 65536 linear /dev/my space name 790528
>
>         device-mapper: reload ioctl on my\x20space\x20name6 failed:
> Invalid argument
>         add map my space name6 : 0 131072 linear /dev/my space name 858112
>
>         device-mapper: reload ioctl on my\x20space\x20name7 failed:
> Invalid argument
>         add map my space name7 : 0 524288 linear /dev/my space name 991232
>
> With patch:
>
>         # kpartx -av "$spacename"
>         add map my space name1 (252:6): 0 262144 linear 7:0 2048
>         add map my space name2 (252:7): 0 524288 linear 7:0 264192
>         add map my space name3 (252:8): 0 2 linear 7:0 788480
>         add map my space name5 (252:9): 0 65536 linear 7:0 790528
>         add map my space name6 (252:10): 0 131072 linear 7:0 858112
>         add map my space name7 (252:11): 0 524288 linear 7:0 991232
>
>         # dmsetup table | sort
>         my space name1: 0 262144 linear 7:0 2048
>         my space name2: 0 524288 linear 7:0 264192
>         my space name3: 0 2 linear 7:0 788480
>         my space name5: 0 65536 linear 7:0 790528
>         my space name6: 0 131072 linear 7:0 858112
>         my space name7: 0 524288 linear 7:0 991232
>
>         # ls -l /dev/mapper/
>         total 0
>         crw------- 1 root root  10, 236 Jan  7 12:00 control
>         brw-rw---- 1 root disk 252,   6 Jan  7 15:50 my\x20space\x20name1
>         brw-rw---- 1 root disk 252,   7 Jan  7 15:50 my\x20space\x20name2
>         brw-rw---- 1 root disk 252,   8 Jan  7 15:50 my\x20space\x20name3
>         brw-rw---- 1 root disk 252,   9 Jan  7 15:50 my\x20space\x20name5
>         brw-rw---- 1 root disk 252,  10 Jan  7 15:50 my\x20space\x20name6
>         brw-rw---- 1 root disk 252,  11 Jan  7 15:50 my\x20space\x20name7
>
> Thanks: Scott Moser (pointers to the error and test-case).
>
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> ---
>  kpartx/kpartx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index a9d4c98..d5eeea9 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
> @@ -477,8 +477,8 @@ main(int argc, char **argv){
>                                 }
>                                 strip_slash(partname);
>
> -                               if (safe_sprintf(params, "%s %" PRIu64 ,
> -                                                device, slices[j].start))
> {
> +                               if (safe_sprintf(params, "%d:%d %" PRIu64 ,
> +                                                MAJOR(buf.st_rdev),
> MINOR(buf.st_rdev), slices[j].start)) {
>                                         fprintf(stderr, "params too
> small\n");
>                                         exit(1);
>                                 }
> @@ -540,8 +540,8 @@ main(int argc, char **argv){
>                                         }
>                                         strip_slash(partname);
>
> -                                       if (safe_sprintf(params, "%s %"
> PRIu64,
> -                                                        device,
> +                                       if (safe_sprintf(params, "%d:%d %"
> PRIu64,
> +
> MAJOR(buf.st_rdev), MINOR(buf.st_rdev),
>                                                          slices[j].start))
> {
>                                                 fprintf(stderr, "params
> too small\n");
>                                                 exit(1);
> @@ -563,8 +563,8 @@ main(int argc, char **argv){
>                                                 &slices[j].minor);
>
>                                         if (verbose)
> -                                               printf("add map %s : 0 %"
> PRIu64 " %s %s\n",
> -                                                      partname,
> slices[j].size,
> +                                               printf("add map %s
> (%d:%d): 0 %" PRIu64 " %s %s\n",
> +                                                      partname,
> slices[j].major, slices[j].minor, slices[j].size,
>                                                        DM_TARGET, params);
>                                         c--;
>                                 }
> --
> 1.8.3.1
>
>

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

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kpartx: support device names with spaces
  2016-01-20  7:15 ` Christophe Varoqui
@ 2016-01-20 13:57   ` Mauricio Faria de Oliveira
  0 siblings, 0 replies; 3+ messages in thread
From: Mauricio Faria de Oliveira @ 2016-01-20 13:57 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development

Hi Christophe,

On 01/20/2016 05:15 AM, Christophe Varoqui wrote:
> shouldn't you either define the MAJOR and MINOR macros, or include the
> appropriate kernel header ? As is the build emits :

Oops. Apologies. I incorrectly submitted a backport of the patch.

I just submitted V2 w/ the right patch, which uses major()/minor()
as elsewhere in the file.  The build and test-case are OK. Thanks!

Regards,

-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-01-20 13:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-07 18:31 [PATCH] kpartx: support device names with spaces Mauricio Faria de Oliveira
2016-01-20  7:15 ` Christophe Varoqui
2016-01-20 13:57   ` Mauricio Faria de Oliveira

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox