April 2020 - Devel - libvirt List Archives

[PATCH libvirt-python] examples: Fix connection error handling 2
by Philipp Hahn 27 Apr '20

27 Apr '20

Fix two more cases in examples as libvirt.open*() does not return None but raises an exception Fixes: 283e2bc693746164b22226e14d6fe3ccd38a07bf Signed-off-by: Philipp Hahn <hahn(a)univention.de> --- examples/dhcpleases.py | 5 +++-- examples/domipaddrs.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/examples/dhcpleases.py b/examples/dhcpleases.py index f394541..6f80fd5 100755 --- a/examples/dhcpleases.py +++ b/examples/dhcpleases.py @@ -22,8 +22,9 @@ else: usage() sys.exit(2) -conn = libvirt.open(uri) -if conn == None: +try: + conn = libvirt.open(uri) +except libvirt.libvirtError: print("Unable to open connection to libvirt") sys.exit(1) diff --git a/examples/domipaddrs.py b/examples/domipaddrs.py index bda308c..e893995 100755 --- a/examples/domipaddrs.py +++ b/examples/domipaddrs.py @@ -21,8 +21,9 @@ else: usage() sys.exit(2) -conn = libvirt.open(uri) -if conn == None: +try: + conn = libvirt.open(uri) +except libvirt.libvirtError: print "Unable to open connection to libvirt" sys.exit(1) -- 2.20.1

2 1

[libvirt PATCH] CONTRIBUTING: Include information on build dependencies
by Andrea Bolognani 27 Apr '20

27 Apr '20

libvirt depends on a ton of packages, so trying to install them all by using the classic approach of repeatedly running configure and reacting to each failure by installing the corresponding missing package will inevitably lead to frustration. Luckily there's an easy solution to get most dependencies installed in one fell swoop, and we just need to document it. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- CONTRIBUTING.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 68c7b547c6..f476700fdd 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -17,3 +17,22 @@ your git clone run: $ make You'll find the freshly-built document in ``docs/contribute.html``. + +If ``configure`` fails because of missing dependencies, you can set +up your system by calling + +:: + + $ sudo dnf builddep libvirt + +if you're on a RHEL-based distribution or + +:: + + $ sudo apt-get build-dep libvirt + +if you're on a Debian-based one. + +You might still be missing some dependencies if your distribution is +shipping an old libvirt version, but that will get you much closer to +where you need to be to build successfully from source. -- 2.25.3

2 3

[PATCH] network: Remove memory leak caused by wrong initialization
by Julio Faracco 27 Apr '20

27 Apr '20

This commit fix a wrong variable initialization. There is a variable called `new_lease` which is being initialized with the content of parameter `lease`. To avoid memory leak, the proper way is initialize with NULL first. This wrong statement was added by commit 97a0aa24. There are some other improvements also. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> --- src/conf/network_conf.c | 38 ++++++++++++++++++------------------- src/network/bridge_driver.c | 8 ++------ 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index cd60ee7548..a7c177f8db 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -412,38 +412,32 @@ static int virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease, xmlNodePtr node) { - virNetworkDHCPLeaseTimeDefPtr new_lease = *lease; - g_autofree char *expiry = NULL; - g_autofree char *unit = NULL; - int unitInt; + virNetworkDHCPLeaseTimeDefPtr new_lease = NULL; + g_autofree char *expirystr = NULL; + g_autofree char *unitstr = NULL; + unsigned long expiry; + int unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES; - if (!(expiry = virXMLPropString(node, "expiry"))) + if (!(expirystr = virXMLPropString(node, "expiry"))) return 0; - if (VIR_ALLOC(new_lease) < 0) - return -1; - new_lease->unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES; - - if (virStrToLong_ul(expiry, NULL, 10, &new_lease->expiry) < 0) + if (virStrToLong_ul(expirystr, NULL, 10, &expiry) < 0) return -1; - if ((unit = virXMLPropString(node, "unit"))) { - if ((unitInt = virNetworkDHCPLeaseTimeUnitTypeFromString(unit)) < 0) { + if ((unitstr = virXMLPropString(node, "unit"))) { + if ((unit = virNetworkDHCPLeaseTimeUnitTypeFromString(unitstr)) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid unit: %s"), unit); + _("Invalid unit: %s"), unitstr); return -1; } - new_lease->unit = unitInt; } /* infinite */ - if (new_lease->expiry > 0) { + if (expiry > 0) { /* This boundary check is related to dnsmasq man page settings: * "The minimum lease time is two minutes." */ - if ((new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && - new_lease->expiry < 120) || - (new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && - new_lease->expiry < 2)) { + if ((unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && expiry < 120) || + (unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && expiry < 2)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("The minimum lease time should be greater " "than 2 minutes")); @@ -451,6 +445,12 @@ virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease, } } + if (VIR_ALLOC(new_lease) < 0) + return -1; + + new_lease->expiry = expiry; + new_lease->unit = unit; + *lease = new_lease; return 0; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 87f0452611..e8f0dcf7d0 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -969,7 +969,6 @@ static int networkConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) static char * networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease) { - char *leasetime = NULL; const char *unit; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; @@ -984,9 +983,7 @@ networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease) virBufferAsprintf(&buf, "%lu%c", lease->expiry, unit[0]); } - leasetime = virBufferContentAndReset(&buf); - - return leasetime; + return virBufferContentAndReset(&buf); } @@ -999,14 +996,13 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx, { size_t i; bool ipv6 = false; - g_autofree char *leasetime = NULL; if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) ipv6 = true; for (i = 0; i < ipdef->nhosts; i++) { virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]); + g_autofree char *leasetime = networkBuildDnsmasqLeaseTime(host->lease); - leasetime = networkBuildDnsmasqLeaseTime(host->lease); if (VIR_SOCKET_ADDR_VALID(&host->ip)) if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip, host->name, host->id, leasetime, -- 2.25.3

2 1

[PATCH v2 0/5] qemu: spport AIO interface io_uring
by Han Han 26 Apr '20

26 Apr '20

Changes from v1: - Separate the news XML patch from the docs patch - More details in the descrition of news XML - Change bus type of unit tests XML from ide to virtio - Fix missing spaces in docs patch - Reorder the sequence of patches git repo: https://github.com/qiankehan/libvirt/tree/io_uring-v2 v1: https://www.redhat.com/archives/libvir-list/2020-April/msg00883.html Han Han (5): qemu_capabilities: Introduce QEMU_CAPS_AIO_IO_URING qemu: Implement the aio mode io_uring docs: Docs and rng schemas for io_uring tests: Tests for io mode io_uring news: qemu: support async IO mode 'io_uring' docs/formatdomain.html.in | 3 +- docs/news.xml | 11 ++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++ tests/qemublocktest.c | 1 + ...le-backing_basic-aio_io_uring-srconly.json | 44 +++++++++++++ .../file-backing_basic-aio_io_uring.json | 66 +++++++++++++++++++ .../file-backing_basic-aio_io_uring.xml | 47 +++++++++++++ .../caps_5.0.0.aarch64.replies | 3 +- .../caps_5.0.0.aarch64.xml | 1 + .../caps_5.0.0.ppc64.replies | 3 +- .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.x86_64.replies | 3 +- .../caps_5.0.0.x86_64.xml | 1 + .../disk-aio-io_uring.x86_64-latest.args | 41 ++++++++++++ tests/qemuxml2argvdata/disk-aio-io_uring.xml | 30 +++++++++ tests/qemuxml2argvtest.c | 1 + .../disk-aio-io_uring.x86_64-latest.xml | 39 +++++++++++ tests/qemuxml2xmltest.c | 1 + 23 files changed, 306 insertions(+), 4 deletions(-) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_io_uring-srconly.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_io_uring.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_io_uring.xml create mode 100644 tests/qemuxml2argvdata/disk-aio-io_uring.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-aio-io_uring.xml create mode 100644 tests/qemuxml2xmloutdata/disk-aio-io_uring.x86_64-latest.xml -- 2.25.0

3 12

[PATCH 0/5] Xen: Add support for xl.cfg passthrough setting
by Jim Fehlig 24 Apr '20

24 Apr '20

Hi All, Note: This series is based on Marek's patches adding support for e820_host https://www.redhat.com/archives/libvir-list/2020-April/msg00633.html which I've ACK'ed, but am waiting to commit until this related setting is hashed out. This series adds support for Xen's xl.cfg(5) 'passthrough' setting. Starting with xen 4.13 this setting must be enabled in order to assign PCI passthrough devices to a guest. libxl will enable it automatically if the guest config has PCI devices at creation time, but otherwise 'passthrough' must be enabled to hotplug PCI devices. The passthrough setting is mapped to a xen hypervisor feature of the same name. Xen hypervisor features were recently added by the series mentioned above. Jim Fehlig (5): conf: add xen hypervisor feature 'passthrough' libxl: make use of passthrough hypervisor feature libxl: refactor cpu and hypervisor feature parser/formatter xenconfig: Add support for 'passthrough' hypervisor feature tests: check conversion of passthrough hypervisor feature docs/formatdomain.html.in | 7 ++ docs/schemas/domaincommon.rng | 12 ++ src/conf/domain_conf.c | 115 ++++++++++++++---- src/conf/domain_conf.h | 11 ++ src/libvirt_private.syms | 2 + src/libxl/libxl_conf.c | 21 ++++ src/libxl/xen_common.c | 86 ++++++++++--- .../test-fullvirt-hypervisor-features.cfg | 26 ++++ .../test-fullvirt-hypervisor-features.xml | 50 ++++++++ .../xlconfigdata/test-paravirt-e820_host.cfg | 1 + .../xlconfigdata/test-paravirt-e820_host.xml | 1 + tests/xlconfigtest.c | 3 + 12 files changed, 290 insertions(+), 45 deletions(-) create mode 100644 tests/xlconfigdata/test-fullvirt-hypervisor-features.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-hypervisor-features.xml -- 2.26.0

3 8

[PATCH] virStorageSourceParseNBDColonString: Rewrite to match what qemu does
by Peter Krempa 24 Apr '20

24 Apr '20

Our implementation wasn't quite able to parse everything that qemu does. This patch rewrites the parser to a code that semantically resembles the combination of 'nbd_parse_filename' and 'inet_parse' methods in qemu to be able to parse the strings in an equivalent manner. The only thing that libvirt doesn't do is to check the lenghts of various components in the nbd string in places where qemu uses constant size buffers. The test cases validate that some of the corner cases involving colons are parsed properly. https://bugzilla.redhat.com/show_bug.cgi?id=1826652 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/virstoragefile.c | 90 +++++++++++++++++++++++---------------- tests/virstoragetest.c | 16 +++++++ 2 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ffc8bdb344..a2ecdc3928 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3072,59 +3072,77 @@ static int virStorageSourceParseNBDColonString(const char *nbdstr, virStorageSourcePtr src) { - VIR_AUTOSTRINGLIST backing = NULL; - const char *exportname; - - if (!(backing = virStringSplit(nbdstr, ":", 0))) - return -1; - - /* we know that backing[0] now equals to "nbd" */ - - if (VIR_ALLOC_N(src->hosts, 1) < 0) - return -1; + g_autofree char *nbd = g_strdup(nbdstr); + char *export_name; + char *host_spec; + char *unixpath; + char *port; + src->hosts = g_new0(virStorageNetHostDef, 1); src->nhosts = 1; - src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + /* We extract the parameters in a similar way qemu does it */ /* format: [] denotes optional sections, uppercase are variable strings * nbd:unix:/PATH/TO/SOCKET[:exportname=EXPORTNAME] * nbd:HOSTNAME:PORT[:exportname=EXPORTNAME] */ - if (!backing[1]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing remote information in '%s' for protocol nbd"), - nbdstr); - return -1; - } else if (STREQ(backing[1], "unix")) { - if (!backing[2]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing unix socket path in nbd backing string %s"), - nbdstr); - return -1; - } - src->hosts->socket = g_strdup(backing[2]); + /* first look for ':exportname=' and cut it off */ + if ((export_name = strstr(nbd, ":exportname="))) { + src->path = g_strdup(export_name + strlen(":exportname=")); + export_name[0] = '\0'; + } + + /* Verify the prefix and contents. Note that we require a + * "host_spec" part to be present. */ + if (!(host_spec = STRSKIP(nbd, "nbd:")) || host_spec[0] == '\0') + goto malformed; + + if ((unixpath = STRSKIP(host_spec, "unix:"))) { src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; - } else { - src->hosts->name = g_strdup(backing[1]); - if (!backing[2]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing port in nbd string '%s'"), - nbdstr); - return -1; + if (unixpath[0] == '\0') + goto malformed; + + src->hosts->socket = g_strdup(unixpath); + } else { + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + if (host_spec[0] == ':') { + /* no host given */ + goto malformed; + } else if (host_spec[0] == '[') { + host_spec++; + /* IPv6 addr */ + if (!(port = strstr(host_spec, "]:"))) + goto malformed; + + port[0] = '\0'; + port += 2; + + if (host_spec[0] == '\0') + goto malformed; + } else { + if (!(port = strchr(host_spec, ':'))) + goto malformed; + + port[0] = '\0'; + port++; } - if (virStringParsePort(backing[2], &src->hosts->port) < 0) + if (virStringParsePort(port, &src->hosts->port) < 0) return -1; - } - if ((exportname = strstr(nbdstr, "exportname="))) { - exportname += strlen("exportname="); - src->path = g_strdup(exportname); + src->hosts->name = g_strdup(host_spec); } return 0; + + malformed: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("malformed nbd string '%s'"), nbdstr); + return -1; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 6d2b21c25f..ac1480de4e 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1261,10 +1261,26 @@ mymain(void) "<source protocol='nbd' name=':test'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("nbd:[::1]:6000:exportname=:test", + "<source protocol='nbd' name=':test'>\n" + " <host name='::1' port='6000'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("nbd:127.0.0.1:6000:exportname=:test", + "<source protocol='nbd' name=':test'>\n" + " <host name='127.0.0.1' port='6000'/>\n" + "</source>\n"); TEST_BACKING_PARSE("nbd:unix:/tmp/sock:exportname=/", "<source protocol='nbd' name='/'>\n" " <host transport='unix' socket='/tmp/sock'/>\n" "</source>\n"); + TEST_BACKING_PARSE("nbd:unix:/tmp/sock:", + "<source protocol='nbd'>\n" + " <host transport='unix' socket='/tmp/sock:'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("nbd:unix:/tmp/sock::exportname=:", + "<source protocol='nbd' name=':'>\n" + " <host transport='unix' socket='/tmp/sock:'/>\n" + "</source>\n"); TEST_BACKING_PARSE("nbd://example.org:1234", "<source protocol='nbd'>\n" " <host name='example.org' port='1234'/>\n" -- 2.26.0

2 2

[libvirt PATCH] travis: explicitly include gnutls to ensure it is updated
by Daniel P. Berrangé 24 Apr '20

24 Apr '20

Travis includes gnutls in the default package set, but it is an outdated version linkng to an incompatible libffi version. The 'update: true' stanza causes the brew toolchain to be updated but not the installed formula. It is possible to run 'brew upgrade' to update installed formula, but this is very slow adding more than 5 minutes to the build time. Listing the gnutls package explicitly causes it to be updated without extending the build time. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI build fix .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 29097cab6c..7c8a114054 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,7 @@ addons: - yajl - glib - docutils + - gnutls matrix: include: -- 2.25.3

1 0

Re: [libvirt-php] libvirt_list_storagepools() gives an error if all the storage pools on the host node are active
by Fernando Casas Schössow 24 Apr '20

24 Apr '20

I can confirm the problem is fixed in current master. Thanks and have a great weekend! On vie, abr 24, 2020 at 4:49 PM, Fernando Casas Schössow <casasfernando(a)outlook.com> wrote: > Hi Michal, > > Thanks for quick reply. > Let me pull from master, rebuild the container and test it. > I will report back with the result. > > Thanks, > > Fernando > > On vie, abr 24, 2020 at 4:16 PM, Michal Privoznik > <mprivozn(a)redhat.com> wrote: >> On 4/23/20 11:02 PM, Fernando Casas Schössow wrote: >>> Hi, >>> >>> While using the function libvirt_list_storagepools() I found that >>> if all the storage pools on the host node are active, the function >>> doesn't return the list of storage pools but it gives the following >>> error instead: >> >> Hey, I've switched the function to use more modern listing API. The >> change us pushed, so can you please fetch the master and see if that >> helped? >> >> Thanks, >> Michal > > >

1 0

[libvirt-php] libvirt_list_storagepools() gives an error if all the storage pools on the host node are active
by Fernando Casas Schössow 24 Apr '20

24 Apr '20

Hi, While using the function libvirt_list_storagepools() I found that if all the storage pools on the host node are active, the function doesn't return the list of storage pools but it gives the following error instead: names in virConnectListDefinedStoragePools must not be NULL Expected result, return an array of all the storage pools names on the host node regardless of their status (active or inactive). virsh output for the equivalent command: $ virsh pool-list --all Name State Autostart -------------------------------------------- default active yes resources active yes templates active yes virtual_machines_hdd active yes virtual_machines_ssd active yes Also in the same scenario (all storage pools active) libvirt_list_inactive_storagepools() gives the same error detailed above. I wouldn't say this is actually wrong but wouldn't it be better if these functions returns FALSE instead if there are no results (-no inactive storage pools in this case-)? System information: OS: Debian Buster PHP version (fast cgi): 7.3.14 Libvirt PHP version: latest from git repository @ gitlab Libvirt version: 5.0.0 KVM host libvirt version: 5.9.0 Thanks, Fernando

2 2

[PATCH 0/8] tests: qemumonitor: Make fake qemu monitor failures more obvious
by Peter Krempa 24 Apr '20

24 Apr '20

See patch 4 and 8 for details. Peter Krempa (8): tests: monitor: Rename qemuMonitorReportError to qemuMonitorTestAddErrorResponse qemuhotplugtest: detach: Add expected 'object-del' to disk-scsi-multipath case qemuhotplugtest: cpu: x86-modern-individual: Remove invalid test case qemumonitortestutils: Make test monitor failures more prominent qemucapabilitiesdata: riscv: Remove call to 'query-machines' qemuhotplugtest: Remove 'drive_del' expectation from failed cases qemumonitortestutils: Store a string identifying test monitor entry qemumonitortestutils: Enforce consumption of all items in test monitor build-aux/syntax-check.mk | 1 + tests/cputest.c | 2 + tests/qemuagenttest.c | 31 +-- .../caps_3.0.0.riscv32.replies | 42 ----- .../caps_3.0.0.riscv64.replies | 42 ----- .../caps_4.0.0.riscv32.replies | 42 ----- .../caps_4.0.0.riscv64.replies | 42 ----- tests/qemuhotplugtest.c | 12 +- tests/qemumonitorjsontest.c | 2 + tests/qemumonitortestutils.c | 176 +++++++++++------- tests/qemumonitortestutils.h | 5 +- 11 files changed, 141 insertions(+), 256 deletions(-) -- 2.26.0

2 10