Re: [PATCH 3/3] qemu: add support for simpler UEFI config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/03/2016 11:49 AM, Daniel P. Berrange wrote:
> This wires up the domain XML post-parse callback so that
> it can fill in the loader/nvram paths, when seeing the
> "firmware" attribute set.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c                            |  6 ++-
>  src/qemu/qemu_conf.c                               | 12 ++---
>  src/qemu/qemu_conf.h                               |  7 +++
>  src/qemu/qemu_domain.c                             | 60 +++++++++++++++++++---
>  .../qemuxml2argv-bios-firmware.args                | 26 ++++++++++
>  .../qemuxml2argv-bios-firmware.xml                 | 41 +++++++++++++++
>  tests/qemuxml2argvtest.c                           |  1 +
>  .../qemuxml2xmlout-bios-firmware.xml               | 48 +++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  tests/testutilsqemu.c                              | 30 ++++++++++-
>  10 files changed, 214 insertions(+), 18 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-firmware.xml
> 

The xml2xml tests could go in patch2 I suppose, but they're fine here
too - at least they're provided.

Could add that empty path for bios/rom type test too if you felt so
compelled.

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 7e52963..a7529bf 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8927,8 +8927,10 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
>  
>      switch ((virDomainLoader) loader->type) {
>      case VIR_DOMAIN_LOADER_TYPE_ROM:
> -        virCommandAddArg(cmd, "-bios");
> -        virCommandAddArg(cmd, loader->path);
> +        if (loader->path) {
> +            virCommandAddArg(cmd, "-bios");
> +            virCommandAddArg(cmd, loader->path);

If !loader->path then -bios has no path? Is that OK from QEMU?

> +        }
>          break;
>  
>      case VIR_DOMAIN_LOADER_TYPE_PFLASH:
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 635fa27..879e2aa 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -124,13 +124,6 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
>  }
>  
>  
> -#define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd"
> -#define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
> -#define VIR_QEMU_OVMF_SEC_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.secboot.fd"
> -#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"
> -
>  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>  {
>      virQEMUDriverConfigPtr cfg;
> @@ -334,6 +327,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>          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)
>          goto error;
> +
> +    cfg->firmwares[0]->arch = VIR_ARCH_AARCH64;
> +    cfg->firmwares[1]->arch = VIR_ARCH_X86_64;
> +    cfg->firmwares[2]->arch = VIR_ARCH_X86_64;
> +    cfg->firmwares[2]->secboot = true;
>  #endif
>  
>      return cfg;
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index d32689a..2f0c91f 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -66,6 +66,13 @@ typedef virQEMUDriver *virQEMUDriverPtr;
>  typedef struct _virQEMUDriverConfig virQEMUDriverConfig;
>  typedef virQEMUDriverConfig *virQEMUDriverConfigPtr;
>  
> +# define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd"
> +# define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
> +# define VIR_QEMU_OVMF_SEC_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.secboot.fd"
> +# 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"
> +
>  /* Main driver config. The data in these object
>   * instances is immutable, so can be accessed
>   * without locking. Threads must, however, hold
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9b1a32e..3badd38 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2346,13 +2346,59 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>          goto cleanup;
>      }
>  
> -    if (def->os.loader &&
> -        def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
> -        def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON &&
> -        !def->os.loader->nvram) {
> -        if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd",
> -                        cfg->nvramDir, def->name) < 0)
> -            goto cleanup;
> +    if (def->os.loader) {
> +        switch (def->os.loader->firmware) {
> +        case VIR_DOMAIN_LOADER_FIRMWARE_DEFAULT:
> +            break;

Again, is having !def->os.loader.path OK here? or will it be a problem?
It seems from qemu-kvm --help it will be:

-bios file      set the filename for the BIOS

> +
> +        case VIR_DOMAIN_LOADER_FIRMWARE_BIOS:
> +            if (def->os.arch != VIR_ARCH_I686 &&
> +                def->os.arch != VIR_ARCH_X86_64) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("BIOS firmware only supported on i686/x86_64"));
> +                goto cleanup;
> +            }
> +            break;
> +
> +        case VIR_DOMAIN_LOADER_FIRMWARE_UEFI:
> +            if (def->os.arch != VIR_ARCH_X86_64 &&
> +                def->os.arch != VIR_ARCH_AARCH64) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("BIOS firmware only supported on x86_64/i686"));

UEFI on x86_64/aarch

Could also use the virArchToString for both failure messages to be precise.

ACK w/ fixed error message and handling that empty -bios arg.

John

> +                goto cleanup;
> +            }
> +
> +            if (def->os.loader->path == NULL) {
> +                virFirmwarePtr firmware;
> +
> +                if (!(firmware = virFirmwareFind(cfg->firmwares,
> +                                                 cfg->nfirmwares,
> +                                                 def->os.arch,
> +                                                 def->os.loader->secure ==
> +                                                 VIR_TRISTATE_SWITCH_ON)))
> +                    goto cleanup;
> +
> +                if (VIR_STRDUP(def->os.loader->path,
> +                               firmware->name) < 0)
> +                    goto cleanup;
> +
> +                if (def->os.loader->templt == NULL &&
> +                    VIR_STRDUP(def->os.loader->templt,
> +                               firmware->nvram) < 0)
> +                    goto cleanup;
> +            }
> +
> +        case VIR_DOMAIN_LOADER_FIRMWARE_LAST:
> +            break;
> +        }
> +
> +        if (def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
> +            def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON &&
> +            !def->os.loader->nvram) {
> +            if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd",
> +                            cfg->nvramDir, def->name) < 0)
> +                goto cleanup;
> +        }
>      }
>  
>      /* check for emulator and create a default one if needed */
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.args b/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.args
> new file mode 100644
> index 0000000..e10f4ec
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.args
> @@ -0,0 +1,26 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name test-bios \
> +-S \
> +-M pc \
> +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,\
> +readonly=on \
> +-drive file=/tmp/nvram/test-bios_VARS.fd,if=pflash,format=raw,unit=1 \
> +-m 1024 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-test-bios/monitor.sock,server,nowait \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-serial pty \
> +-device usb-tablet,id=input0,bus=usb.0,port=1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.xml b/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.xml
> new file mode 100644
> index 0000000..cea1efc
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-firmware.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>test-bios</name>
> +  <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc'>hvm</type>
> +    <loader firmware='uefi'/>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <features>
> +    <acpi/>
> +  </features>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <serial type='pty'>
> +      <target port='0'/>
> +    </serial>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <input type='tablet' bus='usb'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 903276d..5a45f17 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -727,6 +727,7 @@ mymain(void)
>              QEMU_CAPS_MACHINE_OPT,
>              QEMU_CAPS_MACHINE_SMM_OPT,
>              QEMU_CAPS_VIRTIO_SCSI);
> +    DO_TEST("bios-firmware", NONE);
>      DO_TEST("clock-utc", QEMU_CAPS_NODEFCONFIG);
>      DO_TEST("clock-localtime", NONE);
>      DO_TEST("clock-localtime-basis-localtime", QEMU_CAPS_RTC);
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-firmware.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-firmware.xml
> new file mode 100644
> index 0000000..624c60f
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-firmware.xml
> @@ -0,0 +1,48 @@
> +<domain type='qemu'>
> +  <name>test-bios</name>
> +  <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc'>hvm</type>
> +    <loader firmware='uefi' readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
> +    <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/tmp/nvram/test-bios_VARS.fd</nvram>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <features>
> +    <acpi/>
> +  </features>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <serial type='pty'>
> +      <target port='0'/>
> +    </serial>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <input type='tablet' bus='usb'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index fb05c85..5f94c26 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -808,6 +808,7 @@ mymain(void)
>  
>      DO_TEST("bios-nvram", NONE);
>      DO_TEST("bios-nvram-os-interleave", NONE);
> +    DO_TEST("bios-firmware", NONE);
>  
>      DO_TEST("tap-vhost", NONE);
>      DO_TEST("tap-vhost-incorrect", NONE);
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index e66903a..3492f28 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -552,21 +552,24 @@ int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary,
>  int qemuTestDriverInit(virQEMUDriver *driver)
>  {
>      virSecurityManagerPtr mgr = NULL;
> +    virQEMUDriverConfigPtr cfg;
>  
>      memset(driver, 0, sizeof(*driver));
>  
>      if (virMutexInit(&driver->lock) < 0)
>          return -1;
>  
> -    driver->config = virQEMUDriverConfigNew(false);
> +    cfg = driver->config = virQEMUDriverConfigNew(false);
>      if (!driver->config)
>          goto error;
>  
>      /* Overwrite some default paths so it's consistent for tests. */
>      VIR_FREE(driver->config->libDir);
>      VIR_FREE(driver->config->channelTargetDir);
> +    VIR_FREE(driver->config->nvramDir);
>      if (VIR_STRDUP(driver->config->libDir, "/tmp/lib") < 0 ||
> -        VIR_STRDUP(driver->config->channelTargetDir, "/tmp/channel") < 0)
> +        VIR_STRDUP(driver->config->channelTargetDir, "/tmp/channel") < 0 ||
> +        VIR_STRDUP(driver->config->nvramDir, "/tmp/nvram") < 0)
>          goto error;
>  
>      driver->caps = testQemuCapsInit();
> @@ -592,6 +595,29 @@ int qemuTestDriverInit(virQEMUDriver *driver)
>      if (!(driver->securityManager = virSecurityManagerNewStack(mgr)))
>          goto error;
>  
> +    /* The default firmware list may be changed by configure arg,
> +     * so we must clear it and setup a fixed firmware list for testing */
> +    virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
> +    if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)
> +        goto error;
> +    cfg->nfirmwares = 3;
> +    if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||
> +        VIR_ALLOC(cfg->firmwares[2]) < 0)
> +        goto error;
> +
> +    if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
> +        VIR_STRDUP(cfg->firmwares[0]->nvram, VIR_QEMU_AAVMF_NVRAM_PATH) < 0  ||
> +        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)
> +        goto error;
> +
> +    cfg->firmwares[0]->arch = VIR_ARCH_AARCH64;
> +    cfg->firmwares[1]->arch = VIR_ARCH_X86_64;
> +    cfg->firmwares[2]->arch = VIR_ARCH_X86_64;
> +    cfg->firmwares[2]->secboot = true;
> +
>      return 0;
>  
>   error:
> 

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]