October 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] build: avoid -Wno-format on new-enough gcc
by Eric Blake 02 Oct '12

02 Oct '12

Commit c579d6b added a sledgehammer to silence spurious warnings from gcc 4.2, but in the process, it also silenced useful warnings from newer gcc (such as 4.4). As a result, a bug slipped in to commit 0caccb58. Tested with FreeBSD (gcc 4.2.1), RHEL 6.3 (gcc 4.4), and F17 (gcc 4.7.2). * m4/virt-compile-warnings.m4 (-Wno-format): Probe for the actual spurious message, to once again allow gcc 4.4 to use -Wformat. --- m4/virt-compile-warnings.m4 | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index d1173eb..f1b8f39 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -60,6 +60,23 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare" + # gcc 4.2 treats attribute(format) as an implicit attribute(nonnull), + # which triggers spurious warnings for our usage + AC_CACHE_CHECK([whether gcc -Wformat allows NULL strings], + [lv_cv_gcc_wformat_null_works], [ + save_CFLAGS=$CFLAGS + CFLAGS='-Wunknown-pragmas -Werror -Wformat' + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ + #include <stddef.h> + static __attribute__ ((__format__ (__printf__, 1, 2))) int + foo (const char *fmt, ...) { return !fmt; } + ]], [[ + return foo(NULL); + ]])], + [lv_cv_gcc_wformat_null_works=yes], + [lv_cv_gcc_wformat_null_works=no]) + CFLAGS=$save_CFLAGS]) + # Gnulib uses '#pragma GCC diagnostic push' to silence some # warnings, but older gcc doesn't support this. AC_CACHE_CHECK([whether pragma GCC diagnostic push works], @@ -117,7 +134,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # ATTRIBUTE_FMT_PRINT, which causes -Wformat failure on our # intentional use of virReportError(code, NULL). gl_WARN_ADD([-Wno-format-nonliteral]) - if test $lv_cv_gcc_pragma_push_works = no; then + if test $lv_cv_gcc_wformat_null_works = no; then gl_WARN_ADD([-Wno-format]) fi -- 1.7.11.4

2 2

[libvirt] [PATCH] build: fix typo in debug message
by Eric Blake 02 Oct '12

02 Oct '12

Introduced in commit 0caccb58. CC libvirt_driver_qemu_impl_la-qemu_capabilities.lo ../../src/qemu/qemu_capabilities.c: In function 'qemuCapsInitQMP': ../../src/qemu/qemu_capabilities.c:2327:13: error: format '%d' expects argument of type 'int', but argument 8 has type 'const char *' [-Werror=format] * src/qemu/qemu_capabilities.c (qemuCapsInitQMP): Use correct format. --- Pushing under the build-breaker rule. src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 759f5bb..a5eb995 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2324,7 +2324,7 @@ cleanup: int rc; if ((rc = virPidFileReadPath(pidfile, &pid)) < 0) { - VIR_DEBUG("Failed to read pidfile %s: %d", + VIR_DEBUG("Failed to read pidfile %s: %s", pidfile, virStrerror(-rc, ebuf, sizeof(ebuf))); } else { VIR_DEBUG("Killing QMP caps process %lld", (long long) pid); -- 1.7.11.4

1 1

[libvirt] [PATCH] qemu: Kill processes used for QMP caps probing
by Jiri Denemark 02 Oct '12

02 Oct '12

Since libvirt switched to QMP capabilities probing recently, it starts QEMU process used for this probing with -daemonize, which means virCommandAbort can no longer reach these processes. As a result of that, restarting libvirtd will leave several new QEMU processes behind. Let's use QEMU's -pidfile and use it to kill the process when QMP caps probing is done. --- src/qemu/qemu_capabilities.c | 51 ++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 5 +++-- src/qemu/qemu_driver.c | 4 +++- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 20b350a..bfefa92 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -29,6 +29,8 @@ #include "virterror_internal.h" #include "util.h" #include "virfile.h" +#include "virpidfile.h" +#include "virprocess.h" #include "nodeinfo.h" #include "cpu/cpu.h" #include "domain_conf.h" @@ -214,6 +216,7 @@ struct _qemuCapsCache { virMutex lock; virHashTablePtr binaries; char *libDir; + char *runDir; }; @@ -2180,7 +2183,8 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) static int qemuCapsInitQMP(qemuCapsPtr caps, - const char *libDir) + const char *libDir, + const char *runDir) { int ret = -1; virCommandPtr cmd = NULL; @@ -2191,7 +2195,11 @@ qemuCapsInitQMP(qemuCapsPtr caps, virDomainChrSourceDef config; char *monarg = NULL; char *monpath = NULL; + char *pidfile = NULL; + /* the ".sock" sufix is important to avoid a possible clash with a qemu + * domain called "capabilities" + */ if (virAsprintf(&monpath, "%s/%s", libDir, "capabilities.monitor.sock") < 0) { virReportOOMError(); goto cleanup; @@ -2201,6 +2209,14 @@ qemuCapsInitQMP(qemuCapsPtr caps, goto cleanup; } + /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash + * with a qemu domain called "capabilities" + */ + if (virAsprintf(&pidfile, "%s/%s", runDir, "capabilities.pidfile") < 0) { + virReportOOMError(); + goto cleanup; + } + memset(&config, 0, sizeof(config)); config.type = VIR_DOMAIN_CHR_TYPE_UNIX; config.data.nix.path = monpath; @@ -2215,6 +2231,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, "-nographic", "-M", "none", "-qmp", monarg, + "-pidfile", pidfile, "-daemonize", NULL); virCommandAddEnvPassCommon(cmd); @@ -2300,12 +2317,32 @@ cleanup: virCommandFree(cmd); VIR_FREE(monarg); VIR_FREE(monpath); + + if (pidfile) { + char ebuf[1024]; + pid_t pid; + int rc; + + if ((rc = virPidFileReadPath(pidfile, &pid)) < 0) { + VIR_DEBUG("Failed to read pidfile %s: %d", + pidfile, virStrerror(-rc, ebuf, sizeof(ebuf))); + } else { + VIR_DEBUG("Killing QMP caps process %lld", (long long) pid); + if (virProcessKill(pid, SIGKILL) < 0) + VIR_DEBUG("Failed to kill process %lld: %s", + (long long) pid, + virStrerror(errno, ebuf, sizeof(ebuf))); + } + unlink(pidfile); + VIR_FREE(pidfile); + } return ret; } qemuCapsPtr qemuCapsNewForBinary(const char *binary, - const char *libDir) + const char *libDir, + const char *runDir) { qemuCapsPtr caps = qemuCapsNew(); struct stat sb; @@ -2333,7 +2370,7 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary, goto error; } - if ((rv = qemuCapsInitQMP(caps, libDir)) < 0) + if ((rv = qemuCapsInitQMP(caps, libDir, runDir)) < 0) goto error; if (!caps->usedQMP && @@ -2373,7 +2410,7 @@ qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) qemuCapsCachePtr -qemuCapsCacheNew(const char *libDir) +qemuCapsCacheNew(const char *libDir, const char *runDir) { qemuCapsCachePtr cache; @@ -2391,7 +2428,8 @@ qemuCapsCacheNew(const char *libDir) if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree))) goto error; - if (!(cache->libDir = strdup(libDir))) { + if (!(cache->libDir = strdup(libDir)) || + !(cache->runDir = strdup(runDir))) { virReportOOMError(); goto error; } @@ -2420,7 +2458,7 @@ qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary) if (!ret) { VIR_DEBUG("Creating capabilities for %s", binary); - ret = qemuCapsNewForBinary(binary, cache->libDir); + ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runDir); if (ret) { VIR_DEBUG("Caching capabilities %p for %s", ret, binary); @@ -2459,6 +2497,7 @@ qemuCapsCacheFree(qemuCapsCachePtr cache) return; VIR_FREE(cache->libDir); + VIR_FREE(cache->runDir); virHashFree(cache->binaries); virMutexDestroy(&cache->lock); VIR_FREE(cache); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 89f351c..5d343c1 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -163,7 +163,8 @@ typedef qemuCapsCache *qemuCapsCachePtr; qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary, - const char *libDir); + const char *libDir, + const char *runDir); int qemuCapsProbeQMP(qemuCapsPtr caps, qemuMonitorPtr mon); @@ -201,7 +202,7 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, bool qemuCapsIsValid(qemuCapsPtr caps); -qemuCapsCachePtr qemuCapsCacheNew(const char *libDir); +qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, const char *runDir); qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); void qemuCapsCacheFree(qemuCapsCachePtr cache); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0a0bb5..55d8027 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -767,7 +767,9 @@ qemudStartup(int privileged) { if (qemuSecurityInit(qemu_driver) < 0) goto error; - if ((qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir)) == NULL) + qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir, + qemu_driver->stateDir); + if (!qemu_driver->capsCache) goto error; if ((qemu_driver->caps = qemuCreateCapabilities(qemu_driver)) == NULL) -- 1.7.12

2 2

[libvirt] installing libvirt
by Felipe Oliveira Gutierrez 02 Oct '12

02 Oct '12

Hi everyone, I am new using libvirt, Do I have to install libvirt-0.10.2.tar.gz alone, or I need to install libvirt-java-0.4.9.tar.gz after to developing codes using java? I runned "# ant build" from libvirt-java-0.4.9, and got this: Buildfile: /libvirt-java-0.4.9/build.xml init: [copy] Copying 1 file to /libvirt-java-0.4.9 build: [javac] /libvirt-java-0.4.9/build.xml:42: warning: 'includeantruntime' was not set, defaulting to build.sysclasspath=last; set to false for repeatable builds [javac] /libvirt-java-0.4.9/build.xml:43: warning: 'includeantruntime' was not set, defaulting to build.sysclasspath=last; set to false for repeatable builds BUILD SUCCESSFUL On the README file he asks: 2. You must have the libvirt.jar file in your classpath. By default the installs it to /usr/share/java/0.4.9.jar But I don't find the libvirt.jar file to copy. Does anyone can help me please? Bets Regards, Felipe -- *-- Felipe Oliveira Gutierrez* *-- **lipe.82(a)gmail.com* <lipe.82(a)gmail.com> *-- https://sites.google.com/site/lipe82/Home/diaadia*

2 2

[libvirt] [PATCH] qemu: Use proper agent entering function when freezing filesystems
by Peter Krempa 02 Oct '12

02 Oct '12

When doing snapshots, the filesystem freeze function used the agent entering function that expects the qemud_driver unlocked. This might cause a deadlock of the qemu driver if the agent does not respond. The only call path of this function has the qemud_driver locked, so this patch changes the entering functions to those expecting the driver locked. --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0a0bb5..50f44ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10375,6 +10375,7 @@ qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return true; } +/* this function expects the driver lock to be held by the caller */ static int qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, virDomainObjPtr vm) { @@ -10393,9 +10394,9 @@ qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, return -1; } - qemuDomainObjEnterAgent(driver, vm); + qemuDomainObjEnterAgentWithDriver(driver, vm); freezed = qemuAgentFSFreeze(priv->agent); - qemuDomainObjExitAgent(driver, vm); + qemuDomainObjExitAgentWithDriver(driver, vm); return freezed; } -- 1.7.12

2 2

[libvirt] only 256 active networks
by Dan Kenigsberg 02 Oct '12

02 Oct '12

Hi List, Laine, Maybe you can help here. We define 256 bridged networks such as <network> <name>vdsm-test244</name> <uuid>c3acd4c3-37be-3f9d-ee6b-317513015f9e</uuid> <forward mode='bridge'/> <bridge name='test244' /> </network> and end up killing `virsh net-list` with error: Failed to list active networks error: too many remote undefineds: 257 > 256 error: Reconnected to the hypervisor What is this limitation? Why is it there? How can this be extended/avoided? Dan.

2 2

[libvirt] [PATCH] lxc: Correctly report active cgroups
by Michal Privoznik 02 Oct '12

02 Oct '12

There was an inverted return value in lxcCgroupControllerActive(). The function assumes cgroups are active and do couple of checks to prove that. If any of them fails, false is returned. Therefore, at the end, after all checks are done we must return true, not false. --- src/lxc/lxc_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index ae5163e..87305db 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1645,7 +1645,7 @@ static bool lxcCgroupControllerActive(virLXCDriverPtr driver, if (driver->cgroupControllers & (1 << controller)) return true; #endif - return false; + return true; } -- 1.7.8.6

2 2

[libvirt] [PATCH 1/2] build: avoid journald on rhel 5
by Eric Blake 02 Oct '12

02 Oct '12

Commit f6430390 broke builds on RHEL 5, where glibc (2.5) is too old to support mkostemp (2.7) or htole64 (2.9). While gnulib has mkostemp, it still lacks htole64; and it's not worth dragging in replacements on systems where journald is unlikely to exist in the first place, so we just use an extra configure-time check as our witness of whether to attempt compiling the code. * src/util/logging.c (virLogParseOutputs): Don't attempt to compile journald on older glibc. * configure.ac (AC_CHECK_FUNCS_ONCE): Check for htole64. --- Pushing under the build-breaker rule. configure.ac | 2 +- src/util/logging.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 13967e9..c791ed0 100644 --- a/configure.ac +++ b/configure.ac @@ -172,7 +172,7 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \ - getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \ + getpwuid_r getuid htole64 initgroups kill mmap newlocale posix_fallocate \ posix_memalign regexec sched_getaffinity]) dnl Availability of pthread functions (if missing, win32 threading is diff --git a/src/util/logging.c b/src/util/logging.c index 0ce18f1..7a73553 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -50,6 +50,12 @@ #include "virtime.h" #include "intprops.h" +/* Journald output is only supported on Linux new enough to expose + * htole64. */ +#if HAVE_SYSLOG_H && defined(__linux__) && HAVE_HTOLE64 +# define USE_JOURNALD 1 +#endif + #define VIR_FROM_THIS VIR_FROM_NONE VIR_ENUM_DECL(virLogSource) @@ -1029,7 +1035,7 @@ virLogAddOutputToSyslog(virLogPriority priority, } -# ifdef __linux__ +# if USE_JOURNALD # define IOVEC_SET_STRING(iov, str) \ do { \ struct iovec *_i = &(iov); \ @@ -1197,7 +1203,7 @@ static int virLogAddOutputToJournald(int priority) } return 0; } -# endif /* __linux__ */ +# endif /* USE_JOURNALD */ #endif /* HAVE_SYSLOG_H */ #define IS_SPACE(cur) \ @@ -1294,12 +1300,10 @@ virLogParseOutputs(const char *outputs) VIR_FREE(abspath); } else if (STREQLEN(cur, "journald", 8)) { cur += 8; -#if HAVE_SYSLOG_H -# ifdef __linux__ +#if USE_JOURNALD if (virLogAddOutputToJournald(prio) == 0) count++; -# endif /* __linux__ */ -#endif /* HAVE_SYSLOG_H */ +#endif /* USE_JOURNALD */ } else { goto cleanup; } -- 1.7.11.4

2 4

[libvirt] [PATCH] build: avoid infinite autogen loop
by Eric Blake 01 Oct '12

01 Oct '12

Several people have reported that if the .gnulib submodule is dirty, then 'make' will go into an infinite loop attempting to rerun bootstrap, because that never cleans up the dirty submodule. By default, we should halt and make the user investigate, but if the user doesn't know why or care that the submodule is dirty, I also added the ability to 'make CLEAN_SUBMODULE=1' to get things going again. Also, while testing this, I noticed that when a submodule update was needed, 'make' would first run autoreconf, then bootstrap (which reruns autoreconf); adding a strategic dependency allows for less work. * .gnulib: Update to latest, for maint.mk improvements. * cfg.mk (_autogen): Also hook maint.mk, to run before autoreconf. * autogen.sh (bootstrap): Refuse to run if gnulib is dirty, unless user requests discarding gnulib changes. --- * .gnulib 440a1db...b493832 (39): > sockets, sys_stat: restore AC_C_INLINE > localeconv tests: Avoid test failure on OpenIndiana. > havelib: Follow libtool developments. > ChangeLog: mention who reported the problem. > fstatat.c: fix a compile-impeding typo > extern-inline: provide a -Wundef safe config.h > hash-pjw: relax license to LGPLv2+ > autoupdate > maint.mk: fix strict vs. lazy variable issues with RELEASE > maint.mk: formatting changes > maint.mk: provide "make upload" to ease uploading > maint.mk: factor the validation of RELEASE_TYPE > maint.mk: silent rules > localename: port gl_locale_name_thread_unsafe to FreeBSD > binary-io, eealloc, mbfile, mbiter, mbutil, xsize: better 'inline' > pipe-filter-gi, pipe-filter-ii: better use of 'inline' > fdutimensat: omit unnecessary AC_C_INLINE > fchmodat, fchownat, fstatat: use extern-inline > acl, mbchar, priv-set: use extern-inline > sockets, sys_stat: remove AC_C_INLINE in MSVC-only cases > tls-tests: omit unnecessary 'inline' > utimens-tests: avoid unnecessary 'inline' > misc: don't limit commentary to inline functions > non-recursive-gnulib-prefix-hack: new module > ChangeLog: fix indentation > maint.mk: generalize _gl_tight_scope for non-recursive make > maint.mk: exempt trailing blanks found in "binary" files > maint.mk: sc_prohibit_path_max_allocation: don't FP for UNIX_PATH_MAX > maint.mk: teach sc_prohibit_magic_number_exit to accept 77 > maint.mk: relax sc_prohibit_strcmp, to avoid a false positive > localcharset: work around Mac OS X bug with UTF-8 and MB_CUR_MAX > doc: document sticky-EOF issue > poll: fix poll(0, NULL, msec) > poll: fix for systems that can't recv() on a non-socket > poll/select: document portability problems not fixed by Gnulib. > update from texinfo > Fix typo in previous patch: 1 -> 4. > fcntl-h: check for AIX 7.1 bug with O_NOFOLLOW and O_CREAT > net_if: give more details about the bug being fixed .gnulib | 2 +- autogen.sh | 11 +++++++++++ cfg.mk | 16 ++++++++-------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/.gnulib b/.gnulib index 440a1db..b493832 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 +Subproject commit b4938324b7113c9d73f8390d21f3ecbd842e74b9 diff --git a/autogen.sh b/autogen.sh index 72f1e7a..d183397 100755 --- a/autogen.sh +++ b/autogen.sh @@ -63,11 +63,22 @@ bootstrap_hash() if test -d .git; then curr_status=.git-module-status t=$(bootstrap_hash; git diff .gnulib) + case $t:${CLEAN_SUBMODULE+set} in + *:set) ;; + *-dirty*) + echo "error: gnulib submodule is dirty, please investigate" 2>&1 + echo "set env-var CLEAN_SUBMODULE to discard gnulib changes" 2>&1 + exit 1 ;; + esac if test "$t" = "$(cat $curr_status 2>/dev/null)" \ && test -f "po/Makevars"; then # good, it's up to date, all we need is autoreconf autoreconf -if else + if test ${CLEAN_SUBMODULE+set}; then + echo cleaning up submodules... + git submodule foreach 'git clean -dfqx && git reset --hard' + fi echo running bootstrap$no_git... ./bootstrap$no_git --bootstrap-sync && bootstrap_hash > $curr_status \ || { echo "Failed to bootstrap, please investigate."; exit 1; } diff --git a/cfg.mk b/cfg.mk index bbfd4a2..e1fbf4f 100644 --- a/cfg.mk +++ b/cfg.mk @@ -700,10 +700,17 @@ ifeq (0,$(MAKELEVEL)) _clean_requested = $(filter %clean,$(MAKECMDGOALS)) ifeq (1,$(_update_required)$(_clean_requested)) $(info INFO: gnulib update required; running ./autogen.sh first) -Makefile: _autogen +maint.mk Makefile: _autogen endif endif +# It is necessary to call autogen any time gnulib changes. Autogen +# reruns configure, then we regenerate all Makefiles at once. +.PHONY: _autogen +_autogen: + $(srcdir)/autogen.sh + ./config.status + # Give credit where due: # Ensure that each commit author email address (possibly mapped via # git log's .mailmap) appears in our AUTHORS file. @@ -718,13 +725,6 @@ sc_check_author_list: && echo '$(ME): committer(s) not listed in AUTHORS' >&2; \ test $$fail = 0 -# It is necessary to call autogen any time gnulib changes. Autogen -# reruns configure, then we regenerate all Makefiles at once. -.PHONY: _autogen -_autogen: - $(srcdir)/autogen.sh - ./config.status - # regenerate HACKING as part of the syntax-check syntax-check: $(top_srcdir)/HACKING -- 1.7.1

2 3

[libvirt] [PATCH v2] Add systemd journal support
by Daniel P. Berrange 01 Oct '12

01 Oct '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> Add support for logging to the systemd journal, using its simple client library. The benefit over syslog is that it accepts structured log data, so the journald can store individual items like code file/line/func separately from the string message. Tools which require structured log data can then query the journal to extract exactly what they desire without resorting to string parsing While systemd provides a simple client library for logging, it is more convenient for libvirt to directly write its own client code. This lets us build up the iovec's on the stack, avoiding the need to alloc memory when writing log messages. Changed in v2: - Add virFormatIntDecimal instead of using snprintf - Add comment about mkostemp - Fix declaration of linestr var to use size of linenr - Wrap in #ifdef __linux__ Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/libvirt_private.syms | 1 + src/util/logging.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/logging.h | 1 + src/util/util.c | 30 ++++++++ src/util/util.h | 5 +- 5 files changed, 220 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dab607a..eebc52a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1233,6 +1233,7 @@ virFileUnlock; virFileWaitForDevices; virFileWriteStr; virFindFileInPath; +virFormatIntDecimal; virGetGroupID; virGetGroupName; virGetHostname; diff --git a/src/util/logging.c b/src/util/logging.c index cdd94fd..9e44880 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -35,6 +35,10 @@ #if HAVE_SYSLOG_H # include <syslog.h> #endif +#include <sys/socket.h> +#if HAVE_SYS_UN_H +# include <sys/un.h> +#endif #include "virterror_internal.h" #include "logging.h" @@ -44,6 +48,7 @@ #include "threads.h" #include "virfile.h" #include "virtime.h" +#include "intprops.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -146,6 +151,8 @@ virLogOutputString(virLogDestination ldest) return "syslog"; case VIR_LOG_TO_FILE: return "file"; + case VIR_LOG_TO_JOURNALD: + return "journald"; } return "unknown"; } @@ -1020,6 +1027,177 @@ virLogAddOutputToSyslog(virLogPriority priority, } return 0; } + + +# ifdef __linux__ +# define IOVEC_SET_STRING(iov, str) \ + do { \ + struct iovec *_i = &(iov); \ + _i->iov_base = (char*)str; \ + _i->iov_len = strlen(str); \ + } while (0) + +# define IOVEC_SET_INT(iov, buf, val) \ + do { \ + struct iovec *_i = &(iov); \ + _i->iov_base = virFormatIntDecimal(buf, sizeof(buf), val); \ + _i->iov_len = strlen(buf); \ + } while (0) + +static int journalfd = -1; + +static void +virLogOutputToJournald(virLogSource source, + virLogPriority priority, + const char *filename, + int linenr, + const char *funcname, + const char *timestamp ATTRIBUTE_UNUSED, + unsigned int flags, + const char *rawstr, + const char *str ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + virCheckFlags(VIR_LOG_STACK_TRACE,); + int buffd = -1; + size_t niov = 0; + struct msghdr mh; + struct sockaddr_un sa; + union { + struct cmsghdr cmsghdr; + uint8_t buf[CMSG_SPACE(sizeof(int))]; + } control; + struct cmsghdr *cmsg; + /* We use /dev/shm instead of /tmp here, since we want this to + * be a tmpfs, and one that is available from early boot on + * and where unprivileged users can create files. */ + char path[] = "/dev/shm/journal.XXXXXX"; + char priostr[INT_BUFSIZE_BOUND(priority)]; + char linestr[INT_BUFSIZE_BOUND(linenr)]; + + /* First message takes upto 4 iovecs, and each + * other field needs 3, assuming they don't have + * newlines in them + */ +# define IOV_SIZE (4 + (5 * 3)) + struct iovec iov[IOV_SIZE]; + + if (strchr(rawstr, '\n')) { + uint64_t nstr; + /* If 'str' containes a newline, then we must + * encode the string length, since we can't + * rely on the newline for the field separator + */ + IOVEC_SET_STRING(iov[niov++], "MESSAGE\n"); + nstr = htole64(strlen(rawstr)); + iov[niov].iov_base = (char*)&nstr; + iov[niov].iov_len = sizeof(nstr); + niov++; + } else { + IOVEC_SET_STRING(iov[niov++], "MESSAGE="); + } + IOVEC_SET_STRING(iov[niov++], rawstr); + IOVEC_SET_STRING(iov[niov++], "\n"); + + IOVEC_SET_STRING(iov[niov++], "PRIORITY="); + IOVEC_SET_INT(iov[niov++], priostr, priority); + IOVEC_SET_STRING(iov[niov++], "\n"); + + IOVEC_SET_STRING(iov[niov++], "LIBVIRT_SOURCE="); + IOVEC_SET_STRING(iov[niov++], virLogSourceTypeToString(source)); + IOVEC_SET_STRING(iov[niov++], "\n"); + + IOVEC_SET_STRING(iov[niov++], "CODE_FILE="); + IOVEC_SET_STRING(iov[niov++], filename); + IOVEC_SET_STRING(iov[niov++], "\n"); + + IOVEC_SET_STRING(iov[niov++], "CODE_LINE="); + IOVEC_SET_INT(iov[niov++], linestr, linenr); + IOVEC_SET_STRING(iov[niov++], "\n"); + + IOVEC_SET_STRING(iov[niov++], "CODE_FUNC="); + IOVEC_SET_STRING(iov[niov++], funcname); + IOVEC_SET_STRING(iov[niov++], "\n"); + + memset(&sa, 0, sizeof(sa)); + sa.sun_family = AF_UNIX; + if (!virStrcpy(sa.sun_path, "/run/systemd/journal/socket", sizeof(sa.sun_path))) + return; + + memset(&mh, 0, sizeof(mh)); + mh.msg_name = &sa; + mh.msg_namelen = offsetof(struct sockaddr_un, sun_path) + strlen(sa.sun_path); + mh.msg_iov = iov; + mh.msg_iovlen = niov; + + if (sendmsg(journalfd, &mh, MSG_NOSIGNAL) >= 0) + return; + + if (errno != EMSGSIZE && errno != ENOBUFS) + return; + + /* Message was too large, so dump to temporary file + * and pass an FD to the journal + */ + + /* NB: mkostemp is not declared async signal safe by + * POSIX, but this is Linux only code and the GLibc + * impl is safe enough, only using open() and inline + * asm to read a timestamp (falling back to gettimeofday + * on some arches + */ + if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR)) < 0) + return; + + if (unlink(path) < 0) + goto cleanup; + + if (writev(buffd, iov, niov) < 0) + goto cleanup; + + mh.msg_iov = NULL; + mh.msg_iovlen = 0; + + memset(&control, 0, sizeof(control)); + mh.msg_control = &control; + mh.msg_controllen = sizeof(control); + + cmsg = CMSG_FIRSTHDR(&mh); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + memcpy(CMSG_DATA(cmsg), &buffd, sizeof(int)); + + mh.msg_controllen = cmsg->cmsg_len; + + sendmsg(journalfd, &mh, MSG_NOSIGNAL); + +cleanup: + VIR_LOG_CLOSE(buffd); +} + + +static void virLogCloseJournald(void *data ATTRIBUTE_UNUSED) +{ + VIR_LOG_CLOSE(journalfd); +} + + +static int virLogAddOutputToJournald(int priority) +{ + if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) + return -1; + if (virSetInherit(journalfd, false) < 0) { + VIR_LOG_CLOSE(journalfd); + return -1; + } + if (virLogDefineOutput(virLogOutputToJournald, virLogCloseJournald, NULL, + priority, VIR_LOG_TO_JOURNALD, NULL, 0) < 0) { + return -1; + } + return 0; +} +# endif /* __linux__ */ #endif /* HAVE_SYSLOG_H */ #define IS_SPACE(cur) \ @@ -1114,6 +1292,12 @@ virLogParseOutputs(const char *outputs) count++; VIR_FREE(name); VIR_FREE(abspath); + } else if (STREQLEN(cur, "journald", 8)) { + cur += 8; +#if HAVE_SYSLOG_H + if (virLogAddOutputToJournald(prio) == 0) + count++; +#endif /* HAVE_SYSLOG_H */ } else { goto cleanup; } diff --git a/src/util/logging.h b/src/util/logging.h index a3ea821..4fe0c8e 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -80,6 +80,7 @@ typedef enum { VIR_LOG_TO_STDERR = 1, VIR_LOG_TO_SYSLOG, VIR_LOG_TO_FILE, + VIR_LOG_TO_JOURNALD, } virLogDestination; typedef enum { diff --git a/src/util/util.c b/src/util/util.c index 28f9ae3..43fdaf1 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2124,6 +2124,36 @@ virDoubleToStr(char **strp, double number) return ret; } + +/** + * Format @val as a base-10 decimal number, in the + * buffer @buf of size @buflen. To allocate a suitable + * sized buffer, the INT_BUFLEN(int) macro should be + * used + * + * Returns pointer to start of the number in @buf + */ +char * +virFormatIntDecimal(char *buf, size_t buflen, int val) +{ + char *p = buf + buflen - 1; + *p = '\0'; + if (val >= 0) { + do { + *--p = '0' + (val % 10); + val /= 10; + } while (val != 0); + } else { + do { + *--p = '0' - (val % 10); + val /= 10; + } while (val != 0); + *--p = '-'; + } + return p; +} + + const char *virEnumToString(const char *const*types, unsigned int ntypes, int type) diff --git a/src/util/util.h b/src/util/util.h index 5ab36ed..4316ab1 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -210,7 +210,10 @@ char *virStrcpy(char *dest, const char *src, size_t destbytes) # define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest)) int virDoubleToStr(char **strp, double number) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +char *virFormatIntDecimal(char *buf, size_t buflen, int val) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virDiskNameToIndex(const char* str); char *virIndexToDiskName(int idx, const char *prefix); -- 1.7.11.4

2 2