Libguestfs - Mar 2015 - Re: [PATCH 2/2] New API: part_get_part_type for showing partition type

Chen Hanxiao (2):
  parted: introduce enum for whether parted has option -m
  New API: part_get_part_type for showing partition type

 daemon/parted.c      | 136 ++++++++++++++++++++++++++++++++++++++++++++++-----
 generator/actions.ml |  18 +++++++
 src/MAX_PROC_NR      |   2 +-
 3 files changed, 143 insertions(+), 13 deletions(-)

-- 
2.1.0

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/parted.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index a7bcb99..64a7d3c 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -33,6 +33,12 @@ GUESTFSD_EXT_CMD(str_parted, parted);
 GUESTFSD_EXT_CMD(str_sfdisk, sfdisk);
 GUESTFSD_EXT_CMD(str_sgdisk, sgdisk);
 
+enum {
+  PARTED_INVALID = -1,
+  /* parted do not support -m option */
+  PARTED_OPT_NO_M,
+  PARTED_OPT_HAS_M};
+
 /* Notes:
  *
  * Parted 1.9 sends error messages to stdout, hence use of the
@@ -320,7 +326,7 @@ get_table_field (const char *line, int n)
 static int
 test_parted_m_opt (void)
 {
-  static int result = -1;
+  static int result = PARTED_INVALID;
 
   if (result >= 0)
     return result;
@@ -334,9 +340,9 @@ test_parted_m_opt (void)
   }
 
   if (err && strstr (err, "invalid option -- m"))
-    result = 0;
+    result = PARTED_OPT_NO_M;
   else
-    result = 1;
+    result = PARTED_OPT_HAS_M;
   return result;
 }
 
@@ -347,7 +353,7 @@ print_partition_table (const char *device, int
parted_has_m_opt)
   CLEANUP_FREE char *err = NULL;
   int r;
 
-  if (parted_has_m_opt)
+  if (PARTED_OPT_HAS_M == parted_has_m_opt)
     r = command (&out, &err, str_parted, "-m",
"--", device,
                  "unit", "b",
                  "print", NULL);
@@ -370,14 +376,14 @@ char *
 do_part_get_parttype (const char *device)
 {
   int parted_has_m_opt = test_parted_m_opt ();
-  if (parted_has_m_opt == -1)
+  if (parted_has_m_opt == PARTED_INVALID)
     return NULL;
 
   CLEANUP_FREE char *out = print_partition_table (device, parted_has_m_opt);
   if (!out)
     return NULL;
 
-  if (parted_has_m_opt) {
+  if (PARTED_OPT_HAS_M == parted_has_m_opt) {
     /* New-style parsing using the "machine-readable" format from
      * 'parted -m'.
      */
@@ -452,7 +458,7 @@ guestfs_int_partition_list *
 do_part_list (const char *device)
 {
   int parted_has_m_opt = test_parted_m_opt ();
-  if (parted_has_m_opt == -1)
+  if (parted_has_m_opt == PARTED_INVALID)
     return NULL;
 
   CLEANUP_FREE char *out = print_partition_table (device, parted_has_m_opt);
@@ -466,7 +472,7 @@ do_part_list (const char *device)
 
   guestfs_int_partition_list *r;
 
-  if (parted_has_m_opt) {
+  if (PARTED_OPT_HAS_M == parted_has_m_opt) {
     /* New-style parsing using the "machine-readable" format from
      * 'parted -m'.
      *
@@ -578,7 +584,7 @@ do_part_get_bootable (const char *device, int partnum)
   }
 
   int parted_has_m_opt = test_parted_m_opt ();
-  if (parted_has_m_opt == -1)
+  if (parted_has_m_opt == PARTED_INVALID)
     return -1;
 
   CLEANUP_FREE char *out = print_partition_table (device, parted_has_m_opt);
@@ -590,7 +596,7 @@ do_part_get_bootable (const char *device, int partnum)
   if (!lines)
     return -1;
 
-  if (parted_has_m_opt) {
+  if (PARTED_OPT_HAS_M == parted_has_m_opt) {
     /* New-style parsing using the "machine-readable" format from
      * 'parted -m'.
      *
@@ -965,14 +971,14 @@ do_part_get_name (const char *device, int partnum)
 
   if (STREQ (parttype, "gpt")) {
     int parted_has_m_opt = test_parted_m_opt ();
-    if (parted_has_m_opt == -1)
+    if (parted_has_m_opt == PARTED_INVALID)
       return NULL;
 
     CLEANUP_FREE char *out = print_partition_table (device, parted_has_m_opt);
     if (!out)
       return NULL;
 
-    if (parted_has_m_opt) {
+    if (PARTED_OPT_HAS_M == parted_has_m_opt) {
       /* New-style parsing using the "machine-readable" format from
        * 'parted -m'.
        */
-- 
2.1.0

This patch will add support for getting partition type
of a partiton numbered device.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/parted.c      | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/actions.ml |  18 +++++++++
 src/MAX_PROC_NR      |   2 +-
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index 64a7d3c..3aac102 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -1028,3 +1028,109 @@ do_part_get_name (const char *device, int partnum)
   reply_with_error ("cannot get the partition name from '%s'
layouts", parttype);
   return NULL;
 }
+
+char *
+do_part_get_part_type (const char *device, int partnum)
+{
+  CLEANUP_FREE char *parttype;
+  char *part_type;
+
+  parttype = do_part_get_parttype (device);
+  if (parttype == NULL)
+    return NULL;
+
+  /* machine parseable output by 'parted -m' did not provide
+   * partition type info.
+   * Use traditional style.
+   */
+  CLEANUP_FREE char *out = print_partition_table (device, PARTED_OPT_NO_M);
+  if (!out)
+    return NULL;
+
+  CLEANUP_FREE_STRING_LIST char **lines = split_lines (out);
+
+  if (!lines)
+    return NULL;
+
+  size_t start = 0, end = 0, row;
+
+  for (row = 0; lines[row] != NULL; ++row)
+    if (STRPREFIX (lines[row], "Number")) {
+      start = row + 1;
+      break;
+    }
+
+  if (start == 0) {
+    reply_with_error ("parted output has no \"Number\"
line");
+    return NULL;
+  }
+
+  for (row = start; lines[row] != NULL; ++row)
+    if (STREQ (lines[row], "")) {
+      end = row;
+      break;
+    }
+
+  if (end == 0) {
+    reply_with_error ("parted output has no blank after end of
table");
+    return NULL;
+  }
+
+  /* Now parse the lines. */
+  size_t i;
+  int64_t temp_int64;
+  int part_num;
+  char temp_type[16] = {'\0'};
+  for (i = 0, row = start;  row < end; ++i, ++row) {
+    if (STREQ (parttype, "gpt")) {
+      memcpy (temp_type, "primary", strlen("primary"));
+      if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64
"B%" SCNi64 "B",
+               &part_num,
+               &temp_int64,
+               &temp_int64,
+               &temp_int64) != 4) {
+        reply_with_error ("could not parse row from output of parted print
command: %s", lines[row]);
+        return NULL;
+      }
+    } else {
+      if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64
"B%" SCNi64 "B" "%15s",
+               &part_num,
+               &temp_int64,
+               &temp_int64,
+               &temp_int64,
+               temp_type) != 5) {
+        reply_with_error ("could not parse row from output of parted print
command: %s", lines[row]);
+        return NULL;
+      }
+    }
+
+    if (part_num != partnum)
+        continue;
+
+    if (STRPREFIX (temp_type, "primary")) {
+      part_type = strdup("primary");
+      if (part_type == NULL)
+          goto error;
+    } else if (STRPREFIX (temp_type, "logical")) {
+      part_type = strdup("logical");
+      if (part_type == NULL)
+          goto error;
+    } else if (STRPREFIX (temp_type, "extended")) {
+      part_type = strdup("extended");
+      if (part_type == NULL)
+          goto error;
+    } else
+        goto error;
+
+    return part_type;
+  }
+
+  if (row == end) {
+    reply_with_error ("could not find partnum: %d", partnum);
+    return NULL;
+  }
+
+  error:
+    reply_with_error ("strdup failed");
+    return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 786b9a2..ff8fd87 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12527,6 +12527,24 @@ This will Enable extended inode refs." };
     longdesc = "\
 This enable skinny metadata extent refs." };
 
+  { defaults with
+    name = "part_get_part_type";
+    style = RString "partitiontype", [Device "device"; Int
"partnum"], [];
+    proc_nr = Some 453;
+    tests = [
+      InitEmpty, Always, TestResultString (
+        [["part_init"; "/dev/sda"; "mbr"];
+         ["part_add"; "/dev/sda"; "p";
"64"; "204799"];
+         ["part_add"; "/dev/sda"; "e";
"204800"; "614400"];
+         ["part_add"; "/dev/sda"; "l";
"204864"; "205988"];
+         ["part_get_part_type"; "/dev/sda";
"5"]], "logical"), []
+    ];
+
+    shortdesc = "get the partition type";
+    longdesc = "\
+This could get the partition type, such as primary, logical,
+on partition numbered C<partnum> on device C<device>." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 8670c73..534b992 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-452
+453
-- 
2.1.0

On Tuesday 24 March 2015 07:20:16 Chen Hanxiao wrote:
> Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
> ---
>  daemon/parted.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/daemon/parted.c b/daemon/parted.c
> index a7bcb99..64a7d3c 100644
> --- a/daemon/parted.c
> +++ b/daemon/parted.c
> @@ -33,6 +33,12 @@ GUESTFSD_EXT_CMD(str_parted, parted);
>  GUESTFSD_EXT_CMD(str_sfdisk, sfdisk);
>  GUESTFSD_EXT_CMD(str_sgdisk, sgdisk);
>  
> +enum {
> +  PARTED_INVALID = -1,
> +  /* parted do not support -m option */
> +  PARTED_OPT_NO_M,
> +  PARTED_OPT_HAS_M};
(I didn't have even the time to reply to the question about this enum)

PARTED_INVALID does not make much sense, especially that I was
referring just to the parameter for print_partition_table, so for it
only two values (eg PARTED_OUTPUT_MACHINE and PARTED_OUTPUT_NORMAL)
are enough.

> +
>  /* Notes:
>   *
>   * Parted 1.9 sends error messages to stdout, hence use of the
> @@ -320,7 +326,7 @@ get_table_field (const char *line, int n)
>  static int
>  test_parted_m_opt (void)
>  {
> -  static int result = -1;
> +  static int result = PARTED_INVALID;
Commenting here, but it applies to the rest of the changes: if you want
to apply an enum for this case, then do it consistently and for all the
cases. Using an enum value and storing it to an int variable defeats
the point of using an enum at all, as you will not catch non-enum
values or not check to be handling all values where needed.

-- 
Pino Toscano

On Tue, Mar 24, 2015 at 07:20:17AM -0400, Chen Hanxiao
wrote:
> This patch will add support for getting partition type
> of a partiton numbered device.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
I have pushed this.

I changed the API name to part_get_mbr_part_type since it's really
MBR-specific, and also we have an API called part_get_parttype[1] so
there was an opportunity for confusion.

Also I added an extra test and made a small adjustment to the API
description.

Here are the upstream commits:

https://github.com/libguestfs/libguestfs/commit/af9aa2eb0582bfced4e16ade73c19c321b91ceb9
https://github.com/libguestfs/libguestfs/commit/0c396a4bce578486dfc4a38e1f8c47fd5c2836ea

Thanks,

Rich.

[1] returns the partition table type like "mbr".

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top