thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH] resize: properly restore GPT partition types [Feb 2014]

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2014-Feb-03 19:04 UTC

[Libguestfs] [PATCH] resize: properly restore GPT partition types

If there is a GPT partition layout, then what should be read and
restored for each partition is the GPT type and not the MBR ID.

Related to RHBZ#1060404.
---
 resize/resize.ml | 46 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/resize/resize.ml b/resize/resize.ml
index 8683df7..a2670e5 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -48,6 +48,7 @@ type partition = {
   p_part : G.partition;          (* SOURCE partition data from libguestfs. *)
   p_bootable : bool;             (* Is it bootable? *)
   p_mbr_id : int option;         (* MBR ID, if it has one. *)
+  p_gpt_type : string option;    (* GPT ID, if it has one. *)
   p_type : partition_content;    (* Content type and content size. *)
 
   (* What we're going to do: *)
@@ -75,7 +76,14 @@ let rec debug_partition p      p.p_part.G.part_size;
   eprintf "\tbootable: %b\n" p.p_bootable;
   eprintf "\tpartition ID: %s\n"
-    (match p.p_mbr_id with None -> "(none)" | Some i -> sprintf
"0x%x" i);
+    (match p.p_mbr_id, p.p_gpt_type with
+    | None, None -> "(none)"
+    | Some i, None -> sprintf "0x%x" i
+    | None, Some i -> i
+    | Some _, Some _ ->
+      (* This should not happen. *)
+      assert false
+    );
   eprintf "\tcontent: %s\n" (string_of_partition_content p.p_type)
 and string_of_partition_content = function
   | ContentUnknown -> "unknown data"
@@ -440,15 +448,21 @@ read the man page virt-resize(1).
           let part_num = Int32.to_int part_num in
           let name = sprintf "/dev/sda%d" part_num in
           let bootable = g#part_get_bootable "/dev/sda" part_num in
-          let mbr_id -            try Some (g#part_get_mbr_id
"/dev/sda" part_num)
-            with G.Error _ -> None in
+          let mbr_id, gpt_type +            match parttype with
+            | GPT ->
+              None, (try Some (g#part_get_gpt_type "/dev/sda"
part_num)
+                     with G.Error _ -> None)
+            | MBR ->
+              (try Some (g#part_get_mbr_id "/dev/sda" part_num)
+              with G.Error _ -> None), None in
           let typ              if is_extended_partition mbr_id then
ContentExtendedPartition
             else get_partition_content name in
 
           { p_name = name; p_part = part;
-            p_bootable = bootable; p_mbr_id = mbr_id; p_type = typ;
+            p_bootable = bootable; p_mbr_id = mbr_id; p_gpt_type = gpt_type;
+            p_type = typ;
             p_operation = OpCopy; p_target_partnum = 0;
             p_target_start = 0L; p_target_end = 0L }
       ) parts in
@@ -1025,7 +1039,8 @@ read the man page virt-resize(1).
             p_name = "";
             p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
                        part_size = 0L };
-            p_bootable = false; p_mbr_id = None; p_type = ContentUnknown;
+            p_bootable = false; p_mbr_id = None; p_gpt_type = None;
+            p_type = ContentUnknown;
 
             (* Target information is meaningful. *)
             p_operation = OpIgnore;
@@ -1103,11 +1118,20 @@ read the man page virt-resize(1).
       if p.p_bootable then
         g#part_set_bootable "/dev/sdb" p.p_target_partnum true;
 
-      (match p.p_mbr_id with
-      | None -> ()
-      | Some mbr_id ->
-        g#part_set_mbr_id "/dev/sdb" p.p_target_partnum mbr_id
-      );
+      match parttype with
+      | GPT ->
+        (match p.p_gpt_type with
+        | None -> ()
+        | Some gpt_type ->
+          g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type
+        )
+      | MBR ->
+        (match p.p_mbr_id with
+        | None -> ()
+        | Some mbr_id ->
+          g#part_set_mbr_id "/dev/sdb" p.p_target_partnum mbr_id
+        )
+
   ) partitions;
 
   (* Fix the bootloader if we aligned the first partition. *)
-- 
1.8.3.1

Richard W.M. Jones

2014-Feb-03 20:09 UTC

head link

Re: [Libguestfs] [PATCH] resize: properly restore GPT partition types

On Mon, Feb 03, 2014 at 08:04:05PM +0100, Pino Toscano
wrote:> If there is a GPT partition layout, then what should be read and
> restored for each partition is the GPT type and not the MBR ID.
> 
> Related to RHBZ#1060404.
> ---
>  resize/resize.ml | 46 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/resize/resize.ml b/resize/resize.ml
> index 8683df7..a2670e5 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -48,6 +48,7 @@ type partition = {
>    p_part : G.partition;          (* SOURCE partition data from libguestfs.
*)
>    p_bootable : bool;             (* Is it bootable? *)
>    p_mbr_id : int option;         (* MBR ID, if it has one. *)
> +  p_gpt_type : string option;    (* GPT ID, if it has one. *)
>    p_type : partition_content;    (* Content type and content size. *)
>  
>    (* What we're going to do: *)
> @@ -75,7 +76,14 @@ let rec debug_partition p >     
p.p_part.G.part_size;
>    eprintf "\tbootable: %b\n" p.p_bootable;
>    eprintf "\tpartition ID: %s\n"
> -    (match p.p_mbr_id with None -> "(none)" | Some i ->
sprintf "0x%x" i);
> +    (match p.p_mbr_id, p.p_gpt_type with
> +    | None, None -> "(none)"
> +    | Some i, None -> sprintf "0x%x" i
> +    | None, Some i -> i
> +    | Some _, Some _ ->
> +      (* This should not happen. *)
> +      assert false
You can actually make it not happen by having a clearer type.  I
believe something along these lines should work:

type partition = {
  ...
  p_partition_id : partition_id;
  ...
}
and partition_id = No_ID | MBR_ID of int | GPT_ID of string
> +      match parttype with
> +      | GPT ->
> +        (match p.p_gpt_type with
> +        | None -> ()
> +        | Some gpt_type ->
> +          g#part_set_gpt_type "/dev/sdb" p.p_target_partnum
gpt_type
> +        )
> +      | MBR ->
> +        (match p.p_mbr_id with
> +        | None -> ()
> +        | Some mbr_id ->
> +          g#part_set_mbr_id "/dev/sdb" p.p_target_partnum mbr_id
> +        )
With the type above, you could write:

  match parttype, partition_id with
  | GPT, GPT_ID gpt_type -> g#part_set_gpt_type ...
  | MBR, MBR_ID mbr_id -> g#part_set_mbr_id ...
  | GPT, (No_ID | MBR_ID)
  | MBR, (No_ID | GPT_ID) -> ()

The patch generally looks fine so ACK with these changes.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Richard W.M. Jones

2014-Feb-03 20:11 UTC

head link

Re: [Libguestfs] [PATCH] resize: properly restore GPT partition types

On Mon, Feb 03, 2014 at 08:09:51PM +0000, Richard W.M. Jones
wrote:>   | GPT, (No_ID | MBR_ID)
>   | MBR, (No_ID | GPT_ID) -> ()
Oops, missing an '_':

  | GPT, (No_ID | MBR_ID _)
  | MBR, (No_ID | GPT_ID _) -> ()

Rich.

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

Apparently Analagous Threads

Search for more reasonably related threads

Libguestfs - Feb 2014 - Re: [PATCH] resize: properly restore GPT partition types

[Libguestfs] [PATCH] resize: properly restore GPT partition types

Re: [Libguestfs] [PATCH] resize: properly restore GPT partition types

Re: [Libguestfs] [PATCH] resize: properly restore GPT partition types

Apparently Analagous Threads