May 2021 - Devel - libvirt List Archives

[PATCH 0/2] Remove static analysis cruft
by Peter Krempa 20 May '21

20 May '21

Remove comments for silencing coverity and static assers which are no longer needed. CI pipelines: Yesterday's all-green: https://gitlab.com/pipo.sk/libvirt/-/pipelines/305953275 New run started when I've rebased this to current master: https://gitlab.com/pipo.sk/libvirt/-/pipelines/306458103 Peter Krempa (2): Drop magic comments for coverity Remove static analysis assertions src/conf/domain_conf.c | 13 ------------- src/conf/virdomainobjlist.c | 2 -- src/conf/virnwfilterbindingobjlist.c | 1 - src/cpu/cpu_x86.c | 1 - src/libxl/libxl_driver.c | 9 --------- src/qemu/qemu_command.c | 2 -- src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor.c | 3 --- src/qemu/qemu_process.c | 2 -- src/remote/remote_driver.c | 1 - src/rpc/virnetserver.c | 4 ---- src/storage/parthelper.c | 2 -- src/util/vircommand.c | 3 --- src/util/virconf.c | 6 ------ src/util/virhostmem.c | 2 -- src/util/virnetdev.c | 1 - src/util/virobject.c | 1 - src/util/virprocess.c | 1 - src/util/virutil.c | 3 --- tests/commandtest.c | 2 -- tests/virdrivermoduletest.c | 1 - tools/virsh.c | 1 - tools/virt-admin.c | 1 - tools/vsh.c | 1 - 24 files changed, 65 deletions(-) -- 2.31.1

3 4

[PATCH] virNodeDevCapMdevParseXML: Use virXMLPropEnum() for ./start/@type
by Michal Privoznik 20 May '21

20 May '21

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Initially, this started as a fix for broken build, but Peter beat me to it. I figured it's worth sending anyway. src/conf/node_device_conf.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 5598d420fe..d9b7aff7db 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1919,7 +1919,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, g_autofree xmlNodePtr *attrs = NULL; size_t i; g_autofree char *uuidstr = NULL; - g_autofree char *starttype = NULL; + xmlNodePtr startNode = NULL; ctxt->node = node; @@ -1941,17 +1941,14 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, virUUIDFormat(uuidbuf, mdev->uuid); } - if ((starttype = virXPathString("string(./start[1]/@type)", ctxt))) { - int tmp; - if ((tmp = virNodeDevMdevStartTypeFromString(starttype)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown mdev start type '%s' for '%s'"), starttype, def->name); - return -1; - } + mdev->start = VIR_NODE_DEV_MDEV_START_MANUAL; - mdev->start = tmp; - } else { - mdev->start = VIR_NODE_DEV_MDEV_START_MANUAL; + startNode = virXPathNode("./start[1]", ctxt); + if (startNode && + virXMLPropEnum(startNode, "type", + virNodeDevMdevStartTypeFromString, + VIR_XML_PROP_NONE, &mdev->start) < 0) { + return -1; } /* 'iommuGroup' is optional, only report an error if the supplied value is -- 2.26.3

2 1

[PATCH] conf: Report alias name of the detached device in error
by Kristina Hanicova 20 May '21

20 May '21

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367 Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com> --- src/conf/domain_conf.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7044701fac..e21b9fb946 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15781,38 +15781,45 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net) if (matchidx < 0) { if (MACAddrSpecified && PCIAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device matching MAC address %s found on " + _("no device matching MAC address %s and alias %s found on " VIR_PCI_DEVICE_ADDRESS_FMT), virMacAddrFormat(&net->mac, mac), + NULLSTR(net->info.alias), net->info.addr.pci.domain, net->info.addr.pci.bus, net->info.addr.pci.slot, net->info.addr.pci.function); } else if (MACAddrSpecified && CCWAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device matching MAC address %s found on " + _("no device matching MAC address %s and alias %s found on " VIR_CCW_DEVICE_ADDRESS_FMT), virMacAddrFormat(&net->mac, mac), + NULLSTR(net->info.alias), net->info.addr.ccw.cssid, net->info.addr.ccw.ssid, net->info.addr.ccw.devno); } else if (PCIAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device found on " VIR_PCI_DEVICE_ADDRESS_FMT), + _("no device matching alias %s found on " + VIR_PCI_DEVICE_ADDRESS_FMT), + NULLSTR(net->info.alias), net->info.addr.pci.domain, net->info.addr.pci.bus, net->info.addr.pci.slot, net->info.addr.pci.function); } else if (CCWAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device found on " VIR_CCW_DEVICE_ADDRESS_FMT), + _("no device matching alias %s found on " + VIR_CCW_DEVICE_ADDRESS_FMT), + NULLSTR(net->info.alias), net->info.addr.ccw.cssid, net->info.addr.ccw.ssid, net->info.addr.ccw.devno); } else if (MACAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device matching MAC address %s found"), - virMacAddrFormat(&net->mac, mac)); + _("no device matching MAC address %s and alias %s found"), + virMacAddrFormat(&net->mac, mac), + NULLSTR(net->info.alias)); } else { virReportError(VIR_ERR_DEVICE_MISSING, "%s", _("no matching device found")); -- 2.31.1

3 6

[PATCH] conf: node_device: Fix build with clang
by Peter Krempa 20 May '21

20 May '21

Clang complains: ../libvirt/src/conf/node_device_conf.c:1945:74: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero-compare] if ((mdev->start = virNodeDevMdevStartTypeFromString(starttype)) < 0) { Fixes: 42a55854993 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/conf/node_device_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) Pushed as fix for a build-breaker. diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4e2b37c612..5598d420fe 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1942,11 +1942,14 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, } if ((starttype = virXPathString("string(./start[1]/@type)", ctxt))) { - if ((mdev->start = virNodeDevMdevStartTypeFromString(starttype)) < 0) { + int tmp; + if ((tmp = virNodeDevMdevStartTypeFromString(starttype)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown mdev start type '%s' for '%s'"), starttype, def->name); return -1; } + + mdev->start = tmp; } else { mdev->start = VIR_NODE_DEV_MDEV_START_MANUAL; } -- 2.31.1

1 0

[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XII
by Tim Wiederhake 20 May '21

20 May '21

For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainAudioPulseAudioParse: Use virXMLProp* virDomainAudioDef: Change type of "type" to virDomainAudioType virDomainAudioDef: Change type of "sdl.driver" to virDomainAudioSDLDriver virDomainAudioDefParseXML: Use virXMLProp* virDomainAudioDefParseXML: Don't ignore return value of virDomainAudio*Parse() virDomainIOMMUDefParseXML: Use virXMLProp* virStorageAdapterParseXML: Use g_autofree virStorageAdapterFCHost: Change type of "type" to virStorageAdapterType virStorageAdapterParseXML: Use virXMLProp* virDomainDeviceSpaprVioAddressParseXML: Use virXMLProp* src/bhyve/bhyve_command.c | 2 +- src/conf/device_conf.c | 14 +-- src/conf/domain_conf.c | 202 +++++++++++--------------------- src/conf/domain_conf.h | 4 +- src/conf/storage_adapter_conf.c | 38 ++---- src/conf/storage_adapter_conf.h | 2 +- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_validate.c | 2 +- 8 files changed, 97 insertions(+), 171 deletions(-) -- 2.26.3

2 13

[PATCH] virnetdevopenvswitch: Remove redundant declaration
by Peng Liang 20 May '21

20 May '21

virNetDevOpenvswitchInterfaceGetMaster is declared twice in src/util/virnetdevopenvswitch.h. Remove the last one. Signed-off-by: Peng Liang <liangpeng10(a)huawei.com> --- src/util/virnetdevopenvswitch.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 35717085827e..75253768558f 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -57,9 +57,6 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) - ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; - int virNetDevOpenvswitchMaybeUnescapeReply(char *reply) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -- 2.31.1

2 1

[libvirt PATCH] qemu: fs: do not try to fill binary path if we have a socket
by Ján Tomko 20 May '21

20 May '21

We do not need to look for a suitable binary in the vhost-user description files, if we aren't the ones starting it. Otherwise startup will fail with: error: Failed to start domain 'vm1' error: operation failed: Unable to find a satisfying virtiofsd https://bugzilla.redhat.com/show_bug.cgi?id=1855789 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/qemu/qemu_virtiofs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index e310f2e7aa..edaedf0304 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -322,7 +322,7 @@ int qemuVirtioFSPrepareDomain(virQEMUDriver *driver, virDomainFSDef *fs) { - if (fs->binary) + if (fs->binary || fs->sock) return 0; return qemuVhostUserFillDomainFS(driver, fs); -- 2.31.1

2 1

memory allocations for libraries used by libvirt
by Olaf Hering 20 May '21

20 May '21

Currently src/libxl/ allocates a bunch of buffers with variants of g_new0() or g_strdup(), which will be consumed by libxenlight.so. Once the objects which contain these buffers are not needed anymore, libxenlight.so will release them with ordinary calls to free() in its *_dispose() API. In other words: libxenlight.so does not use glib. While the g_malloc docs of today's glib state that (apparently) the mistake of using a private allocator was recognized and corrected in glib 2.46, the same mistake might occur again in the future. I wonder if a patch will be accepted which will add simple wrappers around calloc, strdup, like libxlCallocWrap and libxlStrupWrap, which provides the buffers expected by libxenlight.so and which it can simply free() again. Just so that it looks more "symmetric", as opposed to g_new0()/free() pairs. Maybe this issue was already evaluated at the time libvirt was converted from virAlloc to g_new? Olaf

2 1

[PATCH v2] libxl: adjust handling of libxl_device_nic objects
by Olaf Hering 19 May '21

19 May '21

libxl objects are supposed to be initialized and disposed. Adjust libxlMakeNic to use an already initialized object, it is owned by the caller. Adjust libxlMakeNicList to initialize the list of objects, before they are filled by libxlMakeNic. In case of error the objects are disposed by libxl_domain_config_dispose. The usage of g_new0 is suspicious in the context of libxl because the memory allocated via glib is released with plain free() inside libxl. Signed-off-by: Olaf Hering <olaf(a)aepfle.de> --- src/libxl/libxl_conf.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2d2aab7e66..e4afa578b0 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1345,8 +1345,6 @@ libxlMakeNic(virDomainDef *def, return -1; } - libxl_device_nic_init(x_nic); - virMacAddrGetRaw(&l_nic->mac, x_nic->mac); /* @@ -1532,39 +1530,39 @@ libxlMakeNicList(virDomainDef *def, libxl_domain_config *d_config) { virDomainNetDef **l_nics = def->nets; size_t nnics = def->nnets; - libxl_device_nic *x_nics; size_t i, nvnics = 0; - - x_nics = g_new0(libxl_device_nic, nnics); + int ret = -1; for (i = 0; i < nnics; i++) { if (virDomainNetGetActualType(l_nics[i]) == VIR_DOMAIN_NET_TYPE_HOSTDEV) continue; + nvnics++; + } + if (!nvnics) + return 0; + + d_config->nics = g_new0(libxl_device_nic, nvnics); + d_config->num_nics = nvnics; + + for (i = 0; i < nvnics; i++) + libxl_device_nic_init(&d_config->nics[i]); - if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics], false)) + for (i = 0; i < nvnics; i++) { + if (libxlMakeNic(def, l_nics[i], &d_config->nics[i], false)) goto error; /* * The devid (at least right now) will not get initialized by * libxl in the setup case but is required for starting the * device-model. */ - if (x_nics[nvnics].devid < 0) - x_nics[nvnics].devid = nvnics; - - nvnics++; + if (d_config->nics[i].devid < 0) + d_config->nics[i].devid = i; } - VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); - d_config->nics = x_nics; - d_config->num_nics = nvnics; - - return 0; + ret = 0; error: - for (i = 0; i < nnics; i++) - libxl_device_nic_dispose(&x_nics[i]); - VIR_FREE(x_nics); - return -1; + return ret; } int

2 1

[PATCH v1] libxl: adjust handling of libxl_device_disk objects
by Olaf Hering 19 May '21

19 May '21

libxl objects are supposed to be initialized and disposed. Correct the usage of libxl_device_disk objects which are allocated on the stack. Initialize each one prior usage, and dispose them once done. Adjust libxlMakeDisk to use an already initialized object, it is owned by the caller. Adjust libxlMakeDiskList to initialize the list of objects, before they are filled by libxlMakeDisk. In case of error, the objects are disposed by libxl_domain_config_dispose. The usage of g_new0 is suspicious in the context of libxl because the memory allocated via glib is released with plain free() inside libxl. Signed-off-by: Olaf Hering <olaf(a)aepfle.de> --- src/libxl/libxl_conf.c | 22 +++++++++------------- src/libxl/libxl_driver.c | 6 ++++++ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 3149ee3b4a..2d2aab7e66 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1114,8 +1114,6 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk) int format = virDomainDiskGetFormat(l_disk); int actual_type = virStorageSourceGetActualType(l_disk->src); - libxl_device_disk_init(x_disk); - if (actual_type == VIR_STORAGE_TYPE_NETWORK) { if (STRNEQ_NULLABLE(driver, "qemu")) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1265,26 +1263,24 @@ libxlMakeDiskList(virDomainDef *def, libxl_domain_config *d_config) { virDomainDiskDef **l_disks = def->disks; int ndisks = def->ndisks; - libxl_device_disk *x_disks; size_t i; + int ret = -1; + + d_config->disks = g_new0(libxl_device_disk, ndisks); + d_config->num_disks = ndisks; - x_disks = g_new0(libxl_device_disk, ndisks); + for (i = 0; i < ndisks; i++) + libxl_device_disk_init(&d_config->disks[i]); for (i = 0; i < ndisks; i++) { - if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) + if (libxlMakeDisk(l_disks[i], &d_config->disks[i]) < 0) goto error; } - d_config->disks = x_disks; - d_config->num_disks = ndisks; - - return 0; + ret = 0; error: - for (i = 0; i < ndisks; i++) - libxl_device_disk_dispose(&x_disks[i]); - VIR_FREE(x_disks); - return -1; + return ret; } /* diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d54cd41785..2b844bb3b5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2978,6 +2978,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk) size_t i; int ret = -1; + libxl_device_disk_init(&x_disk); for (i = 0; i < vm->def->ndisks; i++) { if (vm->def->disks[i]->bus == disk->bus && STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -3018,6 +3019,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk) ret = 0; cleanup: + libxl_device_disk_dispose(&x_disk); virObjectUnref(cfg); return ret; } @@ -3030,6 +3032,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) libxl_device_disk x_disk; int ret = -1; + libxl_device_disk_init(&x_disk); switch (l_disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: ret = libxlDomainChangeEjectableMedia(vm, l_disk); @@ -3091,6 +3094,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) } cleanup: + libxl_device_disk_dispose(&x_disk); virObjectUnref(cfg); return ret; } @@ -3329,6 +3333,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) int idx; int ret = -1; + libxl_device_disk_init(&x_disk); switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_DISK: if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { @@ -3380,6 +3385,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) } cleanup: + libxl_device_disk_dispose(&x_disk); virObjectUnref(cfg); return ret; }

2 1