[PATCH 0/2] Remove static analysis cruft
by Peter Krempa
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
4 years
[PATCH] virNodeDevCapMdevParseXML: Use virXMLPropEnum() for ./start/@type
by Michal Privoznik
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
4 years
[PATCH] conf: Report alias name of the detached device in error
by Kristina Hanicova
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
4 years
[PATCH] conf: node_device: Fix build with clang
by Peter Krempa
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
4 years
[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XII
by Tim Wiederhake
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
4 years
[PATCH] virnetdevopenvswitch: Remove redundant declaration
by Peng Liang
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
4 years
[libvirt PATCH] qemu: fs: do not try to fill binary path if we have a socket
by Ján Tomko
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
4 years
memory allocations for libraries used by libvirt
by Olaf Hering
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
4 years
[PATCH v2] libxl: adjust handling of libxl_device_nic objects
by Olaf Hering
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
4 years
[PATCH v1] libxl: adjust handling of libxl_device_disk objects
by Olaf Hering
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;
}
4 years
