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