[libvirt] [PATCH 00/26] Resolve more Coverity issues
by John Ferlan
Sorry for the large dump, but before I got too involved in other things
I figured I'd go through the list of the remaining 68 Coverity issues
from the new version in order to reduce the pile. Many are benign, some
seemingly false positives, and I think most are error paths. The one
non error path that does stick out is the qemu_driver.c changes in the
qemuDomainSetBlkioParameters() routine where 'param' and 'params' were
used differently between LIVE and CONFIG. In particular, in CONFIG the
use of 'params->field' instead of 'param->field'.
One that does bear looking at more closely and if someone has a better
idea is avoiding a false positive resource_leak in remote_driver.c. I
left a healthy comment in the code - you'll know when you see it.
These patches get the numbers down to 19 issues. Of the remaining
issues - some are related to Coverity thinking that 'mgetgroups' could
return a negative value with an allocated groups structure (which I'm
still scratching my head over). There is also a few calls to
virJSONValueObjectGetNumberUlong() in qemu_monitor_json.c that don't
check status, but I'm not sure why - just didn't have the research
cycles for that.
John Ferlan (26):
qemu_driver: Resolve Coverity COPY_PASTE_ERROR
remote_driver: Resolve Coverity RESOURCE_LEAK
storage: Resolve Coverity UNUSED_VALUE
vbox: Resolve Coverity UNUSED_VALUE
qemu: Resolve Coverity REVERSE_INULL
storage: Resolve Coverity OVERFLOW_BEFORE_WIDEN
virsh: Resolve Coverity DEADCODE
virfile: Resolve Coverity DEADCODE
virsh: Resolve Coverity DEADCODE
qemu: Resolve Coverity DEADCODE
tests: Resolve Coverity DEADCODE
virsh: Resolve Coverity DEADCODE
qemu: Resolve Coverity FORWARD_NULL
lxc: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
network: Resolve Coverity FORWARD_NULL
virstring: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
network_conf: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity NEGATIVE_RETURNS
nodeinfo: Resolve Coverity NEGATIVE_RETURNS
virsh: Resolve Coverity NEGATIVE_RETURNS
xen: Resolve Coverity NEGATIVE_RETURNS
qemu: Resolve Coverity NEGATIVE_RETURNS
qemu: Resolve Coverity NEGATIVE_RETURNS
libxl: Resolve Coverity NULL_RETURNS
src/conf/network_conf.c | 4 ++--
src/libxl/libxl_migration.c | 1 -
src/lxc/lxc_driver.c | 6 ++++--
src/network/leaseshelper.c | 3 +--
src/nodeinfo.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_driver.c | 26 +++++++++++++++-----------
src/qemu/qemu_migration.c | 3 ++-
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_process.c | 5 +++--
src/remote/remote_driver.c | 12 ++++++++++++
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 1 -
src/util/virfile.c | 5 ++---
src/util/virstring.c | 3 +++
src/vbox/vbox_common.c | 9 +++++++--
src/xen/xend_internal.c | 3 ++-
tests/virstringtest.c | 5 +++++
tools/virsh-domain.c | 22 ++++++++--------------
tools/virsh-edit.c | 9 ---------
tools/virsh-interface.c | 3 ---
tools/virsh-network.c | 12 +++++-------
tools/virsh-nwfilter.c | 3 ---
tools/virsh-pool.c | 3 ---
tools/virsh-snapshot.c | 3 ---
26 files changed, 76 insertions(+), 74 deletions(-)
--
1.9.3
10 years, 7 months
[libvirt] [PATCH] util/virprocess.c: fix MinGW build
by Pavel Hrdina
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/util/virprocess.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 15d8309..3dae1bd 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -28,7 +28,6 @@
#include <stdlib.h>
#include <sys/wait.h>
#include <unistd.h>
-#include <sys/syscall.h>
#if HAVE_SETRLIMIT
# include <sys/time.h>
# include <sys/resource.h>
@@ -78,10 +77,21 @@ VIR_LOG_INIT("util.process");
#endif
#ifndef HAVE_SETNS
+# ifndef WIN32
+# include <sys/syscall.h>
+
static inline int setns(int fd, int nstype)
{
return syscall(__NR_setns, fd, nstype);
}
+# else
+static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Namespaces are not supported on windows."));
+ return -1;
+}
+# endif /* WIN32 */
#endif /* HAVE_SETNS */
/**
--
1.8.5.5
10 years, 7 months
[libvirt] [PATCH] selinux: Properly check TAP FD label
by Michal Privoznik
After a4431931 the TAP FDs ale labeled with image label instead
of the process label. On the other hand, the commit was
incomplete as a few lines above, there's still old check for the
process label presence while it should be check for the image
label instead.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Pushed under trivial rule.
After this commit, the function is completely the same as
virSecuritySELinuxSetImageFDLabel(). However I'd like to keep
them separate because there's an ongoing bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1095636
so with fair chance the TapFDLabel() function will be rewritten
soon.
src/security/security_selinux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 7064158..bf67fb5 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2347,7 +2347,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virSecurityLabelDefPtr secdef;
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
- if (!secdef || !secdef->label)
+ if (!secdef || !secdef->imagelabel)
return 0;
return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel);
--
1.8.5.5
10 years, 7 months
[libvirt] [PATCH] libxl: support domainReset
by Jim Fehlig
Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET
option, but domainReset can be implemented in the libxl driver by
forcibly destroying the domain and starting it again.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 67fd7bc6..08018d4 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -979,6 +979,62 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
}
static int
+libxlDomainReset(virDomainPtr dom, unsigned int flags)
+{
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ int ret = -1;
+ libxlDomainObjPrivatePtr priv;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = libxlDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainResetEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("Domain is not running"));
+ goto cleanup;
+ }
+
+ /*
+ * The semantics of reset can be achieved by forcibly destroying
+ * the domain and starting it again.
+ */
+ priv = vm->privateData;
+ if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reset domain '%d'"), vm->def->id);
+ goto cleanup;
+ }
+
+ if (!libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to cleanup domain '%d' after reset"),
+ vm->def->id);
+ vm = NULL;
+ goto cleanup;
+ }
+
+ if (libxlDomainStart(driver, vm, false, -1) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to start domain '%d' after reset"),
+ vm->def->id);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+static int
libxlDomainDestroyFlags(virDomainPtr dom,
unsigned int flags)
{
@@ -4758,6 +4814,7 @@ static virDriver libxlDriver = {
.domainShutdown = libxlDomainShutdown, /* 0.9.0 */
.domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */
.domainReboot = libxlDomainReboot, /* 0.9.0 */
+ .domainReset = libxlDomainReset, /* 1.2.8 */
.domainDestroy = libxlDomainDestroy, /* 0.9.0 */
.domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */
.domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
--
1.8.4.5
10 years, 7 months
[libvirt] [PATCH] qemu: remove leftover virResetLastError
by Ján Tomko
As of commit 5d29ca0:
qemu: switch PCI address set from hash table to an array
There is no error to be reset.
---
src/qemu/qemu_command.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5856fe7..665a590 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1770,7 +1770,6 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video"
" device will not be possible without manual"
" intervention");
- virResetLastError();
} else if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) {
goto cleanup;
}
--
1.8.5.5
10 years, 7 months
[libvirt] [PATCH 0/5] Fix starting of VMs with backing chains containing unknown backing file specification
by Peter Krempa
When a backing chain element specifies a parent in a format not known to libvirt
we'd fail to start the VM as the chain would appear broken.
To prevent this happening introduce a new disk type to collect
unknow format specs and avoid startup failures with such disk type.
Peter Krempa (5):
util: storage: Convert disk locality check to switch statement
conf: Introduce raw disk string passthrough
conf: Mark backing chain ending with "raw" volume as broken
tests: Add tests for disk type 'raw'
utils: storage: Fall back to "raw" disk on backing store parse
failure
docs/schemas/domaincommon.rng | 14 ++++
src/conf/domain_conf.c | 12 ++-
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_driver.c | 4 +
src/storage/storage_driver.c | 8 ++
src/util/virstoragefile.c | 35 +++++++-
src/util/virstoragefile.h | 1 +
.../qemuxml2argv-disk-backing-chains-raw.xml | 94 +++++++++++++++++++++
.../qemuxml2argv-disk-virtio-raw.args | 9 ++
.../qemuxml2argv-disk-virtio-raw.xml | 45 ++++++++++
tests/qemuxml2argvtest.c | 1 +
...muxml2xmlout-disk-backing-chains-raw-active.xml | 95 ++++++++++++++++++++++
...xml2xmlout-disk-backing-chains-raw-inactive.xml | 59 ++++++++++++++
tests/qemuxml2xmltest.c | 2 +
14 files changed, 375 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-backing-chains-raw.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-raw.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-raw.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-raw-active.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-raw-inactive.xml
--
2.0.2
10 years, 7 months
[libvirt] [PATCH] virsh: desc command in --title mode mentions description instead of title
by Peter Krempa
Tweak the messages so that they mention "title" rather than
"description" when operating in title mode. Also fixes one missing "%s"
before non-formatted gettext message.
Before:
$ virsh desc --title dom
No description for domain: dom
After:
$ virsh desc --title dom
No title for domain: dom
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140034
---
tools/virsh-domain.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 86deae6..6aa8631 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7219,7 +7219,9 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
/* Compare original XML with edited. Has it changed at all? */
if (STREQ(desc, desc_edited)) {
- vshPrint(ctl, _("Domain description not changed.\n"));
+ vshPrint(ctl, "%s",
+ title ? _("Domain title not changed\n") :
+ _("Domain description not changed\n"));
ret = true;
goto cleanup;
}
@@ -7231,10 +7233,13 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
if (virDomainSetMetadata(dom, type, desc, NULL, NULL, flags) < 0) {
vshError(ctl, "%s",
- _("Failed to set new domain description"));
+ title ? _("Failed to set new domain title") :
+ _("Failed to set new domain description"));
goto cleanup;
}
- vshPrint(ctl, "%s", _("Domain description updated successfully"));
+ vshPrint(ctl, "%s",
+ title ? _("Domain title updated successfully") :
+ _("Domain description updated successfully"));
} else {
desc = vshGetDomainDescription(ctl, dom, title,
config?VIR_DOMAIN_XML_INACTIVE:0);
@@ -7244,7 +7249,9 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
if (strlen(desc) > 0)
vshPrint(ctl, "%s", desc);
else
- vshPrint(ctl, _("No description for domain: %s"),
+ vshPrint(ctl,
+ title ? _("No title for domain: %s") :
+ _("No description for domain: %s"),
virDomainGetName(dom));
}
--
2.0.2
10 years, 7 months
[libvirt] lxc: shutdown $domain broken in libvirt 1.2.3
by Steven Wilson
We recently ran into libvirt lxc shutdown failing because of mount namespaces not being available:
# virsh -c lxc:/// start test01
Domain test01 started
# virsh -c lxc:/// list
Id Name State
----------------------------------------------------
9336 test01 running
# virsh -c lxc:/// shutdown test01
error: Failed to shutdown domain test01
error: Mount namespaces are not available on this platform: Function not implemented
/var/log/libvirt/libvirtd.log:
2014-09-03 15:50:50.682+0000: 3030: error : virProcessRunInMountNamespace:999 : Mount namespaces are not available on this platform: Function not implemented
We're running Debian wheezy with stock libc6 2.13-38, a custom 3.12.15 kernel (with namespace support) and libvirt 1.2.3.
The patch in this previous thread fixes shutdown for us and we'd like to see it merged upstream:
https://www.redhat.com/archives/libvir-list/2014-May/msg00175.html
Thanks,
Steve
10 years, 7 months
[libvirt] [PATCH 0/2] [RFC] Use the power of SystemTap to get rid of all* deadlocks forever*
by Martin Kletzander
Many moons ago, I wanted to make locking more debuggable from logs.
An idea appeared that this might be better off with SystemTap. I let
it rot for a while and some time back I had a deadlock that I wanted
to try it on. So I started scripting it and here's the work (in
progress).
Well, the idea is simple. For each mutex that is about to get locked,
append it's symname and pid that locked it into the list A, when it
get's locked move it to list B and when it's unlocked, remove it.
Then whenever the user presses ^C, the script prints all the locks
that were locked (with their particular backtraces) and locks that are
waiting to be locked. This could be enhanced that it would print both
pieces of information only for the locks that are waiting to be
acquired. But I don't really care what color the shed will have.
Here are some problems I'd love to get any help with:
- I cannot run it against built git version, two problems with that:
- When I'm not root, staprun needs setuid bit set and that is, of
course, incompatible with the needed LD_LIBRARY_PATH.
- Even when I am root, I'm unable to get any backtrace that goes
beyond virLogMutex() no matter how I'm loading the debug symbols.
- When running with installed libvirtd, I am not getting as nice
backtrace as I would in gdb (but that's probably the only way how
systemtap will get any backtrace so it's better than nothing). And
I'm unable to get line numbers even with addr2line.
I'd be interested in hearing how this works for others, or any hints
on how to make the script so good that I could post it for the
inclusion upstream. Any best practices about systemtap scripts are
welcomed as well.
_______________________________________________________
(*) Not quite, I just wanted you to notice this mail ;)
Martin Kletzander (2):
lock-debug.stp
DO NOT APPLY: deadlock test
daemon/libvirtd.c | 1 +
examples/systemtap/lock-debug.stp | 115 ++++++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
create mode 100644 examples/systemtap/lock-debug.stp
--
2.1.0
10 years, 7 months
[libvirt] [PATCH 0/4] Few unrelated fixes in save/dump code
by Peter Krempa
I spent some time in the save/dump code today and found a few nits to fix.
Peter Krempa (4):
util: process: Don't report OOM errors in helper
virsh: domain: Clean up handling of "dom" in "save" command
qemu: dump: Fix formatting of function headers and code inline
qemu: dump: Resume CPUs only when the VM is still alive
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++------------------------
src/util/virprocess.c | 10 +++++-----
tools/virsh-domain.c | 6 +++---
3 files changed, 35 insertions(+), 32 deletions(-)
--
2.0.2
10 years, 7 months
