1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths

Michal Privoznik posted 5 patches 4 years, 8 months ago
Test syntax-check passed
Failed in applying to current master (apply log)
src/libvirt_private.syms     |  1 +
src/qemu/qemu_capabilities.c | 19 +++++++--
src/qemu/qemu_firmware.c     | 81 +++++++++++++++++++++++++++++++++++-
src/qemu/qemu_firmware.h     |  5 ++-
src/util/virfirmware.c       |  2 +-
src/util/virfirmware.h       |  5 +++
tests/qemufirmwaretest.c     | 78 ++++++++++++++++++++++++++++++----
7 files changed, 177 insertions(+), 14 deletions(-)
[libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
Posted by Michal Privoznik 4 years, 8 months ago 
It feels a bit odd to report a built in list of FW images when we have
FW descriptor files. Especially, when some weird architectures are
concerned. For instance, OVMF_CODE.fd is reported even for
non-x86_64/non-i386 arches, like ppc. But if FW descriptor files are
taken into the picture then no OVMF_CODE.fd is ever reported.

One can argue, that these patches are not necessary, because the whole
point of FW descriptor files is that users do not have to bother with
paths to FW images. And that is true. However, the whole ecosystem of
FW descriptor files allows sys admins and regular users to write their
own FW descriptor files and thus reporting what paths libvirt found
might come handy when writing those descriptors.

Michal Prívozník (5):
  virfirmware: Expose and define autoptr for virFirmwareFree
  qemu_firmware: Document qemuFirmwareGetSupported
  qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths
  qemufirmwaretest: Test FW path getting through
    qemuFirmwareGetSupported()
  qemu: Use FW descriptors to report FW image paths

 src/libvirt_private.syms     |  1 +
 src/qemu/qemu_capabilities.c | 19 +++++++--
 src/qemu/qemu_firmware.c     | 81 +++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_firmware.h     |  5 ++-
 src/util/virfirmware.c       |  2 +-
 src/util/virfirmware.h       |  5 +++
 tests/qemufirmwaretest.c     | 78 ++++++++++++++++++++++++++++++----
 7 files changed, 177 insertions(+), 14 deletions(-)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
Posted by Michal Privoznik 4 years, 7 months ago 
On 8/5/19 6:14 PM, Michal Privoznik wrote:
 >

Ping.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
Posted by Cole Robinson 4 years, 7 months ago 
On 8/5/19 12:14 PM, Michal Privoznik wrote:
> It feels a bit odd to report a built in list of FW images when we have
> FW descriptor files. Especially, when some weird architectures are
> concerned. For instance, OVMF_CODE.fd is reported even for
> non-x86_64/non-i386 arches, like ppc. But if FW descriptor files are
> taken into the picture then no OVMF_CODE.fd is ever reported.
> 
> One can argue, that these patches are not necessary, because the whole
> point of FW descriptor files is that users do not have to bother with
> paths to FW images. And that is true. However, the whole ecosystem of
> FW descriptor files allows sys admins and regular users to write their
> own FW descriptor files and thus reporting what paths libvirt found
> might come handy when writing those descriptors.
> 
> Michal Prívozník (5):
>   virfirmware: Expose and define autoptr for virFirmwareFree
>   qemu_firmware: Document qemuFirmwareGetSupported
>   qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths
>   qemufirmwaretest: Test FW path getting through
>     qemuFirmwareGetSupported()
>   qemu: Use FW descriptors to report FW image paths
> 
>  src/libvirt_private.syms     |  1 +
>  src/qemu/qemu_capabilities.c | 19 +++++++--
>  src/qemu/qemu_firmware.c     | 81 +++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_firmware.h     |  5 ++-
>  src/util/virfirmware.c       |  2 +-
>  src/util/virfirmware.h       |  5 +++
>  tests/qemufirmwaretest.c     | 78 ++++++++++++++++++++++++++++++----
>  7 files changed, 177 insertions(+), 14 deletions(-)
> 

Reviewed-by: Cole Robinson <crobinso@redhat.com>

One problem with the output though, but I think it can be fixed as a
follow on.

$ sudo ./tools/virsh domcapabilities --machine q35
...
    <loader supported='yes'>
      <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
      <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
      <value>/usr/share/edk2/ovmf/OVMF_CODE.fd</value>
      <enum name='type'>
        <value>rom</value>
        <value>pflash</value>
      </enum>
      <enum name='readonly'>
        <value>yes</value>
        <value>no</value>
      </enum>
      <enum name='secure'>
        <value>yes</value>
        <value>no</value>
      </enum>
    </loader>


Notice the double secboot listing. This is on f31 with stock packages.
Probably due to 50-edk2-ovmf-x64-sb.json and
40-edk2-ovmf-x64-sb-enrolled.json using the same firmware path. I guess
dupes should be filtered out?

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
Posted by Kashyap Chamarthy 4 years, 7 months ago 
On Mon, Aug 05, 2019 at 06:14:20PM +0200, Michal Privoznik wrote:

[...]

> Michal Prívozník (5):
>   virfirmware: Expose and define autoptr for virFirmwareFree
>   qemu_firmware: Document qemuFirmwareGetSupported
>   qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths
>   qemufirmwaretest: Test FW path getting through
>     qemuFirmwareGetSupported()
>   qemu: Use FW descriptors to report FW image paths

[...]

Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

I've just tested this patchset on Fedora 30.  (I too can reproduce the
behaviour Cole saw - duplicate 'secboot' binaries.)

Build libvirt with this:

    $> git describe
    v5.7.0-107-gb6e6d35f3f

Stop the system libvirt daemons:

    $> systemctl stop libvirtd virtlockd virtlogd

Start the daemons built from Git:

    $> sudo ./run src/virtlockd &
    $> sudo ./run src/virtlogd &
    $> sudo ./run src/libvirtd &

Make sure your EDK2/OVMF RPM has the 'secboot' binaries/VARS files:

    $> rpm -q edk2-ovmf
    edk2-ovmf-20190501stable-3.fc30.noarch

    $> rpm -ql edk2-ovmf | grep secboot
    /usr/share/OVMF/OVMF_CODE.secboot.fd
    /usr/share/OVMF/OVMF_VARS.secboot.fd
    /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd
    /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd

(The top two files are a symlink to the bottom two.)

Before invoking domCapabilities API, ensure the relevant firmware
descriptor files for x86_64 have the secboot binary listed:

    $> grep CODE.secboot /usr/share/qemu/firmware/40-edk2-ovmf-x64-sb-enrolled.json /usr/share/qemu/firmware/50-edk2-ovmf-x64-sb.json
    /usr/share/qemu/firmware/40-edk2-ovmf-x64-sb-enrolled.json:            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd",
    /usr/share/qemu/firmware/50-edk2-ovmf-x64-sb.json:            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd",

Now run 'domcapabilities' (with 'q35'):

    $> sudo tools/virsh domcapabilities --machine q35 --arch x86_64
    [...]
    <os supported='yes'>
      <enum name='firmware'>
        <value>bios</value>
        <value>efi</value>
      </enum>
      <loader supported='yes'>
        <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
        <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
        <value>/usr/share/edk2/ovmf/OVMF_CODE.fd</value>
        <value>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd</value>
        <value>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-with-csm.fd</value>
        <enum name='type'>
          <value>rom</value>
          <value>pflash</value>
        </enum>
        <enum name='readonly'>
          <value>yes</value>
          <value>no</value>
        </enum>
        <enum name='secure'>
          <value>yes</value>
          <value>no</value>
        </enum>
      </loader>
    </os>
    [...]


-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
Posted by Michal Privoznik 4 years, 7 months ago 
On 9/13/19 12:25 PM, Kashyap Chamarthy wrote:
> On Mon, Aug 05, 2019 at 06:14:20PM +0200, Michal Privoznik wrote:
> 
> [...]
> 
>> Michal Prívozník (5):
>>    virfirmware: Expose and define autoptr for virFirmwareFree
>>    qemu_firmware: Document qemuFirmwareGetSupported
>>    qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths
>>    qemufirmwaretest: Test FW path getting through
>>      qemuFirmwareGetSupported()
>>    qemu: Use FW descriptors to report FW image paths
> 
> [...]
> 
> Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

Thanks, but I've pushed it yesterday and thus I can no longer add you to 
the commit messages. Sorry.

> 
> I've just tested this patchset on Fedora 30.  (I too can reproduce the
> behaviour Cole saw - duplicate 'secboot' binaries.)
> 
> Build libvirt with this:
> 
>      $> git describe
>      v5.7.0-107-gb6e6d35f3f
> 
> Stop the system libvirt daemons:
> 
>      $> systemctl stop libvirtd virtlockd virtlogd
> 
> Start the daemons built from Git:
> 
>      $> sudo ./run src/virtlockd &
>      $> sudo ./run src/virtlogd &
>      $> sudo ./run src/libvirtd &
> 
> Make sure your EDK2/OVMF RPM has the 'secboot' binaries/VARS files:
> 
>      $> rpm -q edk2-ovmf
>      edk2-ovmf-20190501stable-3.fc30.noarch
> 
>      $> rpm -ql edk2-ovmf | grep secboot
>      /usr/share/OVMF/OVMF_CODE.secboot.fd
>      /usr/share/OVMF/OVMF_VARS.secboot.fd
>      /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd
>      /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd
> 
> (The top two files are a symlink to the bottom two.)
> 
> Before invoking domCapabilities API, ensure the relevant firmware
> descriptor files for x86_64 have the secboot binary listed:
> 
>      $> grep CODE.secboot /usr/share/qemu/firmware/40-edk2-ovmf-x64-sb-enrolled.json /usr/share/qemu/firmware/50-edk2-ovmf-x64-sb.json
>      /usr/share/qemu/firmware/40-edk2-ovmf-x64-sb-enrolled.json:            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd",
>      /usr/share/qemu/firmware/50-edk2-ovmf-x64-sb.json:            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd",

Yeah, I've posted a patch for that:

https://www.redhat.com/archives/libvir-list/2019-September/msg00485.html

The reason is that (I suspect) 40-edk2-ovmf-x64-sb-enrolled.json and 
50-edk2-ovmf-x64-sb.json have the same _CODE but different _VARS. 
Therefore, in qemuFirmwareGetSupported() (which is called when 
constructing domcaps) we effectivelly see two different firmwares:

https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_firmware.c;h=f316a26a5ba5c14d993fa8b96df32eba3c1f0997;hb=HEAD#l1536

Therefore, they are appended onto @fws list which is then passed to 
virQEMUCapsFillDomainLoaderCaps() in virQEMUCapsFillDomainOSCaps() which 
doesn't deduplicate them.

But the same thing can happen even without FW descriptors: ./configure 
--with-loader-nvram="/dev/null:X:/dev/null:Y" (a very dumb config, but 
serves the point) which declares [{CODE = "/dev/null", NVRAM = "X"}, 
{CODE = "/dev/null", NVRAM  "Y"}] pair. In both cases the FW image is 
the same and therefore it would be printed twice. To test this with 
latest git just revert e9d51a221c1871 because now libvirt picks FW 
descriptors and thus configure arg (or correspondinf qemu.conf knob) is 
ignrored.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.