Skip to content
  • Markus Armbruster's avatar
    vl: Fix -boot order and once regressions, and related bugs · 8281abd5
    Markus Armbruster authored
    Option "once" sets up a different boot order just for the initial
    boot.  Boot order reverts back to normal on reset.  Option "order"
    changes the normal boot order.
    
    The reversal is implemented by reset handler restore_boot_devices(),
    which takes the boot order to revert to as argument.
    restore_boot_devices() does nothing on its first call, because that
    must be the initial machine reset.  On its second call, it changes the
    boot order back, and unregisters itself.
    
    Because we register the handler right when -boot gets parsed, we can
    revert to an incorrect normal boot order, and multiple -boot can
    interact in funny ways.
    
    Here's how things work without -boot once or order:
    
    * boot_devices is "".
    
    * main() passes machine->boot_order to to machine->init(), because
      boot_devices is "".  machine->init() configures firmware
      accordingly.  For PC machines, machine->boot_order is "cad", and
      pc_cmos_init() writes it to RTC CMOS, where SeaBIOS picks it up.
    
    Now consider -boot order=:
    
    * boot_devices is "".
    
    * -boot order= sets boot_devices to "" (no change).
    
    * main() passes machine->boot_order to to machine->init(), because
      boot_devices is "", as above.
    
      Bug: -boot order= has no effect.  Broken in commit e4ada29e.
    
    Next, consider -boot once=a:
    
    * boot_devices is "".
    
    * -boot once=a registers restore_boot_devices() with argument "", and
      sets boot_devices to "a".
    
    * main() passes boot_devices "a" to machine->init(), which configures
      firmware accordingly.  For PC machines, pc_cmos_init() writes the
      boot order to RTC CMOS.
    
    * main() calls qemu_system_reset().  This runs reset handlers.
    
      - restore_boot_devices() gets called with argument "".  Does
        nothing, because it's the first call.
    
    * Machine boots, boot order is "a".
    
    * Machine resets (e.g. monitor command).  Reset handlers run.
    
      - restore_boot_devices() gets called with argument "".  Calls
        qemu_boot_set("") to reconfigure firmware.  For PC machines,
        pc_boot_set() writes it into RTC CMOS.  Reset handler
        unregistered.
    
        Bug: boot order reverts to "" instead of machine->boot_order.  The
        actual boot order depends on how firmware interprets "".  Broken
        in commit e4ada29e.
    
    Next, consider -boot once=a -boot order=c:
    
    * boot_devices is "".
    
    * -boot once=a registers restore_boot_devices() with argument "", and
      sets boot_devices to "a".
    
    * -boot order=c sets boot_devices to "c".
    
    * main() passes boot_devices "c" to machine->init(), which configures
      firmware accordingly.  For PC machines, pc_cmos_init() writes the
      boot order to RTC CMOS.
    
    * main() calls qemu_system_reset().  This runs reset handlers.
    
      - restore_boot_devices() gets called with argument "".  Does
        nothing, because it's the first call.
    
    * Machine boots, boot order is "c".
    
      Bug: it should be "a".  I figure this has always been broken.
    
    * Machine resets (e.g. monitor command).  Reset handlers run.
    
      - restore_boot_devices() gets called with argument "".  Calls
        qemu_boot_set("") to reconfigure firmware.  For PC machines,
        pc_boot_set() writes it into RTC CMOS.  Reset handler
        unregistered.
    
        Bug: boot order reverts to "" instead of "c".  I figure this has
        always been broken, just differently broken before commit
        e4ada29e.
    
    Next, consider -boot once=a -boot once=b -boot once=c:
    
    * boot_devices is "".
    
    * -boot once=a registers restore_boot_devices() with argument "", and
      sets boot_devices to "a".
    
    * -boot once=b registers restore_boot_devices() with argument "a", and
      sets boot_devices to "b".
    
    * -boot once=c registers restore_boot_devices() with argument "b", and
      sets boot_devices to "c".
    
    * main() passes boot_devices "c" to machine->init(), which configures
      firmware accordingly.  For PC machines, pc_cmos_init() writes the
      boot order to RTC CMOS.
    
    * main() calls qemu_system_reset().  This runs reset handlers.
    
      - restore_boot_devices() gets called with argument "".  Does
        nothing, because it's the first call.
    
      - restore_boot_devices() gets called with argument "a".  Calls
        qemu_boot_set("a") to reconfigure firmware.  For PC machines,
        pc_boot_set() writes it into RTC CMOS.  Reset handler
        unregistered.
    
      - restore_boot_devices() gets called with argument "b".  Calls
        qemu_boot_set("b") to reconfigure firmware.  For PC machines,
        pc_boot_set() writes it into RTC CMOS.  Reset handler
        unregistered.
    
    * Machine boots, boot order is "b".
    
      Bug: should really be "c", because that came last, and for all other
      -boot options, the last one wins.  I figure this was broken some
      time before commit 37905d6a
    
    , and fixed there only for a single
      occurence of "once".
    
    * Machine resets (e.g. monitor command).  Reset handlers run.
    
      - restore_boot_devices() gets called with argument "".  Calls
        qemu_boot_set("") to reconfigure firmware.  For PC machines,
        pc_boot_set() writes it into RTC CMOS.  Reset handler
        unregistered.
    
        Same bug as above: boot order reverts to "" instead of
        machine->boot_order.
    
    Fix by acting upon -boot options order, once and menu only after
    option parsing is complete, and the machine is known.  This is how the
    other -boot options work already.
    
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Reviewed-by: default avatarAnthony Liguori <aliguori@us.ibm.com>
    Message-id: 1371208516-7857-4-git-send-email-armbru@redhat.com
    Signed-off-by: default avatarAnthony Liguori <aliguori@us.ibm.com>
    8281abd5