Libguestfs - Feb 2015 - Re: resize: Preserve GPT GUID so we don't break EFI bootloaders (RHBZ#1189284)

virt-resize didn't preserve the per-partition GPT GUID.

Now that guests using UEFI are becoming common (basically it's the
default on aarch64) we need to take into account that sometimes the
partition GUID is used by the bootloader NVRAM variables to identify
the boot partition, so it must be preserved across resize.

This bug caused the 'virt-builder --size' option to fail on aarch64.

Rich.

---
 daemon/parted.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index 2f10144..877ef10 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -787,7 +787,7 @@ optgroup_gdisk_available (void)
 }
 
 int
-do_part_set_gpt_type(const char *device, int partnum, const char *guid)
+do_part_set_gpt_type (const char *device, int partnum, const char *guid)
 {
   if (partnum <= 0) {
     reply_with_error ("partition number must be >= 1");
-- 
2.1.0

In GPT, each partition has a GUID assigned randomly.  Allow this GUID
to be written and read.

Note this is different from the GUID type code which is used to
identify the type of the partition.
---
 daemon/parted.c      | 33 +++++++++++++++++++++++++++++++++
 generator/actions.ml | 36 ++++++++++++++++++++++++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index 877ef10..a7bcb99 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -812,6 +812,32 @@ do_part_set_gpt_type (const char *device, int partnum,
const char *guid)
   return 0;
 }
 
+int
+do_part_set_gpt_guid (const char *device, int partnum, const char *guid)
+{
+  if (partnum <= 0) {
+    reply_with_error ("partition number must be >= 1");
+    return -1;
+  }
+
+  CLEANUP_FREE char *typecode = NULL;
+  if (asprintf (&typecode, "%i:%s", partnum, guid) == -1) {
+    reply_with_perror ("asprintf");
+    return -1;
+  }
+
+  CLEANUP_FREE char *err = NULL;
+  int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+                    str_sgdisk, device, "-u", typecode, NULL);
+
+  if (r == -1) {
+    reply_with_error ("%s %s -u %s: %s", str_sgdisk, device,
typecode, err);
+    return -1;
+  }
+
+  return 0;
+}
+
 static char *
 sgdisk_info_extract_field (const char *device, int partnum, const char *field,
                            char *(*extract) (const char *path))
@@ -922,6 +948,13 @@ do_part_get_gpt_type (const char *device, int partnum)
 }
 
 char *
+do_part_get_gpt_guid (const char *device, int partnum)
+{
+  return sgdisk_info_extract_field (device, partnum,
+                                    "Partition unique GUID",
extract_uuid);
+}
+
+char *
 do_part_get_name (const char *device, int partnum)
 {
   CLEANUP_FREE char *parttype;
diff --git a/generator/actions.ml b/generator/actions.ml
index 985bb81..6b37712 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12413,6 +12413,42 @@ Recover the chunk tree of btrfs filesystem by scannning
the devices one by one."
     longdesc = "\
 Recover bad superblocks from good copies." };
 
+  { defaults with
+    name = "part_set_gpt_guid";
+    style = RErr, [Device "device"; Int "partnum"; GUID
"guid"], [];
+    proc_nr = Some 446;
+    optional = Some "gdisk";
+    tests = [
+      InitGPT, Always, TestLastFail (
+        [["part_set_gpt_guid"; "/dev/sda"; "1";
"f"]]), [];
+      InitGPT, Always, TestResultString (
+        [["part_set_gpt_guid"; "/dev/sda"; "1";
+          "01234567-89AB-CDEF-0123-456789ABCDEF"];
+         ["part_get_gpt_guid"; "/dev/sda"; "1"]],
+        "01234567-89AB-CDEF-0123-456789ABCDEF"), [];
+    ];
+    shortdesc = "set the GUID of a GPT partition";
+    longdesc = "\
+Set the GUID of numbered GPT partition C<partnum> to C<guid>. 
Return an
+error if the partition table of C<device> isn't GPT, or if
C<guid> is not a
+valid GUID." };
+
+  { defaults with
+    name = "part_get_gpt_guid";
+    style = RString "guid", [Device "device"; Int
"partnum"], [];
+    proc_nr = Some 447;
+    optional = Some "gdisk";
+    tests = [
+      InitGPT, Always, TestResultString (
+        [["part_set_gpt_guid"; "/dev/sda"; "1";
+          "01234567-89AB-CDEF-0123-456789ABCDEF"];
+         ["part_get_gpt_guid"; "/dev/sda"; "1"]],
+        "01234567-89AB-CDEF-0123-456789ABCDEF"), [];
+    ];
+    shortdesc = "get the GUID of a GPT partition";
+    longdesc = "\
+Return the GUID of numbered GPT partition C<partnum>." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index e5a135a..e9b7520 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-445
+447
-- 
2.1.0

When copying disks that use EFI, we created a new partition table,
randomizing the GPT GUID of the first partition.  Since EFI may store
the GUID in its NVRAM variables, this could make the guest unbootable.
---
 resize/resize.ml | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/resize/resize.ml b/resize/resize.ml
index 871c6a4..84fd6d4 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -50,6 +50,7 @@ type partition = {
   p_id : partition_id;           (* Partition (MBR/GPT) ID. *)
   p_type : partition_content;    (* Content type and content size. *)
   p_label : string option;       (* Label/name. *)
+  p_guid : string option;        (* Partition GUID (GPT only). *)
 
   (* What we're going to do: *)
   mutable p_operation : partition_operation;
@@ -93,6 +94,11 @@ let rec debug_partition p      (match p.p_label with
     | Some label -> label
     | None -> "(none)"
+    );
+  printf "\tGUID: %s\n"
+    (match p.p_guid with
+    | Some guid -> guid
+    | None -> "(none)"
     )
 and string_of_partition_content = function
   | ContentUnknown -> "unknown data"
@@ -479,10 +485,16 @@ read the man page virt-resize(1).
           let label              try Some (g#part_get_name "/dev/sda"
part_num)
             with G.Error _ -> None in
+          let guid +            match parttype with
+            | MBR -> None
+            | GPT ->
+              try Some (g#part_get_gpt_guid "/dev/sda" part_num)
+              with G.Error _ -> None in
 
           { p_name = name; p_part = part;
             p_bootable = bootable; p_id = id; p_type = typ;
-            p_label = label;
+            p_label = label; p_guid = guid;
             p_operation = OpCopy; p_target_partnum = 0;
             p_target_start = 0L; p_target_end = 0L }
       ) parts in
@@ -1068,7 +1080,7 @@ read the man page virt-resize(1).
           p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
                      part_size = 0L };
           p_bootable = false; p_id = No_ID; p_type = ContentUnknown;
-          p_label = None;
+          p_label = None; p_guid = None;
 
           (* Target information is meaningful. *)
           p_operation = OpIgnore;
@@ -1167,6 +1179,12 @@ read the man page virt-resize(1).
       | None -> ()
       );
 
+      (match p.p_guid with
+      | Some guid ->
+        g#part_set_gpt_guid "/dev/sdb" p.p_target_partnum guid;
+      | None -> ()
+      );
+
       match parttype, p.p_id with
       | GPT, GPT_Type gpt_type ->
         g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type
-- 
2.1.0

On Thursday 05 February 2015 08:40:37 Richard W.M. Jones
wrote:
> virt-resize didn't preserve the per-partition GPT GUID.
> 
> Now that guests using UEFI are becoming common (basically it's the
> default on aarch64) we need to take into account that sometimes the
> partition GUID is used by the bootloader NVRAM variables to identify
> the boot partition, so it must be preserved across resize.
> 
> This bug caused the 'virt-builder --size' option to fail on
aarch64.
LGTM.

-- 
Pino Toscano

On Thu, Feb 05, 2015 at 11:14:33AM +0100, Pino Toscano
wrote:
> On Thursday 05 February 2015 08:40:37 Richard W.M. Jones wrote:
> > virt-resize didn't preserve the per-partition GPT GUID.
> > 
> > Now that guests using UEFI are becoming common (basically it's the
> > default on aarch64) we need to take into account that sometimes the
> > partition GUID is used by the bootloader NVRAM variables to identify
> > the boot partition, so it must be preserved across resize.
> > 
> > This bug caused the 'virt-builder --size' option to fail on
aarch64.
> 
> LGTM.
Thanks - pushed.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW