- Devel - libvirt List Archives

[libvirt] [PATCH v2] virtlockd: make re-exec more robust
by Michael Chapman 22 Jan '14

22 Jan '14

- Use $XDG_RUNTIME_DIR for re-exec state file when running unprivileged. - argv[0] may not contain a full path to the binary, however it should contain something that can be looked up in the PATH. Use execvp() to do path lookup on re-exec. - As per list discussion [1], ignore --daemon on re-exec. [1] https://www.redhat.com/archives/libvir-list/2013-December/msg00514.html Signed-off-by: Michael Chapman <mike(a)very.puzzling.org> --- src/locking/lock_daemon.c | 128 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 94 insertions(+), 34 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index a6be43c..b405e3a 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -925,7 +925,41 @@ error: } -#define VIR_LOCK_DAEMON_RESTART_EXEC_FILE LOCALSTATEDIR "/run/virtlockd-restart-exec.json" +static int +virLockDaemonExecRestartStatePath(bool privileged, + char **state_file) +{ + if (privileged) { + if (VIR_STRDUP(*state_file, LOCALSTATEDIR "/run/virtlockd-restart-exec.json") < 0) + goto error; + } else { + char *rundir = NULL; + mode_t old_umask; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + + old_umask = umask(077); + if (virFileMakePath(rundir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(state_file, "%s/virtlockd-restart-exec.json", rundir) < 0) { + VIR_FREE(rundir); + goto error; + } + + VIR_FREE(rundir); + } + + return 0; + +error: + return -1; +} + static char * virLockDaemonGetExecRestartMagic(void) @@ -938,7 +972,10 @@ virLockDaemonGetExecRestartMagic(void) static int -virLockDaemonPostExecRestart(bool privileged) +virLockDaemonPostExecRestart(const char *state_file, + const char *pid_file, + int *pid_file_fd, + bool privileged) { const char *gotmagic; char *wantmagic = NULL; @@ -948,14 +985,14 @@ virLockDaemonPostExecRestart(bool privileged) VIR_DEBUG("Running post-restart exec"); - if (!virFileExists(VIR_LOCK_DAEMON_RESTART_EXEC_FILE)) { - VIR_DEBUG("No restart file %s present", - VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + if (!virFileExists(state_file)) { + VIR_DEBUG("No restart state file %s present", + state_file); ret = 0; goto cleanup; } - if (virFileReadAll(VIR_LOCK_DAEMON_RESTART_EXEC_FILE, + if (virFileReadAll(state_file, 1024 * 1024 * 10, /* 10 MB */ &state) < 0) goto cleanup; @@ -982,13 +1019,18 @@ virLockDaemonPostExecRestart(bool privileged) goto cleanup; } + /* Re-claim PID file now as we will not be daemonizing */ + if (pid_file && + (*pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) + goto cleanup; + if (!(lockDaemon = virLockDaemonNewPostExecRestart(object, privileged))) goto cleanup; ret = 1; cleanup: - unlink(VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + unlink(state_file); VIR_FREE(wantmagic); VIR_FREE(state); virJSONValueFree(object); @@ -997,7 +1039,8 @@ cleanup: static int -virLockDaemonPreExecRestart(virNetServerPtr srv, +virLockDaemonPreExecRestart(const char *state_file, + virNetServerPtr srv, char **argv) { virJSONValuePtr child; @@ -1065,15 +1108,15 @@ virLockDaemonPreExecRestart(virNetServerPtr srv, VIR_DEBUG("Saving state %s", state); - if (virFileWriteStr(VIR_LOCK_DAEMON_RESTART_EXEC_FILE, + if (virFileWriteStr(state_file, state, 0700) < 0) { virReportSystemError(errno, _("Unable to save state file %s"), - VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + state_file); goto cleanup; } - if (execv(argv[0], argv) < 0) { + if (execvp(argv[0], argv) < 0) { virReportSystemError(errno, "%s", _("Unable to restart self")); goto cleanup; @@ -1153,6 +1196,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; int pid_file_fd = -1; char *sock_file = NULL; + char *state_file = NULL; bool implicit_conf = false; mode_t old_umask; bool privileged = false; @@ -1276,21 +1320,13 @@ int main(int argc, char **argv) { VIR_DEBUG("Decided on socket paths '%s'", sock_file); - if (godaemon) { - char ebuf[1024]; - - if (chdir("/") < 0) { - VIR_ERROR(_("cannot change to root directory: %s"), - virStrerror(errno, ebuf, sizeof(ebuf))); - goto cleanup; - } - - if ((statuswrite = virLockDaemonForkIntoBackground(argv[0])) < 0) { - VIR_ERROR(_("Failed to fork as daemon: %s"), - virStrerror(errno, ebuf, sizeof(ebuf))); - goto cleanup; - } + if (virLockDaemonExecRestartStatePath(privileged, + &state_file) < 0) { + VIR_ERROR(_("Can't determine restart state file path")); + exit(EXIT_FAILURE); } + VIR_DEBUG("Decided on restart state file path '%s'", + state_file); /* Ensure the rundir exists (on tmpfs on some systems) */ if (privileged) { @@ -1317,20 +1353,41 @@ int main(int argc, char **argv) { } umask(old_umask); - /* If we have a pidfile set, claim it now, exiting if already taken */ - if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) { - ret = VIR_LOCK_DAEMON_ERR_PIDFILE; - goto cleanup; - } - - if ((rv = virLockDaemonPostExecRestart(privileged)) < 0) { + if ((rv = virLockDaemonPostExecRestart(state_file, + pid_file, + &pid_file_fd, + privileged)) < 0) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; } /* rv == 1, means we setup everything from saved state, - * so we only setup stuff from scratch if rv == 0 */ + * so only (possibly) daemonize and setup stuff from + * scratch if rv == 0 + */ if (rv == 0) { + if (godaemon) { + char ebuf[1024]; + + if (chdir("/") < 0) { + VIR_ERROR(_("cannot change to root directory: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto cleanup; + } + + if ((statuswrite = virLockDaemonForkIntoBackground(argv[0])) < 0) { + VIR_ERROR(_("Failed to fork as daemon: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto cleanup; + } + } + + /* If we have a pidfile set, claim it now, exiting if already taken */ + if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) { + ret = VIR_LOCK_DAEMON_ERR_PIDFILE; + goto cleanup; + } + if (!(lockDaemon = virLockDaemonNew(config, privileged))) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; @@ -1388,7 +1445,9 @@ int main(int argc, char **argv) { virNetServerRun(lockDaemon->srv); if (execRestart && - virLockDaemonPreExecRestart(lockDaemon->srv, argv) < 0) + virLockDaemonPreExecRestart(state_file, + lockDaemon->srv, + argv) < 0) ret = VIR_LOCK_DAEMON_ERR_REEXEC; else ret = 0; @@ -1410,6 +1469,7 @@ cleanup: virPidFileReleasePath(pid_file, pid_file_fd); VIR_FREE(pid_file); VIR_FREE(sock_file); + VIR_FREE(state_file); VIR_FREE(run_dir); return ret; -- 1.8.4.2

2 2

[libvirt] [PATCH] docs: refer to the correct event ID for DomainEventIOErrorReasonCallback
by Claudio Bley 22 Jan '14

22 Jan '14

s/_ID_IO_ERROR/_ID_IO_ERROR_REASON/ Signed-off-by: Claudio Bley <cbley(a)av-test.de> --- Pushed under the trivial rule. include/libvirt/libvirt.h.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a448411..47a896b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4664,7 +4664,7 @@ typedef void (*virConnectDomainEventIOErrorCallback)(virConnectPtr conn, * @opaque: application specified data * * The callback signature to use when registering for an event of type - * VIR_DOMAIN_EVENT_ID_IO_ERROR with virConnectDomainEventRegisterAny() + * VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON with virConnectDomainEventRegisterAny() * */ typedef void (*virConnectDomainEventIOErrorReasonCallback)(virConnectPtr conn, -- 1.7.9.5

1 0

[libvirt] [PATCH 0/2] Add a test for linuxNodeGetCPUStats
by Ján Tomko 22 Jan '14

22 Jan '14

Ján Tomko (2): Add test for linuxNodeGetCPUStats Simplify linuxNodeGetCPUStats src/libvirt_linux.syms | 1 + src/nodeinfo.c | 83 +++++++-------- tests/nodeinfodata/linux-cpustat-24cpu.out | 150 ++++++++++++++++++++++++++++ tests/nodeinfodata/linux-cpustat-24cpu.stat | 25 +++++ tests/nodeinfotest.c | 128 ++++++++++++++++++++++++ 5 files changed, 339 insertions(+), 48 deletions(-) create mode 100644 tests/nodeinfodata/linux-cpustat-24cpu.out create mode 100644 tests/nodeinfodata/linux-cpustat-24cpu.stat -- 1.8.3.2

1 3

[libvirt] [PATCH] qemu: snapshot: Forbid snapshots when backing is a scsi passthrough disk
by Peter Krempa 22 Jan '14

22 Jan '14

https://bugzilla.redhat.com/show_bug.cgi?id=1034993 SCSI passthrough disks (<disk .. device="lun">) can't be used as backing for snapshots. Currently with upstream qemu the vm crashes on such attempt. This patch adds a early check to catch an attempt to do such a snapshot and rejects it right away. qemu will fix the issue but this will let us control the error message. --- Notes: You need a domain with a similar disk config to test this patch: <disk type='block' device='lun'> <driver name='qemu' type='raw'/> <source dev='/dev/disk/by-path/ip-192.168.122.223:3260-iscsi-iqn.2011-03.org.example.istgt:test-lun-0'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='virtio-scsi'> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b101d77..bc29714 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12206,7 +12206,7 @@ endjob: } static int -qemuDomainSnapshotPrepareDiskExternalBacking(virDomainDiskDefPtr disk) +qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) { int actualType = qemuDiskGetActualType(disk); @@ -12250,6 +12250,23 @@ qemuDomainSnapshotPrepareDiskExternalBacking(virDomainDiskDefPtr disk) static int +qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk) +{ + int actualType = qemuDiskGetActualType(disk); + + if (actualType == VIR_DOMAIN_DISK_TYPE_BLOCK && + disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("external active snapshots are not supported on scsi " + "passthrough devices")); + return -1; + } + + return 0; +} + + +static int qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr disk) { int actualType = qemuSnapshotDiskGetActualType(disk); @@ -12315,12 +12332,15 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, if (qemuTranslateDiskSourcePool(conn, disk) < 0) return -1; - if (qemuDomainSnapshotPrepareDiskExternalBacking(disk) < 0) + if (qemuDomainSnapshotPrepareDiskExternalBackingInactive(disk) < 0) return -1; if (qemuDomainSnapshotPrepareDiskExternalOverlayInactive(snapdisk) < 0) return -1; } else { + if (qemuDomainSnapshotPrepareDiskExternalBackingActive(disk) < 0) + return -1; + if (qemuDomainSnapshotPrepareDiskExternalOverlayActive(snapdisk) < 0) return -1; } -- 1.8.5.3

2 2

[libvirt] [vmware] Fix VMware driver version checking on Linux
by ryan woodsmall 22 Jan '14

22 Jan '14

Recent changes to the VMware driver allow for VMware Fusion version checking on OS X. It looks like the version checking on Linux for VMware Workstation and Player could possibly be broken by the changes. The version check in vmware_conf.c bundled STDOUT and STDERR into a common buffer; when/if something is printed to STDERR, at least on Player, version checking fails. This example output causes the version check to fail on a RHEL6-based Linux install: ** [ryan@os-sl6-controller ~]$ vmplayer -v Gtk-Message: Failed to load module "canberra-gtk-module": libcanberra-gtk-module.so: cannot open shared object file: No such file or directory VMware Player 6.0.1 build-1379776 [ryan@os-sl6-controller ~]$ virsh -c vmwareplayer:///session error: failed to connect to the hypervisor error: internal error: failed to parse VMware Player version ** This patch bundles STDERR into the checked buffer only with the VMware Fusion driver, allowing the VMware Player version check to succeed on Linux even with the above warning. It looks like there's still an issue (see below) but virsh now starts at least. ** [ryan@os-sl6-controller ~]$ virsh -c vmwareplayer:///session Welcome to virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # version Compiled against library: libvirt 1.2.1 Using library: libvirt 1.2.1 Using API: VMware 1.2.1 Cannot extract running VMware hypervisor version ** This is virsh running against VMware Player 6.0.1 on Linux x86_64. I have not tested on OS X since I don't have a license for VMware Fusion. Thanks everyone! -r diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..502d5c9 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -293,7 +293,8 @@ vmwareExtractVersion(struct vmware_driver *driver) cmd = virCommandNewArgList(bin, "-v", NULL); virCommandSetOutputBuffer(cmd, &outbuf); - virCommandSetErrorBuffer(cmd, &outbuf); + if (driver->type == VMWARE_DRIVER_FUSION) + virCommandSetErrorBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) goto cleanup;

2 3

Re: [libvirt] [Users] libvirt migration port configuration and virPortAllocator
by Dan Kenigsberg 21 Jan '14

21 Jan '14

On Wed, Jan 08, 2014 at 01:26:23AM +0100, Gianluca Cecchi wrote: > Hello, > following the bugzilla here: > https://bugzilla.redhat.com/show_bug.cgi?id=1019053 > > we have > " > This is now fixed by v1.1.3-188-g0196845 and v1.1.3-189-ge3ef20d > " > > Does this mean that f19 that has > libvirt-1.0.5.8-1.fc19.x86_64 > is out? > Any time soon to update it? > I see at > http://libvirt.org/sources/ > files such as > libvirt-1.1.4-1.fc19.x86_64.rpm > since beginning of November... > any particular reason for no packages neither in updates-testing repo? > > Also, Fedora 20 has libvirt-1.1.3.2-1.fc20.x86_64.rpm > Did it receive the patch update? I see nothing particular in its changelog... I suspect that you'd see this bugfix only on libvirt>=1.2, but the best place for such questions is the libvirt mailing list (CCed). Unfortunately, that version is not even on virt-preview http://fedorapeople.org/groups/virt/virt-preview/fedora-19 (but it is on http://fedorapeople.org/groups/virt/virt-preview/fedora-20 !) Dan.

5 5

[libvirt] [[PATCH v2]] virsh nodecpustats returns incorrect stats of cpu on Linux when the number of online cpu exceed 9.
by mars＠linux.vnet.ibm.com 21 Jan '14

21 Jan '14

From: Bing Bu Cao <mars(a)linux.vnet.ibm.com> To retrieve node cpu statistics on Linux system, the linuxNodeGetCPUstats function simply uses STRPREFIX() to match the cpuid with the cpuid read from /proc/stat, it will cause obvious error. For example: 'virsh nodecpustats 1' will display stats of cpu1* if the latter is online and cpu1 is offline. This patch fixes this bug. Signed-off-by: Bing Bu Cao <mars(a)linux.vnet.ibm.com> --- src/nodeinfo.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 05bc038..cba2fc1 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -708,9 +708,9 @@ linuxNodeGetCPUStats(FILE *procstat, } if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) { - strcpy(cpu_header, "cpu"); + strcpy(cpu_header, "cpu "); } else { - snprintf(cpu_header, sizeof(cpu_header), "cpu%d", cpuNum); + snprintf(cpu_header, sizeof(cpu_header), "cpu%d ", cpuNum); } while (fgets(line, sizeof(line), procstat) != NULL) { -- 1.7.7.6

2 1

[libvirt] [PATCH] Honour prefix in wireshark install dir
by Daniel P. Berrange 21 Jan '14

21 Jan '14

Trying to run $ ./configure --prefix=$HOME/usr/libvirt-git $ make install results in libvirt trying to install in /usr/lib/wireshark/plugins/.... with predictable amounts of fail. The configure script should not be hardcoding /usr/lib by default but rather honour $libdir Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index b9375f1..3a70375 100644 --- a/configure.ac +++ b/configure.ac @@ -2566,7 +2566,7 @@ AC_ARG_WITH([ws-plugindir], if test "$with_wireshark_dissector" != "no" && test -z "$ws_plugindir"; then ws_version=`$WIRESHARK -v | head -1 | cut -f 2 -d' '` - ws_plugindir=`dirname $WIRESHARK`/../lib/wireshark/plugins/$ws_version + ws_plugindir="$libdir/wireshark/plugins/$ws_version" fi AC_SUBST([ws_plugindir]) -- 1.8.4.2

2 1

Re: [libvirt] [PATCH] Be more clever and verbose about localization-initialization.
by andreas.fuchs＠sit.fraunhofer.de 21 Jan '14

21 Jan '14

I should have been more specific. If e.g. an LC_MESSAGE is set that cannot be found it returns "initialization failed" and nothing more... See the bug report.Spent >1h this morning trying to get some demonstrator running... So I thought rather share it... ----- Reply message ----- Von: "Daniel P. Berrange" <berrange(a)redhat.com> An: "Fuchs, Andreas" <andreas.fuchs(a)sit.fraunhofer.de> Cc: "libvir-list(a)redhat.com" <libvir-list(a)redhat.com> Betreff: [libvirt] [PATCH] Be more clever and verbose about localization-initialization. Datum: Mo., Okt 7, 2013 17:52 On Mon, Oct 07, 2013 at 03:47:48PM +0000, Fuchs, Andreas wrote: > Currently libvirt fails with an uninformative error message > if no translations are found. I don't see any failure if I delete all the libvirt.mo files. What are you actually doing to trigger an error, and what is the exact error you get. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

5 9

[libvirt] RFC: Move tests into same directory as the sources
by Daniel P. Berrange 21 Jan '14

21 Jan '14

Currently we have a split with all the "source" code under various subdirectories of src/, while the test cases are (mostly) all under the tests/ directory. I think there could be some benefits to moving the test files to be alongside the source files being tested. eg instead of tests/virhashtest.c have src/util/virhashtest.c The tests/ directory is getting reasonably large, and is likely to grow even more as we push for greater unit tests. We could split it up into various sub-directories mirroring the split of dirs under src/, but it seems easier to put have them directory under src/ and remove the arbitrary split of source and tests. Having the test files alongside the source files makes it obvious to anyone looking in a directory that tests exist for a file. This might encourage people to think about adding tests when changing a source file. It also makes it more slightly obvious which files lack proper tests. In essence I'm suggesting that our code file naming should be thus virfoo.h - exports of module foo for general internal usage virfoo.c - implementation of module foo virfoopriv.h - exports of module foo for test suite usage only virfootest.c - implementation of tests for module foo virfoomock.c - implementation of LD_PRELOAD hack for module foo tests Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

3 3