[v3,1/2] qemu: Add AAVMF32 to the list of known UEFIs

Message ID 20170720195656.20904-2-dann.frazier@canonical.com
State New
Headers show
Series
  • qemu: Add AAVMF32 to the list of known UEFIs
Related show

Commit Message

dann frazier July 20, 2017, 7:56 p.m.
Add a path for UEFI VMs for AArch32 VMs, based on the path Debian is using.
libvirt is the de facto canonical location for defining where distros
should place these firmware images, so let's define this path here to try
and minimize distro fragmentation.
---
 src/qemu/qemu.conf                                           |  3 ++-
 src/qemu/qemu_conf.c                                         | 12 ++++++++----
 src/qemu/test_libvirtd_qemu.aug.in                           |  1 +
 tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml             |  1 +
 tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml |  1 +
 tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml |  1 +
 tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            |  1 +
 tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            |  1 +
 tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml             |  1 +
 tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              |  1 +
 tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml         |  1 +
 tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              |  1 +
 tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml             |  1 +
 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml         |  1 +
 tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml         |  1 +
 tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml             |  1 +
 tests/domaincapstest.c                                       |  1 +
 17 files changed, 25 insertions(+), 5 deletions(-)

-- 
2.13.2

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

Comments

John Ferlan July 20, 2017, 8:46 p.m. | #1
On 07/20/2017 03:56 PM, dann frazier wrote:
> Add a path for UEFI VMs for AArch32 VMs, based on the path Debian is using.

> libvirt is the de facto canonical location for defining where distros

> should place these firmware images, so let's define this path here to try

> and minimize distro fragmentation.

> ---

>  src/qemu/qemu.conf                                           |  3 ++-

>  src/qemu/qemu_conf.c                                         | 12 ++++++++----

>  src/qemu/test_libvirtd_qemu.aug.in                           |  1 +

>  tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml             |  1 +

>  tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml |  1 +

>  tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml |  1 +

>  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            |  1 +

>  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            |  1 +

>  tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml             |  1 +

>  tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              |  1 +

>  tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml         |  1 +

>  tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              |  1 +

>  tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml             |  1 +

>  tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml         |  1 +

>  tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml         |  1 +

>  tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml             |  1 +

>  tests/domaincapstest.c                                       |  1 +

>  17 files changed, 25 insertions(+), 5 deletions(-)

> 


FWIW: I did consult with the QEMU UEFI (Laszlo Ersek) who was fine with
the /usr/share/AAVMF/AAVMF32_{CODE,VARS}.fd. He also forwarded along to
Leif Lindholm and Ard Biesheuvel and there were no objections there.

I think what's here does look good - I'd like to give it a couple of
days to "percolate" to ensure no one has heartburn before pushing though
as there are some folks on this list far more involved in the AAVMF than
I am.

John

> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf

> index e6c0832662..1d81472df0 100644

> --- a/src/qemu/qemu.conf

> +++ b/src/qemu/qemu.conf

> @@ -677,7 +677,8 @@

>  #nvram = [

>  #   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",

>  #   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd",

> -#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"

> +#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd",

> +#   "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd"

>  #]

>  

>  # The backend to use for handling stdout/stderr output from

> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c

> index a65c92a262..9dd9442d63 100644

> --- a/src/qemu/qemu_conf.c

> +++ b/src/qemu/qemu_conf.c

> @@ -131,6 +131,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)

>  #define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"

>  #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"

>  #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"

> +#define VIR_QEMU_AAVMF32_LOADER_PATH "/usr/share/AAVMF/AAVMF32_CODE.fd"

> +#define VIR_QEMU_AAVMF32_NVRAM_PATH "/usr/share/AAVMF/AAVMF32_VARS.fd"

>  

>  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>  {

> @@ -335,11 +337,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>          goto error;

>  

>  #else

> -    if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)

> +    if (VIR_ALLOC_N(cfg->firmwares, 4) < 0)

>          goto error;

> -    cfg->nfirmwares = 3;

> +    cfg->nfirmwares = 4;

>      if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||

> -        VIR_ALLOC(cfg->firmwares[2]) < 0)

> +        VIR_ALLOC(cfg->firmwares[2]) < 0 || VIR_ALLOC(cfg->firmwares[3]) < 0)

>          goto error;

>  

>      if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||

> @@ -347,7 +349,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>          VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||

>          VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||

>          VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 0 ||

> -        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0)

> +        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0 ||

> +        VIR_STRDUP(cfg->firmwares[3]->name, VIR_QEMU_AAVMF32_LOADER_PATH) < 0 ||

> +        VIR_STRDUP(cfg->firmwares[3]->nvram, VIR_QEMU_AAVMF32_NVRAM_PATH) < 0)

>          goto error;

>  #endif

>  

> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in

> index 3e317bc7e9..676d48cf5c 100644

> --- a/src/qemu/test_libvirtd_qemu.aug.in

> +++ b/src/qemu/test_libvirtd_qemu.aug.in

> @@ -90,6 +90,7 @@ module Test_libvirtd_qemu =

>      { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" }

>      { "2" = "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd" }

>      { "3" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" }

> +    { "4" = "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd" }

>  }

>  { "stdio_handler" = "logd" }

>  { "gluster_debug_level" = "9" }

> diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml

> index 1eadba393f..8d1ad86570 100644

> --- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml

> index 54b89dc72b..7c019b2308 100644

> --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml

> index 60bf2f54f7..700dc618b2 100644

> --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml

> index 1a980927cf..3b14280621 100644

> --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml

> index 4ecf8651b4..5b986e52f7 100644

> --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml

> +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml

> index 843bdc2b73..de81886237 100644

> --- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml

> index dc6d2d8f0c..b93d00ece1 100644

> --- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml

> +++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml

> index 962cd0557f..d860cd833f 100644

> --- a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml

> index 53c3190f20..ee40d1e84e 100644

> --- a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml

> +++ b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml

> index 7d5ac063fa..33161f7b82 100644

> --- a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml

> index f6d54d9a12..07e1db641a 100644

> --- a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml

> index 413f3fa7ce..af606493aa 100644

> --- a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml

> index e8fe01d85e..2fc3b72138 100644

> --- a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml

> +++ b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml

> @@ -7,6 +7,7 @@

>    <os supported='yes'>

>      <loader supported='yes'>

>        <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>

> +      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>

>        <value>/usr/share/OVMF/OVMF_CODE.fd</value>

>        <enum name='type'>

>          <value>rom</value>

> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c

> index 5a36fcf29d..78ed4109a2 100644

> --- a/tests/domaincapstest.c

> +++ b/tests/domaincapstest.c

> @@ -240,6 +240,7 @@ fillQemuCaps(virDomainCapsPtr domCaps,

>  

>      if (fillStringValues(&loader->values,

>                           "/usr/share/AAVMF/AAVMF_CODE.fd",

> +                         "/usr/share/AAVMF/AAVMF32_CODE.fd",

>                           "/usr/share/OVMF/OVMF_CODE.fd",

>                           NULL) < 0)

>          goto cleanup;

> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Andrea Bolognani July 21, 2017, 10:52 a.m. | #2
On Thu, 2017-07-20 at 13:56 -0600, dann frazier wrote:
> @@ -131,6 +131,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
>  #define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
>  #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"
>  #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"
> +#define VIR_QEMU_AAVMF32_LOADER_PATH "/usr/share/AAVMF/AAVMF32_CODE.fd"
> +#define VIR_QEMU_AAVMF32_NVRAM_PATH "/usr/share/AAVMF/AAVMF32_VARS.fd"
>  
>  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>  {
> @@ -335,11 +337,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>          goto error;
>  
>  #else
> -    if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)
> +    if (VIR_ALLOC_N(cfg->firmwares, 4) < 0)
>          goto error;
> -    cfg->nfirmwares = 3;
> +    cfg->nfirmwares = 4;
>      if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||
> -        VIR_ALLOC(cfg->firmwares[2]) < 0)
> +        VIR_ALLOC(cfg->firmwares[2]) < 0 || VIR_ALLOC(cfg->firmwares[3]) < 0)
>          goto error;
>  
>      if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
> @@ -347,7 +349,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>          VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
>          VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||
>          VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 0 ||
> -        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0)
> +        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0 ||
> +        VIR_STRDUP(cfg->firmwares[3]->name, VIR_QEMU_AAVMF32_LOADER_PATH) < 0 ||
> +        VIR_STRDUP(cfg->firmwares[3]->nvram, VIR_QEMU_AAVMF32_NVRAM_PATH) < 0)
>          goto error;
>  #endif

Not your fault of course, but the way this is done at the
moment is *extremely* yucky.

I just posted a patch[1] that makes it saner, and although I
realize that's a bit more work I'd ask you to wait for it to
land in master and then respin on top of it.

Everything else looks good though :)


[1] https://www.redhat.com/archives/libvir-list/2017-July/msg00870.html
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
John Ferlan July 21, 2017, 6:51 p.m. | #3
On 07/21/2017 06:52 AM, Andrea Bolognani wrote:
> On Thu, 2017-07-20 at 13:56 -0600, dann frazier wrote:

>> @@ -131,6 +131,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)

>>   #define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"

>>   #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"

>>   #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"

>> +#define VIR_QEMU_AAVMF32_LOADER_PATH "/usr/share/AAVMF/AAVMF32_CODE.fd"

>> +#define VIR_QEMU_AAVMF32_NVRAM_PATH "/usr/share/AAVMF/AAVMF32_VARS.fd"

>>   

>>   virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>>   {

>> @@ -335,11 +337,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>>           goto error;

>>   

>>   #else

>> -    if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)

>> +    if (VIR_ALLOC_N(cfg->firmwares, 4) < 0)

>>           goto error;

>> -    cfg->nfirmwares = 3;

>> +    cfg->nfirmwares = 4;

>>       if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||

>> -        VIR_ALLOC(cfg->firmwares[2]) < 0)

>> +        VIR_ALLOC(cfg->firmwares[2]) < 0 || VIR_ALLOC(cfg->firmwares[3]) < 0)

>>           goto error;

>>   

>>       if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||

>> @@ -347,7 +349,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>>           VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||

>>           VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||

>>           VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 0 ||

>> -        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0)

>> +        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0 ||

>> +        VIR_STRDUP(cfg->firmwares[3]->name, VIR_QEMU_AAVMF32_LOADER_PATH) < 0 ||

>> +        VIR_STRDUP(cfg->firmwares[3]->nvram, VIR_QEMU_AAVMF32_NVRAM_PATH) < 0)

>>           goto error;

>>   #endif

> 

> Not your fault of course, but the way this is done at the

> moment is *extremely* yucky.

> 

> I just posted a patch[1] that makes it saner, and although I

> realize that's a bit more work I'd ask you to wait for it to

> land in master and then respin on top of it.


IMO: Probably should have had Dann's patch go first, then do the
adjustment. Water under the bridge though...

So in order to make progress and since it was a simple modification, I
updated Dann's code to utilize the new model in qemu_conf.c from commit
id '6c2c04e75':

# define DEFAULT_LOADER_NVRAM \
    "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:" \
    "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:" \
    "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:" \
    "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd"

Since there's another "looks good though" - I've pushed this to master
(and now pray that some CI build doesn't break ;-))

John

Congrats on your first libvirt patch

> 

> Everything else looks good though :)

> 

> 

> [1] https://www.redhat.com/archives/libvir-list/2017-July/msg00870.html

> -- 

> Andrea Bolognani / Red Hat / Virtualization

> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Andrea Bolognani July 24, 2017, 7:52 a.m. | #4
On Fri, 2017-07-21 at 14:51 -0400, John Ferlan wrote:
> > Not your fault of course, but the way this is done at the
> > moment is *extremely* yucky.
> > 
> > I just posted a patch[1] that makes it saner, and although I
> > realize that's a bit more work I'd ask you to wait for it to
> > land in master and then respin on top of it.

> IMO: Probably should have had Dann's patch go first, then do the
> adjustment. Water under the bridge though...

Either approach results in the same code at the end of the
day, but having my cleanup in first meant that the patch
adding AAVMF32 could be much shorter and to the point, and
that's a win in my book.

> So in order to make progress and since it was a simple modification, I
> updated Dann's code to utilize the new model in qemu_conf.c from commit
> id '6c2c04e75':

> # define DEFAULT_LOADER_NVRAM \
>     "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:" \
>     "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:" \
>     "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:" \
>     "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd"

> Since there's another "looks good though" - I've pushed this to master
> (and now pray that some CI build doesn't break ;-))

Thanks for that! And it looks like your prayers have been
heard: CI is covered in a beautiful shade of green today ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Patch

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e6c0832662..1d81472df0 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -677,7 +677,8 @@ 
 #nvram = [
 #   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
 #   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd",
-#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"
+#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd",
+#   "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd"
 #]
 
 # The backend to use for handling stdout/stderr output from
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a65c92a262..9dd9442d63 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -131,6 +131,8 @@  void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
 #define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
 #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"
 #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"
+#define VIR_QEMU_AAVMF32_LOADER_PATH "/usr/share/AAVMF/AAVMF32_CODE.fd"
+#define VIR_QEMU_AAVMF32_NVRAM_PATH "/usr/share/AAVMF/AAVMF32_VARS.fd"
 
 virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
 {
@@ -335,11 +337,11 @@  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
         goto error;
 
 #else
-    if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)
+    if (VIR_ALLOC_N(cfg->firmwares, 4) < 0)
         goto error;
-    cfg->nfirmwares = 3;
+    cfg->nfirmwares = 4;
     if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||
-        VIR_ALLOC(cfg->firmwares[2]) < 0)
+        VIR_ALLOC(cfg->firmwares[2]) < 0 || VIR_ALLOC(cfg->firmwares[3]) < 0)
         goto error;
 
     if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
@@ -347,7 +349,9 @@  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
         VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
         VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||
         VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 0 ||
-        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0)
+        VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0 ||
+        VIR_STRDUP(cfg->firmwares[3]->name, VIR_QEMU_AAVMF32_LOADER_PATH) < 0 ||
+        VIR_STRDUP(cfg->firmwares[3]->nvram, VIR_QEMU_AAVMF32_NVRAM_PATH) < 0)
         goto error;
 #endif
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 3e317bc7e9..676d48cf5c 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -90,6 +90,7 @@  module Test_libvirtd_qemu =
     { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" }
     { "2" = "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd" }
     { "3" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" }
+    { "4" = "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd" }
 }
 { "stdio_handler" = "logd" }
 { "gluster_debug_level" = "9" }
diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
index 1eadba393f..8d1ad86570 100644
--- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
index 54b89dc72b..7c019b2308 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
index 60bf2f54f7..700dc618b2 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
index 1a980927cf..3b14280621 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
index 4ecf8651b4..5b986e52f7 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
index 843bdc2b73..de81886237 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
index dc6d2d8f0c..b93d00ece1 100644
--- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
index 962cd0557f..d860cd833f 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
index 53c3190f20..ee40d1e84e 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
index 7d5ac063fa..33161f7b82 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
index f6d54d9a12..07e1db641a 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
index 413f3fa7ce..af606493aa 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
index e8fe01d85e..2fc3b72138 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
@@ -7,6 +7,7 @@ 
   <os supported='yes'>
     <loader supported='yes'>
       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
+      <value>/usr/share/AAVMF/AAVMF32_CODE.fd</value>
       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 5a36fcf29d..78ed4109a2 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -240,6 +240,7 @@  fillQemuCaps(virDomainCapsPtr domCaps,
 
     if (fillStringValues(&loader->values,
                          "/usr/share/AAVMF/AAVMF_CODE.fd",
+                         "/usr/share/AAVMF/AAVMF32_CODE.fd",
                          "/usr/share/OVMF/OVMF_CODE.fd",
                          NULL) < 0)
         goto cleanup;