Libguestfs - Jan 2015 - [PATCH] daemon: parted: use --part-type with recent sfdisk

Check whether --part-type is supported (in the rewritten sfdisk in
util-linux >= 2.26), and use it instead of --print-id & --change-id.

The actual result should be the same, just not using a (recently)
deprecated command line API.
---
 daemon/parted.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index 8fbb6e4..2f10144 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -681,6 +681,30 @@ do_part_get_bootable (const char *device, int partnum)
   }
 }
 
+/* Test if sfdisk is recent enough to have --part-type, to be used instead
+ * of --print-id and --change-id.
+ */
+static int
+test_sfdisk_has_part_type (void)
+{
+  static int tested = -1;
+
+  if (tested != -1)
+    return tested;
+
+  int r;
+  CLEANUP_FREE char *out = NULL, *err = NULL;
+
+  r = command (&out, &err, str_sfdisk, "--help", NULL);
+  if (r == -1) {
+    reply_with_error ("%s: %s", "sfdisk --help", err);
+    return -1;
+  }
+
+  tested = strstr (out, "--part-type") != NULL;
+  return tested;
+}
+
 /* Currently we use sfdisk for getting and setting the ID byte.  In
  * future, extend parted to provide this functionality.  As a result
  * of using sfdisk, this won't work for non-MBR-style partitions, but
@@ -695,6 +719,8 @@ do_part_get_mbr_id (const char *device, int partnum)
     return -1;
   }
 
+  const char *param = test_sfdisk_has_part_type () ? "--part-type" :
"--print-id";
+
   char partnum_str[16];
   snprintf (partnum_str, sizeof partnum_str, "%d", partnum);
 
@@ -703,9 +729,9 @@ do_part_get_mbr_id (const char *device, int partnum)
 
   udev_settle ();
 
-  r = command (&out, &err, str_sfdisk, "--print-id", device,
partnum_str, NULL);
+  r = command (&out, &err, str_sfdisk, param, device, partnum_str,
NULL);
   if (r == -1) {
-    reply_with_error ("sfdisk --print-id: %s", err);
+    reply_with_error ("sfdisk %s: %s", param, err);
     return -1;
   }
 
@@ -729,6 +755,8 @@ do_part_set_mbr_id (const char *device, int partnum, int
idbyte)
     return -1;
   }
 
+  const char *param = test_sfdisk_has_part_type () ? "--part-type" :
"--change-id";
+
   char partnum_str[16];
   snprintf (partnum_str, sizeof partnum_str, "%d", partnum);
 
@@ -741,9 +769,9 @@ do_part_set_mbr_id (const char *device, int partnum, int
idbyte)
   udev_settle ();
 
   r = command (NULL, &err, str_sfdisk,
-               "--change-id", device, partnum_str, idbyte_str, NULL);
+               param, device, partnum_str, idbyte_str, NULL);
   if (r == -1) {
-    reply_with_error ("sfdisk --change-id: %s", err);
+    reply_with_error ("sfdisk %s: %s", param, err);
     return -1;
   }
 
-- 
1.9.3

On Wed, Jan 28, 2015 at 02:43:10PM +0100, Pino Toscano
wrote:
> Check whether --part-type is supported (in the rewritten sfdisk in
> util-linux >= 2.26), and use it instead of --print-id & --change-id.
> 
> The actual result should be the same, just not using a (recently)
> deprecated command line API.
If this is a fix for RHBZ#1183234 then remember to put that
into the first line of the git commit message.

Apart from that, it all looks good to me, so ACK.

Rich.
>  daemon/parted.c | 36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/daemon/parted.c b/daemon/parted.c
> index 8fbb6e4..2f10144 100644
> --- a/daemon/parted.c
> +++ b/daemon/parted.c
> @@ -681,6 +681,30 @@ do_part_get_bootable (const char *device, int partnum)
>    }
>  }
>  
> +/* Test if sfdisk is recent enough to have --part-type, to be used instead
> + * of --print-id and --change-id.
> + */
> +static int
> +test_sfdisk_has_part_type (void)
> +{
> +  static int tested = -1;
> +
> +  if (tested != -1)
> +    return tested;
> +
> +  int r;
> +  CLEANUP_FREE char *out = NULL, *err = NULL;
> +
> +  r = command (&out, &err, str_sfdisk, "--help", NULL);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", "sfdisk --help", err);
> +    return -1;
> +  }
> +
> +  tested = strstr (out, "--part-type") != NULL;
> +  return tested;
> +}
> +
>  /* Currently we use sfdisk for getting and setting the ID byte.  In
>   * future, extend parted to provide this functionality.  As a result
>   * of using sfdisk, this won't work for non-MBR-style partitions, but
> @@ -695,6 +719,8 @@ do_part_get_mbr_id (const char *device, int partnum)
>      return -1;
>    }
>  
> +  const char *param = test_sfdisk_has_part_type () ?
"--part-type" : "--print-id";
> +
>    char partnum_str[16];
>    snprintf (partnum_str, sizeof partnum_str, "%d", partnum);
>  
> @@ -703,9 +729,9 @@ do_part_get_mbr_id (const char *device, int partnum)
>  
>    udev_settle ();
>  
> -  r = command (&out, &err, str_sfdisk, "--print-id",
device, partnum_str, NULL);
> +  r = command (&out, &err, str_sfdisk, param, device, partnum_str,
NULL);
>    if (r == -1) {
> -    reply_with_error ("sfdisk --print-id: %s", err);
> +    reply_with_error ("sfdisk %s: %s", param, err);
>      return -1;
>    }
>  
> @@ -729,6 +755,8 @@ do_part_set_mbr_id (const char *device, int partnum,
int idbyte)
>      return -1;
>    }
>  
> +  const char *param = test_sfdisk_has_part_type () ?
"--part-type" : "--change-id";
> +
>    char partnum_str[16];
>    snprintf (partnum_str, sizeof partnum_str, "%d", partnum);
>  
> @@ -741,9 +769,9 @@ do_part_set_mbr_id (const char *device, int partnum,
int idbyte)
>    udev_settle ();
>  
>    r = command (NULL, &err, str_sfdisk,
> -               "--change-id", device, partnum_str, idbyte_str,
NULL);
> +               param, device, partnum_str, idbyte_str, NULL);
>    if (r == -1) {
> -    reply_with_error ("sfdisk --change-id: %s", err);
> +    reply_with_error ("sfdisk %s: %s", param, err);
>      return -1;
>    }
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html