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

Message ID 20170628160232.31664-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 June 28, 2017, 4:02 p.m.
Add a path for UEFI VMs for AArch32 VMs. This is the path Debian is
currently using in experimental. libvirt is the de facto canonical
location for where distros should place these firmware images, so let's
define this path here to try and minimize distro fragmentation.
---
 src/qemu/qemu_conf.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.13.2

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

Comments

John Ferlan July 10, 2017, 6:23 p.m. | #1
On 06/28/2017 12:02 PM, dann frazier wrote:
> Add a path for UEFI VMs for AArch32 VMs. This is the path Debian is

> currently using in experimental. libvirt is the de facto canonical


"experimental"?

Is this written in stone some where yet?

> location for where distros should place these firmware images, so let's

> define this path here to try and minimize distro fragmentation.


hmmm... This makes it appear that nothing is decided upon yet.

> ---

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

>  1 file changed, 8 insertions(+), 4 deletions(-)

> 


How would anyone know to use this unless qemu.conf was modified to
describe the possibility?

See commit id '436dcf0b' for when AAVMF was originally added keeping in
mind that commit id 'f2f1e388' fixed the names/path. This will give a
better idea of what additional files should be changed.

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

> index 73c33d6788..c1bd91935b 100644

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

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

> @@ -130,6 +130,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"


Not that it's in my wheelhouse, but I'm somewhat surprised by the notion
to have the AAVMF directory have both 64 and 32 bit files... Guess I'm
surprised it's not AAVMF32/ since it would seem easier to be able to
have consistent names of "%s_VARS.fd" and "%s_CODE.fd" to go with the
/usr/share/%s/ path when trying to "build" a name.  I've asked the
QEMU/OVMF maintainer in this space and get his insights as well.

John

>  

>  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

>  {

> @@ -334,11 +336,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 ||

> @@ -346,7 +348,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

>  

> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
dann frazier July 13, 2017, 9:07 p.m. | #2
On Mon, Jul 10, 2017 at 12:23 PM, John Ferlan <jferlan@redhat.com> wrote:
>

>

> On 06/28/2017 12:02 PM, dann frazier wrote:

>> Add a path for UEFI VMs for AArch32 VMs. This is the path Debian is

>> currently using in experimental. libvirt is the de facto canonical

>

> "experimental"?


Hi John!

experimental is a staging archive for Debian. I uploaded it there
because Debian was in freeze at the time, not because we're continuing
to weigh options.

> Is this written in stone some where yet?


I'm not sure what qualifies as stone here. I've planted a flag via
Debian, and tried to build consensus on the cross-distro list:
  https://lists.linaro.org/pipermail/cross-distro/2017-April/000865.html

As you can see, mostly crickets, other than a suggestion to lock it in
via libvirt.

>> location for where distros should place these firmware images, so let's

>> define this path here to try and minimize distro fragmentation.

>

> hmmm... This makes it appear that nothing is decided upon yet.


If you know of someone else you need me to convince, point me at 'em! :)

>> ---

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

>>  1 file changed, 8 insertions(+), 4 deletions(-)

>>

>

> How would anyone know to use this unless qemu.conf was modified to

> describe the possibility?

>

> See commit id '436dcf0b' for when AAVMF was originally added keeping in

> mind that commit id 'f2f1e388' fixed the names/path. This will give a

> better idea of what additional files should be changed.


OK, thanks - I'll take a look.

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

>> index 73c33d6788..c1bd91935b 100644

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

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

>> @@ -130,6 +130,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"

>

> Not that it's in my wheelhouse, but I'm somewhat surprised by the notion

> to have the AAVMF directory have both 64 and 32 bit files... Guess I'm

> surprised it's not AAVMF32/ since it would seem easier to be able to

> have consistent names of "%s_VARS.fd" and "%s_CODE.fd" to go with the

> /usr/share/%s/ path when trying to "build" a name.  I've asked the

> QEMU/OVMF maintainer in this space and get his insights as well.


One of my goals here is to avoid further pollution of the /usr/share
directory. I've had complaints from other Debian Developers about the
existing AAVMF and OVMF subdirs, but we've retained this for
compatibility with libvirt upstream.

  -dann

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

Patch

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 73c33d6788..c1bd91935b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -130,6 +130,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)
 {
@@ -334,11 +336,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 ||
@@ -346,7 +348,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