[Libguestfs] virt-resize: support to MBR logical partitions and some question
Hu Tao
hutao at cn.fujitsu.com
Tue Jul 15 07:45:19 UTC 2014
Hi Rich,
On Wed, Jun 04, 2014 at 10:02:55AM +0100, Richard W.M. Jones wrote:
>
> On Wed, Jun 04, 2014 at 10:21:41AM +0800, Hu Tao wrote:
> > Hi,
> >
> > I'm adding support to resizing logical partitions(patch is in progess).
> > But encounter an error when adding a logical partition in dest image:
> >
> > virt-resize: libguestfs error: part_add: parted: /dev/sdb: Warning: The resulting partition is not properly aligned for best performance.
> > Error: Error informing the kernel about modifications to partition /dev/sdb5 -- Device or resource busy. This means Linux won't know about any changes you made to /dev/sdb5 until you reboot -- so you shouldn't mount it or use it in any way before rebooting.
> > Error: Failed to add partition 5 (Device or resource busy)
> >
> > The error is actually no harm since the logical partition has been added
> > successfully, and I don't want to inform kernel at all. But it prevents
> > virt-resize from adding further logical partitions.
> >
> > I can ignore the error when adding logical partitions manually using parted.
> > The question is, is there any way to ignore such errors in virt-resize?
>
> This may indicate a bug in the daemon.
>
> Normally after parted runs, it tries to get the kernel to re-read
> partition tables. This can fail because for several reasons -- for
> example, because a partition is mounted (and for many other reasons,
> including udev and/or the kernel just getting confused.
>
> We try to avoid this if we can by various tricks, eg. see use of
> udev_settle function in the daemon.
>
> Can you come up with a minimal reproducer (eg. using just guestfish
> commands)? If so, then file a bug.
guestfish just works with my patch(attached):
---
[root at f20-x64 libguestfs-git]# rm -f delme.img && truncate -s 5G delme.img && ./run guestfish -a delme.img <<EOF
run
part-init /dev/sda msdos
part-add /dev/sda extended 512 -1
part-add /dev/sda logical 2049 -1
EOF
[root at f20-x64 libguestfs-git]# fish/guestfish -a delme.img <<EOF
run
list-partitions
EOF
/dev/sda1
/dev/sda5
---
But virt-resize doesn't. The error message is:
Setting up initial partition table on test.img ...
virt-resize: libguestfs error: part_add: parted: /dev/sdb: Warning: The resulting partition is not properly aligned for best performance.
Error: Error informing the kernel about modifications to partition /dev/sdb5 -- Device or resource busy. This means Linux won't know about any changes you made to /dev/sdb5 until you reboot -- so you shouldn't mount it or use it in any way before rebooting.
Error: Failed to add partition 5 (Device or resource busy)
This is the same as I reported earlier. At this time /dev/sdb has been added
successfully despite of the error message. But the bad is I can't write
to /dev/sdb5, aka copy /dev/sda5 to /dev/sdb5. The error message is:
---
Setting up initial partition table on test.img ...
Copying /dev/sda1 ...
100% ⟦▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒⟧ 00:00
Copying /dev/sda2 ...
100% ⟦▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒⟧ 00:00
Copying /dev/sda3 ...
Copying /dev/sda5 ...
virt-resize: libguestfs error: copy_device_to_device: copy_device_to_device_stub: /dev/sdb5: No such file or directory
---
The solution could be:
1. as the error message suggests, rebooting. But I don't known how since it
runs in an applicance if I'm right.
2. fix parted informing the kernel about modifications to partitions. Do
you have any insight of this? If you think this is a possible
solution I'll start invetigate it.
Regards,
Hu
-------------- next part --------------
>From ea4ff4a7db571a65476b43f5993bd23d6e05aec3 Mon Sep 17 00:00:00 2001
From: Hu Tao <hutao at cn.fujitsu.com>
Date: Mon, 2 Jun 2014 23:30:57 -0400
Subject: [PATCH] resize: add support for MBR logical partitions
---
resize/resize.ml | 202 ++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 171 insertions(+), 31 deletions(-)
diff --git a/resize/resize.ml b/resize/resize.ml
index 9614ec7..9705efc 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;
@@ -62,6 +66,7 @@ and partition_content =
| ContentPV of int64 (* physical volume (size of PV) *)
| ContentFS of string * int64 (* mountable filesystem (FS type, FS size) *)
| ContentExtendedPartition (* MBR extended partition *)
+ | ContentLogicalPartition (* MBR logical partition *)
and partition_operation =
| OpCopy (* copy it as-is, no resizing *)
| OpIgnore (* ignore it (create on target, but don't
@@ -96,11 +101,13 @@ and string_of_partition_content = function
| ContentPV sz -> sprintf "LVM PV (%Ld bytes)" sz
| ContentFS (fs, sz) -> sprintf "filesystem %s (%Ld bytes)" fs sz
| ContentExtendedPartition -> "extended partition"
+ | ContentLogicalPartition -> "logical partition"
and string_of_partition_content_no_size = function
| ContentUnknown -> "unknown data"
| ContentPV _ -> "LVM PV"
| ContentFS (fs, _) -> sprintf "filesystem %s" fs
| ContentExtendedPartition -> "extended partition"
+ | ContentLogicalPartition -> "logical partition"
(* Data structure describing LVs on the source disk. This is only
* used if the user gave the --lv-expand option.
@@ -443,6 +450,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 +468,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 +489,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 +564,16 @@ 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 } } :: _
- 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_name = name; p_part = { G.part_start = part_start }; p_type = typ } :: _
+ when end_of_prev > part_start && prev_typ <> ContentExtendedPartition && typ <> ContentLogicalPartition ->
+ 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 partitions;
+ loop 0L ContentUnknown partitions;
+
+ (* TODO: check logical partitions don't overlap *)
partitions in
@@ -526,7 +586,7 @@ read the man page virt-resize(1).
let typ = get_partition_content name in
assert (
match typ with
- | ContentPV _ | ContentExtendedPartition -> false
+ | ContentPV _ | ContentExtendedPartition | ContentLogicalPartition -> false
| ContentUnknown | ContentFS _ -> true
);
@@ -553,6 +613,7 @@ read the man page virt-resize(1).
| ContentFS (("btrfs"), _) when !btrfs_available -> true
| ContentFS (_, _) -> false
| ContentExtendedPartition -> false
+ | ContentLogicalPartition -> false (* FIXME *)
else
fun _ -> false
@@ -566,6 +627,7 @@ read the man page virt-resize(1).
| ContentFS (("btrfs"), _) when !btrfs_available -> BtrfsFilesystemResize
| ContentFS (_, _) -> assert false
| ContentExtendedPartition -> assert false
+ | ContentLogicalPartition -> assert false (* FIXME *)
else
fun _ -> assert false
in
@@ -578,6 +640,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
@@ -659,6 +727,9 @@ read the man page virt-resize(1).
| ContentExtendedPartition ->
error (f_"%s: This extended partition contains logical partitions which might be damaged by shrinking it. If you want to shrink this partition, you need to use the '--resize-force' option, but that could destroy logical partitions within this partition. (This error came from '%s' option on the command line.)")
name option
+ | ContentLogicalPartition -> (* FIXME *)
+ error (f_"%s: This extended partition contains logical partitions which might be damaged by shrinking it. If you want to shrink this partition, you need to use the '--resize-force' option, but that could destroy logical partitions within this partition. (This error came from '%s' option on the command line.)")
+ name option
);
p.p_operation <- OpResize newsize
@@ -692,6 +763,30 @@ 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
+ eprintf "size: %Ld\n" size;
+ let logical_sizes = List.fold_left (
+ fun total p ->
+ match p.p_operation with
+ | OpDelete -> eprintf " d--- 0\n"; total +^ 0L
+ | OpCopy | OpIgnore -> eprintf "c--- %Ld\n"
+ p.p_part.G.part_size; total +^ p.p_part.G.part_size
+ | OpResize newsize -> eprintf "r--- %Ld\n" newsize; total +^ newsize
+ ) 0L p.p_partitions in
+ eprintf "logical size: %Ld\n" logical_sizes;
+ 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 true ps (* skip p *)
| OpIgnore | OpCopy -> (* same size *)
(* Size in sectors. *)
@@ -1026,9 +1124,10 @@ 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 } :: loop (partnum+1) next 0L true 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. *)
@@ -1036,16 +1135,17 @@ read the man page virt-resize(1).
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 true 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,12 +1180,22 @@ 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
+ 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 ->
+ g#part_add "/dev/sdb" "logical" p.p_target_start p.p_target_end
+ ) p.p_partitions
) partitions;
(* Copy over the data. *)
@@ -1113,6 +1225,7 @@ 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
@@ -1120,6 +1233,32 @@ read the man page virt-resize(1).
*)
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
+
+ | ContentLogicalPartition -> () (* FIXME *)
)
| OpIgnore | OpDelete -> ()
) partitions;
@@ -1181,6 +1320,7 @@ read the man page virt-resize(1).
| { p_type =
(ContentFS _|ContentUnknown|ContentPV _
|ContentExtendedPartition) } :: _
+ | { p_type = (ContentLogicalPartition) } :: _
| [] -> ()
);
--
1.9.3
More information about the Libguestfs
mailing list