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

[Richard W.M. Jones]

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/