resize: Add --align-first auto|never|always option.

The first partition can now be aligned.  We fix the bootloader
correctly for Windows by adjusting the "Hidden Sectors" field.

3 files changed, 129 insertions, 16 deletions

diff --git a/resize/resize.ml b/resize/resize.ml
index aed0e439..ef279cda 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -28,8 +28,10 @@ let min_extra_partition = 10L *^ 1024L *^ 1024L
 (* Command line argument parsing. *)
 let prog = Filename.basename Sys.executable_name
 
-let infile, outfile, alignment, copy_boot_loader, debug, deletes, dryrun,
-  expand, expand_content, extra_partition, format, ignores,
+type align_first_t = [ `Never | `Always | `Auto ]
+
+let infile, outfile, align_first, alignment, copy_boot_loader, debug, deletes,
+  dryrun, expand, expand_content, extra_partition, format, ignores,
   lv_expands, machine_readable, ntfsresize_force, output_format,
   quiet, resizes, resizes_force, shrink =
   let display_version () =
@@ -42,6 +44,7 @@ let infile, outfile, alignment, copy_boot_loader, debug, deletes, dryrun,
 
   let add xs s = xs := s :: !xs in
 
+  let align_first = ref "auto" in
   let alignment = ref 128 in
   let copy_boot_loader = ref true in
   let debug = ref false in
@@ -72,6 +75,7 @@ let infile, outfile, alignment, copy_boot_loader, debug, deletes, dryrun,
   in
 
   let argspec = Arg.align [
+    "--align-first", Arg.Set_string align_first, "never|always|auto Align first partition (default: auto)";
     "--alignment", Arg.Set_int alignment,   "sectors Set partition alignment (default: 128 sectors)";
     "--no-copy-boot-loader", Arg.Clear copy_boot_loader, " Don't copy boot loader";
     "-d",        Arg.Set debug,             " Enable debugging messages";
@@ -142,6 +146,14 @@ read the man page virt-resize(1).
     error "alignment cannot be < 1";
   let alignment = Int64.of_int alignment in
 
+  let align_first =
+    match !align_first with
+    | "never" -> `Never
+    | "always" -> `Always
+    | "auto" -> `Auto
+    | _ ->
+      error "unknown --align-first option: use never|always|auto" in
+
   (* No arguments and machine-readable mode?  Print out some facts
    * about what this binary supports.  We only need to print out new
    * things added since this option, or things which depend on features
@@ -153,6 +165,7 @@ read the man page virt-resize(1).
     printf "32bitok\n";
     printf "128-sector-alignment\n";
     printf "alignment\n";
+    printf "align-first\n";
     let g = new G.guestfs () in
     g#add_drive_opts "/dev/null";
     g#launch ();
@@ -170,8 +183,8 @@ read the man page virt-resize(1).
     | _ ->
         error "usage is: %s [--options] indisk outdisk" prog in
 
-  infile, outfile, alignment, copy_boot_loader, debug, deletes, dryrun,
-  expand, expand_content, extra_partition, format, ignores,
+  infile, outfile, align_first, alignment, copy_boot_loader, debug, deletes,
+  dryrun, expand, expand_content, extra_partition, format, ignores,
   lv_expands, machine_readable, ntfsresize_force, output_format,
   quiet, resizes, resizes_force, shrink
 
@@ -807,6 +820,22 @@ let () =
     ignore (g#pwrite_device "/dev/sdb" loader start)
   )
 
+(* Are we going to align the first partition and fix the bootloader? *)
+let align_first_partition_and_fix_bootloader =
+  (* Bootloaders that we know how to fix. *)
+  let can_fix_boot_loader =
+    match partitions with
+    | { p_type = ContentFS ("ntfs", _); p_bootable = true;
+        p_operation = OpCopy | OpIgnore | OpResize _ } :: _ -> true
+    | _ -> false
+  in
+
+  match align_first, can_fix_boot_loader with
+  | `Never, _
+  | `Auto, false -> false
+  | `Always, _
+  | `Auto, true -> true
+
 (* Repartition the target disk. *)
 
 (* Calculate the location of the partitions on the target disk.  This
@@ -869,12 +898,18 @@ let partitions =
         []
   in
 
-  (* The first partition must start at the same position as the old
-   * first partition.  Old virt-resize used to align this to 64
-   * sectors, but I suspect this is the cause of boot failures, so
-   * let's not do this.
+  (* Choose the alignment of the first partition based on the
+   * '--align-first' option.  Old virt-resize used to always align this
+   * to 64 sectors, but this causes boot failures unless we are able to
+   * adjust the bootloader accordingly.
    *)
-  let start = (List.hd partitions).p_part.G.part_start /^ sectsize in
+  let start =
+    if align_first_partition_and_fix_bootloader then
+      alignment
+    else
+      (* Preserve the existing start, but convert to sectors. *)
+      (List.hd partitions).p_part.G.part_start /^ sectsize in
+
   loop 1 start partitions
 
 (* Now partition the target disk. *)
@@ -922,6 +957,41 @@ let () =
       | _ -> ()
   ) partitions
 
+(* Fix the bootloader if we aligned the first partition. *)
+let () =
+  if align_first_partition_and_fix_bootloader then (
+    (* See can_fix_boot_loader above. *)
+    match partitions with
+    | { p_type = ContentFS ("ntfs", _); p_bootable = true;
+        p_target_partnum = partnum; p_target_start = start } :: _ ->
+      (* If the first partition is NTFS and bootable, set the "Number of
+       * Hidden Sectors" field in the NTFS Boot Record so that the
+       * filesystem is still bootable.
+       *)
+
+      (* Should always be /dev/sdb1? *)
+      let target = sprintf "/dev/sdb%d" partnum in
+
+      (* Sanity check: it contains the NTFS magic. *)
+      let magic = g#pread_device target 8 3L in
+      if magic <> "NTFS    " then
+        eprintf "warning: first partition is NTFS but does not contain NTFS boot loader magic\n%!"
+      else (
+        if not quiet then
+          printf "Fixing first NTFS partition boot record ...\n%!";
+
+        if debug then (
+          let old_hidden = int_of_le32 (g#pread_device target 4 0x1c_L) in
+          eprintf "old hidden sectors value: 0x%Lx\n%!" old_hidden
+        );
+
+        let new_hidden = le32_of_int start in
+        ignore (g#pwrite_device target new_hidden 0x1c_L)
+      )
+
+    | _ -> ()
+  )
+
 (* After copying the data over we must shut down and restart the
  * appliance in order to expand the content.  The reason for this may
  * not be obvious, but it's because otherwise we'll have duplicate VGs
diff --git a/resize/utils.ml b/resize/utils.ml
index 38519757..dc134be6 100644
--- a/resize/utils.ml
+++ b/resize/utils.ml
@@ -27,6 +27,29 @@ let ( /^ ) = Int64.div
 let ( &^ ) = Int64.logand
 let ( ~^ ) = Int64.lognot
 
+let int_of_le32 str =
+  assert (String.length str = 4);
+  let c0 = Char.code (String.unsafe_get str 0) in
+  let c1 = Char.code (String.unsafe_get str 1) in
+  let c2 = Char.code (String.unsafe_get str 2) in
+  let c3 = Char.code (String.unsafe_get str 3) in
+  Int64.of_int c0 +^
+    (Int64.shift_left (Int64.of_int c1) 8) +^
+    (Int64.shift_left (Int64.of_int c2) 16) +^
+    (Int64.shift_left (Int64.of_int c3) 24)
+
+let le32_of_int i =
+  let c0 = i &^ 0xffL in
+  let c1 = Int64.shift_right (i &^ 0xff00L) 8 in
+  let c2 = Int64.shift_right (i &^ 0xff0000L) 16 in
+  let c3 = Int64.shift_right (i &^ 0xff000000L) 24 in
+  let s = String.create 4 in
+  String.unsafe_set s 0 (Char.unsafe_chr (Int64.to_int c0));
+  String.unsafe_set s 1 (Char.unsafe_chr (Int64.to_int c1));
+  String.unsafe_set s 2 (Char.unsafe_chr (Int64.to_int c2));
+  String.unsafe_set s 3 (Char.unsafe_chr (Int64.to_int c3));
+  s
+
 let output_spaces chan n = for i = 0 to n-1 do output_char chan ' ' done
 
 let wrap ?(chan = stdout) ?(hanging = 0) str =
diff --git a/resize/virt-resize.pod b/resize/virt-resize.pod
index 8ae4894f..4ce3a4ea 100644
--- a/resize/virt-resize.pod
+++ b/resize/virt-resize.pod
@@ -246,6 +246,28 @@ C<dd if=/dev/zero of=outdisk bs=1M count=..>)
 
 Display help.
 
+=item B<--align-first auto>
+
+=item B<--align-first never>
+
+=item B<--align-first always>
+
+Align the first partition for improved performance (see also the
+I<--alignment> option).
+
+The default is I<--align-first auto> which only aligns the first
+partition if it is safe to do so.  That is, only when we know how to
+fix the bootloader automatically, and at the moment that can only be
+done for Windows guests.
+
+I<--align-first never> means we never move the first partition.
+This is the safest option.  Try this if the guest does not boot
+after resizing.
+
+I<--align-first always> means we always align the first partition (if
+it needs to be aligned).  For some guests this will break the
+bootloader, making the guest unbootable.
+
 =item B<--alignment N>
 
 Set the alignment of partitions to C<N> sectors.  The default in
@@ -590,10 +612,10 @@ not required by any modern operating system.
 
 In Windows Vista and later versions, Microsoft switched to using a
 separate boot partition.  In these VMs, typically C</dev/sda1> is the
-boot partition and C</dev/sda2> is the main (C:) drive.  We have not
-had any luck resizing the boot partition.  Doing so seems to break the
-guest completely.  However expanding the second partition (ie. C:
-drive) should work.
+boot partition and C</dev/sda2> is the main (C:) drive.  Resizing the
+first (boot) partition causes the bootloader to fail with
+C<0xC0000225> error.  Resizing the second partition (ie. C: drive)
+should work.
 
 Windows may initiate a lengthy "chkdsk" on first boot after a resize,
 if NTFS partitions have been expanded.  This is just a safety check
@@ -602,9 +624,7 @@ and (unless it find errors) is nothing to worry about.
 =head2 GUEST BOOT STUCK AT "GRUB"
 
 If a Linux guest does not boot after resizing, and the boot is stuck
-after printing C<GRUB> on the console, try reinstalling grub.  This
-sometimes happens on older (RHEL 5-era) guests, for reasons we don't
-fully understand, although we think is to do with partition alignment.
+after printing C<GRUB> on the console, try reinstalling grub.
 
  guestfish -i -a newdisk
  ><fs> cat /boot/grub/device.map