util-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libfdisk: Fix multipath partition seperators for user-friendly names
@ 2018-06-21 16:07 kmahlkuc
  2018-06-22  9:52 ` Karel Zak
  0 siblings, 1 reply; 4+ messages in thread
From: kmahlkuc @ 2018-06-21 16:07 UTC (permalink / raw)
  To: util-linux; +Cc: KyleMahlkuch

From: KyleMahlkuch <Kyle.Mahlkuch@ibm.com>

The current code assumes "-part" is the only partition sepereator
but this is not true for some distros.

For example in Ubuntu 18.04 fdisk does not print the correct names for
mpatha:

~# ls -l /dev/mapper/mpatha*
lrwxrwxrwx 1 root root 7 Feb  1 04:39 /dev/mapper/mpatha -> ../dm-0
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha1 -> ../dm-4
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha2 -> ../dm-5
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha3 -> ../dm-6

~# fdisk -l /dev/mapper/mpatha
Device                   Boot     Start        End   Sectors  Size Id Type
/dev/mapper/mpatha-part1           2048  419432447 419430400  200G 83 Linux
/dev/mapper/mpatha-part2      419432448  838862847 419430400  200G 83 Linux
/dev/mapper/mpatha-part3      838862848 1258291199 419428352  200G 83 Linux

Instead of assuming a partition seperator of "-part" this patch uses
access to check the file system for a partition seperator of "p" or
the absense of a partition seperator. If neither of these work the patch
defaults to "-part" like we had before this patch.

Signed-off-by: Kyle Mahlkuch <Kyle.Mahlkuch@ibm.com>
---
 libfdisk/src/utils.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libfdisk/src/utils.c b/libfdisk/src/utils.c
index 5ba9e04..031c783 100644
--- a/libfdisk/src/utils.c
+++ b/libfdisk/src/utils.c
@@ -153,7 +153,18 @@ char *fdisk_partname(const char *dev, size_t partno)
 	if ((strncmp(dev, _PATH_DEV_BYID, sizeof(_PATH_DEV_BYID) - 1) == 0) ||
 	     strncmp(dev, _PATH_DEV_BYPATH, sizeof(_PATH_DEV_BYPATH) - 1) == 0 ||
 	     strncmp(dev, "/dev/mapper", sizeof("/dev/mapper") - 1) == 0) {
-	       p = "-part";
+		asprintf(&res, "%.*s%zu", w, dev, partno);
+		if (access(res, F_OK) == 0){
+			p = "";
+		} else {
+			/* check for partition seperator "p" */
+			p = "p";
+			asprintf(&res, "%.*s%s%zu", w, dev, p, partno);
+			if (access(res, F_OK) != 0){
+				/* otherwise, default to "-path" */
+				p = "-part";
+			}
+		}
 	}
 
 	if (asprintf(&res, "%.*s%s%zu", w, dev, p, partno) <= 0)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] libfdisk: Fix multipath partition seperators for user-friendly names
  2018-06-21 16:07 [PATCH] libfdisk: Fix multipath partition seperators for user-friendly names kmahlkuc
@ 2018-06-22  9:52 ` Karel Zak
  2018-06-25 19:52   ` [PATCH v2] " kmahlkuc
  0 siblings, 1 reply; 4+ messages in thread
From: Karel Zak @ 2018-06-22  9:52 UTC (permalink / raw)
  To: kmahlkuc; +Cc: util-linux, KyleMahlkuch

On Thu, Jun 21, 2018 at 11:07:34AM -0500, kmahlkuc@linux.vnet.ibm.com wrote:
> diff --git a/libfdisk/src/utils.c b/libfdisk/src/utils.c
> index 5ba9e04..031c783 100644
> --- a/libfdisk/src/utils.c
> +++ b/libfdisk/src/utils.c
> @@ -153,7 +153,18 @@ char *fdisk_partname(const char *dev, size_t partno)
>  	if ((strncmp(dev, _PATH_DEV_BYID, sizeof(_PATH_DEV_BYID) - 1) == 0) ||
>  	     strncmp(dev, _PATH_DEV_BYPATH, sizeof(_PATH_DEV_BYPATH) - 1) == 0 ||
>  	     strncmp(dev, "/dev/mapper", sizeof("/dev/mapper") - 1) == 0) {
> -	       p = "-part";
> +		asprintf(&res, "%.*s%zu", w, dev, partno);
> +		if (access(res, F_OK) == 0){
> +			p = "";
> +		} else {
> +			/* check for partition seperator "p" */
> +			p = "p";
> +			asprintf(&res, "%.*s%s%zu", w, dev, p, partno);

asprintf() always allocates a new string, right? ... in this case I
see a memory leak :-)

    Karel

> +			if (access(res, F_OK) != 0){
> +				/* otherwise, default to "-path" */
> +				p = "-part";
> +			}
> +		}
>  	}
>  
>  	if (asprintf(&res, "%.*s%s%zu", w, dev, p, partno) <= 0)
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH v2] libfdisk: Fix multipath partition seperators for user-friendly names
  2018-06-22  9:52 ` Karel Zak
@ 2018-06-25 19:52   ` kmahlkuc
  2018-07-04 13:20     ` Karel Zak
  0 siblings, 1 reply; 4+ messages in thread
From: kmahlkuc @ 2018-06-25 19:52 UTC (permalink / raw)
  To: util-linux; +Cc: KyleMahlkuch

From: KyleMahlkuch <Kyle.Mahlkuch@ibm.com>

The current code assumes "-part" is the only partition sepereator
but this is not true for some distros.

For example in Ubuntu 18.04 fdisk does not print the correct names for
mpatha:

~# ls -l /dev/mapper/mpatha*
lrwxrwxrwx 1 root root 7 Feb  1 04:39 /dev/mapper/mpatha -> ../dm-0
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha1 -> ../dm-4
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha2 -> ../dm-5
lrwxrwxrwx 1 root root 7 Feb  1 04:38 /dev/mapper/mpatha3 -> ../dm-6

~# fdisk -l /dev/mapper/mpatha
Device                   Boot     Start        End   Sectors  Size Id Type
/dev/mapper/mpatha-part1           2048  419432447 419430400  200G 83 Linux
/dev/mapper/mpatha-part2      419432448  838862847 419430400  200G 83 Linux
/dev/mapper/mpatha-part3      838862848 1258291199 419428352  200G 83 Linux

Instead of assuming a partition seperator of "-part" this patch uses
access to check the file system for a partition seperator of "p" or
the absense of a partition seperator. If neither of these work the patch
defaults to "-part" like we had before this patch.
---
 libfdisk/src/utils.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libfdisk/src/utils.c b/libfdisk/src/utils.c
index 5ba9e04..54e28b2 100644
--- a/libfdisk/src/utils.c
+++ b/libfdisk/src/utils.c
@@ -153,7 +153,20 @@ char *fdisk_partname(const char *dev, size_t partno)
 	if ((strncmp(dev, _PATH_DEV_BYID, sizeof(_PATH_DEV_BYID) - 1) == 0) ||
 	     strncmp(dev, _PATH_DEV_BYPATH, sizeof(_PATH_DEV_BYPATH) - 1) == 0 ||
 	     strncmp(dev, "/dev/mapper", sizeof("/dev/mapper") - 1) == 0) {
-	       p = "-part";
+		asprintf(&res, "%.*s%zu", w, dev, partno);
+		if (access(res, F_OK) == 0){
+			p = "";
+		} else {
+			/* check for partition seperator "p" */
+			p = "p";
+			free(res);
+			asprintf(&res, "%.*s%s%zu", w, dev, p, partno);
+			if (access(res, F_OK) != 0){
+				/* otherwise, default to "-path" */
+				p = "-part";
+			}
+		}
+		free(res);
 	}
 
 	if (asprintf(&res, "%.*s%s%zu", w, dev, p, partno) <= 0)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] libfdisk: Fix multipath partition seperators for user-friendly names
  2018-06-25 19:52   ` [PATCH v2] " kmahlkuc
@ 2018-07-04 13:20     ` Karel Zak
  0 siblings, 0 replies; 4+ messages in thread
From: Karel Zak @ 2018-07-04 13:20 UTC (permalink / raw)
  To: kmahlkuc; +Cc: util-linux, KyleMahlkuch

On Mon, Jun 25, 2018 at 02:52:01PM -0500, kmahlkuc@linux.vnet.ibm.com wrote:
>  libfdisk/src/utils.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Applied, thanks!


I have a little clean upped the code by next patch to reduce number
of asprintf() calls and to check for asprintf() return value. 

Please, re-test if it still works for your use case :-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-04 13:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 16:07 [PATCH] libfdisk: Fix multipath partition seperators for user-friendly names kmahlkuc
2018-06-22  9:52 ` Karel Zak
2018-06-25 19:52   ` [PATCH v2] " kmahlkuc
2018-07-04 13:20     ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).