[libvirt] [PATCH v3] storage: Report error from VolOpen by default
by Cole Robinson
Currently VolOpen notifies the user of a potentially non-fatal failure by
returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most
callers treat -2 as fatal but don't actually report any message with
the error APIs.
Rename the VOL_OPEN_ERROR flag to VOL_OPEN_NOERROR. If NOERROR is specified,
we preserve the current behavior of returning -2 (there's only one caller
that wants this).
However in the default case, only return -1, and actually use the error
APIs. Fix up a couple callers as a result.
---
src/storage/storage_backend.c | 77 ++++++++++++++++++++++++--------------
src/storage/storage_backend.h | 7 ++--
src/storage/storage_backend_fs.c | 28 +++++---------
src/storage/storage_backend_scsi.c | 3 --
4 files changed, 62 insertions(+), 53 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 8fe3687..f44e323 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1279,8 +1279,9 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
/*
* Allows caller to silently ignore files with improper mode
*
- * Returns -1 on error, -2 if file mode is unexpected or the
- * volume is a dangling symbolic link.
+ * Returns -1 on error. If VIR_STORAGE_VOL_OPEN_NOERROR is passed, we
+ * return -2 if file mode is unexpected or the volume is a dangling
+ * symbolic link.
*/
int
virStorageBackendVolOpen(const char *path, struct stat *sb,
@@ -1288,9 +1289,10 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
{
int fd, mode = 0;
char *base = last_component(path);
+ bool noerror = (flags * VIR_STORAGE_VOL_OPEN_NOERROR);
if (lstat(path, sb) < 0) {
- if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) {
+ if (errno == ENOENT && noerror) {
VIR_WARN("ignoring missing file '%s'", path);
return -2;
}
@@ -1301,11 +1303,21 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
}
if (S_ISFIFO(sb->st_mode)) {
- VIR_WARN("ignoring FIFO '%s'", path);
- return -2;
+ if (noerror) {
+ VIR_WARN("ignoring FIFO '%s'", path);
+ return -2;
+ }
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Volume path '%s' is a FIFO"), path);
+ return -1;
} else if (S_ISSOCK(sb->st_mode)) {
- VIR_WARN("ignoring socket '%s'", path);
- return -2;
+ if (noerror) {
+ VIR_WARN("ignoring socket '%s'", path);
+ return -2;
+ }
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Volume path '%s' is a socket"), path);
+ return -1;
}
/* O_NONBLOCK should only matter during open() for fifos and
@@ -1316,25 +1328,21 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
* race. */
if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
if ((errno == ENOENT || errno == ELOOP) &&
- S_ISLNK(sb->st_mode)) {
+ S_ISLNK(sb->st_mode) && noerror) {
VIR_WARN("ignoring dangling symlink '%s'", path);
return -2;
}
- if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) {
+ if (errno == ENOENT && noerror) {
VIR_WARN("ignoring missing file '%s'", path);
return -2;
}
- virReportSystemError(errno,
- _("cannot open volume '%s'"),
- path);
+ virReportSystemError(errno, _("cannot open volume '%s'"), path);
return -1;
}
if (fstat(fd, sb) < 0) {
- virReportSystemError(errno,
- _("cannot stat file '%s'"),
- path);
+ virReportSystemError(errno, _("cannot stat file '%s'"), path);
VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -1351,33 +1359,46 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
if (STREQ(base, ".") ||
STREQ(base, "..")) {
VIR_FORCE_CLOSE(fd);
- VIR_INFO("Skipping special dir '%s'", base);
- return -2;
+ if (noerror) {
+ VIR_INFO("Skipping special dir '%s'", base);
+ return -2;
+ }
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot use volume path '%s'"), path);
+ return -1;
}
} else {
- VIR_WARN("ignoring unexpected type for file '%s'", path);
VIR_FORCE_CLOSE(fd);
- return -2;
+ if (noerror) {
+ VIR_WARN("ignoring unexpected type for file '%s'", path);
+ return -2;
+ }
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected type for file '%s'"), path);
+ return -1;
}
if (virSetBlocking(fd, true) < 0) {
+ VIR_FORCE_CLOSE(fd);
+ if (noerror) {
+ VIR_WARN("unable to set blocking mode for '%s'", path);
+ return -2;
+ }
virReportSystemError(errno, _("unable to set blocking mode for '%s'"),
path);
- VIR_FORCE_CLOSE(fd);
- return -2;
+ return -1;
}
if (!(mode & flags)) {
VIR_FORCE_CLOSE(fd);
- VIR_INFO("Skipping volume '%s'", path);
-
- if (mode & VIR_STORAGE_VOL_OPEN_ERROR) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected storage mode for '%s'"), path);
- return -1;
+ if (noerror) {
+ VIR_INFO("Skipping volume '%s'", path);
+ return -2;
}
- return -2;
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected storage mode for '%s'"), path);
+ return -1;
}
return fd;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 89511f8..c695ef7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -120,16 +120,15 @@ virStorageBackendPtr virStorageBackendForType(int type);
/* VolOpenCheckMode flags */
enum {
- VIR_STORAGE_VOL_OPEN_ERROR = 1 << 0, /* warn if unexpected type
- * encountered */
+ VIR_STORAGE_VOL_OPEN_NOERROR = 1 << 0, /* don't error if unexpected type
+ * encountered, just warn */
VIR_STORAGE_VOL_OPEN_REG = 1 << 1, /* regular files okay */
VIR_STORAGE_VOL_OPEN_BLOCK = 1 << 2, /* block files okay */
VIR_STORAGE_VOL_OPEN_CHAR = 1 << 3, /* char files okay */
VIR_STORAGE_VOL_OPEN_DIR = 1 << 4, /* directories okay */
};
-# define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_ERROR |\
- VIR_STORAGE_VOL_OPEN_REG |\
+# define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_REG |\
VIR_STORAGE_VOL_OPEN_BLOCK)
int virStorageBackendVolOpen(const char *path, struct stat *sb,
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de6521c..9b958f0 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -56,10 +56,10 @@
VIR_LOG_INIT("storage.storage_backend_fs");
-#define VIR_STORAGE_VOL_FS_OPEN_FLAGS (VIR_STORAGE_VOL_OPEN_DEFAULT |\
- VIR_STORAGE_VOL_OPEN_DIR)
-#define VIR_STORAGE_VOL_FS_REFRESH_FLAGS (VIR_STORAGE_VOL_FS_OPEN_FLAGS &\
- ~VIR_STORAGE_VOL_OPEN_ERROR)
+#define VIR_STORAGE_VOL_FS_OPEN_FLAGS (VIR_STORAGE_VOL_OPEN_DEFAULT &\
+ VIR_STORAGE_VOL_OPEN_DIR)
+#define VIR_STORAGE_VOL_FS_PROBE_FLAGS (VIR_STORAGE_VOL_FS_OPEN_FLAGS &\
+ VIR_STORAGE_VOL_OPEN_NOERROR)
static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virStorageBackendProbeTarget(virStorageSourcePtr target,
@@ -80,7 +80,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
*encryption = NULL;
if ((ret = virStorageBackendVolOpen(target->path, &sb,
- VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0)
+ VIR_STORAGE_VOL_FS_PROBE_FLAGS)) < 0)
goto error; /* Take care to propagate ret, it is not always -1 */
fd = ret;
@@ -902,19 +902,11 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
vol->backingStore.path = backingStore;
vol->backingStore.format = backingStoreFormat;
- if (virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
- false,
- VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
- /* The backing file is currently unavailable, the capacity,
- * allocation, owner, group and mode are unknown. Just log the
- * error and continue.
- * Unfortunately virStorageBackendProbeTarget() might already
- * have logged a similar message for the same problem, but only
- * if AUTO format detection was used. */
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot probe backing volume info: %s"),
- vol->backingStore.path);
- }
+ virStorageBackendUpdateVolTargetInfo(&vol->backingStore, false,
+ VIR_STORAGE_VOL_OPEN_DEFAULT);
+ /* If this failed, the backing file is currently unavailable,
+ * the capacity, allocation, owner, group and mode are unknown.
+ * An error message was raised, but we just continue. */
}
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index cf546fb..c448d7f 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -201,9 +201,6 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
if (virStorageBackendUpdateVolInfo(vol, true,
VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to update volume for '%s'"),
- devpath);
retval = -1;
goto free_vol;
}
--
1.9.0
10 years, 8 months
[libvirt] Clock problems on live migration
by Paul Boven
Hi everyone,
When doing a live migration, Linux guests will frequently get stuck and
become unresponsive, while the CPU utilization on the host for that
guest goes to 100%. If you wait long enough, the guest seems to always
recover, with a dmesg entry such as:
Clocksource tsc unstable (delta = 35882846234 ns)
So the TSC did a jump of nearly 36 seconds. I've actually had a
(test-vm) stay stuck for over 11 minutes before becoming responsive
again (delta = 662463064082 ns).
Migrations often fail when going from server A to B, but will then work
fine in the other direction.
Both servers and guests are locked to the same NTP servers, and are well
within 1ms from one another.
Both hosts are running Ubuntu 13.04 with these versions (from Ubuntu
packages):
Kernel: 3.8.0-35-generic x86_64
Libvirt: 1.0.2
Qemu: 1.4.0
Gluster-fs: 3.4.2 (libvirt access the images via the filesystem, not
using libgfapi yet).
The interconnect between both machines (both for migration and gluster)
is 10GbE.
We have different guests (all Ubuntu releases, 13.04 and 13.10), and
they all seem to be affected.
Clocksource: kvm-clock on all guests.
Clock entry from the guest XML: <clock offset='utc'/>
Now as far as I've udnerstood the documentation of kvm-clock, it
specifically supports live migrations, so I'm a bit surprised by these
problems. There isn't all that much information to find on this issue,
although I have found postings by others that seem to have run into the
same issues, but without a solution.
Any help would be much appreciated.
Regards, Paul Boven.
--
Paul Boven <boven(a)jive.nl> +31 (0)521-596547
Unix/Linux/Networking specialist
Joint Institute for VLBI in Europe - www.jive.nl
VLBI - It's a fringe science
10 years, 8 months
[libvirt] [PATCHv2] Fix coverity-reported leak in virSecurityManagerGenLabel
by Ján Tomko
Coverity complains about a possible leak of seclabel if
!sec_managers[i]->drv->domainGenSecurityLabel is true
and the seclabel might be overwritten by the next iteration
of the loop.
This leak should never happen, because every security driver
has domainGenSecurityLabel defined.
---
v1: 'Remove useless NULL check in virSecurityManagerGenLabel'
https://www.redhat.com/archives/libvir-list/2014-April/msg00087.html
src/security/security_manager.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index d68c7e9..79edb07 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -514,6 +514,8 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
if (!sec_managers[i]->drv->domainGenSecurityLabel) {
virReportUnsupportedError();
+ virSecurityLabelDefFree(seclabel);
+ seclabel = NULL;
} else {
/* The seclabel must be added to @vm prior calling domainGenSecurityLabel
* which may require seclabel to be presented already */
--
1.8.3.2
10 years, 8 months
[libvirt] [PATCH v3] Introduce --without-pm-utils to get rid of pm-is-supported dependency
by Cédric Bosdonnat
This uses the dbus api of systemd to check the power management
capabilities of the node.
---
Diff with v2:
* Fixed a few dbus call problems
configure.ac | 11 +++++++++
libvirt.spec.in | 9 ++++++++
src/libvirt_private.syms | 3 +++
src/util/virnodesuspend.c | 32 +++++++++++++++++++++++++
src/util/virsystemd.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virsystemd.h | 6 +++++
6 files changed, 120 insertions(+)
diff --git a/configure.ac b/configure.ac
index 73efffa..807cf0f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -563,6 +563,10 @@ AC_ARG_WITH([chrdev-lock-files],
[location for UUCP style lock files for character devices
(use auto for default paths on some platforms) @<:@default=auto@:>@])])
m4_divert_text([DEFAULTS], [with_chrdev_lock_files=auto])
+AC_ARG_WITH([pm-utils],
+ [AS_HELP_STRING([--with-pm-utils],
+ [use pm-utils for power management @<:@default=yes@:>@])])
+m4_divert_text([DEFAULTS], [with_pm_utils=yes])
dnl
dnl in case someone want to build static binaries
@@ -1621,6 +1625,12 @@ fi
AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"])
+dnl Should we build with pm-utils support?
+if test "$with_pm_utils" = "yes"; then
+ AC_DEFINE_UNQUOTED([WITH_PM_UTILS], 1, [whether to use pm-utils])
+fi
+AM_CONDITIONAL([WITH_PM_UTILS], [test "$with_pm_utils" = "yes"])
+
dnl virsh libraries
VIRSH_LIBS="$VIRSH_LIBS $READLINE_LIBS"
AC_SUBST([VIRSH_LIBS])
@@ -2845,6 +2855,7 @@ AC_MSG_NOTICE([ rbd: $LIBRBD_LIBS])
else
AC_MSG_NOTICE([ rbd: no])
fi
+AC_MSG_NOTICE([pm-utils: $with_pm_utils])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Test suite])
diff --git a/libvirt.spec.in b/libvirt.spec.in
index eab9b23..5c20955 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -132,6 +132,7 @@
%define with_libssh2 0%{!?_without_libssh2:0}
%define with_wireshark 0%{!?_without_wireshark:0}
%define with_systemd_daemon 0%{!?_without_systemd_daemon:0}
+%define with_pm_utils 1
# Non-server/HV driver defaults which are always enabled
%define with_sasl 0%{!?_without_sasl:1}
@@ -182,6 +183,7 @@
%if 0%{?fedora} >= 17 || 0%{?rhel} >= 7
%define with_systemd 1
%define with_systemd_daemon 1
+ %define with_pm_utils 0
%endif
# Fedora 18 / RHEL-7 are first where firewalld support is enabled
@@ -1138,8 +1140,10 @@ Requires: nc
Requires: gettext
# Needed by virt-pki-validate script.
Requires: gnutls-utils
+%if %{with_pm_utils}
# Needed for probing the power management features of the host.
Requires: pm-utils
+%{endif}
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
@@ -1395,6 +1399,10 @@ driver
%define _without_systemd_daemon --without-systemd-daemon
%endif
+%if ! %{with_pm_utils}
+ %define _without_pm_utils --without-pm-utils
+%endif
+
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1471,6 +1479,7 @@ rm -f po/stamp-po
%{?_with_firewalld} \
%{?_without_wireshark} \
%{?_without_systemd_daemon} \
+ %{?_without_pm_utils} \
%{with_packager} \
%{with_packager_version} \
--with-qemu-user=%{qemu_user} \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 38fbf63..ce51bdf 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1879,6 +1879,9 @@ virSysinfoSetup;
# util/virsystemd.h
+virSystemdCanHibernate;
+virSystemdCanHybridSleep;
+virSystemdCanSuspend;
virSystemdCreateMachine;
virSystemdMakeMachineName;
virSystemdMakeScopeName;
diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c
index 8088931..ba4a338 100644
--- a/src/util/virnodesuspend.c
+++ b/src/util/virnodesuspend.c
@@ -22,6 +22,9 @@
#include <config.h>
#include "virnodesuspend.h"
+#ifndef WITH_PM_UTILS
+# include "virsystemd.h"
+#endif
#include "vircommand.h"
#include "virthread.h"
#include "datatypes.h"
@@ -260,6 +263,7 @@ int nodeSuspendForDuration(unsigned int target,
*
* Returns 0 if the query was successful, -1 on failure.
*/
+#ifdef WITH_PM_UTILS
static int
virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
{
@@ -300,6 +304,34 @@ virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
virCommandFree(cmd);
return ret;
}
+#else /* ! WITH_PM_UTILS */
+static int
+virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
+{
+ int ret = -1;
+
+ if (virNodeSuspendInitialize() < 0)
+ return -1;
+
+ *supported = false;
+
+ switch (target) {
+ case VIR_NODE_SUSPEND_TARGET_MEM:
+ ret = virSystemdCanSuspend(supported);
+ break;
+ case VIR_NODE_SUSPEND_TARGET_DISK:
+ ret = virSystemdCanHibernate(supported);
+ break;
+ case VIR_NODE_SUSPEND_TARGET_HYBRID:
+ ret = virSystemdCanHybridSleep(supported);
+ break;
+ default:
+ return ret;
+ }
+
+ return ret;
+}
+#endif /* WITH_PM_UTILS */
/**
* virNodeSuspendGetTargetMask:
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 93b3f9c..d144d12 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -325,3 +325,62 @@ virSystemdNotifyStartup(void)
sd_notify(0, "READY=1");
#endif
}
+
+static int
+virSystemdPMSupportTarget(const char *methodName, bool *result)
+{
+ int ret;
+ DBusConnection *conn;
+ DBusMessage *message = NULL;
+ char *response;
+
+ ret = virDBusIsServiceEnabled("org.freedesktop.login1");
+ if (ret < 0)
+ return ret;
+
+ if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) < 0)
+ return ret;
+
+ if (!(conn = virDBusGetSystemBus()))
+ return -1;
+
+ ret = -1;
+
+ if (virDBusCallMethod(conn,
+ &message,
+ NULL,
+ "org.freedesktop.login1",
+ "/org/freedesktop/login1",
+ "org.freedesktop.login1.Manager",
+ methodName,
+ NULL) < 0)
+ return ret;
+
+ if ((ret = virDBusMessageRead(message, "s", &response)) < 0)
+ goto cleanup;
+
+ *result = STRNEQ("no", response);
+
+ ret = 0;
+
+ cleanup:
+ dbus_message_unref(message);
+ VIR_FREE(response);
+
+ return ret;
+}
+
+int virSystemdCanSuspend(bool *result)
+{
+ return virSystemdPMSupportTarget("CanSuspend", result);
+}
+
+int virSystemdCanHibernate(bool *result)
+{
+ return virSystemdPMSupportTarget("CanHibernate", result);
+}
+
+int virSystemdCanHybridSleep(bool *result)
+{
+ return virSystemdPMSupportTarget("CanHybridSleep", result);
+}
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 7fed456..491c9b7 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -48,4 +48,10 @@ int virSystemdTerminateMachine(const char *name,
void virSystemdNotifyStartup(void);
+int virSystemdCanSuspend(bool *result);
+
+int virSystemdCanHibernate(bool *result);
+
+int virSystemdCanHybridSleep(bool *result);
+
#endif /* __VIR_SYSTEMD_H__ */
--
1.8.4.5
10 years, 8 months
[libvirt] [PATCH] Remove useless NULL check in virSecurityManagerGenLabel
by Ján Tomko
Every security driver has domainGenSecurityLabel defined.
Coverity complains about a possible leak of seclabel if
!sec_managers[i]->drv->domainGenSecurityLabel is true
and the seclabel might be overwritten by the next iteration
of the loop.
---
src/security/security_manager.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index d68c7e9..24855db 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -512,24 +512,20 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
}
}
- if (!sec_managers[i]->drv->domainGenSecurityLabel) {
- virReportUnsupportedError();
- } else {
- /* The seclabel must be added to @vm prior calling domainGenSecurityLabel
- * which may require seclabel to be presented already */
- if (generated &&
- VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0)
- goto cleanup;
-
- if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) {
- if (VIR_DELETE_ELEMENT(vm->seclabels,
- vm->nseclabels -1, vm->nseclabels) < 0)
- vm->nseclabels--;
- goto cleanup;
- }
+ /* The seclabel must be added to @vm prior calling domainGenSecurityLabel
+ * which may require seclabel to be presented already */
+ if (generated &&
+ VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0)
+ goto cleanup;
- seclabel = NULL;
+ if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) {
+ if (VIR_DELETE_ELEMENT(vm->seclabels,
+ vm->nseclabels -1, vm->nseclabels) < 0)
+ vm->nseclabels--;
+ goto cleanup;
}
+
+ seclabel = NULL;
}
ret = 0;
--
1.8.3.2
10 years, 8 months
[libvirt] [PATCHv2 0/4] Time setting and getting in qemu guests
by Michal Privoznik
If we'd come up with a struct to interpret the time, it's written
in stone and there's nothing we can do about it. Even if we break
up the struct into function arguments. However, we can use typed
parameters and have the API extensible. For example, I'm
implementing seconds granularity only for now. If later somebody
feels like he/she wants to use even finer granularity, we can do
that by inventing a new typed param. Same goes for timezone, etc.
Michal Privoznik (4):
Introduce virDomain{Get,Set}Time APIs
remote: Implement remote{Get,Set}Time
virsh: Expose virDomain{Get,Set}Time
qemu: Implement virDomain{Get,Set}Time
daemon/remote.c | 50 +++++++++++++++
include/libvirt/libvirt.h.in | 24 ++++++++
src/access/viraccessperm.c | 2 +-
src/access/viraccessperm.h | 7 ++-
src/driver.h | 14 +++++
src/libvirt.c | 96 +++++++++++++++++++++++++++++
src/libvirt_public.syms | 6 ++
src/qemu/qemu_agent.c | 95 ++++++++++++++++++++++++++++
src/qemu/qemu_agent.h | 6 ++
src/qemu/qemu_driver.c | 139 +++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 47 ++++++++++++++
src/remote/remote_protocol.x | 32 +++++++++-
src/remote_protocol-structs | 20 ++++++
tools/virsh-domain-monitor.c | 143 +++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 16 +++++
15 files changed, 694 insertions(+), 3 deletions(-)
--
1.9.0
10 years, 8 months
Re: [libvirt] RFC: viridentitytest Failure on CentOS 6.5?
by Nehal J Wani
On Sun, Mar 16, 2014 at 1:28 PM, Nehal J Wani <nehaljw.kkd1(a)gmail.com> wrote:
> I followed the following steps to build a clean development version of
> libvirt from source. Can't seem to understand what is wrong. (Entire
> buildLog has been attached):
>
> ➜ /tmp cat /etc/issue
> CentOS release 6.5 (Final)
> Kernel \r on an \m
> ➜ /tmp sestatus
> SELinux status: enabled
> SELinuxfs mount: /selinux
> Current mode: enforcing
> Mode from config file: enforcing
> Policy version: 24
> Policy from config file: targeted
> ➜ /tmp git clone git://libvirt.org/libvirt.git
> Initialized empty Git repository in /tmp/libvirt/.git/
> remote: Counting objects: 138935, done.
> remote: Compressing objects: 100% (21143/21143), done.
> remote: Total 138935 (delta 117282), reused 138935 (delta 117282)
> Receiving objects: 100% (138935/138935), 137.72 MiB | 559 KiB/s, done.
> Resolving deltas: 100% (117282/117282), done.
> ➜ /tmp cd libvirt
> ➜ libvirt git:(master) ./autogen.sh --system
> Running ./configure with --prefix=/usr --sysconfdir=/etc
> --localstatedir=/var --libdir=/usr/lib64
> ..
> ..
> Now type 'make' to compile libvirt.
> ➜ libvirt git:(master) make
> ..
> ..
> make[1]: Leaving directory `/tmp/libvirt'
> ➜ libvirt git:(master) ✗ cd tests
> ➜ tests git:(master) ✗ ./viridentitytest
> /tmp/libvirt/tests/.libs/libsecurityselinuxhelper.so: No such file or directory
> ➜ tests git:(master) ✗
>
>
> --
> Nehal J Wani
Ping!
--
Nehal J Wani
10 years, 8 months
[libvirt] [PATCH] qemu: cleanup error checking on agent replies
by Martin Kletzander
On all the places where qemuAgentComand() was called, we did a check
for errors in the reply. Unfortunately, some of the places called
qemuAgentCheckError() without checking for non-null reply which might
have resulted in a crash.
So this patch makes the error-checking part of qemuAgentCommand()
itself, which:
a) makes it look better,
b) makes the check mandatory and, most importantly,
c) checks for the errors if and only if it is appropriate.
This actually fixes a potential crashers when qemuAgentComand()
returned 0, but reply was NULL. Having said that, it *should* fix the
following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_agent.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 27af1be..4c83d7d 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -117,6 +117,8 @@ struct _qemuAgent {
qemuAgentEvent await_event;
};
+static int qemuAgentCheckError(virJSONValuePtr cmd, virJSONValuePtr reply);
+
static virClassPtr qemuAgentClass;
static void qemuAgentDispose(void *obj);
@@ -1009,6 +1011,7 @@ qemuAgentCommand(qemuAgentPtr mon,
}
} else {
*reply = msg.rxObject;
+ ret = qemuAgentCheckError(cmd, *reply);
}
}
@@ -1277,9 +1280,6 @@ int qemuAgentShutdown(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1308,8 +1308,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon)
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@@ -1346,8 +1345,7 @@ int qemuAgentFSThaw(qemuAgentPtr mon)
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@@ -1386,9 +1384,6 @@ qemuAgentSuspend(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1419,9 +1414,6 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon,
if ((ret = qemuAgentCommand(mon, cmd, &reply, timeout)) < 0)
goto cleanup;
- if ((ret = qemuAgentCheckError(cmd, reply)) < 0)
- goto cleanup;
-
if (!(*result = virJSONValueToString(reply, false)))
ret = -1;
@@ -1449,9 +1441,6 @@ qemuAgentFSTrim(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1472,8 +1461,7 @@ qemuAgentGetVCPUs(qemuAgentPtr mon,
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (!(data = virJSONValueObjectGet(reply, "return"))) {
@@ -1581,8 +1569,7 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
cpus = NULL;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
--
1.9.1
10 years, 8 months
[libvirt] [PATCH] virsh: man: delete the unexpected character in snapshot-list
by Shanzhi Yu
---
tools/virsh.pod | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ba2da20..98d891a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3162,7 +3162,7 @@ with I<--current>.
=item B<snapshot-list> I<domain> [I<--metadata>] [I<--no-metadata>]
[{I<--parent> | I<--roots> | [{I<--tree> | I<--name>}]}]
[{[I<--from>] B<snapshot> | I<--current>} [I<--descendants>]]
-[I<--leaves>] [I<--no-leaves>] p[I<--inactive>] [I<--active>]
+[I<--leaves>] [I<--no-leaves>] [I<--inactive>] [I<--active>]
[I<--disk-only>] [I<--internal>] [I<--external>]
List all of the available snapshots for the given domain, defaulting
--
1.9.0
10 years, 8 months
[libvirt] [BUG] xenInotify/keep-alive double-free
by Philipp Hahn
Hello,
There seems to be a double-free bug in libvirt; I've checked libvirt
0.9.12 and 1.2.3:
With 0.9.12 xenUnifiedOpen() passes a pointer of "conn" to
xenInotifyOpen(), which passes it to virEventAddHandle(...opaque=conn...).
After successfully defining a new domain inotify picks up the newly
created directory and generates an event, which is processes when virsh
already dropped its last public reference to the domain and subsequently
already freed the connection. Since *conn is not zero-filled, the data
is still valid and usable, but virsh the terminates on double-freeing
some internal data.
I can reproduce by it doing:
virsh undefine $DOM
virsh -c xen:// define $DOM.xml
For 1.2.3 something similar seems to happen with the keep-alive:
$ grep unref ~/BUG/31032_virsh-define-segv.log
virUnrefDomain:276 : unref domain 0x7f4ec4003fe0 ucs32-64-segv 1
virReleaseDomain:246 : unref connection 0x917650 2
virUnrefDomain:276 : unref domain 0x934460 ucs32-64-segv 1
virReleaseDomain:246 : unref connection 0x917650 2
virUnrefConnect:145 : unref connection 0x917650 1
virUnrefDomain:276 : unref domain 0x7f4ec4004060 ucs32-64-segv 1
virReleaseDomain:246 : unref connection 0x917650 1
Notice that there are two lines for "unref connection ... 1"!
My gut feeling is that libvirt should also increment the reference
counter for internal references to delay freeing still used data and add
a second counter to track external references, which is used to start
closing down things.
Comments and ideas welcomed.
We're tracking this as
<https://forge.univention.org/bugzilla/show_bug.cgi?id=31032>
Sincerely
Philipp
--
Philipp Hahn
Open Source Software Engineer
Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen
Tel.: +49 421 22232-0
Fax : +49 421 22232-99
hahn(a)univention.de
http://www.univention.de/
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876
10 years, 8 months
