9ae3a8
From a49a3e6984fdb8562003cff96a82b2ac7d9bcc0d Mon Sep 17 00:00:00 2001
9ae3a8
Message-Id: <a49a3e6984fdb8562003cff96a82b2ac7d9bcc0d.1383564115.git.minovotn@redhat.com>
9ae3a8
In-Reply-To: <5575e0aec51f40ebec46e98ec085cda053283aba.1383564115.git.minovotn@redhat.com>
9ae3a8
References: <5575e0aec51f40ebec46e98ec085cda053283aba.1383564115.git.minovotn@redhat.com>
9ae3a8
From: Markus Armbruster <armbru@redhat.com>
9ae3a8
Date: Fri, 27 Sep 2013 13:31:13 +0200
9ae3a8
Subject: [PATCH 03/14] vl: Fix -boot order and once regressions, and related
9ae3a8
 bugs
9ae3a8
9ae3a8
RH-Author: Markus Armbruster <armbru@redhat.com>
9ae3a8
Message-id: <1380288680-26645-4-git-send-email-armbru@redhat.com>
9ae3a8
Patchwork-id: 54567
9ae3a8
O-Subject: [PATCH 7.0 qemu-kvm 03/10] vl: Fix -boot order and once regressions, and related bugs
9ae3a8
Bugzilla: 997817
9ae3a8
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
9ae3a8
RH-Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
9ae3a8
9ae3a8
From: Markus Armbruster <armbru@redhat.com>
9ae3a8
9ae3a8
Option "once" sets up a different boot order just for the initial
9ae3a8
boot.  Boot order reverts back to normal on reset.  Option "order"
9ae3a8
changes the normal boot order.
9ae3a8
9ae3a8
The reversal is implemented by reset handler restore_boot_devices(),
9ae3a8
which takes the boot order to revert to as argument.
9ae3a8
restore_boot_devices() does nothing on its first call, because that
9ae3a8
must be the initial machine reset.  On its second call, it changes the
9ae3a8
boot order back, and unregisters itself.
9ae3a8
9ae3a8
Because we register the handler right when -boot gets parsed, we can
9ae3a8
revert to an incorrect normal boot order, and multiple -boot can
9ae3a8
interact in funny ways.
9ae3a8
9ae3a8
Here's how things work without -boot once or order:
9ae3a8
9ae3a8
* boot_devices is "".
9ae3a8
9ae3a8
* main() passes machine->boot_order to to machine->init(), because
9ae3a8
  boot_devices is "".  machine->init() configures firmware
9ae3a8
  accordingly.  For PC machines, machine->boot_order is "cad", and
9ae3a8
  pc_cmos_init() writes it to RTC CMOS, where SeaBIOS picks it up.
9ae3a8
9ae3a8
Now consider -boot order=:
9ae3a8
9ae3a8
* boot_devices is "".
9ae3a8
9ae3a8
* -boot order= sets boot_devices to "" (no change).
9ae3a8
9ae3a8
* main() passes machine->boot_order to to machine->init(), because
9ae3a8
  boot_devices is "", as above.
9ae3a8
9ae3a8
  Bug: -boot order= has no effect.  Broken in commit e4ada29e.
9ae3a8
9ae3a8
Next, consider -boot once=a:
9ae3a8
9ae3a8
* boot_devices is "".
9ae3a8
9ae3a8
* -boot once=a registers restore_boot_devices() with argument "", and
9ae3a8
  sets boot_devices to "a".
9ae3a8
9ae3a8
* main() passes boot_devices "a" to machine->init(), which configures
9ae3a8
  firmware accordingly.  For PC machines, pc_cmos_init() writes the
9ae3a8
  boot order to RTC CMOS.
9ae3a8
9ae3a8
* main() calls qemu_system_reset().  This runs reset handlers.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Does
9ae3a8
    nothing, because it's the first call.
9ae3a8
9ae3a8
* Machine boots, boot order is "a".
9ae3a8
9ae3a8
* Machine resets (e.g. monitor command).  Reset handlers run.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Calls
9ae3a8
    qemu_boot_set("") to reconfigure firmware.  For PC machines,
9ae3a8
    pc_boot_set() writes it into RTC CMOS.  Reset handler
9ae3a8
    unregistered.
9ae3a8
9ae3a8
    Bug: boot order reverts to "" instead of machine->boot_order.  The
9ae3a8
    actual boot order depends on how firmware interprets "".  Broken
9ae3a8
    in commit e4ada29e.
9ae3a8
9ae3a8
Next, consider -boot once=a -boot order=c:
9ae3a8
9ae3a8
* boot_devices is "".
9ae3a8
9ae3a8
* -boot once=a registers restore_boot_devices() with argument "", and
9ae3a8
  sets boot_devices to "a".
9ae3a8
9ae3a8
* -boot order=c sets boot_devices to "c".
9ae3a8
9ae3a8
* main() passes boot_devices "c" to machine->init(), which configures
9ae3a8
  firmware accordingly.  For PC machines, pc_cmos_init() writes the
9ae3a8
  boot order to RTC CMOS.
9ae3a8
9ae3a8
* main() calls qemu_system_reset().  This runs reset handlers.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Does
9ae3a8
    nothing, because it's the first call.
9ae3a8
9ae3a8
* Machine boots, boot order is "c".
9ae3a8
9ae3a8
  Bug: it should be "a".  I figure this has always been broken.
9ae3a8
9ae3a8
* Machine resets (e.g. monitor command).  Reset handlers run.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Calls
9ae3a8
    qemu_boot_set("") to reconfigure firmware.  For PC machines,
9ae3a8
    pc_boot_set() writes it into RTC CMOS.  Reset handler
9ae3a8
    unregistered.
9ae3a8
9ae3a8
    Bug: boot order reverts to "" instead of "c".  I figure this has
9ae3a8
    always been broken, just differently broken before commit
9ae3a8
    e4ada29e.
9ae3a8
9ae3a8
Next, consider -boot once=a -boot once=b -boot once=c:
9ae3a8
9ae3a8
* boot_devices is "".
9ae3a8
9ae3a8
* -boot once=a registers restore_boot_devices() with argument "", and
9ae3a8
  sets boot_devices to "a".
9ae3a8
9ae3a8
* -boot once=b registers restore_boot_devices() with argument "a", and
9ae3a8
  sets boot_devices to "b".
9ae3a8
9ae3a8
* -boot once=c registers restore_boot_devices() with argument "b", and
9ae3a8
  sets boot_devices to "c".
9ae3a8
9ae3a8
* main() passes boot_devices "c" to machine->init(), which configures
9ae3a8
  firmware accordingly.  For PC machines, pc_cmos_init() writes the
9ae3a8
  boot order to RTC CMOS.
9ae3a8
9ae3a8
* main() calls qemu_system_reset().  This runs reset handlers.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Does
9ae3a8
    nothing, because it's the first call.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "a".  Calls
9ae3a8
    qemu_boot_set("a") to reconfigure firmware.  For PC machines,
9ae3a8
    pc_boot_set() writes it into RTC CMOS.  Reset handler
9ae3a8
    unregistered.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "b".  Calls
9ae3a8
    qemu_boot_set("b") to reconfigure firmware.  For PC machines,
9ae3a8
    pc_boot_set() writes it into RTC CMOS.  Reset handler
9ae3a8
    unregistered.
9ae3a8
9ae3a8
* Machine boots, boot order is "b".
9ae3a8
9ae3a8
  Bug: should really be "c", because that came last, and for all other
9ae3a8
  -boot options, the last one wins.  I figure this was broken some
9ae3a8
  time before commit 37905d6a, and fixed there only for a single
9ae3a8
  occurence of "once".
9ae3a8
9ae3a8
* Machine resets (e.g. monitor command).  Reset handlers run.
9ae3a8
9ae3a8
  - restore_boot_devices() gets called with argument "".  Calls
9ae3a8
    qemu_boot_set("") to reconfigure firmware.  For PC machines,
9ae3a8
    pc_boot_set() writes it into RTC CMOS.  Reset handler
9ae3a8
    unregistered.
9ae3a8
9ae3a8
    Same bug as above: boot order reverts to "" instead of
9ae3a8
    machine->boot_order.
9ae3a8
9ae3a8
Fix by acting upon -boot options order, once and menu only after
9ae3a8
option parsing is complete, and the machine is known.  This is how the
9ae3a8
other -boot options work already.
9ae3a8
9ae3a8
Signed-off-by: Markus Armbruster <armbru@redhat.com>
9ae3a8
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
9ae3a8
Message-id: 1371208516-7857-4-git-send-email-armbru@redhat.com
9ae3a8
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
9ae3a8
(cherry picked from commit 8281abd548d840d84223e66812491918c713e56c)
9ae3a8
---
9ae3a8
 vl.c | 59 ++++++++++++++++++++++++++++++-----------------------------
9ae3a8
 1 file changed, 30 insertions(+), 29 deletions(-)
9ae3a8
9ae3a8
Signed-off-by: Michal Novotny <minovotn@redhat.com>
9ae3a8
---
9ae3a8
 vl.c | 59 ++++++++++++++++++++++++++++++-----------------------------
9ae3a8
 1 file changed, 30 insertions(+), 29 deletions(-)
9ae3a8
9ae3a8
diff --git a/vl.c b/vl.c
9ae3a8
index a5663ad..1c3236c 100644
9ae3a8
--- a/vl.c
9ae3a8
+++ b/vl.c
9ae3a8
@@ -2795,7 +2795,7 @@ int main(int argc, char **argv, char **envp)
9ae3a8
     const char *icount_option = NULL;
9ae3a8
     const char *initrd_filename;
9ae3a8
     const char *kernel_filename, *kernel_cmdline;
9ae3a8
-    char boot_devices[33] = "";
9ae3a8
+    const char *boot_order = NULL;
9ae3a8
     DisplayState *ds;
9ae3a8
     int cyls, heads, secs, translation;
9ae3a8
     QemuOpts *hda_opts = NULL, *opts, *machine_opts;
9ae3a8
@@ -3086,31 +3086,9 @@ int main(int argc, char **argv, char **envp)
9ae3a8
                 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
9ae3a8
                 break;
9ae3a8
             case QEMU_OPTION_boot:
9ae3a8
-                {
9ae3a8
-                    char *standard_boot_devices;
9ae3a8
-                    const char *order, *once;
9ae3a8
-
9ae3a8
-                    opts = qemu_opts_parse(qemu_find_opts("boot-opts"),
9ae3a8
-                                           optarg, 1);
9ae3a8
-                    if (!opts) {
9ae3a8
-                        exit(1);
9ae3a8
-                    }
9ae3a8
-
9ae3a8
-                    order = qemu_opt_get(opts, "order");
9ae3a8
-                    if (order) {
9ae3a8
-                        validate_bootdevices(order);
9ae3a8
-                        pstrcpy(boot_devices, sizeof(boot_devices), order);
9ae3a8
-                    }
9ae3a8
-
9ae3a8
-                    once = qemu_opt_get(opts, "once");
9ae3a8
-                    if (once) {
9ae3a8
-                        validate_bootdevices(once);
9ae3a8
-                        standard_boot_devices = g_strdup(boot_devices);
9ae3a8
-                        pstrcpy(boot_devices, sizeof(boot_devices), once);
9ae3a8
-                        qemu_register_reset(restore_boot_devices,
9ae3a8
-                                            standard_boot_devices);
9ae3a8
-                    }
9ae3a8
-                    boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
9ae3a8
+                opts = qemu_opts_parse(qemu_find_opts("boot-opts"), optarg, 1);
9ae3a8
+                if (!opts) {
9ae3a8
+                    exit(1);
9ae3a8
                 }
9ae3a8
                 break;
9ae3a8
             case QEMU_OPTION_fda:
9ae3a8
@@ -4049,6 +4027,31 @@ int main(int argc, char **argv, char **envp)
9ae3a8
     initrd_filename = qemu_opt_get(machine_opts, "initrd");
9ae3a8
     kernel_cmdline = qemu_opt_get(machine_opts, "append");
9ae3a8
 
9ae3a8
+    if (!boot_order) {
9ae3a8
+        boot_order = machine->boot_order;
9ae3a8
+    }
9ae3a8
+    opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
9ae3a8
+    if (opts) {
9ae3a8
+        char *normal_boot_order;
9ae3a8
+        const char *order, *once;
9ae3a8
+
9ae3a8
+        order = qemu_opt_get(opts, "order");
9ae3a8
+        if (order) {
9ae3a8
+            validate_bootdevices(order);
9ae3a8
+            boot_order = order;
9ae3a8
+        }
9ae3a8
+
9ae3a8
+        once = qemu_opt_get(opts, "once");
9ae3a8
+        if (once) {
9ae3a8
+            validate_bootdevices(once);
9ae3a8
+            normal_boot_order = g_strdup(boot_order);
9ae3a8
+            boot_order = once;
9ae3a8
+            qemu_register_reset(restore_boot_devices, normal_boot_order);
9ae3a8
+        }
9ae3a8
+
9ae3a8
+        boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
9ae3a8
+    }
9ae3a8
+
9ae3a8
     if (!kernel_cmdline) {
9ae3a8
         kernel_cmdline = "";
9ae3a8
     }
9ae3a8
@@ -4213,9 +4216,7 @@ int main(int argc, char **argv, char **envp)
9ae3a8
     qdev_machine_init();
9ae3a8
 
9ae3a8
     QEMUMachineInitArgs args = { .ram_size = ram_size,
9ae3a8
-                                 .boot_device = (boot_devices[0] == '\0') ?
9ae3a8
-                                                machine->boot_order :
9ae3a8
-                                                boot_devices,
9ae3a8
+                                 .boot_device = boot_order,
9ae3a8
                                  .kernel_filename = kernel_filename,
9ae3a8
                                  .kernel_cmdline = kernel_cmdline,
9ae3a8
                                  .initrd_filename = initrd_filename,
9ae3a8
-- 
9ae3a8
1.7.11.7
9ae3a8