Hi, The attached patch adds support for resizing MBR logical partitions. The failure is still there, I can't get any helpful information from lsof. Any suggestions? From: root <root(a)localhost.localdomain&gt; Date: Mon, 2 Jun 2014 23:30:57 -0400 Subject: [PATCH] resize: add support for MBR logical partitions Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; --- resize/resize.ml | 243 +++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 202 insertions(+), 41 deletions(-) diff --git a/resize/resize.ml b/resize/resize.ml index 9614ec7..a476076 100644 --- a/resize/resize.ml +++ b/resize/resize.ml @@ -50,6 +50,10 @@ 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_part_num: int; (* partition number *) + + mutable p_partitions : partition list; (* MBR logical partitions. Non-empty + list implies extended partition *) (* What we're going to do: *) mutable p_operation : partition_operation; @@ -90,7 +94,8 @@ let rec debug_partition p = (match p.p_label with | Some label -> label | None -> "(none)" - ) + ); + List.iter debug_partition p.p_partitions and string_of_partition_content = function | ContentUnknown -> "unknown data" | ContentPV sz -> sprintf "LVM PV (%Ld bytes)" sz @@ -443,6 +448,14 @@ read the man page virt-resize(1). if List.length parts = 0 then error (f_"the source disk has no partitions"); + let logical_partitions = + match parttype with + | GPT -> [] + | MBR -> + List.filter (fun ({ G.part_num = part_num }) -> + part_num >= 5_l + ) parts in + (* Filter out logical partitions. See note above. *) let parts = match parttype with @@ -453,10 +466,8 @@ read the man page virt-resize(1). | _ -> true ) parts in - let partitions = - List.map ( - fun ({ G.part_num = part_num } as part) -> - let part_num = Int32.to_int part_num in + let to_partition part = + let part_num = Int32.to_int part.G.part_num in let name = sprintf "/dev/sda%d" part_num in let bootable = g#part_get_bootable "/dev/sda" part_num in let id = @@ -476,13 +487,58 @@ read the man page virt-resize(1). { p_name = name; p_part = part; p_bootable = bootable; p_id = id; p_type = typ; - p_label = label; + p_label = label; p_part_num = part_num; + p_partitions = []; p_operation = OpCopy; p_target_partnum = 0; p_target_start = 0L; p_target_end = 0L } + in + + let logical_partitions = + List.map ( + fun part -> to_partition part + ) logical_partitions in + + let logical_partitions_num = List.length logical_partitions in + + let partitions = + List.map ( + fun part -> to_partition part ) parts in + let extended_partitions = + match parttype with + | GPT -> [] + | MBR -> + List.filter (fun ({ p_type = typ }) -> + typ = ContentExtendedPartition + ) partitions in + + + (* Filter out extended partition. *) + let partitions = + match parttype with + | GPT -> partitions + | MBR -> + List.filter (fun ({ p_type = typ }) -> + typ <> ContentExtendedPartition + ) partitions in + + assert ((List.length extended_partitions) <= 1); + + let cmp_partition parta partb = + if parta.p_part_num < partb.p_part_num then -1 + else if parta.p_part_num = partb.p_part_num then 0 + else 1 in + + List.iter (fun part -> + part.p_partitions <- List.merge cmp_partition part.p_partitions logical_partitions + ) extended_partitions; + + let partitions = List.merge cmp_partition partitions extended_partitions in + if verbose then ( - eprintf "%d partitions found\n" (List.length partitions); + eprintf "%d partitions found\n" ((List.length partitions) + + logical_partitions_num); List.iter debug_partition partitions ); @@ -506,14 +562,27 @@ read the man page virt-resize(1). ) partitions; (* Check partitions don't overlap. *) - let rec loop end_of_prev = function + let rec loop end_of_prev prev_typ = function + | [] -> () + | { p_name = name; p_part = { G.part_start = part_start }; p_type = typ } :: _ + when end_of_prev > part_start && prev_typ <> ContentExtendedPartition -> + error (f_"%s: this partition overlaps the previous one") name + | { p_part = { G.part_end = part_end }; p_type = typ } :: parts -> loop part_end typ parts + in + loop 0L ContentUnknown partitions; + + (* check logical partitions don't overlap *) + let rec loop end_of_prev extended_part = function | [] -> () - | { p_name = name; p_part = { G.part_start = part_start } } :: _ - when end_of_prev > part_start -> + | {p_name = name; p_part = {G.part_start = part_start } } :: _ + when end_of_prev > part_start -> error (f_"%s: this partition overlaps the previous one") name - | { p_part = { G.part_end = part_end } } :: parts -> loop part_end parts + | { p_part = { G.part_end = part_end; G.part_start = part_start } } :: parts -> + loop part_end extended_part parts in - loop 0L partitions; + List.iter (fun part -> + loop part.p_part.G.part_start part part.p_partitions + ) extended_partitions; partitions in @@ -578,6 +647,12 @@ read the man page virt-resize(1). let hash = Hashtbl.create 13 in List.iter (fun ({ p_name = name } as p) -> Hashtbl.add hash name p) partitions; + List.iter (fun p -> + if p.p_type = ContentExtendedPartition then ( + List.iter (fun ({ p_name = name } as p) -> Hashtbl.add hash name p) + p.p_partitions + ) + ) partitions; fun ~option name -> let name = if String.length name < 5 || String.sub name 0 5 <> "/dev/" then @@ -692,6 +767,26 @@ read the man page virt-resize(1). List.iter (do_resize ~option:"--resize") resizes; List.iter (do_resize ~option:"--resize-force" ~force:true) resizes_force; + (* handle resizing of logical partitions *) + List.iter ( + fun p -> + if p.p_type = ContentExtendedPartition then ( + let sectsize = Int64.of_int sectsize in + let size = roundup64 p.p_part.G.part_size sectsize in + let logical_sizes = List.fold_left ( + fun total p -> + match p.p_operation with + | OpDelete -> total +^ 0L + | OpCopy | OpIgnore -> total +^ p.p_part.G.part_size + | OpResize newsize -> total +^ newsize + ) 0L p.p_partitions in + if logical_sizes > size then + p.p_operation <- OpResize logical_sizes + (* don't touch the extended partition if logical sizes less + * then the original size *) + ) + ) partitions; + (* Helper function calculates the surplus space, given the total * required so far for the current partition layout, compared to * the size of the target disk. If the return value >= 0 then it's @@ -816,29 +911,31 @@ read the man page virt-resize(1). printf "**********\n\n"; printf "Summary of changes:\n\n"; - List.iter ( - fun ({ p_name = name; p_part = { G.part_size = oldsize }} as p) -> + let rec print_summary p = let text = match p.p_operation with | OpCopy -> - sprintf (f_"%s: This partition will be left alone.") name + sprintf (f_"%s: This partition will be left alone.") p.p_name | OpIgnore -> - sprintf (f_"%s: This partition will be created, but the contents will be ignored (ie. not copied to the target).") name + sprintf (f_"%s: This partition will be created, but the contents will be ignored (ie. not copied to the target).") p.p_name | OpDelete -> - sprintf (f_"%s: This partition will be deleted.") name + sprintf (f_"%s: This partition will be deleted.") p.p_name | OpResize newsize -> sprintf (f_"%s: This partition will be resized from %s to %s.") - name (human_size oldsize) (human_size newsize) ^ + p.p_name (human_size p.p_part.G.part_size) (human_size newsize) ^ if can_expand_content p.p_type then ( sprintf (f_" The %s on %s will be expanded using the '%s' method.") (string_of_partition_content_no_size p.p_type) - name + p.p_name (string_of_expand_content_method (expand_content_method p.p_type)) ) else "" in - wrap ~indent:4 (text ^ "\n\n") - ) partitions; + wrap ~indent:4 (text ^ "\n\n"); + + List.iter print_summary p.p_partitions in + + List.iter print_summary partitions; List.iter ( fun ({ lv_name = name } as lv) -> @@ -1009,10 +1106,11 @@ read the man page virt-resize(1). let partitions = let sectsize = Int64.of_int sectsize in - let rec loop partnum start = function + let rec loop partnum start gap create_surplus = function | p :: ps -> + let start = start +^ gap in (match p.p_operation with - | OpDelete -> loop partnum start ps (* skip p *) + | OpDelete -> loop partnum start 0L create_surplus ps (* skip p *) | OpIgnore | OpCopy -> (* same size *) (* Size in sectors. *) @@ -1026,26 +1124,28 @@ read the man page virt-resize(1). partnum start (end_ -^ 1L); { p with p_target_start = start; p_target_end = end_ -^ 1L; - p_target_partnum = partnum } :: loop (partnum+1) next ps + p_target_partnum = partnum; p_partitions = loop 5 start 1L + false p.p_partitions } :: loop (partnum+1) next 0L create_surplus ps | OpResize newsize -> (* resized partition *) + let oldsize = p.p_part.G.part_size in (* New size in sectors. *) let size = (newsize +^ sectsize -^ 1L) /^ sectsize in (* Start of next partition + alignment. *) let next = start +^ size in let next = roundup64 next alignment in - if verbose then - eprintf "target partition %d: resize: newsize=%Ld start=%Ld end=%Ld\n%!" - partnum newsize start (next -^ 1L); + eprintf "target partition %d: resize: oldsize=%Ld newsize=%Ld start=%Ld end=%Ld\n%!" + partnum oldsize newsize start (next -^ 1L); { p with p_target_start = start; p_target_end = next -^ 1L; - p_target_partnum = partnum } :: loop (partnum+1) next ps + p_target_partnum = partnum; p_partitions = loop 5 start 1L + false p.p_partitions } :: loop (partnum+1) next 0L create_surplus ps ) | [] -> (* Create the surplus partition if there is room for it. *) - if extra_partition && surplus >= min_extra_partition then ( + if create_surplus && extra_partition && surplus >= min_extra_partition then ( [ { (* Since this partition has no source, this data is * meaningless and not used since the operation is @@ -1056,6 +1156,8 @@ read the man page virt-resize(1). part_size = 0L }; p_bootable = false; p_id = No_ID; p_type = ContentUnknown; p_label = None; + p_part_num = 0; + p_partitions = []; (* Target information is meaningful. *) p_operation = OpIgnore; @@ -1078,14 +1180,52 @@ read the man page virt-resize(1). (* Preserve the existing start, but convert to sectors. *) (List.hd partitions).p_part.G.part_start /^ sectsize in - loop 1 start partitions in + loop 1 start 0L true partitions in + + let mbr_part_type x = + if x.p_part_num <= 4 && x.p_type <> ContentExtendedPartition then "primary" + else if x.p_part_num <= 4 && x.p_type = ContentExtendedPartition then "extended" + else "logical" in (* Now partition the target disk. *) List.iter ( fun p -> - g#part_add "/dev/sdb" "primary" p.p_target_start p.p_target_end + try + if p.p_operation <> OpIgnore then ( + g#part_add "/dev/sdb" (mbr_part_type p) p.p_target_start p.p_target_end + ); + if p.p_type = ContentExtendedPartition then + List.iter ( + fun p -> + if p.p_operation <> OpIgnore then + try + g#part_add "/dev/sdb" "logical" p.p_target_start p.p_target_end + with G.Error msg -> () + ) p.p_partitions + with + G.Error msg -> + (* eprintf "lsof:\n---\n%s\n---\n" (g#debug "sh" [| "lsof" * |]); *) + eprintf "original error: %s\n" msg; ) partitions; + let g = + g#shutdown (); + g#close (); + + let g = new G.guestfs () in + if trace then g#set_trace true; + if verbose then g#set_verbose true; + let _, { URI.path = path; protocol = protocol; + server = server; username = username; + password = password } = infile in + g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password path; + (* The output disk is being created, so use cache=unsafe here. *) + g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe" + outfile; + if not quiet then Progress.set_up_progress_bar ~machine_readable g; + g#launch (); + g in + (* Copy over the data. *) List.iter ( fun p -> @@ -1113,13 +1253,31 @@ read the man page virt-resize(1). g#copy_device_to_device ~size:copysize ~sparse source target | ContentExtendedPartition -> - (* You can't just copy an extended partition by name, eg. - * source = "/dev/sda2", because the device name only covers - * the first 1K of the partition. Instead, copy the - * source bytes from the parent disk (/dev/sda). - *) - let srcoffset = p.p_part.G.part_start in - g#copy_device_to_device ~srcoffset ~size:copysize "/dev/sda" target + List.iter ( + fun p -> + match p.p_operation with + | OpCopy | OpResize _ -> + let oldsize = p.p_part.G.part_size in + let newsize = + match p.p_operation with OpResize s -> s | _ -> oldsize in + + let copysize = if newsize < oldsize then + newsize else oldsize in + + let source = p.p_name in + let target = sprintf "/dev/sdb%d" p.p_target_partnum in + + if not quiet then + printf (f_"Copying %s ...\n%!") source; + + (match p.p_type with + | ContentUnknown | ContentPV _ | ContentFS _ -> + g#copy_device_to_device ~size:copysize ~sparse source target + | _ -> () + ) + | OpIgnore | OpDelete -> () + ) p.p_partitions + ) | OpIgnore | OpDelete -> () ) partitions; @@ -1240,8 +1398,7 @@ read the man page virt-resize(1). in (* Expand partition content as required. *) - List.iter ( - function + let expand_partition = function | ({ p_operation = OpResize _ } as p) when can_expand_content p.p_type -> let source = p.p_name in @@ -1256,7 +1413,11 @@ read the man page virt-resize(1). do_expand_content target meth | { p_operation = (OpCopy | OpIgnore | OpDelete | OpResize _) } - -> () + -> () in + + List.iter ( + fun p -> expand_partition p; List.iter expand_partition + p.p_partitions ) partitions; (* Expand logical volume content as required. *) -- 1.9.3

On Mon, Sep 15, 2014 at 05:47:03PM +0800, Hu Tao wrote: This weekend I found a bug that might be similar to this one. See: https://bugzilla.redhat.com/show_bug.cgi?id=1141451 If it's the same thing, it might be fixed by calling 'udev_settle' after copy_device_to_device (see daemon/copy.c). So let's make that clear by having a separate list. Yes, definitely it needs a test. Rich. -- 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