[libvirt] [PATCH v2 0/2] Expose UEFI binary path
This is practically reworked v1 from Cole. Cole Robinson (1): domaincaps: Expose UEFI binary path, if it exists Michal Privoznik (1): qemu_capabilities: Change virQEMUCapsFillDomainCaps signature docs/formatdomaincaps.html.in | 6 ++ docs/schemas/domaincaps.rng | 17 ++++-- src/conf/domain_capabilities.c | 29 ++++++++++ src/conf/domain_capabilities.h | 8 +++ src/qemu/qemu_capabilities.c | 53 +++++++++++++---- src/qemu/qemu_capabilities.h | 9 ++- src/qemu/qemu_driver.c | 7 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 66 ++++++++++++++++++---- 10 files changed, 167 insertions(+), 31 deletions(-) -- 1.8.5.5
Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++++++++++++++++--------- src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 ++++++++++++------- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9f8868d..c306899 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3608,7 +3608,7 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps) } -static void +static int virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, virDomainCapsLoaderPtr loader, virArch arch) @@ -3629,10 +3629,11 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(loader->readonly, VIR_TRISTATE_BOOL_YES, VIR_TRISTATE_BOOL_NO); + return 0; } -static void +static int virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, virDomainCapsOSPtr os, virArch arch) @@ -3640,11 +3641,13 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, virDomainCapsLoaderPtr loader = &os->loader; os->device.supported = true; - virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch); + if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch) < 0) + return -1; + return 0; } -static void +static int virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceDiskPtr disk) { @@ -3667,10 +3670,11 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); + return 0; } -static void +static int virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceHostdevPtr hostdev) { @@ -3715,10 +3719,11 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM); } + return 0; } -void +int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps) { @@ -3729,7 +3734,9 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, domCaps->maxvcpus = maxvcpus; - virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch); - virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk); - virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev); + if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch) < 0 || + virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk) < 0 || + virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0980c00..828bba3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -324,7 +324,7 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, virQEMUCapsPtr kvmbinCaps, virArch guestarch); -void virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, - virQEMUCapsPtr qemuCaps); +int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, + virQEMUCapsPtr qemuCaps); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15ad64d..4fe5909 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17357,7 +17357,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; - virQEMUCapsFillDomainCaps(domCaps, qemuCaps); + if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0) + goto cleanup; ret = virDomainCapsFormat(domCaps); cleanup: diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index f240643..0c4b09f 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -28,13 +28,13 @@ #define VIR_FROM_THIS VIR_FROM_NONE -typedef void (*virDomainCapsFill)(virDomainCapsPtr domCaps, - void *opaque); +typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps, + void *opaque); #define SET_ALL_BITS(x) \ memset(&(x.values), 0xff, sizeof(x.values)) -static void +static int fillAll(virDomainCapsPtr domCaps, void *opaque ATTRIBUTE_UNUSED) { @@ -60,18 +60,20 @@ fillAll(virDomainCapsPtr domCaps, SET_ALL_BITS(hostdev->subsysType); SET_ALL_BITS(hostdev->capsType); SET_ALL_BITS(hostdev->pciBackend); + return 0; } #ifdef WITH_QEMU # include "testutilsqemu.h" -static void +static int fillQemuCaps(virDomainCapsPtr domCaps, void *opaque) { virQEMUCapsPtr qemuCaps = (virQEMUCapsPtr) opaque; - virQEMUCapsFillDomainCaps(domCaps, qemuCaps); + if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0) + return -1; /* The function above tries to query host's KVM & VFIO capabilities by * calling qemuHostdevHostSupportsPassthroughLegacy() and @@ -82,6 +84,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + return 0; } #endif /* WITH_QEMU */ @@ -99,8 +102,10 @@ buildVirDomainCaps(const char *emulatorbin, if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, type))) goto cleanup; - if (fillFunc) - fillFunc(domCaps, opaque); + if (fillFunc && fillFunc(domCaps, opaque) < 0) { + virObjectUnref(domCaps); + domCaps = NULL; + } cleanup: return domCaps; -- 1.8.5.5
On 09/17/14 14:15, Michal Privoznik wrote:
Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++++++++++++++++--------- src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 ++++++++++++------- 4 files changed, 32 insertions(+), 19 deletions(-)
Seems reasonable. Acked-by: Laszlo Ersek <lersek@redhat.com>
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++++++++++++++++--------- src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 ++++++++++++------- 4 files changed, 32 insertions(+), 19 deletions(-)
ACK - Cole
From: Cole Robinson <crobinso@redhat.com> Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like <loader ...> <value>/path/to/ovmf</value> </loader> We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomaincaps.html.in | 6 +++ docs/schemas/domaincaps.rng | 17 +++++--- src/conf/domain_capabilities.c | 29 +++++++++++++ src/conf/domain_capabilities.h | 8 ++++ src/qemu/qemu_capabilities.c | 32 +++++++++++--- src/qemu/qemu_capabilities.h | 7 +++- src/qemu/qemu_driver.c | 6 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 49 +++++++++++++++++++--- 10 files changed, 140 insertions(+), 17 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 34d746d..6959dfe 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -105,6 +105,7 @@ ... <os supported='yes'> <loader supported='yes'> + <value>/usr/share/OVMF/OVMF_CODE.fd</value> <enum name='type'> <value>rom</value> <value>pflash</value> @@ -122,6 +123,11 @@ <p>For the <code>loader</code> element, the following can occur:</p> <dl> + <dt>value</dt> + <dd>List of known loader paths. Currently this is only used + to advertise known locations of OVMF binaries for qemu. Binaries + will only be listed if they actually exist on disk.</dd> + <dt>type</dt> <dd>Whether loader is a typical BIOS (<code>rom</code>) or an UEFI binary (<code>pflash</code>). This refers to diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index ad8d966..f4a555f 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -47,6 +47,9 @@ <define name='loader'> <element name='loader'> <ref name='supported'/> + <optional> + <ref name='value'/> + </optional> <ref name='enum'/> </element> </define> @@ -85,6 +88,14 @@ </element> </define> + <define name='value'> + <zeroOrMore> + <element name='value'> + <text/> + </element> + </zeroOrMore> + </define> + <define name='supported'> <attribute name='supported'> <choice> @@ -100,11 +111,7 @@ <attribute name='name'> <text/> </attribute> - <zeroOrMore> - <element name='value'> - <text/> - </element> - </zeroOrMore> + <ref name='value'/> </element> </zeroOrMore> </define> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 5a3c8e7..7c59912 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -48,12 +48,28 @@ VIR_ONCE_GLOBAL_INIT(virDomainCaps) static void +virDomainCapsStringValuesFree(virDomainCapsStringValuesPtr values) +{ + size_t i; + + if (!values || !values->values) + return; + + for (i = 0; i < values->nvalues; i++) + VIR_FREE(values->values[i]); + VIR_FREE(values->values); +} + + +static void virDomainCapsDispose(void *obj) { virDomainCapsPtr caps = obj; VIR_FREE(caps->path); VIR_FREE(caps->machine); + + virDomainCapsStringValuesFree(&caps->os.loader.values); } @@ -156,6 +172,18 @@ virDomainCapsEnumFormat(virBufferPtr buf, return ret; } + +static void +virDomainCapsStringValuesFormat(virBufferPtr buf, + virDomainCapsStringValuesPtr values) +{ + size_t i; + + for (i = 0; i < values->nvalues; i++) + virBufferEscapeString(buf, "<value>%s</value>\n", values->values[i]); +} + + #define FORMAT_PROLOGUE(item) \ do { \ virBufferAsprintf(buf, "<" #item " supported='%s'%s\n", \ @@ -185,6 +213,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf, { FORMAT_PROLOGUE(loader); + virDomainCapsStringValuesFormat(buf, &loader->values); ENUM_PROCESS(loader, type, virDomainLoaderTypeToString); ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 768646b..597ac75 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -37,6 +37,13 @@ struct _virDomainCapsEnum { unsigned int values; /* Bitmask of values supported in the corresponding enum */ }; +typedef struct _virDomainCapsStringValues virDomainCapsStringValues; +typedef virDomainCapsStringValues *virDomainCapsStringValuesPtr; +struct _virDomainCapsStringValues { + char **values; /* raw string values */ + size_t nvalues; /* number of strings */ +}; + typedef struct _virDomainCapsDevice virDomainCapsDevice; typedef virDomainCapsDevice *virDomainCapsDevicePtr; struct _virDomainCapsDevice { @@ -47,6 +54,7 @@ typedef struct _virDomainCapsLoader virDomainCapsLoader; typedef virDomainCapsLoader *virDomainCapsLoaderPtr; struct _virDomainCapsLoader { virDomainCapsDevice device; + virDomainCapsStringValues values; /* Info about values for the element */ virDomainCapsEnum type; /* Info about virDomainLoader */ virDomainCapsEnum readonly; /* Info about readonly:virTristateBool */ }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c306899..9246813 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3611,10 +3611,30 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps) static int virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, virDomainCapsLoaderPtr loader, - virArch arch) + virArch arch, + virQEMUDriverConfigPtr cfg) { + size_t i; + loader->device.supported = true; + if (VIR_ALLOC_N(loader->values.values, cfg->nloader) < 0) + return -1; + + for (i = 0; i < cfg->nloader; i++) { + const char *filename = cfg->loader[i]; + + if (!virFileExists(filename)) { + VIR_DEBUG("loader filename=%s does not exist", filename); + continue; + } + + if (VIR_STRDUP(loader->values.values[loader->values.nvalues], + filename) < 0) + return -1; + loader->values.nvalues++; + } + VIR_DOMAIN_CAPS_ENUM_SET(loader->type, VIR_DOMAIN_LOADER_TYPE_ROM); @@ -3636,12 +3656,13 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, static int virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, virDomainCapsOSPtr os, - virArch arch) + virArch arch, + virQEMUDriverConfigPtr cfg) { virDomainCapsLoaderPtr loader = &os->loader; os->device.supported = true; - if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch) < 0) + if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch, cfg) < 0) return -1; return 0; } @@ -3725,7 +3746,8 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg) { virDomainCapsOSPtr os = &domCaps->os; virDomainCapsDeviceDiskPtr disk = &domCaps->disk; @@ -3734,7 +3756,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, domCaps->maxvcpus = maxvcpus; - if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch) < 0 || + if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch, cfg) < 0 || virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk) < 0 || virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0) return -1; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 828bba3..cf69e59 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -324,7 +324,12 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, virQEMUCapsPtr kvmbinCaps, virArch guestarch); +/* Forward declaration */ +typedef struct _virQEMUDriverConfig virQEMUDriverConfig; +typedef virQEMUDriverConfig *virQEMUDriverConfigPtr; + int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, - virQEMUCapsPtr qemuCaps); + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4fe5909..822a4eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17285,12 +17285,15 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, int virttype; /* virDomainVirtType */ virDomainCapsPtr domCaps = NULL; int arch = virArchFromHost(); /* virArch */ + virQEMUDriverConfigPtr cfg = NULL; virCheckFlags(0, ret); if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0) return ret; + cfg = virQEMUDriverGetConfig(driver); + if (qemuHostdevHostSupportsPassthroughLegacy()) virttype = VIR_DOMAIN_VIRT_KVM; else @@ -17357,11 +17360,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; - if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0) + if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg) < 0) goto cleanup; ret = virDomainCapsFormat(domCaps); cleanup: + virObjectUnref(cfg); virObjectUnref(domCaps); virObjectUnref(qemuCaps); return ret; diff --git a/tests/domaincapsschemadata/domaincaps-full.xml b/tests/domaincapsschemadata/domaincaps-full.xml index 9722772..96202bc 100644 --- a/tests/domaincapsschemadata/domaincaps-full.xml +++ b/tests/domaincapsschemadata/domaincaps-full.xml @@ -6,6 +6,8 @@ <vcpu max='255'/> <os supported='yes'> <loader supported='yes'> + <value>/foo/bar</value> + <value>/tmp/my_path</value> <enum name='type'> <value>rom</value> <value>pflash</value> diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml index 568cecb..346ef65 100644 --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml @@ -5,6 +5,7 @@ <arch>x86_64</arch> <os supported='yes'> <loader supported='yes'> + <value>/usr/share/OVMF/OVMF_CODE.fd</value> <enum name='type'> <value>rom</value> <value>pflash</value> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 0c4b09f..8543963 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -34,6 +34,27 @@ typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps, #define SET_ALL_BITS(x) \ memset(&(x.values), 0xff, sizeof(x.values)) +static int ATTRIBUTE_SENTINEL +fillStringValues(virDomainCapsStringValuesPtr values, ...) +{ + int ret = 0; + va_list list; + const char *str; + + va_start(list, values); + while ((str = va_arg(list, const char *))) { + if (VIR_REALLOC_N(values->values, values->nvalues + 1) < 0 || + VIR_STRDUP(values->values[values->nvalues], str) < 0) { + ret = -1; + break; + } + values->nvalues++; + } + va_end(list); + + return ret; +} + static int fillAll(virDomainCapsPtr domCaps, void *opaque ATTRIBUTE_UNUSED) @@ -49,6 +70,11 @@ fillAll(virDomainCapsPtr domCaps, loader->device.supported = true; SET_ALL_BITS(loader->type); SET_ALL_BITS(loader->readonly); + if (fillStringValues(&loader->values, + "/foo/bar", + "/tmp/my_path", + NULL) < 0) + return -1; disk->device.supported = true; SET_ALL_BITS(disk->diskDevice); @@ -66,13 +92,21 @@ fillAll(virDomainCapsPtr domCaps, #ifdef WITH_QEMU # include "testutilsqemu.h" + +struct fillQemuCapsData { + virQEMUCapsPtr qemuCaps; + virQEMUDriverConfigPtr cfg; +}; + static int fillQemuCaps(virDomainCapsPtr domCaps, void *opaque) { - virQEMUCapsPtr qemuCaps = (virQEMUCapsPtr) opaque; + struct fillQemuCapsData *data = (struct fillQemuCapsData *) opaque; + virQEMUCapsPtr qemuCaps = data->qemuCaps; + virQEMUDriverConfigPtr cfg = data->cfg; - if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0) + if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg) < 0) return -1; /* The function above tries to query host's KVM & VFIO capabilities by @@ -97,7 +131,7 @@ buildVirDomainCaps(const char *emulatorbin, virDomainCapsFill fillFunc, void *opaque) { - virDomainCapsPtr domCaps; + virDomainCapsPtr domCaps, ret = NULL; if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, type))) goto cleanup; @@ -107,8 +141,9 @@ buildVirDomainCaps(const char *emulatorbin, domCaps = NULL; } + ret = domCaps; cleanup: - return domCaps; + return ret; } struct test_virDomainCapsFormatData { @@ -182,13 +217,16 @@ mymain(void) #ifdef WITH_QEMU + virQEMUDriverConfigPtr cfg = virQEMUDriverConfigNew(false); + # define DO_TEST_QEMU(Filename, QemuCapsFile, Emulatorbin, Machine, Arch, Type, ...) \ do { \ const char *capsPath = abs_srcdir "/qemucapabilitiesdata/" QemuCapsFile ".caps"; \ virQEMUCapsPtr qemuCaps = qemuTestParseCapabilities(capsPath); \ + struct fillQemuCapsData fillData = {.qemuCaps = qemuCaps, .cfg = cfg}; \ struct test_virDomainCapsFormatData data = {.filename = Filename, \ .emulatorbin = Emulatorbin, .machine = Machine, .arch = Arch, \ - .type = Type, .fillFunc = fillQemuCaps, .opaque = qemuCaps}; \ + .type = Type, .fillFunc = fillQemuCaps, .opaque = &fillData}; \ if (!qemuCaps) { \ fprintf(stderr, "Unable to build qemu caps from %s\n", capsPath); \ ret = -1; \ @@ -199,6 +237,7 @@ mymain(void) DO_TEST_QEMU("qemu_1.6.50-1", "caps_1.6.50-1", "/usr/bin/qemu-system-x86_64", "pc-1.2", VIR_ARCH_X86_64, VIR_DOMAIN_VIRT_KVM); + virObjectUnref(cfg); #endif /* WITH_QEMU */ return ret; -- 1.8.5.5
On 09/17/14 14:15, Michal Privoznik wrote:
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 0c4b09f..8543963 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -34,6 +34,27 @@ typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps, #define SET_ALL_BITS(x) \ memset(&(x.values), 0xff, sizeof(x.values))
+static int ATTRIBUTE_SENTINEL +fillStringValues(virDomainCapsStringValuesPtr values, ...) +{ + int ret = 0; + va_list list; + const char *str; + + va_start(list, values); + while ((str = va_arg(list, const char *))) { + if (VIR_REALLOC_N(values->values, values->nvalues + 1) < 0 || + VIR_STRDUP(values->values[values->nvalues], str) < 0) { + ret = -1; + break; + } + values->nvalues++; + } + va_end(list); + + return ret; +}
Okay, you increment "values->nvalues" only after. The rest too looks good to me. Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
From: Cole Robinson <crobinso@redhat.com>
Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like
<loader ...> <value>/path/to/ovmf</value> </loader>
We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACK, thanks for cleaning it up. But please change authorship since the patch is reasonably altered, I won't be offended :) - Cole
On 17.09.2014 14:57, Cole Robinson wrote:
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
From: Cole Robinson <crobinso@redhat.com>
Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like
<loader ...> <value>/path/to/ovmf</value> </loader>
We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACK, thanks for cleaning it up. But please change authorship since the patch is reasonably altered, I won't be offended :)
- Cole
Thanks. Fixed and pushed. Michal
On 09/17/2014 06:15 AM, Michal Privoznik wrote:
This is practically reworked v1 from Cole.
Cole Robinson (1): domaincaps: Expose UEFI binary path, if it exists
Michal Privoznik (1): qemu_capabilities: Change virQEMUCapsFillDomainCaps signature
docs/formatdomaincaps.html.in | 6 ++ docs/schemas/domaincaps.rng | 17 ++++-- src/conf/domain_capabilities.c | 29 ++++++++++ src/conf/domain_capabilities.h | 8 +++ src/qemu/qemu_capabilities.c | 53 +++++++++++++---- src/qemu/qemu_capabilities.h | 9 ++- src/qemu/qemu_driver.c | 7 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 66 ++++++++++++++++++---- 10 files changed, 167 insertions(+), 31 deletions(-)
I'm now seeing test failures: FAIL: domaincapstest ==================== TEST: domaincapstest 1) basic ... OK 2) full ... OK 3) qemu_1.6.50-1 ... Offset 196 Expect [value>/usr/share/OVMF/OVMF_CODE.fd</value> <e] Actual [e] ... FAILED -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
As of f05b6a918e28 the test produces the list of paths that can be passed to <loader/> and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/domaincapstest.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + + /* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ + if (!domCaps->os.loader.values.nvalues) { + virDomainCapsLoaderPtr loader = &domCaps->os.loader; + + if (fillStringValues(&loader->values, + "/usr/share/OVMF/OVMF_CODE.fd", + NULL) < 0) + return -1; + } return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5
On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:
As of f05b6a918e28 the test produces the list of paths that can be passed to <loader/> and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/domaincapstest.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + + /* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ + if (!domCaps->os.loader.values.nvalues) { + virDomainCapsLoaderPtr loader = &domCaps->os.loader; + + if (fillStringValues(&loader->values, + "/usr/share/OVMF/OVMF_CODE.fd", + NULL) < 0) + return -1; + } return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5
ACK, build-breaker (at least for me). Martin
On 17.09.2014 17:40, Martin Kletzander wrote:
On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:
As of f05b6a918e28 the test produces the list of paths that can be passed to <loader/> and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/domaincapstest.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + + /* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ + if (!domCaps->os.loader.values.nvalues) { + virDomainCapsLoaderPtr loader = &domCaps->os.loader; + + if (fillStringValues(&loader->values, + "/usr/share/OVMF/OVMF_CODE.fd", + NULL) < 0) + return -1; + } return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5
ACK, build-breaker (at least for me).
Thank you, pushed now. Michal
participants (5)
-
Cole Robinson -
Eric Blake -
Laszlo Ersek -
Martin Kletzander -
Michal Privoznik