Re: [libvirt] [Qemu-devel] [Bug 1585533] [NEW] cache-miss-rate / Invalid JSON
by Eric Blake
On 05/25/2016 02:46 AM, Marc Brothier wrote:
> Public bug reported:
>
> Hi,
>
> We have VMs which were started with an older version than qemu 2.1 which
> added "cache-miss-rate" property for XBZRLECacheStats. While trying to
> migrate the VM to a new host which is running a higher version (2.3) of
> Qemu we got an exception:
>
> virJSONValueFromString:1642 : internal error: cannot parse json {"return": {"expected-downtime": 1, "xbzrle-cache": {"bytes": 0, "cache-size": 67108864, "cache-miss-rate": -nan, "pages": 0, "overflow": 0, "cache-miss": 8933}, "status": "active", "disk": {"total": 429496729600, "dirty-sync-count": 0, "remaining": 193896382464, "mbps": 0, "transferred": 235600347136, "duplicate": 0, "dirty-pages-rate": 0, "skipped": 0, "normal-bytes": 0, "normal": 0}, "setup-time": 13, "total-time": 1543124, "ram": {"total": 8599183360, "dirty-sync-count": 4, "remaining": 30695424, "mbps": 830.636997, "transferred": 3100448901, "duplicate": 1358341, "dirty-pages-rate": 7, "skipped": 0, "normal-bytes": 3082199040, "normal": 752490}}, "id": "libvirt-186200"}: lexical error: malformed number, a digit is required after the minus sign.
> 67108864, "cache-miss-rate": -nan, "pages": 0, "overflow": 0
> (right here) ------^
>
> virNetClientStreamRaiseError:191 : stream aborted at client request
Wow - I've known we have a problem with qemu emitting non-compliant
JSON, but this proves that it is fatal to libvirt. I guess my series on
improving the JSON parser [1] should consider doing a fallback to
s/NaN/0/ and s/Inf/DBL_MAX/ rather than completely erroring out when a
client tries to request it. Meanwhile, it's an easy patch to qemu to
avoid division by zero when generating cache-miss-rate.
[1] https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03424.html
>
>
> Would it be possible to improve the JSON parser to skip the key if the value is incorrect
Libvirt uses libyajl to parse JSON, and libyajl has an outstanding bug
request to support extensions to JSON such as parsing non-finite floats.
Since there has been no upstream reaction to the bug request, I
seriously doubt it will happen any time soon, so any change to tolerate
NaN in libvirt would have to be a one-off patch. It sounds like the
better fix is to make qemu emit valid JSON in the first place, rather
than making libvirt deal with broken JSON from qemu.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8 years, 7 months
[libvirt] [PATCH 0/3] qemu: Allow update of certain disk options during runtime
by Peter Krempa
Until this series it was possible to update floppies and cdroms only. With this
you can update startup policy and snapshot behavior for regular disks too.
Peter Krempa (3):
qemu: driver: Move around code to avoid need to rollback
qemu: driver: Remove unnecessary cleanup label from
qemuDomainChangeDiskLive
qemu: driver: Allow disk update of startupPolicy/snapshot for all
disks
src/qemu/qemu_driver.c | 74 ++++++++++++++++++--------------------------------
1 file changed, 26 insertions(+), 48 deletions(-)
--
2.8.2
8 years, 7 months
[libvirt] [PATCH/RFC] Add missing delta from Ubuntu to apparmor profiles
by Stefan Bader
This had been on the Debian package list before but its time to take
this onwards. So the goal would be to have one set to rule them all
(when using apparmor) and drop the seperate set of definitions which
exist at least in the Ubuntu packaging.
Right now the patch would be at a state which adds all missing files
and rules to the current examples in libvirt and installs them when
using --with-apparmor-profiles.
One problem seems to be that some of the definitions might cause
parse failures on certain versions of apparmor. I checked this morning
and this looks a bit hairy. So some apparmor 2.8 versions potentially
have issues, but not all apparmor 2.8 are the same (gah).
I could imagine (but John, we really could use some guidance here ;))
that at least some changes could be related to version 2.8.95~2430:
+ debian/patches/mediate-signals.patch,
debian/patches/change-signal-syntax.patch: Parse signal rules with
apparmor_parser. See the apparmor.d(5) man page for syntax details.
+ debian/patches/change-ptrace-syntax.patch,
debian/patches/mediate-ptrace.patch: Parse ptrace rules with
apparmor_parser. See the apparmor.d(5) man page for syntax details.
But, regardless of the when, the apparmor rules maybe need a way to handle
versioned features of the parser. One proposal was to comment out problematic
rules and allow the packager to re-enable things. Maybe going one step
further and have some pre-processing that handles version based sections
(like #if (APPARMOR_VERSION >= xxx)).
So that is where we stand. Ideas are very welcome.
-Stefan
---
>From aec5cf8cc30c80492a37856626264c3d4c27a31f Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader(a)canonical.com>
Date: Thu, 18 Sep 2014 14:15:17 +0200
Subject: [PATCH] Add missing delta from Ubuntu to apparmor profiles
This fixes up the upstream profiles and would allow to drop apparmor
related delta from the Ubuntu package.
Thanks to Serge Hallyn for the Makefile.am install hook that allows
to rename the local file.
Signed-off-by: Stefan Bader <stefan.bader(a)canonical.com>
---
examples/apparmor/Makefile.am | 10 ++++++++
examples/apparmor/libvirt-lxc | 15 +++++++++++-
examples/apparmor/libvirt-qemu | 31 +++++++++++++++++++++++-
examples/apparmor/local-usr.sbin.libvirtd | 2 ++
examples/apparmor/usr.lib.libvirt.virt-aa-helper | 25 ++++++++++++++++---
examples/apparmor/usr.sbin.libvirtd | 17 ++++++++++++-
6 files changed, 94 insertions(+), 6 deletions(-)
create mode 100644 examples/apparmor/local-usr.sbin.libvirtd
diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
index 7a20e16..aa46cb9 100644
--- a/examples/apparmor/Makefile.am
+++ b/examples/apparmor/Makefile.am
@@ -20,6 +20,7 @@ EXTRA_DIST= \
libvirt-qemu \
libvirt-lxc \
usr.lib.libvirt.virt-aa-helper \
+ local-usr.sbin.libvirtd \
usr.sbin.libvirtd
if WITH_APPARMOR_PROFILES
@@ -29,6 +30,15 @@ apparmor_DATA = \
usr.sbin.libvirtd \
$(NULL)
+localdir = $(apparmordir)/local
+local_DATA = \
+ local-usr.sbin.libvirtd \
+ $(NULL)
+
+install-data-hook:
+ mv $(DESTDIR)$(localdir)/local-usr.sbin.libvirtd \
+ $(DESTDIR)$(localdir)/usr.sbin.libvirtd
+
abstractionsdir = $(apparmordir)/abstractions
abstractions_DATA = \
libvirt-qemu \
diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
index 4bfb503..4705e0a 100644
--- a/examples/apparmor/libvirt-lxc
+++ b/examples/apparmor/libvirt-lxc
@@ -1,12 +1,18 @@
-# Last Modified: Fri Feb 7 13:01:36 2014
+# Last Modified: Thu, 18 Sep 2014 13:56:49 +0200
#include <abstractions/base>
umount,
+ dbus,
+ signal,
+ ptrace,
# ignore DENIED message on / remount
deny mount options=(ro, remount) -> /,
+ # support use of cgmanager proxy
+ mount options=(move) /sys/fs/cgroup/cgmanager/ -> /sys/fs/cgroup/cgmanager.lower/,
+
# allow tmpfs mounts everywhere
mount fstype=tmpfs,
@@ -33,8 +39,15 @@
mount fstype=fusectl -> /sys/fs/fuse/connections/,
mount fstype=securityfs -> /sys/kernel/security/,
mount fstype=debugfs -> /sys/kernel/debug/,
+ deny mount fstype=debugfs -> /var/lib/ureadahead/debugfs/,
mount fstype=proc -> /proc/,
mount fstype=sysfs -> /sys/,
+
+ mount options=(rw nosuid nodev noexec remount) -> /sys/,
+ mount options=(rw remount) -> /sys/kernel/security/,
+ mount options=(rw remount) -> /sys/fs/pstore/,
+ mount options=(ro remount) -> /sys/fs/pstore/,
+
deny /sys/firmware/efi/efivars/** rwklx,
deny /sys/kernel/security/** rwklx,
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index c6de6dd..b69e64c 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -1,4 +1,4 @@
-# Last Modified: Wed Sep 3 21:52:03 2014
+# Last Modified: Thu, 18 Sep 2014 16:41:21 +0200
#include <abstractions/base>
#include <abstractions/consoles>
@@ -13,15 +13,22 @@
capability setgid,
capability setuid,
+ # this is needed with libcap-ng support, however it breaks a lot of things
+ # atm, so just silence the denial until libcap-ng works right. LP: #522845
+ deny capability setpcap,
+
network inet stream,
network inet6 stream,
/dev/net/tun rw,
+ /dev/tap* rw,
/dev/kvm rw,
/dev/ptmx rw,
/dev/kqemu rw,
@{PROC}/*/status r,
@{PROC}/sys/kernel/cap_last_cap r,
+ owner @{PROC}/*/auxv r,
+ @{PROC}/sys/vm/overcommit_memory r,
# For hostdev access. The actual devices will be added dynamically
/sys/bus/usb/devices/ r,
@@ -38,6 +45,9 @@
/dev/snd/* rw,
capability ipc_lock,
# spice
+ /usr/bin/qemu-system-i386-spice rmix,
+ /usr/bin/qemu-system-x86_64-spice rmix,
+ /{dev,run}/shm/ r,
owner /{dev,run}/shm/spice.* rw,
# 'kill' is not required for sound and is a security risk. Do not enable
# unless you absolutely need it.
@@ -73,6 +83,7 @@
# the various binaries
/usr/bin/kvm rmix,
/usr/bin/qemu rmix,
+ /usr/bin/qemu-system-aarch64 rmix,
/usr/bin/qemu-system-arm rmix,
/usr/bin/qemu-system-cris rmix,
/usr/bin/qemu-system-i386 rmix,
@@ -91,6 +102,7 @@
/usr/bin/qemu-system-sparc rmix,
/usr/bin/qemu-system-sparc64 rmix,
/usr/bin/qemu-system-x86_64 rmix,
+ /usr/bin/qemu-system-x86_64-spice rmix,
/usr/bin/qemu-alpha rmix,
/usr/bin/qemu-arm rmix,
/usr/bin/qemu-armeb rmix,
@@ -117,6 +129,16 @@
/bin/dash rmix,
/bin/dd rmix,
/bin/cat rmix,
+ /etc/pki/CA/ r,
+ /etc/pki/CA/* r,
+ /etc/pki/libvirt/ r,
+ /etc/pki/libvirt/** r,
+
+ # for rbd
+ /etc/ceph/ceph.conf r,
+
+ # for access to hugepages
+ owner "/run/hugepages/kvm/libvirt/qemu/**" rw,
# for usb access
/dev/bus/usb/ r,
@@ -124,6 +146,13 @@
/sys/bus/ r,
/sys/class/ r,
+ signal (receive) peer=/usr/sbin/libvirtd,
+ ptrace (tracedby) peer=/usr/sbin/libvirtd,
+
+ # for ppc device-tree access
+ @{PROC}/device-tree/ r,
+ @{PROC}/device-tree/** r,
+
/usr/{lib,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper,
# child profile for bridge helper process
profile qemu_bridge_helper {
diff --git a/examples/apparmor/local-usr.sbin.libvirtd b/examples/apparmor/local-usr.sbin.libvirtd
new file mode 100644
index 0000000..6e19f20
--- /dev/null
+++ b/examples/apparmor/local-usr.sbin.libvirtd
@@ -0,0 +1,2 @@
+# Site-specific additions and overrides for usr.sbin.libvirtd.
+# For more details, please see /etc/apparmor.d/local/README.
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index bceaaff..4df86b0 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -1,8 +1,9 @@
-# Last Modified: Mon Apr 5 15:10:27 2010
+# Last Modified: Thu, 18 Sep 2014 14:05:36 +0200
#include <tunables/global>
/usr/lib/libvirt/virt-aa-helper {
#include <abstractions/base>
+ #include <abstractions/user-tmp>
# needed for searching directories
capability dac_override,
@@ -19,6 +20,12 @@
# for hostdev
/sys/devices/ r,
/sys/devices/** r,
+ /sys/bus/usb/devices/ r,
+ /sys/bus/usb/devices/** r,
+ deny /dev/sd* r,
+ deny /dev/dm-* r,
+ deny /dev/mapper/ r,
+ deny /dev/mapper/* r,
/usr/lib/libvirt/virt-aa-helper mr,
/sbin/apparmor_parser Ux,
@@ -26,8 +33,11 @@
/etc/apparmor.d/libvirt/* r,
/etc/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]* rw,
- # for backingstore -- allow access to non-hidden files in @{HOME} as well
- # as storage pools
+ # For backingstore, virt-aa-helper needs to peek inside the disk image, so
+ # allow access to non-hidden files in @{HOME} as well as storage pools, and
+ # removable media and filesystems, and certain file extentions. A
+ # virt-aa-helper failure when checking a disk for backinsgstore is non-fatal
+ # (but obviously the backingstore won't be added).
audit deny @{HOME}/.* mrwkl,
audit deny @{HOME}/.*/ rw,
audit deny @{HOME}/.*/** mrwkl,
@@ -35,8 +45,17 @@
audit deny @{HOME}/bin/** mrwkl,
@{HOME}/ r,
@{HOME}/** r,
+ @{HOME}/.Private/** mrwlk,
+ @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk,
+
/var/lib/libvirt/images/ r,
/var/lib/libvirt/images/** r,
+ /var/lib/nova/images/** r,
+ /var/lib/nova/instances/_base/** r,
+ /var/lib/nova/instances/snapshots/** r,
+ /var/lib/eucalyptus/instances/**/disk* r,
+ /var/lib/eucalyptus/instances/**/loader* r,
+ /var/lib/uvtool/libvirt/images/** r,
/{media,mnt,opt,srv}/** r,
/**.img r,
diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd
index 3011eff..814b4d81 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -1,10 +1,12 @@
-# Last Modified: Mon Apr 5 15:03:58 2010
+# Last Modified: Tue, 23 Sep 2014 09:28:07 +0200
#include <tunables/global>
@{LIBVIRT}="libvirt"
/usr/sbin/libvirtd {
#include <abstractions/base>
#include <abstractions/dbus>
+ # Site-specific additions and overrides. See local/README for details.
+ #include <local/usr.sbin.libvirtd>
capability kill,
capability net_admin,
@@ -23,6 +25,7 @@
capability setpcap,
capability mknod,
capability fsetid,
+ capability ipc_lock,
capability audit_write,
# Needed for vfio
@@ -33,6 +36,12 @@
network inet6 stream,
network inet6 dgram,
network packet dgram,
+ network netlink,
+
+ dbus bus=system,
+ signal,
+ ptrace,
+ unix,
# Very lenient profile for libvirtd since we want to first focus on confining
# the guests. Guests will have a very restricted profile.
@@ -45,6 +54,12 @@
/usr/sbin/* PUx,
/lib/udev/scsi_id PUx,
/usr/lib/xen-common/bin/xen-toolstack PUx,
+ /usr/lib/xen-*/bin/pygrub PUx,
+ /usr/lib/xen-*/bin/libxl-save-helper PUx,
+
+ # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to
+ # write and run an ebtables script.
+ /var/lib/libvirt/virtd* ixr,
# force the use of virt-aa-helper
audit deny /sbin/apparmor_parser rwxl,
--
1.9.1
8 years, 7 months
[libvirt] [PATCH] Call qemuDomainObjEndJob when qemuCaps is null during hotplug
by Shivaprasad G Bhat
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 249393a..3f26f1f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8237,7 +8237,7 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
if (priv->qemuCaps)
qemuCaps = virObjectRef(priv->qemuCaps);
else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator)))
- goto cleanup;
+ goto endjob;
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
@@ -8490,7 +8490,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
if (priv->qemuCaps)
qemuCaps = virObjectRef(priv->qemuCaps);
else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator)))
- goto cleanup;
+ goto endjob;
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
8 years, 7 months
[libvirt] [PATCH] xenconfig: xm: check for driver on disk format
by Joao Martins
When reviewing libxl vif typename series[0] I found out a bug
on xen-xm formatter where this "virsh domxml-to-native xen-xm file.xml"
can lead to a NULL dereference if the disk driver isn't specified.
Fix this by checking for driver before writing/testing it down.
[0] https://www.redhat.com/archives/libvir-list/2016-April/msg01434.html
Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
---
src/xenconfig/xen_xm.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 34d57de..3658c59 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -297,9 +297,12 @@ xenFormatXMDisk(virConfValuePtr list,
type = "aio";
else
type = virStorageFileFormatTypeToString(format);
- virBufferAsprintf(&buf, "%s:", driver);
- if (STREQ(driver, "tap"))
- virBufferAsprintf(&buf, "%s:", type);
+
+ if (driver) {
+ virBufferAsprintf(&buf, "%s:", driver);
+ if (STREQ(driver, "tap"))
+ virBufferAsprintf(&buf, "%s:", type);
+ }
} else {
switch (virDomainDiskGetType(disk)) {
case VIR_STORAGE_TYPE_FILE:
--
2.1.4
8 years, 7 months
[libvirt] [PATCH] Unref the cfg in qemuDomainAttachHostPCIDevice()
by Shivaprasad G Bhat
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_hotplug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 807aaf8..41a0e4f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1164,7 +1164,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
unsigned int flags = 0;
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
- return -1;
+ goto cleanup;
if (!cfg->relaxedACS)
flags |= VIR_HOSTDEV_STRICT_ACS_CHECK;
8 years, 7 months
[libvirt] [PATCH v2 0/3] Fix a couple of recently found coverity issues
by John Ferlan
v1:
http://www.redhat.com/archives/libvir-list/2016-May/msg01611.html
Although initially ACK'd by Erik, Jan pointed out something with one
of the patches and well this just lagged on my todo list. Since then
a change was made to lxc_driver.c (commit id '306b3a850') which caused
a merge conflict.
So rather than push without checking first, I figure I'll make a v2.
Patch 1 was already ACK'd
Patch 2 is new to remove persistentAddrs from qemu
Patch 3 is altered... I think the return from virDomainObjListRemove
will cause the 'vm' to be NULL anyway; however, since the next
called API virDomainObjEndAPI checks for that - it seems that
making that call won't hurt. I also moved the check to after
endjob, so that some sort of "failure" and goto cleanup doesn't
unexpectedly remove a transient domain.
John Ferlan (3):
qemu: Remove dead code
qemu: Remove unused persistentAddrs
lxc: Fix lxcDomainDestroyFlags endjob processing
src/lxc/lxc_driver.c | 6 ++----
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_domain_address.c | 17 ++++-------------
src/qemu/qemu_process.c | 18 ++++++++----------
4 files changed, 14 insertions(+), 28 deletions(-)
--
2.5.5
8 years, 7 months
[libvirt] [PATCH 0/7] qemu: Fix removable devices without a tray
by Peter Krempa
Peter Krempa (7):
qemu: Move struct qemuDomainDiskInfo to qemu_domain.h
qemu: Extract more information about qemu drives
qemu: Move and rename qemuDomainCheckEjectableMedia to
qemuProcessRefreshDisks
qemu: process: Fix and improve disk data extraction
qemu: hotplug: Extract code for waiting for tray eject
qemu: hotplug: Fix error reported when cdrom tray is locked
qemu: hotplug: wait for the tray to eject only for drives with a tray
src/qemu/qemu_conf.h | 7 ---
src/qemu/qemu_domain.h | 13 ++++
src/qemu/qemu_hotplug.c | 141 ++++++++++++++++---------------------------
src/qemu/qemu_hotplug.h | 3 -
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_monitor.c | 18 ------
src/qemu/qemu_monitor.h | 3 -
src/qemu/qemu_monitor_json.c | 12 ++--
src/qemu/qemu_process.c | 58 +++++++++++++++++-
src/qemu/qemu_process.h | 5 ++
tests/qemumonitorjsontest.c | 25 +++++++-
11 files changed, 157 insertions(+), 130 deletions(-)
--
2.8.2
8 years, 7 months
[libvirt] [PATCH] lxc: completely rework reference counting
by Katerina Koukiou
This patch follows the pattern used in qemu driver regarding
reference counting.
It changes lxcDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds virDomainObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Katerina Koukiou <k.koukiou(a)gmail.com>
---
src/lxc/lxc_domain.c | 16 +---
src/lxc/lxc_domain.h | 5 +-
src/lxc/lxc_driver.c | 216 +++++++++++++++++++--------------------------------
3 files changed, 85 insertions(+), 152 deletions(-)
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index bca2bb2..6fd5423 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -80,7 +80,7 @@ virLXCDomainObjFreeJob(virLXCDomainObjPrivatePtr priv)
* in any way
*
* Upon successful return, the object will have its ref count increased,
- * successful calls must be followed by EndJob eventually
+ * Successful calls must be followed by EndJob eventually
*/
int
virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
@@ -95,8 +95,6 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
return -1;
then = now + LXC_JOB_WAIT_TIME;
- virObjectRef(obj);
-
while (priv->job.active) {
VIR_DEBUG("Wait normal job condition for starting job: %s",
virLXCDomainJobTypeToString(job));
@@ -126,23 +124,17 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
else
virReportSystemError(errno,
"%s", _("cannot acquire job mutex"));
-
- virObjectUnref(obj);
return -1;
}
/*
- * obj must be locked before calling
+ * obj must be locked and have a reference before calling
*
* To be called after completing the work associated with the
* earlier virLXCDomainBeginJob() call
- *
- * Returns true if the remaining reference count on obj is
- * non-zero, false if the reference count has dropped to zero
- * and obj is disposed.
*/
-bool
+void
virLXCDomainObjEndJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
virDomainObjPtr obj)
{
@@ -154,8 +146,6 @@ virLXCDomainObjEndJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
virLXCDomainObjResetJob(priv);
virCondSignal(&priv->job.cond);
-
- return virObjectUnref(obj);
}
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index e82c719..5c4f31e 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -102,10 +102,9 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver,
enum virLXCDomainJob job)
ATTRIBUTE_RETURN_CHECK;
-bool
+void
virLXCDomainObjEndJob(virLXCDriverPtr driver,
- virDomainObjPtr obj)
- ATTRIBUTE_RETURN_CHECK;
+ virDomainObjPtr obj);
#endif /* __LXC_DOMAIN_H__ */
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 143089d..4aef78d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -126,11 +126,11 @@ static virNWFilterCallbackDriver lxcCallbackDriver = {
* lxcDomObjFromDomain:
* @domain: Domain pointer that has to be looked up
*
- * This function looks up @domain and returns the appropriate
- * virDomainObjPtr.
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI.
*
- * Returns the domain object which is locked on success, NULL
- * otherwise.
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
*/
static virDomainObjPtr
lxcDomObjFromDomain(virDomainPtr domain)
@@ -139,7 +139,7 @@ lxcDomObjFromDomain(virDomainPtr domain)
virLXCDriverPtr driver = domain->conn->privateData;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
virReportError(VIR_ERR_NO_DOMAIN,
@@ -283,7 +283,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- vm = virDomainObjListFindByUUID(driver->domains, uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -301,8 +301,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn,
dom->id = vm->def->id;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return dom;
}
@@ -347,8 +346,7 @@ static int lxcDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -367,8 +365,7 @@ static int lxcDomainIsPersistent(virDomainPtr dom)
ret = obj->persistent;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -386,8 +383,7 @@ static int lxcDomainIsUpdated(virDomainPtr dom)
ret = obj->updated;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -492,13 +488,14 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
driver->xmlopt,
0, &oldDef)))
goto cleanup;
+
+ virObjectRef(vm);
def = NULL;
vm->persistent = 1;
if (virDomainSaveConfig(cfg->configDir, driver->caps,
vm->newDef ? vm->newDef : vm->def) < 0) {
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
goto cleanup;
}
@@ -515,8 +512,7 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
cleanup:
virDomainDefFree(def);
virDomainDefFree(oldDef);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(caps);
@@ -566,14 +562,12 @@ static int lxcDomainUndefineFlags(virDomainPtr dom,
vm->persistent = 0;
} else {
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
}
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(cfg);
@@ -628,8 +622,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -654,8 +647,7 @@ lxcDomainGetState(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -674,8 +666,7 @@ static char *lxcDomainGetOSType(virDomainPtr dom)
goto cleanup;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -695,8 +686,7 @@ lxcDomainGetMaxMemory(virDomainPtr dom)
ret = virDomainDefGetMemoryActual(vm->def);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -790,12 +780,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -940,12 +928,10 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -1042,8 +1028,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -1069,8 +1054,7 @@ static char *lxcDomainGetXMLDesc(virDomainPtr dom,
virDomainDefFormatConvertXMLFlags(flags));
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -1166,12 +1150,10 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(cfg);
@@ -1301,13 +1283,11 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
dom->id = vm->def->id;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
virDomainDefFree(def);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(caps);
@@ -1390,8 +1370,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -1568,12 +1547,10 @@ lxcDomainDestroyFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
return ret;
@@ -1907,8 +1884,7 @@ static char *lxcDomainGetSchedulerType(virDomainPtr dom,
ignore_value(VIR_STRDUP(ret, "posix"));
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2089,13 +2065,11 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
virDomainDefFree(vmdef);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -2206,8 +2180,7 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -2461,12 +2434,10 @@ lxcDomainBlockStats(virDomainPtr dom,
&stats->wr_req);
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2594,12 +2565,10 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
*nparams = tmp;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2809,12 +2778,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -3209,8 +3176,7 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -3258,12 +3224,10 @@ lxcDomainInterfaceStats(virDomainPtr dom,
_("Invalid path, '%s' is not a known interface"), path);
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
#else
@@ -3293,8 +3257,7 @@ static int lxcDomainGetAutostart(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3365,13 +3328,12 @@ static int lxcDomainSetAutostart(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3503,13 +3465,12 @@ static int lxcDomainSuspend(virDomainPtr dom)
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3559,13 +3520,12 @@ static int lxcDomainResume(virDomainPtr dom)
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3630,8 +3590,7 @@ lxcDomainOpenConsole(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3707,12 +3666,10 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3814,12 +3771,10 @@ lxcDomainShutdownFlags(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3899,12 +3854,10 @@ lxcDomainReboot(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5208,15 +5161,14 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5314,15 +5266,14 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
}
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5419,15 +5370,14 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5484,12 +5434,10 @@ static int lxcDomainLxcOpenNamespace(virDomainPtr dom,
ret = nfds;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5586,11 +5534,10 @@ lxcDomainMemoryStats(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5738,12 +5685,10 @@ lxcDomainSetMetadata(virDomainPtr dom,
driver->xmlopt, cfg->stateDir,
cfg->configDir, flags);
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5773,7 +5718,7 @@ lxcDomainGetMetadata(virDomainPtr dom,
ret = virDomainObjGetMetadata(vm, type, uri, caps, driver->xmlopt, flags);
cleanup:
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -5820,7 +5765,7 @@ lxcDomainGetCPUStats(virDomainPtr dom,
ret = virCgroupGetPercpuStats(priv->cgroup, params,
nparams, start_cpu, ncpus, NULL);
cleanup:
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5881,8 +5826,7 @@ lxcDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
--
2.7.4
8 years, 7 months
