On 08/23/13 11:27, Michael Chang wrote:
> On Thu, Aug 22, 2013 at 06:05:58PM -0700, Jordan Justen wrote:
>> On Mon, Aug 19, 2013 at 3:00 AM, Michael Chang <[email protected]> wrote:
>>> On Sat, Aug 17, 2013 at 11:16:16PM -0700, Jordan Justen wrote:
>>>> The volatile 'NvVars' variable indicates that the variables do
>>>> not need to be loaded from the file again. After we write the
>>>> variables out to the file, there is clearly no need to load
>>>> them back from the file.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Jordan Justen <[email protected]>
>>>> Cc: Michael Chang <[email protected]>
>>>> Cc: Laszlo Ersek <[email protected]>
>>>> ---
>>>> Michael,
>>>>
>>>> Does this patch fix the issue you highlighted (way back) on June
>>>> 21st in your patch:
>>>> * OvmfPkg/NvVarsFileLib: handle the inital file not found
>>>
>>> It seems not fix the problem if I use regular openSUSE 12.3
>>> installation to a new virtual disk or chooses to recreate the ESP
>>> if it already exists.
>>>
>>> However the manual test step works, if without your patch it fails. I
>>> list the steps for reference.
>>>
>>> Fistly to simulate the initial (disk) condition.
>>>
>>> Delete NvVars File
>>>  $ rm /boot/efi/NvVars
>>> Delete openSUSE boot entry
>>>  $ efibootmgr -b 0005 -B
>>> Delete NvVars variables
>>>  $ cd /sys/firmware/efi/vars
>>>  $ cat NvVars-<GUID ..>/raw_var > del_var
>>> Power off
>>>  $ poweroff
>>>
>>> Now we can start vm to install the bootloader, use efi shell to
>>> launch the installed grub2. In the booted system, use
>>>  $ grub2-install
>>>
>>> Check if boot variable are written correctly
>>>  $ efibootmgr -v
>>>
>>> Reboot and check if opensuse shows in the EFI boot manager ..
>>>
>>> So there could be some other corner cases, but I cannot tell it now.
>>> I can continue to investigate it or do you have any other better
>>> idea?
>>
>> Do these failing scenarios work with your original patch?
> 
> My patch fails as well.

I agree.

The problematic sequence is this:
1. LoadNvVarsFromFs() doesn't find L"NvVars" in RAM
2. LoadNvVarsFromFs() doesn't find file store --> doesn't set L"NvVars"
3. SaveNvVarsToFs() *succeeds*
4. ExitBootServices()
5. RT variable change, in RAM only
6. warm reboot
7. LoadNvVarsFromFs() doesn't find L"NvVars" in RAM (due to step 2)
8. LoadNvVarsFromFs() loads the file store (due to step 3),
   overwriting/losing step 5

This sequence (bug) doesn't apply when we start out with an
unpartitioned disk, because step 3 doesn't work there.

The sequence (bug) doesn't apply either when we start with a formatted
EFI System Partition that contains the file store, because step 2
succeeds then, and step 7 and step 8 don't happen.

The sequence (bug) applies when we start with a formatted EFI System
Partition that has no file store. Under those circumstances Jordan's
patch prevents the bug too. And it is pretty much to the point: step 3
is the immediate cause of the lossage in step 8, and the patch prevents
steps 7 and 8 right in step 3, by setting L"NvVars".

For Jordan's patch (...too, because I acked Michael's as well):

Reviewed-by: Laszlo Ersek <[email protected]>

(The commit message does not reflect the above steps precisely, but
let's not obsess about this any longer!)


> I think the problem is I'm using virt-intall, and there might be some
> tricks on how libvirt restarts domain after installation finished that
> may actually restart from power-down ? Laszlo did you aware any issue
> reported for libvirt before ?

If you power off the VM at step 6, you lose all changes from step 5
right there, independently of your original "partitioning / file store
status".

Regarding virt-install... Check out '--print-step' in the virt-install
manual: virt-install distinguishes three steps (three first boots) of a VM.

    --print-step
      Acts similarly to --print-xml, except requires specifying which
      install step to print XML for. Possible values are 1, 2, 3, or
      all. Stage 1 is typically booting from the install media, and
      stage 2 is typically the final guest config booting off hardisk.
      Stage 3 is only relevant for windows installs, which by default
      have a second install stage. This option implies --quiet.

I guess this distinction between steps is mainly important so that the
install media can be ejected in a "safe way".

However, when I install an OVMF guest, I don't allow virt-install to
actually start the VM. I documented my method in
<http://www.linux-kvm.org/page/OVMF>.

In a nutshell, I only prepare a domain XML file with virt-install
(--print-step=1).

Then I post-process the XML (because these things are not directly
supported by virt-install on my RHEL-6 laptop):
- setting OVMF.fd as boot firmware binary,
- setting a wrapper script around qemu-kvm as emulator,
- potentially setting the main disk type to virtio-scsi.

Then I virsh-define the VM from the template XML, perhaps effect further
changes in virt-manager (which are easiest to do there), and finally I
start the VM in the usual way.

I need the wrapper script around qemu-kvm because I want to control
aspects of the VM that libvirt (in RHEL-6) doesn't enable me to, or not
in a way that I wanted to. And libvirt + wrapper script is way more
convenient than the raw qemu command line.


Closing note... Even with Jordan's patch in place (or with Michael's,
makes no difference in this regard), the OS installer can *still* store
a wrong device path in the boot option (step 5) before the warm reboot
(step 6). So, even if
- this issue is fixed, and
- the reboot is warm too, and
- the installer media has been removed (or the qemu boot order lists it
  as second),
the user may still not see an automatic reboot into the OS just
installed, if the device path is borked.

... This is exactly what happens with the RHEL-6 / Fedora(-19?)
installer, on virtio (-blk or -scsi) target disk :)

https://bugzilla.redhat.com/show_bug.cgi?id=998611

Thanks
Laszlo

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to