Libguestfs - Jan 2017 - [PATCH 1/2] daemon: Fix part-to-dev when the partition name includes p<N>.

From: Pino Toscano <ptoscano@redhat.com>

If the device name ends with a number, Linux uses partition names of
the form <device>p<N>.  Handle this case by knocking off the
'p'
character.
---
 daemon/devsparts.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index 5862ae2..b764f63 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -203,6 +203,9 @@ do_part_to_dev (const char *part)
     return NULL;
   }
 
+  if (part[n-1] == 'p')
+    n--;
+
   char *r = strndup (part, n);
   if (r == NULL) {
     reply_with_perror ("strdup");
-- 
2.9.3

From: Pino Toscano <ptoscano@redhat.com>

---
 daemon/devsparts.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index b764f63..852ee99 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -20,6 +20,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -36,7 +37,7 @@ typedef int (*block_dev_func_t) (const char *dev, struct
stringsbuf *r);
 
 /* Execute a given function for each discovered block device */
 static char **
-foreach_block_device (block_dev_func_t func)
+foreach_block_device (block_dev_func_t func, bool return_md)
 {
   CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (r);
   DIR *dir;
@@ -60,7 +61,9 @@ foreach_block_device (block_dev_func_t func)
         STREQLEN (d->d_name, "hd", 2) ||
         STREQLEN (d->d_name, "ubd", 3) ||
         STREQLEN (d->d_name, "vd", 2) ||
-        STREQLEN (d->d_name, "sr", 2)) {
+        STREQLEN (d->d_name, "sr", 2) ||
+        (return_md &&
+         STREQLEN (d->d_name, "md", 2) && c_isdigit
(d->d_name[2]))) {
       snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name);
 
       /* Ignore the root device. */
@@ -131,7 +134,12 @@ add_device (const char *device, struct stringsbuf *r)
 char **
 do_list_devices (void)
 {
-  return foreach_block_device (add_device);
+  /* For backwards compatibility, don't return MD devices in the
+   * list returned by guestfs_list_devices.  This is because most
+   * API users expect that this list is effectively the same as
+   * the list of devices added by guestfs_add_drive.
+   */
+  return foreach_block_device (add_device, false);
 }
 
 static int
@@ -184,7 +192,7 @@ add_partitions (const char *device, struct stringsbuf *r)
 char **
 do_list_partitions (void)
 {
-  return foreach_block_device (add_partitions);
+  return foreach_block_device (add_partitions, true);
 }
 
 char *
-- 
2.9.3

On Thursday, 19 January 2017 11:24:36 CET Richard W.M. Jones
wrote:
> From: Pino Toscano <ptoscano@redhat.com>
> 
> If the device name ends with a number, Linux uses partition names of
> the form <device>p<N>.  Handle this case by knocking off the
'p'
> character.
> ---
>  daemon/devsparts.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/daemon/devsparts.c b/daemon/devsparts.c
> index 5862ae2..b764f63 100644
> --- a/daemon/devsparts.c
> +++ b/daemon/devsparts.c
> @@ -203,6 +203,9 @@ do_part_to_dev (const char *part)
>      return NULL;
>    }
>  
Can you please add here a small comment for the if that follow? I.e.
something like:

 /* Deal with <device>p<N> partition names. */
> +  if (part[n-1] == 'p')
> +    n--;
> +
>    char *r = strndup (part, n);
>    if (r == NULL) {
>      reply_with_perror ("strdup");
> 
LGTM with the small addition above.

Thanks,
-- 
Pino Toscano

On Thursday, 19 January 2017 11:24:37 CET Richard W.M. Jones
wrote:
> From: Pino Toscano <ptoscano@redhat.com>
> 
> ---
>  daemon/devsparts.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/daemon/devsparts.c b/daemon/devsparts.c
> index b764f63..852ee99 100644
> --- a/daemon/devsparts.c
> +++ b/daemon/devsparts.c
> @@ -20,6 +20,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <stdbool.h>
>  #include <string.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> @@ -36,7 +37,7 @@ typedef int (*block_dev_func_t) (const char *dev, struct
stringsbuf *r);
>  
>  /* Execute a given function for each discovered block device */
>  static char **
> -foreach_block_device (block_dev_func_t func)
> +foreach_block_device (block_dev_func_t func, bool return_md)
>  {
>    CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (r);
>    DIR *dir;
> @@ -60,7 +61,9 @@ foreach_block_device (block_dev_func_t func)
>          STREQLEN (d->d_name, "hd", 2) ||
>          STREQLEN (d->d_name, "ubd", 3) ||
>          STREQLEN (d->d_name, "vd", 2) ||
> -        STREQLEN (d->d_name, "sr", 2)) {
> +        STREQLEN (d->d_name, "sr", 2) ||
> +        (return_md &&
> +         STREQLEN (d->d_name, "md", 2) && c_isdigit
(d->d_name[2]))) {
>        snprintf (dev_path, sizeof dev_path, "/dev/%s",
d->d_name);
>  
>        /* Ignore the root device. */
> @@ -131,7 +134,12 @@ add_device (const char *device, struct stringsbuf *r)
>  char **
>  do_list_devices (void)
>  {
> -  return foreach_block_device (add_device);
> +  /* For backwards compatibility, don't return MD devices in the
> +   * list returned by guestfs_list_devices.  This is because most
> +   * API users expect that this list is effectively the same as
> +   * the list of devices added by guestfs_add_drive.
Small addendum:

 * 
 * Also, MD devices are special devices (unlike the devices exposed
 * by QEMU, and there is a special API for them,
 * guestfs_list_md_devices.
> +   */
> +  return foreach_block_device (add_device, false);
>  }
>  
>  static int
> @@ -184,7 +192,7 @@ add_partitions (const char *device, struct stringsbuf
*r)
>  char **
>  do_list_partitions (void)
>  {
> -  return foreach_block_device (add_partitions);
> +  return foreach_block_device (add_partitions, true);
>  }
>  
>  char *
LGTM with the above addition.

Thanks,
-- 
Pino Toscano