- Devel - libvirt List Archives

[libvirt] Potential bug: spice port collision on different addresses
by Michael Edlinger 14 Sep '19

14 Sep '19

Hi all, I think I've found a bug in either libvirt or spice. I'll give a short description here, but I have a stack exchange question that also has information here: https://unix.stackexchange.com/questions/541127/how-to-listen-on-same-port-… Basically, have spice for VM1 listen to port 5900 on 10.0.0.2 and spice for VM2 listen to port 5900 on 10.0.0.3, and you'll get an error message saying the ports collide, even though they shouldn't be (they are listening on different addresses). I've tried going to the #virt irc channel with very limited responses, and would appreciate hearing what you all have to say about this issue. Thanks! - Michael

2 2

[libvirt] [PATCH] qemu_blockjob: Don't leak old job when registering a new one
by Michal Privoznik 14 Sep '19

14 Sep '19

If a disk that a block job is registered with already has a job (stored in private data) it needs to be unrefed to keep the refcounter in balance (and properly free the object): 2,577 (104 direct, 2,473 indirect) bytes in 1 blocks are definitely lost in loss record 168 of 174 at 0x4838B86: calloc (vg_replace_malloc.c:752) by 0x4A0AD42: virAllocVar (viralloc.c:346) by 0x4A84D12: virObjectNew (virobject.c:243) by 0x21EE13: qemuBlockJobDataNew (qemu_blockjob.c:111) by 0x184EBC: qemuDomainObjPrivateXMLParseBlockjobData (qemu_domain.c:3120) by 0x1854B5: qemuDomainObjPrivateXMLParseBlockjobs (qemu_domain.c:3191) by 0x186D7D: qemuDomainObjPrivateXMLParse (qemu_domain.c:3622) by 0x4B198CC: virDomainObjParseXML (domain_conf.c:21490) by 0x4B19DDF: virDomainObjParseNode (domain_conf.c:21630) by 0x4B19E7D: virDomainObjParseFile (domain_conf.c:21649) by 0x13FDD5: testCompareStatusXMLToXMLFiles (qemuxml2xmltest.c:61) by 0x16F7D2: virTestRun (testutils.c:115) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_blockjob.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index a991309ee7..67b1f677f8 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -151,6 +151,7 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job, if (disk) { job->disk = disk; job->chain = virObjectRef(disk->src); + virObjectUnref(QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob); QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); } -- 2.21.0

3 2

[libvirt] [PATCH] network: fix connection usage counts after restart
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

Since the introduction of the virNetworkPort object, the network driver has a persistent record of ports that have been created against the networks. Thus the hypervisor drivers no longer communicate to the network driver during libvirtd restart. This change, however, meant that the connection usage counts were no longer re-initialized during a libvirtd restart. To deal with this we must iterate over all virNetworkPortDefPtr objects we have and invoke the notify callback to record the connection usage count. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/virnetworkobj.c | 33 +++++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 9 +++++++++ src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 17 +++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index d63ead7fac..ca1d598cf9 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1844,6 +1844,39 @@ virNetworkObjPortListExport(virNetworkPtr net, } +typedef struct _virNetworkObjPortListForEachData virNetworkObjPortListForEachData; +struct _virNetworkObjPortListForEachData { + virNetworkPortListIter iter; + void *opaque; + bool err; +}; + +static int +virNetworkObjPortForEachCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNetworkObjPortListForEachData *data = opaque; + + if (!data->iter(payload, data->opaque)) + data->err = true; + + return 0; +} + +int +virNetworkObjPortForEach(virNetworkObjPtr obj, + virNetworkPortListIter iter, + void *opaque) +{ + virNetworkObjPortListForEachData data = { iter, opaque, false }; + virHashForEach(obj->ports, virNetworkObjPortForEachCallback, &data); + if (data.err) + return -1; + return 0; +} + + static int virNetworkObjLoadAllPorts(virNetworkObjPtr net, const char *stateDir) diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h index 1c28f0888c..a91b4304c6 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -190,6 +190,15 @@ virNetworkObjPortListExport(virNetworkPtr net, virNetworkPortPtr **ports, virNetworkPortListFilter filter); +typedef bool +(*virNetworkPortListIter)(virNetworkPortDefPtr portdef, + void *opaque); + +int +virNetworkObjPortForEach(virNetworkObjPtr obj, + virNetworkPortListIter iter, + void *opaque); + int virNetworkObjSaveStatus(const char *statusDir, virNetworkObjPtr net, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7fe10d2286..37afb07e21 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1097,6 +1097,7 @@ virNetworkObjLookupPort; virNetworkObjMacMgrAdd; virNetworkObjMacMgrDel; virNetworkObjNew; +virNetworkObjPortForEach; virNetworkObjPortListExport; virNetworkObjRemoveInactive; virNetworkObjReplacePersistentDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7b44184616..0fee153cb8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -527,6 +527,21 @@ networkBridgeDummyNicName(const char *brname) } +static int +networkNotifyPort(virNetworkObjPtr obj, + virNetworkPortDefPtr port); + +static bool +networkUpdatePort(virNetworkPortDefPtr port, + void *opaque) +{ + virNetworkObjPtr obj = opaque; + + networkNotifyPort(obj, port); + + return false; +} + static int networkUpdateState(virNetworkObjPtr obj, void *opaque) @@ -591,6 +606,8 @@ networkUpdateState(virNetworkObjPtr obj, goto cleanup; } + virNetworkObjPortForEach(obj, networkUpdatePort, obj); + /* Try and read dnsmasq/radvd pids of active networks */ if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { pid_t radvdPid; -- 2.21.0

2 3

[libvirt] [PATCH] network: apply bandwidth settings for forward mode=bridge
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

We previously allowed bandwidth settings when attaching NICs to networks with forward mode=bridge: commit 42a92ee93d5432ebd9ebfd409903b5287fc7d7ff Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Tue Nov 20 11:30:05 2018 +0000 network: add missing bandwidth limits for bridge forward type In the case of a network with forward=bridge, which has a bridge device listed, we are capable of setting bandwidth limits but fail to call the function to register them. Reviewed-by: Cole Robinson <crobinso(a)redhat.com> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> Unfortunately the wrong version of this patch was posted and reviewed and thus it lacked the code to actually apply the bandwidth settings to the bridge itself. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0fee153cb8..6a97bb17e2 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2743,21 +2743,39 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver, static int networkStartNetworkBridge(virNetworkObjPtr obj) { + virNetworkDefPtr def = virNetworkObjGetDef(obj); + /* put anything here that needs to be done each time a network of * type BRIDGE, is started. On failure, undo anything you've done, * and return -1. On success return 0. */ - return networkStartHandleMACTableManagerMode(obj, NULL); + if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) + goto error; + + if (networkStartHandleMACTableManagerMode(obj, NULL) < 0) + goto error; + + return 0; + + error: + if (def->bandwidth) + virNetDevBandwidthClear(def->bridge); + return -1; } static int networkShutdownNetworkBridge(virNetworkObjPtr obj ATTRIBUTE_UNUSED) { + virNetworkDefPtr def = virNetworkObjGetDef(obj); + /* put anything here that needs to be done each time a network of * type BRIDGE is shutdown. On failure, undo anything you've done, * and return -1. On success return 0. */ + if (def->bandwidth) + virNetDevBandwidthClear(def->bridge); + return 0; } -- 2.21.0

2 1

[libvirt] [PATCH] network: add debug when bandwidth settings are not applied
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

To aid in troubleshooting add some debug messages wrt bandwidth settings and networks. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6a97bb17e2..c54be96407 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5192,9 +5192,14 @@ networkCheckBandwidth(virNetworkObjPtr obj, return -1; } + if (!netBand || !netBand->in) { + VIR_DEBUG("No network bandwidth controls present"); + /* no QoS required, claim success */ + return 1; + } if (((!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor) && - (!oldBandwidth || !oldBandwidth->in || !oldBandwidth->in->floor)) || - !netBand || !netBand->in) { + (!oldBandwidth || !oldBandwidth->in || !oldBandwidth->in->floor))) { + VIR_DEBUG("No old/new interface bandwidth floor"); /* no QoS required, claim success */ return 1; } -- 2.21.0

2 1

[libvirt] [PATCH 0/9] Pass identity information between daemons
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

This was previously posted as part of the daemon split series: https://www.redhat.com/archives/libvir-list/2019-July/msg01754.html The patches were not merged at the time since they needed more work which is now done by this series. This is all about enabling the polkit fine grain auth checks to work correctly with the split daemons A mgmt app running non-root will connect to virtqemud. virtqemud sees the client identity as the non-root user. virtqemud in turn may connect to virtnetworkd, forwarding API calls that the mgmt app makes. virtnetworkd sees the client identity as the root user. This series allows virtqemud to pass on the non-root identity of the mgmt app to virtnetworkd, so that polkit checks are done against the correct identity. Daniel P. Berrangé (9): api: introduce virConnectSetIdentity for passing uid, gid, selinux info util: change identity class attribute names tests: fix debug messages wrt selinux context when test fails util: make generic identity accessors private util: removed unused virIdentityIsEqual method util: sanitize return values for virIdentity getters util: store identity attrs as virTypedParameter internally util: allow identity to be imported/exported as typed parameters remote: pass identity across to newly opened daemons include/libvirt/libvirt-host.h | 74 +++++ src/access/viraccessdriverpolkit.c | 22 +- src/admin/admin_server.c | 52 +-- src/driver-hypervisor.h | 7 + src/libvirt-host.c | 51 +++ src/libvirt_private.syms | 21 +- src/libvirt_public.syms | 4 + src/libvirt_remote.syms | 1 + src/remote/remote_daemon_dispatch.c | 112 ++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 +- src/remote_protocol-structs | 8 + src/rpc/virnetserverclient.c | 20 +- src/rpc/virnetserverclient.h | 2 + src/util/viridentity.c | 488 ++++++++++++++++------------ src/util/viridentity.h | 71 ++-- tests/viridentitytest.c | 115 ++----- tests/virnetserverclienttest.c | 40 +-- 18 files changed, 672 insertions(+), 433 deletions(-) -- 2.21.0

2 12

[libvirt] [PATCH] tests: remove use of virTestOOMActive from bhyve testsuite
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

The virTestOOMActive method was deleted in commit 2c52ecd96086b4643b99b4570b5823d40ce2787b Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Aug 29 13:04:07 2019 +0100 util: purge all code for testing OOM handling Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI build fix for FreeBSD tests/bhyveargv2xmltest.c | 44 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/bhyveargv2xmltest.c b/tests/bhyveargv2xmltest.c index 1684f03355..d108366682 100644 --- a/tests/bhyveargv2xmltest.c +++ b/tests/bhyveargv2xmltest.c @@ -39,38 +39,36 @@ testCompareXMLToArgvFiles(const char *xmlfile, if (!(vmdef = bhyveParseCommandLineString(cmd, driver.bhyvecaps, driver.xmlopt))) { - if ((flags & FLAG_EXPECT_FAILURE) && !virTestOOMActive()) { - VIR_TEST_DEBUG("Got expected failure from " - "bhyveParseCommandLineString."); + if ((flags & FLAG_EXPECT_FAILURE)) { + VIR_TEST_DEBUG("Got expected failure from " + "bhyveParseCommandLineString."); } else { goto fail; } - } else if ((flags & FLAG_EXPECT_FAILURE) && !virTestOOMActive()) { + } else if ((flags & FLAG_EXPECT_FAILURE)) { VIR_TEST_DEBUG("Did not get expected failure from " "bhyveParseCommandLineString."); goto fail; } - if (!virTestOOMActive()) { - if ((log = virTestLogContentAndReset()) == NULL) + if ((log = virTestLogContentAndReset()) == NULL) + goto fail; + if (flags & FLAG_EXPECT_WARNING) { + if (*log) { + VIR_TEST_DEBUG("Got expected warning from " + "bhyveParseCommandLineString:\n%s", + log); + } else { + VIR_TEST_DEBUG("bhyveParseCommandLineString " + "should have logged a warning"); + goto fail; + } + } else { /* didn't expect a warning */ + if (*log) { + VIR_TEST_DEBUG("Got unexpected warning from " + "bhyveParseCommandLineString:\n%s", + log); goto fail; - if (flags & FLAG_EXPECT_WARNING) { - if (*log) { - VIR_TEST_DEBUG("Got expected warning from " - "bhyveParseCommandLineString:\n%s", - log); - } else { - VIR_TEST_DEBUG("bhyveParseCommandLineString " - "should have logged a warning"); - goto fail; - } - } else { /* didn't expect a warning */ - if (*log) { - VIR_TEST_DEBUG("Got unexpected warning from " - "bhyveParseCommandLineString:\n%s", - log); - goto fail; - } } } -- 2.21.0

1 0

[libvirt] [PATCH] apparmor: avoid copying empty profile name
by Jim Fehlig 13 Sep '19

13 Sep '19

AppArmorGetSecurityProcessLabel copies the VM's profile name to the label member of virSecurityLabel struct. If the profile is not loaded, the name is set empty before calling virStrcpy to copy it. However, virStrcpy will fail if src is empty (0 length), causing AppArmorGetSecurityProcessLabel to needlessly fail. Simple operations that report security driver information will subsequently fail virsh dominfo test Id: 248 Name: test ... Security model: apparmor Security DOI: 0 error: internal error: error copying profile name Avoid copying an empty profile name when the profile is not loaded. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/security/security_apparmor.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 6d16b15c65..77eee9410c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -525,14 +525,13 @@ AppArmorGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, "%s", _("error getting profile status")); goto cleanup; } else if (status == -1) { - profile_name[0] = '\0'; - } - - if (virStrcpy(sec->label, profile_name, - VIR_SECURITY_LABEL_BUFLEN) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("error copying profile name")); - goto cleanup; + sec->label[0] = '\0'; + } else { + if (virStrcpy(sec->label, profile_name, VIR_SECURITY_LABEL_BUFLEN) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("error copying profile name")); + goto cleanup; + } } sec->enforcing = status == 1; -- 2.23.0

2 2

[libvirt] [PATCH] qemu: fix detach of hostdev based network interface
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

This fixes bug in commit bbe2aa627f621e6749af374b22856184d1f351dc Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Jul 26 17:24:30 2018 +0100 conf: simplify link from hostdev back to network device hostdevs have a link back to the original network device. This is fairly generic accepting any type of device, however, we don't intend to make use of this approach in future. It can thus be specialized to network devices. Reviewed-by: Cole Robinson <crobinso(a)redhat.com> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> which mistakenly deleted the assignment to the 'net' variable, which meant we never invoked the network driver release callback Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/qemu/qemu_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bd8868b0f7..16070f2a57 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4561,6 +4561,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, } if (hostdev->parentnet) { + net = hostdev->parentnet; for (i = 0; i < vm->def->nnets; i++) { if (vm->def->nets[i] == hostdev->parentnet) { virDomainNetRemove(vm->def, i); -- 2.21.0

2 1

[libvirt] [PATCH 0/5] qemu: Use FW descriptors to report FW image paths
by Michal Privoznik 13 Sep '19

13 Sep '19

It feels a bit odd to report a built in list of FW images when we have FW descriptor files. Especially, when some weird architectures are concerned. For instance, OVMF_CODE.fd is reported even for non-x86_64/non-i386 arches, like ppc. But if FW descriptor files are taken into the picture then no OVMF_CODE.fd is ever reported. One can argue, that these patches are not necessary, because the whole point of FW descriptor files is that users do not have to bother with paths to FW images. And that is true. However, the whole ecosystem of FW descriptor files allows sys admins and regular users to write their own FW descriptor files and thus reporting what paths libvirt found might come handy when writing those descriptors. Michal Prívozník (5): virfirmware: Expose and define autoptr for virFirmwareFree qemu_firmware: Document qemuFirmwareGetSupported qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths qemufirmwaretest: Test FW path getting through qemuFirmwareGetSupported() qemu: Use FW descriptors to report FW image paths src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 19 +++++++-- src/qemu/qemu_firmware.c | 81 +++++++++++++++++++++++++++++++++++- src/qemu/qemu_firmware.h | 5 ++- src/util/virfirmware.c | 2 +- src/util/virfirmware.h | 5 +++ tests/qemufirmwaretest.c | 78 ++++++++++++++++++++++++++++++---- 7 files changed, 177 insertions(+), 14 deletions(-) -- 2.21.0

3 10

[libvirt] [PATCH] maint: Use pep8 to implement sc_prohibit_semicolon_at_eol_in_python
by Shi Lei 12 Sep '19

12 Sep '19

Now sc_prohibit_semicolon_at_eol_in_python can't handle semicolon within multiline strings(comments) properly. I suggest that we could use pep8 to check python code style error, such as 'statement ends with a semicolon'. In future, we could use '--select' to introduce other rules. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com> --- cfg.mk | 6 ++---- configure.ac | 4 ++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cfg.mk b/cfg.mk index 42e1abf0..c8eaf74e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -813,10 +813,8 @@ sc_require_enum_last_marker: # In Python files we don't want to end lines with a semicolon like in C sc_prohibit_semicolon_at_eol_in_python: - @prohibit='^[^#].*\;$$' \ - in_vc_files='\.py$$' \ - halt='python does not require to end lines with a semicolon' \ - $(_sc_search_regexp) + @$(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs $(PEP8) --select E703 \ + | $(GREP) . && { exit 1; } || : # mymain() in test files should use return, not exit, for nicer output sc_prohibit_exit_in_tests: diff --git a/configure.ac b/configure.ac index bf9e7681..37fa9924 100644 --- a/configure.ac +++ b/configure.ac @@ -704,6 +704,10 @@ AC_PATH_PROGS([PYTHON], [python3 python2 python]) if test -z "$PYTHON"; then AC_MSG_ERROR(['python3', 'python2' or 'python' binary is required to build libvirt]) fi +AC_PATH_PROG([PEP8], [pep8]) +if test -z "$PEP8"; then + AC_MSG_ERROR(['pep8' binary is required to check python code style]) +fi AC_PATH_PROG([PERL], [perl]) if test -z "$PERL"; then AC_MSG_ERROR(['perl' binary is required to build libvirt]) -- 2.17.1

5 7

[libvirt] [PATCH] conf: correctly convert 'managed' attribute from network port
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

The virNetworkPortDef config stores the 'managed' attribute as the virTristate type. The virDomainDef config stores the 'managed' attribute as the bool type. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/domain_conf.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3dc638f0de..ae196cac52 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30688,7 +30688,15 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, actual->data.hostdev.def.parentnet = iface; actual->data.hostdev.def.info = &iface->info; actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - actual->data.hostdev.def.managed = port->plug.hostdevpci.managed; + switch (port->plug.hostdevpci.managed) { + case VIR_TRISTATE_BOOL_YES: + actual->data.hostdev.def.managed = true; + break; + case VIR_TRISTATE_BOOL_ABSENT: + case VIR_TRISTATE_BOOL_NO: + actual->data.hostdev.def.managed = false; + break; + } actual->data.hostdev.def.source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; actual->data.hostdev.def.source.subsys.u.pci.addr = port->plug.hostdevpci.addr; switch ((virNetworkForwardDriverNameType)port->plug.hostdevpci.driver) { @@ -30820,7 +30828,10 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, iface->ifname); goto error; } - port->plug.hostdevpci.managed = actual->data.hostdev.def.managed; + if (actual->data.hostdev.def.managed) + port->plug.hostdevpci.managed = VIR_TRISTATE_BOOL_YES; + else + port->plug.hostdevpci.managed = VIR_TRISTATE_BOOL_NO; port->plug.hostdevpci.addr = actual->data.hostdev.def.source.subsys.u.pci.addr; switch ((virDomainHostdevSubsysPCIBackendType)actual->data.hostdev.def.source.subsys.u.pci.backend) { case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: -- 2.21.0

2 1

[libvirt] [PATCH python] sanitytest: whitelist 'network' method as having no C impl
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI build fix for the previous patch. sanitytest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanitytest.py b/sanitytest.py index e87b57d..0b415fd 100644 --- a/sanitytest.py +++ b/sanitytest.py @@ -357,7 +357,7 @@ for klass in gotfunctions: for func in sorted(gotfunctions[klass]): # These are pure python methods with no C APi if func in ["connect", "getConnect", "domain", "getDomain", - "virEventInvokeFreeCallback", + "virEventInvokeFreeCallback", "network", "sparseRecvAll", "sparseSendAll"]: continue -- 2.21.0

1 0

[libvirt] [PATCH v2 0/4] util: abort when out of memory
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

This is the first part of a previously posted series: https://www.redhat.com/archives/libvir-list/2019-August/msg01374.html I've temporarily split the addition of glib into a separate branch until we get clarity on guarantees around g_malloc() and mallc() using the same allocator. I published a docs update https://gitlab.gnome.org/GNOME/glib/merge_requests/1099/ to attempt to get confirmation from glib maintainers on this matter. Changed in v2: - Keep ATTRIBUTE_RETURN_CHECK annotations. We're aborting on OOM, but don't want to update all callers to drop return value checks yet - Update docs for virAsprintfQuiet too Daniel P. Berrangé (4): util: purge all code for testing OOM handling util: make allocation functions abort on OOM util: remove several unused _QUIET allocation macro variants util: make string functions abort on OOM configure.ac | 17 -- docs/docs.html.in | 3 - docs/internals/oomtesting.html.in | 213 -------------------- src/libvirt_private.syms | 4 - src/util/viralloc.c | 314 +++++------------------------- src/util/viralloc.h | 192 ++++-------------- src/util/virstring.c | 93 +++------ src/util/virstring.h | 73 +++---- tests/Makefile.am | 1 - tests/oomtrace.pl | 41 ---- tests/qemuxml2argvtest.c | 12 +- tests/testutils.c | 189 +----------------- tests/testutils.h | 2 - tests/virfirewalltest.c | 12 -- 14 files changed, 136 insertions(+), 1030 deletions(-) delete mode 100644 docs/internals/oomtesting.html.in delete mode 100755 tests/oomtrace.pl -- 2.21.0

2 8

[libvirt] [PATCH] fix memleak in virNodeDeviceGetPCISRIOVCaps
by Yi Wang 12 Sep '19

12 Sep '19

From: Jiang Kun <jiang.kun2(a)zte.com.cn> it always alloc new memory when get dumpxml of pf device,but never free it. Now free the old first,then alloc new memory. Signed-off-by: Jiang kun <jiang.kun2(a)zte.com.cn> --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index e51371d..618ce8e 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2509,6 +2509,7 @@ virNodeDeviceGetPCISRIOVCaps(const char *sysfsPath, for (i = 0; i < pci_dev->num_virtual_functions; i++) VIR_FREE(pci_dev->virtual_functions[i]); VIR_FREE(pci_dev->virtual_functions); + VIR_FREE(pci_dev->physical_function); pci_dev->num_virtual_functions = 0; pci_dev->max_virtual_functions = 0; pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; -- 1.8.3.1

2 1

[libvirt] [python PATCH] generator: fix constructor for virNetworkPort
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

The virNetworkPort class is passed both the virNetwork parent python class and the virNetworkPort C object. This needs special handling in the generator, similar to how virDomainSnapshots are dealt with. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- generator.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/generator.py b/generator.py index 0b0a5a7..433c1f2 100755 --- a/generator.py +++ b/generator.py @@ -1083,6 +1083,10 @@ class_domain_impl = { "virDomainSnapshot": True, } +class_network_impl = { + "virNetworkPort": True, +} + functions_noexcept = { 'virDomainGetID': True, 'virDomainGetName': True, @@ -1551,6 +1555,8 @@ def buildWrappers(module): classes.write(" def __init__(self, conn, _obj=None):\n") elif classname in [ "virDomainCheckpoint", "virDomainSnapshot" ]: classes.write(" def __init__(self, dom, _obj=None):\n") + elif classname in [ "virNetworkPort" ]: + classes.write(" def __init__(self, net, _obj=None):\n") else: classes.write(" def __init__(self, _obj=None):\n") if classname in [ "virDomain", "virNetwork", "virInterface", @@ -1564,6 +1570,9 @@ def buildWrappers(module): elif classname in [ "virDomainCheckpoint", "virDomainSnapshot" ]: classes.write(" self._dom = dom\n") classes.write(" self._conn = dom.connect()\n") + elif classname in [ "virNetworkPort" ]: + classes.write(" self._net = net\n") + classes.write(" self._conn = net.connect()\n") classes.write(" if type(_obj).__name__ not in [\"PyCapsule\", \"PyCObject\"]:\n") classes.write(" raise Exception(\"Expected a wrapped C Object but got %s\" % type(_obj))\n") classes.write(" self._o = _obj\n\n") @@ -1585,6 +1594,10 @@ def buildWrappers(module): classes.write(" def domain(self):\n") classes.write(" return self._dom\n\n") + if classname in class_network_impl: + classes.write(" def network(self):\n") + classes.write(" return self._net\n\n") + classes.write(" def c_pointer(self):\n") classes.write(" \"\"\"Get C pointer to underlying object\"\"\"\n") classes.write(" return libvirtmod.%s_pointer(self._o)\n\n" % -- 2.21.0

2 1

[libvirt] [PATCH] tools: add virsh docs for network port commands
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tools/virsh.pod | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tools/virsh.pod b/tools/virsh.pod index 59fb4bfc2e..cf2798e71a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3780,6 +3780,42 @@ specified. =back +=head1 NETWORK PORT COMMANDS + +The following commands manipulate network ports. Libvirt virtual networks +have ports created when a virtual machine has a virtual network interface +added. In general there should be no need to use any of the commands +here, since the hypervisor drivers run these commands are the right +point in a virtual machine's lifecycle. They can be useful for debugging +problems and / or recovering from bugs / stale state. + +=over 4 + +=item B<net-port-list> { [I<--table>] | I<--uuid> } + I<network> + +List all network ports recorded against the network. + +If I<--uuid> is specified network ports' UUID's are printed +instead of a table. Flag I<--table> specifies that the legacy +table-formatted output should be used. This is the default. +All of these are mutually exclusive. + +=item B<net-port-create> I<network> I<file> + +Allocate a new network port reserving resources based on the +port description. + +=item B<net-port-dumpxml> I<network> I<port> + +Output the network port information as an XML dump to stdout. + +=item B<net-port-delete> I<network> I<port> + +Delete record of the network port and release its resources + +=back + =head1 INTERFACE COMMANDS The following commands manipulate host interfaces. Often, these host -- 2.21.0

2 1

[libvirt] [PATCH] tools: fix XML validator detection of network port XML schema
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tools/virt-xml-validate.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 5cb7dcd276..249bcf7eef 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -80,6 +80,9 @@ if [ -z "$TYPE" ]; then *domain*) TYPE="domain" ;; + *networkport*) + TYPE="networkport" + ;; *network*) TYPE="network" ;; -- 2.21.0

2 1

[libvirt] [PATCH] conf: avoid looking up network port that doesn't exist
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

If the hypervisor driver has not yet created the network port, the portid field will be "00000000-0000-0000-0000-000000000000". If a failure occurs during early VM startup, the hypervisor driver may none the less try to release the network port, resulting in an undesirable warning: 2019-09-12 13:17:42.349+0000: 16544: error : virNetworkObjLookupPort:1679 : network port not found: Network port with UUID 00000000-0000-0000-0000-000000000000 does not exist By checking if the portid UUID is valid, we can avoid polluting the logs in this way. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3dc638f0de..b1f8a13319 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30994,6 +30994,12 @@ virDomainNetReleaseActualDevice(virConnectPtr conn, virNetworkPortPtr port = NULL; int ret = -1; + /* Port might not exist if a failure occurred during VM startup */ + if (!virUUIDIsValid(iface->data.network.portid)) { + ret = 0; + goto cleanup; + } + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) goto cleanup; -- 2.21.0

2 1

[libvirt] [PATCH 0/7] util: abort when out of memory
by Daniel P. Berrangé 12 Sep '19

12 Sep '19

See this previous thread: https://www.redhat.com/archives/libvir-list/2019-May/msg00273.html The executive summary is that catching & reporting ENOMEM is not worth the maint cost because: - this code will almost never run on Linux hosts - if it does run it will likely have bad behaviour silently dropping data or crashing the process - apps using libvirt often do so via a non-C language that aborts/exits the app on OOM regardless, or use other C libraries that abort - we can build a system that is more reliable when OOM happens by not catching OOM, instead simply letting apps exit, restart and carry on where they left off The first part of the series does the bare minimum to cull OOM handling. With this done, the main reason why we never adopted glib is now removed. Thus the second part of this series introduces use of glib into libvirt and converts our our allocation APIs to use the glib allocation APIs internally. In future I'd especially like to use glib to replace virObject code, which I knowingly wrote as a somewhat pathetic clone of GObject. Eliminating all our DBus code is also another thing I'd like, since Glib's DBus client code is better IMHO. Note that none of the callers are updated at this point, so they are all still attempting to handle errors from VIR_ALLOC, VIR_STRDUP, etc. If we convert VIR_ALLOC calls to remove return value checks, and then later convert to glib's g_new0 API, we go through two lots of churn which I think is undesirable. Thus I think it is highly desirable to introduce glib straight away and do a single conversion step. It also means we can introduce other data structures from glib to replace ours and avoid wasting time converting those too. Overall the code in this series is all quite simple and a nice cleanup. 50% of the lines culled come from the first patch removing OOM testing, the other 50% come from viralloc.c impl simplification The interesting question is the impact is has on downstreams who have to backport patches to older versions. If we start converting callers to g_new0, etc, then downstream have to either * Change g_new0 back into VIR_ALLOC and likely re-add many goto calls we purged Or * Backport all the patches in this series that drop OOM handling and introduce glib usage If we start adopting other glib features beyond g_new0, then downstreams are pretty much forced into option 2. Given the benefit I think we'll see from this series in our codebase, I'm inclined to say we should prioritize the future, over prioritizing the past (backports), and thus freely adopt use of glib APIs rightaway. IOW, I think we should expect vendors to backport this series as a starting point, before any other patches. I've not tested but I'm hopeful that such a backport of this series is fairly easy. The viralloc.{c,h} file hasn't seen much change in recent times so ought to be a clean cherry-pick. The glib additions might hit some conflicts in makefiles, but not too terrible I hope. Probably worth doing a test to see just how easy backports are over the past 1 year of releases. Daniel P. Berrangé (7): util: purge all code for testing OOM handling util: make allocation functions abort on OOM util: remove several unused _QUIET allocation macro variants util: make string functions abort on OOM build: probe for glib-2 library in configure build: link to glib library util: use glib allocation functions configure.ac | 19 +- docs/docs.html.in | 3 - docs/internals/oomtesting.html.in | 213 -------------------- libvirt.spec.in | 1 + m4/virt-glib.m4 | 30 +++ mingw-libvirt.spec.in | 2 + src/Makefile.am | 2 + src/internal.h | 1 + src/libvirt_private.syms | 4 - src/lxc/Makefile.inc.am | 2 + src/remote/Makefile.inc.am | 1 + src/util/Makefile.inc.am | 1 + src/util/viralloc.c | 313 ++++-------------------------- src/util/viralloc.h | 204 ++++--------------- src/util/virstring.c | 93 +++------ src/util/virstring.h | 79 +++----- tests/Makefile.am | 4 +- tests/oomtrace.pl | 41 ---- tests/qemuxml2argvtest.c | 12 +- tests/testutils.c | 189 +----------------- tests/testutils.h | 2 - tests/virfirewalltest.c | 12 -- tools/Makefile.am | 1 + 23 files changed, 179 insertions(+), 1050 deletions(-) delete mode 100644 docs/internals/oomtesting.html.in create mode 100644 m4/virt-glib.m4 delete mode 100755 tests/oomtrace.pl -- 2.21.0

4 21

[libvirt] [dockerfiles PATCH] Add 'trigger' and 'monitor' scripts
by Andrea Bolognani 12 Sep '19

12 Sep '19

These are extremely crude scripts that are nonetheless useful when it's time to rebuild all images. Usage is along these lines: $ ls libvirt-debian-10* >in $ ./trigger in out $ ./monitor out 3 Error handling is almost non-existent, but realistically speaking at most three people will ever run these scripts anyway :) Quay has limits on both the number of jobs that can be running at the same time and the rate of job submission: rebuilding containers in batches, eg. all Debian 10 containers, followed by all Debian 9 containers, and so on, allows us to remain within those limits. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- monitor | 40 ++++++++++++++++++++++++++++++++++++++++ trigger | 23 +++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100755 monitor create mode 100755 trigger diff --git a/monitor b/monitor new file mode 100755 index 0000000..fd27140 --- /dev/null +++ b/monitor @@ -0,0 +1,40 @@ +#!/bin/sh + +QUAYADMIN="../libvirt-jenkins-ci/guests/quayadmin" + +usage() { + echo "Usage: monitor INFILE INTERVAL" >&2 + exit 1 +} + +infile="$1" +interval="$2" + +test "$infile" || usage +test "$interval" || usage + +while true; do + reset + output=no + + while read _ build _ _ _ repository; do + repository="${repository#*/}" + + out=$("$QUAYADMIN" show-build libvirt "$repository" "$build") + phase=$(echo "$out" | grep phase: | sed -E 's/.*: //g') + + if test "$phase" = "complete"; then + continue + fi + + echo "libvirt/$repository: $phase" + output=yes + + done <"$infile" + + if test "$output" == "no"; then + break + fi + + sleep "$interval" +done diff --git a/trigger b/trigger new file mode 100755 index 0000000..d2e853d --- /dev/null +++ b/trigger @@ -0,0 +1,23 @@ +#!/bin/sh + +QUAYADMIN="../libvirt-jenkins-ci/guests/quayadmin" + +usage() { + echo "Usage: trigger INFILE OUTFILE" >&2 + exit 1 +} + +infile="$1" +outfile="$2" + +test "$infile" || usage +test "$outfile" || usage + +while read zip +do + repository="${zip%.zip}" + url="https://github.com/libvirt/libvirt-dockerfiles/raw/master/$zip" + + "$QUAYADMIN" create-build libvirt "$repository" "$url" + +done <"$infile" | tee "$outfile" -- 2.21.0

2 2

[libvirt] [PATCH v2 00/10] Adding resolution properties for video models
by jcfaracco＠gmail.com 11 Sep '19

11 Sep '19

From: Julio Faracco <jcfaracco(a)gmail.com> This serie adds 'xres' and 'yres' QEMU display properties into a new element called 'resolution'. This element is covered by model element: <model type='foo'> <resolution x='800' y='600'/> </model> Only types VGA, QXL, Virtio and Bochs support passing resolution to driver. That's why some test cases were added too. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1485793 Julio Faracco (10): docs: Adding 'xres' and 'yres' into qxl XML definition qemu: Include {xres,yres} QEMU capabilities for video models conf: Adding resolution property for model element qemu: Include {xres,yres} for QEMU command line tests: Include {xres,yres} QEMU capabilities into tests tests: Include bochs-display as capability test too tests: Introduce resolution test for QXL model tests: Introduce resolution test for Virtio model tests: Introduce resolution test for Bochs model tests: Introduce resolution test for VGA model docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c | 75 +++++++- src/conf/domain_conf.h | 5 + src/conf/virconftypes.h | 3 + src/qemu/qemu_capabilities.c | 16 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 20 +++ .../caps_2.10.0.aarch64.xml | 2 + .../caps_2.10.0.ppc64.xml | 2 + .../caps_2.10.0.s390x.xml | 2 + .../caps_2.10.0.x86_64.xml | 2 + .../caps_2.11.0.s390x.xml | 2 + .../caps_2.11.0.x86_64.xml | 2 + .../caps_2.12.0.aarch64.xml | 2 + .../caps_2.12.0.ppc64.xml | 2 + .../caps_2.12.0.s390x.xml | 2 + .../caps_2.12.0.x86_64.xml | 2 + .../caps_3.0.0.ppc64.replies | 139 +++++++++++---- .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 2 + .../caps_3.0.0.riscv32.xml | 2 + .../caps_3.0.0.riscv64.xml | 2 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 2 + .../caps_3.0.0.x86_64.replies | 167 +++++++++++++----- .../caps_3.0.0.x86_64.xml | 2 + .../caps_3.1.0.ppc64.replies | 139 +++++++++++---- .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 2 + .../caps_3.1.0.x86_64.replies | 167 +++++++++++++----- .../caps_3.1.0.x86_64.xml | 2 + .../caps_4.0.0.aarch64.replies | 139 +++++++++++---- .../caps_4.0.0.aarch64.xml | 2 + .../caps_4.0.0.ppc64.replies | 139 +++++++++++---- .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 + .../caps_4.0.0.riscv32.replies | 131 +++++++++++--- .../caps_4.0.0.riscv32.xml | 2 + .../caps_4.0.0.riscv64.replies | 131 +++++++++++--- .../caps_4.0.0.riscv64.xml | 2 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 + .../caps_4.0.0.x86_64.replies | 167 +++++++++++++----- .../caps_4.0.0.x86_64.xml | 2 + .../caps_4.1.0.x86_64.replies | 159 ++++++++++++----- .../caps_4.1.0.x86_64.xml | 2 + ...ochs-display-resolution.x86_64-latest.args | 35 ++++ .../video-bochs-display-resolution.xml | 33 ++++ .../video-qxl-resolution.x86_64-latest.args | 35 ++++ .../qemuxml2argvdata/video-qxl-resolution.xml | 33 ++++ ...o-virtio-gpu-resolution.x86_64-latest.args | 35 ++++ .../video-virtio-gpu-resolution.xml | 33 ++++ tests/qemuxml2argvtest.c | 4 + ...bochs-display-resolution.x86_64-latest.xml | 33 ++++ .../video-qxl-resolution.x86_64-latest.xml | 33 ++++ ...eo-virtio-gpu-resolution.x86_64-latest.xml | 33 ++++ tests/qemuxml2xmltest.c | 5 + 52 files changed, 1604 insertions(+), 365 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/video-bochs-display-resolution.xml create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml create mode 100644 tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml -- 2.20.1

3 13

[libvirt] [PATCH 1/2] libxl: add acpi slic table support
by Marek Marczykowski-Górecki 11 Sep '19

11 Sep '19

From: Ivan Kardykov <kardykov(a)tabit.pro> Libxl driver did not support setup additional acpi firmware to xen guest. It is necessary to activate OEM Windows installs. This patch allow to define in OS section acpi table param (which supported domain common schema). Signed-off-by: Ivan Kardykov <kardykov(a)tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com> --- docs/formatdomain.html.in | 3 ++- src/libxl/libxl_conf.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fcb7c59c00..de612ae870 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -363,7 +363,8 @@ <dd>The <code>table</code> element contains a fully-qualified path to the ACPI table. The <code>type</code> attribute contains the ACPI table type (currently only <code>slic</code> is supported) - <span class="since">Since 1.3.5 (QEMU only)</span></dd> + <span class="since">Since 1.3.5 (QEM)</span> + <span class="since">Since 5.8.0 (Xen)</span></dd> </dl> <h4><a id="elementsOSContainer">Container boot</a></h4> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 766a726ebc..c1e248d98c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -506,6 +506,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON); + /* copy SLIC table path to acpi_firmware */ + if (def->os.slic_table && + VIR_STRDUP(b_info->u.hvm.acpi_firmware, def->os.slic_table) < 0) + return -1; + if (def->nsounds > 0) { /* * Use first sound device. man xl.cfg(5) describes soundhw as -- 2.21.0

2 8

[libvirt] [PATCH v1] docs: Expand the "BIOS bootloader" documentation for domainCaps
by Kashyap Chamarthy 11 Sep '19

11 Sep '19

Rewrite some parts for clarity, elaborate the meaning of some of the XML attributes. And where necessary, distinguish that we're dealing with two different XML documents here: - the domainCapabilities XML, to detect the host "hypervisor" (QEMU/KVM) capabilities, and what libvirt knows about them. - the guest XML definition, i.e. what features a guest can use, based on the capabilities (of QEMU and libvirt and the host) reported in the domainCapabilities XML. Signed-off-by: Kashyap Chamarthy <kchamart(a)redhat.com> --- docs/formatdomaincaps.html.in | 53 ++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index bc99d378567a553afe682bc522e7a753b2d805fc..a8d970934df2c0ce8c41eb4958c94fbdf96ef8e0 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -127,7 +127,7 @@ <value>/usr/share/OVMF/OVMF_CODE.fd</value> <enum name='type'> <value>rom</value> - <value>pflash</value> + <value>pflapsh</value> </enum> <enum name='readonly'> <value>yes</value> @@ -143,38 +143,53 @@ <domainCapabilities> </pre> - <p>The <code>firmware</code> enum corresponds to - <code>firmware</code> attribute of the <code>os</code> element. - Plain presence of this enum means that libvirt is capable of so - called firmware auto selection. The listed values then represent - accepted values for the domain attribute. Only values for which - there exists a firmware descriptor that matches machine type and - architecture are listed, i.e. those which won't cause a failure - on domain startup. + <p>The <code>firmware</code> enum corresponds to the + <code>firmware</code> attribute of the <code>os</code> element in + the domain XML. The presence of this enum means libvirt is capable + of the so-called firmware auto-selection feature. And the listed + firmware values represent the accepted input in the domain + XML. Note that the <code>firmware</code> enum reports only those + values for which a firmware "descriptor file" exists on the host + -- a small JSON document that describes details about a given UEFI + binary on the host, e.g. the fimware binary path, its + architecture, supported machine type, NVRAM template, etc. This + ensures that the reported values won't cause a failure on guest + boot. (The firmware "descriptor files" are typically shipped + Linux distribution as part of the firmware package, + e.g. EDK2/OVMF.) </p> <p>For the <code>loader</code> element, the following can occur:</p> <dl> <dt><code>value</code></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> + <dd>List of known firmware binary paths. Currently this is used + only to advertise the known location of OVMF binaries for + QEMU. OVMF binaries will only be listed if they actually exist on + host.</dd> <dt><code>type</code></dt> - <dd>Whether loader is a typical BIOS (<code>rom</code>) or - an UEFI binary (<code>pflash</code>). This refers to - <code>type</code> attribute of the <loader/> - element.</dd> + <dd>Whether the boot loader is a typical BIOS (<code>rom</code>) + or a UEFI firmware (<code>pflash</code>). Each <code>value</code> + sub-element under the <code>type</code> enum represents a possible + value for the <code>type</code> attribute for the <loader/> + element in the domain XML. E.g. the presence + of <code>pfalsh</code> under the <code>type</code> enum means that + a domain XML can use UEFI firmware via: <loader/> + type="pflash" ...>/path/to/the/firmware/binary/</loader>. + </dd> <dt><code>readonly</code></dt> <dd>Options for the <code>readonly</code> attribute of the - <loader/> element.</dd> + <loader/> element in the domain XML.</dd> <dt><code>secure</code></dt> <dd>Options for the <code>secure</code> attribute of the - <loader/> element. Note, that <code>yes</code> is listed - only if there is a firmware that supports it.</dd> + <loader/> element in the domain XML. Note that the + value <code>yes</code> is listed only if libvirt detects a + firmware descriptor file that has path to an OVMF binary that + supports Secure boot, and lists its architecture and supported + machine type.</dd> </dl> <h3><a id="elementsCPU">CPU configuration</a></h3> -- 2.21.0

2 3

[libvirt] [PATCH] libvirt.spec.in: Add the Secure Boot-variant OVMF binaries
by Kashyap Chamarthy 11 Sep '19

11 Sep '19

Currently the RPM spec doesn't add the 'secboot'-variant OVMF binaries (an unintentional omission, checking with Cole on #virt, OFTC) for 'x86_64' and 'ia32'. Add them. This way, getDomainCapabilities() will report all the OVMF binaries that are present on the system. E.g. on Fedora 29, if you only have the edk2-ovmf-20190308stable-1.fc29.noarch package installed, then running `virsh domcapabilities` will enumerate _both_ the OVMF binaries (instead of just the OVMF_CODE.fd): $> virsh getdomcapabilities ... <loader supported='yes'> <value>/usr/share/edk2/ovmf/OVMF_CODE.fd</value> <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value> ... ( Learnt this from a discussion with Michal Privoznik in this bug, comment#2: https://bugzilla.redhat.com/show_bug.cgi?id=1733940 -- RFE: Report firmware (FW) paths in domainCapabilities based on FW descriptor files ) Signed-off-by: Kashyap Chamarthy <kchamart(a)redhat.com> --- I only did a cursory check on if I missed to add any other valid paths for other architectures. --- libvirt.spec.in | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b13b863928..99950495a7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1135,10 +1135,17 @@ exit 1 # Nightly edk2.git-arm LOADERS="$LOADERS:/usr/share/edk2.git/arm/QEMU_EFI-pflash.raw:/usr/share/edk2.git/arm/vars-template-pflash.raw" - # Fedora edk2-ovmf + # Fedora edk2-ovmf, x86_64 LOADERS="$LOADERS:/usr/share/edk2/ovmf/OVMF_CODE.fd:/usr/share/edk2/ovmf/OVMF_VARS.fd" + # Fedora edk2-ovmf, x86_64, with Secure Boot + LOADERS="$LOADERS:/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd:/usr/share/edk2/ovmf/OVMF_VARS.secboot.fd" # Fedora edk2-ovmf-ia32 LOADERS="$LOADERS:/usr/share/edk2/ovmf-ia32/OVMF_CODE.fd:/usr/share/edk2/ovmf-ia32/OVMF_VARS.fd" + # Fedora edk2-ovmf-ia32, with Secure Boot. (NB: Unlike x86_64, for + # 'ia32', there is no secboot-variant "VARS" file (NVRAM template). + # So the NVRAM template for 'ovmf-ia32/OVMF_CODE.secboot.fd' is the + # same as the one for the non-secboot variant.) + LOADERS="$LOADERS:/usr/share/edk2/ovmf-ia32/OVMF_CODE.secboot.fd:/usr/share/edk2/ovmf-ia32/OVMF_VARS.fd" # Fedora edk2-aarch64 LOADERS="$LOADERS:/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw:/usr/share/edk2/aarch64/vars-template-pflash.raw" # Fedora edk2-arm -- 2.20.1

3 2