September 2019 - Devel - libvirt List Archives

[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