- Devel - libvirt List Archives

[libvirt] PATCH: Fix non-raw storage volume cloning
by Daniel P. Berrange 17 Jul '09

17 Jul '09

The recent storage patches broke cloning of non-raw volumes, due to a small typo. THis was identified by the libvirt TCK storage tests! Daniel diff --git a/src/storage_backend.c b/src/storage_backend.c index 1664804..67815d7 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -567,7 +567,7 @@ virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, (inputvol->type == VIR_STORAGE_VOL_FILE && inputvol->target.format != VIR_STORAGE_VOL_FILE_RAW)) { - if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { + if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("creation of non-raw file images is " "not supported without qemu-img.")); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 2

[libvirt] [PATCH] build: do not emit a trailing blank line into VC'd file, NEWS
by Jim Meyering 16 Jul '09

16 Jul '09

Cole Robinson noticed that the generated NEWS file would fail the "make syntax-check" tests. This adjusts the rule that generates it not to emit any of the offending trailing blank lines. At the same time, it regenerates NEWS. >From 53af8d848d78bcc5144b2c06b2109bbdc65803cf Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 16 Jul 2009 22:46:43 +0200 Subject: [PATCH] build: do not emit a trailing blank line into VC'd file, NEWS * docs/Makefile.am ($(top_builddir)/NEWS): Adjust rule to filter out any trailing blank lines when generating this file. * NEWS: Regenerate. --- NEWS | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ docs/Makefile.am | 7 ++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 5733fa8..447d2b4 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,62 @@ http://libvirt.org/news.html +0.6.5: Jul 3 2009: + - New features: create storage columes on disk backend (Henrik Persson), + drop of capabilities based on libcap-ng when possible (Daniel + Berrange) + - Portability: fix build on non-Linux targets (Daniel Berrange) + - Documentation: typo and english fixes (Runa Bhattacharjee and + Garry Dolley), Docs on extending APIs (Dave Allan), cleanup + of debug and logging documentation (Amy Griffis), add + HACKING doc to the website (Daniel Berrange), + documentation for OpenNebula driver (Abel Miguez Rodriguez) + - Bug fixes: forbid autostart on transcient networks, + xen device removal crash (Daniel Berrange), re-detection of + transient VMs after libvirtd restart(Daniel Berrange), + bug in virFindFileInPath (Daniel Berrange), handle new + availheap sysctl in Xen (Daniel Berrange), allow USB hostdev + product 0 (Cole Robinson), cleanup when creating a storage pool + fails (Henrik Persson), domain id fix on redefinition in + test driver (Cole Robinson), fix raw storage allocation (Cole + Robinson), memory reporting for inactive qemu drivers (Cole + Robinson), segfault if storage pool has no type attribute (Cole + Robinson), OpenNebula compilation issues (Javier Fontan), + dominfo command without security driver (Daniel Berrange), + domain state problems after migration or destroy (Federico + Simoncelli), leak in node device parsing (Dave Allan), + storage pool definitions reading at startup (Cole Robinson), + bogus WWN in NPIV support (David Allan), avoid a segfault with + recent Xen (Sascha), cope with libnuma failures on weird + topologies (Dan Berrange), crash in QEMU driver with bad capabilities + data (Dan Berrange), trying to re-create a pool should not destroy + it (Dave Allan), endless loop in node device XML dump (Cole Robinson), + Re-label shared and readonly images (Dan Berrange) + - Improvements: create and destroy NPIV support (David Allan), + networking in UML driver (Daniel Berrange), HAL driver restart + thread safety (Daniel Berrange), capabilities and nodeinfo + APIs for LXC (Daniel Berrange), iNUMA API for VBox (Daniel Berrange), + dynamically search and use kvm-img qemu-img or qcow-create (Doug + Goldstein), fix qemu and kvm version parsing (Mark McLoughlin), + serial number for HAL storage (Dave Allan), improve error reporting + for virConnectOpen URIs (Daniel Berrange), include OS driver name + in device XML (Daniel Berrange), fix qemu command flags fetching + (Cole Robinson), check that qemu support -drive format= (Cole + Robinson), improve emulator detection (Cole Robinson), changes + to config parser to accomodate VMX syntax (Matthias Bolte), + update network schemas and driver for missing elements (Satoru SATOH), + avoid changing file context if not needed (Tim Waugh), + skip labelling if no src path (Cole Robinson), add arm emulation + if qemu-system-arm is present (C.J. Adams-Collier) + - Cleanups: daemon check logging env variables (Daniel Berrange), + User Mode Linux start and stop cleanups (Daniel Berrange), + share the NUMA api implementations (Daniel Berrange), storage + module dependancies (Dave Allan), refactor storage XML parsing + (Cole Robinson), big cleanup of logging code (Amy Griffis), + superfluous % on format (Matthias Bolte), cleanups and updates + on OpenNebula driver (Daniel Berrange and Abel Miguez Rodriguez) + + 0.6.4: May 29 2009: - New features: new API virStorageVolCreateXMLFrom (Cole Robinson), full VBox graphic capabilities (Pritesh Kothari), Interface config diff --git a/docs/Makefile.am b/docs/Makefile.am index 4bdb827..9ac2f6d 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -127,8 +127,11 @@ libvirt-api.xml libvirt-refs.xml: apibuild.py \ -(./apibuild.py) $(top_builddir)/NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in - -@(if [ -x $(XSLTPROC) ] ; then \ - $(XSLTPROC) --nonet $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in > $(top_builddir)/NEWS ; fi ); + -@(if [ -x $(XSLTPROC) ] ; then \ + $(XSLTPROC) --nonet $(top_srcdir)/docs/news.xsl \ + $(top_srcdir)/docs/news.html.in \ + | perl -0777 -pe 's/\n\n+$$/\n/' \ + > $@-t && mv $@-t $@ ; fi ); clean-local: rm -f *~ *.bak *.hierarchy *.signals *-unused.txt -- 1.6.4.rc0.127.g81400

2 2

[libvirt] [PATCH 0/2] updated/working patches for netcf backend and virsh commands for virInterface*
by Laine Stump 16 Jul '09

16 Jul '09

These are updated versions of patches I previously sent: 1 - add iface-* commands to virsh to expose virInterface* functions to the cli. 2 - add the netcf backend driver. These haven't been exhaustively tested, but appear to work in simple smoke tests. Note that they all depend on the submitted-but-not-yet-committed virConnectListDefinedInterfaces() patch.

2 4

[libvirt] [PATCH 0/4] Storage disk backend fixes
by Cole Robinson 16 Jul '09

16 Jul '09

The following series fixes some issues in the disk storage backend, most wrt the recently added ability to create disk volumes. Cole Robinson (4): storage: disk: Fix parthelper '-g' option handling. storage: disk: Fix segfault creating volume without target path storage: disk: Default to 'ext2' for new volumes. storage: disk: Use capacity, not allocation, when creating volume. src/parthelper.c | 4 ++-- src/storage_backend_disk.c | 33 +++++++++++++++++++-------------- src/storage_conf.c | 2 +- 3 files changed, 22 insertions(+), 17 deletions(-)

3 14

[libvirt] [PATCH 0/9] Storage cloning for LVM and Disk backends
by Cole Robinson 16 Jul '09

16 Jul '09

This following series implements cloning for LVM and disk backends. Most of the functionality is already here, it just needed some reorganization to be accessible for every backend. I verified the following scenarios produced a bootable image: - Clone within a disk pool - Clone within a logical pool - Clone a raw file to a disk pool - Clone a disk pool to a logical pool Cole Robinson (9): storage: Refactor FS backend 'create' function choosing. storage: Always assume we are tracking alloc progress in FS backend storage: Move most of the FS creation functions to common backend. storage: cleanup: do away with 'createFile' storage: Break out actual raw cloning to separate function. storage: Implement 'CreateBlockFrom' helper. storage: Don't try sparse detection if writing to block device. storage: Implement CreateVolFrom for logical and disk backend. storage: Fix deadlock when cloning across pools. src/storage_backend.c | 471 +++++++++++++++++++++++++++++++++++++++++ src/storage_backend.h | 15 ++ src/storage_backend_disk.c | 15 ++ src/storage_backend_fs.c | 378 ++------------------------------- src/storage_backend_logical.c | 17 ++ src/storage_driver.c | 6 +- 6 files changed, 540 insertions(+), 362 deletions(-)

3 21

[libvirt] [PATCH 0/5] Add virConnectListDefinedInterfaces/NumOfDefinedInterfaces
by Laine Stump 16 Jul '09

16 Jul '09

This patch adds everything but the backend implementation of two APIs that were requested during review of my patch to add virsh commands that expose the virInterface* API. These count and list interfaces on the host that are currently inactive (ie "down"). virConnectListInterfaces and virConnectNumOfInterfaces will list/count *only* those interfaces that are currently active. With these functions in place, I'll be able to add the "--inactive" and "--all" flags to iface-list, as suggested by danpb. Implementation of the backend of these two functions is waiting for two things: 1) supporting code in libnetcf, and 2) implementation of the rest of the backend driver functions for virInterface* (ready in the wings and waiting to go).

3 14

[libvirt] [PATCH] implement virsh dump for qemu guests
by Paolo Bonzini 16 Jul '09

16 Jul '09

This patch uses a "migrate"+"cont" combination to implement "virsh dump" for QEMU guests (BZ507551). The code is mostly based on qemudDomainSave, except that the XML prolog is not included as it is not needed to examine the dump with e.g. crash. 2009-07-08 Paolo Bonzini <bonzini(a)gnu.org> * qemu_driver.c (qemudDomainCoreDump): New. (qemuDriver): Add core dump function. --- src/qemu_driver.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 84 insertions(+), 1 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 95ea882..546a691 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2844,6 +2844,89 @@ cleanup: } +static int qemudDomainCoreDump(virDomainPtr dom, + const char *path, + int flags ATTRIBUTE_UNUSED) { + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + char *command = NULL; + char *info = NULL; + char *safe_path = NULL; + int ret = -1; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + /* Migrate to file */ + safe_path = qemudEscapeShellArg(path); + if (!safe_path) { + virReportOOMError(dom->conn); + goto cleanup; + } + if (virAsprintf(&command, "migrate \"exec:" + "dd of='%s' 2>/dev/null" + "\"", safe_path) == -1) { + virReportOOMError(dom->conn); + command = NULL; + goto cleanup; + } + + if (qemudMonitorCommand(vm, command, &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("migrate operation failed")); + goto cleanup; + } + + DEBUG ("%s: migrate reply: %s", vm->def->name, info); + + /* If the command isn't supported then qemu prints: + * unknown command: migrate" */ + if (strstr(info, "unknown command:")) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, + "%s", + _("'migrate' not supported by this qemu")); + goto cleanup; + } + + /* Migrate always stops the VM. However, since the monitor is always + attached to a pty for libvirt, it will support synchronous + operations so we get here just after the end of the migration. */ + if (vm->state == VIR_DOMAIN_RUNNING) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("continue operation failed")); + goto cleanup; + } + DEBUG("Reply %s", info); + VIR_FREE(info); + } + ret = 0; + +cleanup: + VIR_FREE(safe_path); + VIR_FREE(command); + VIR_FREE(info); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + + static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -5310,7 +5393,7 @@ static virDriver qemuDriver = { qemudDomainGetInfo, /* domainGetInfo */ qemudDomainSave, /* domainSave */ qemudDomainRestore, /* domainRestore */ - NULL, /* domainCoreDump */ + qemudDomainCoreDump, /* domainCoreDump */ qemudDomainSetVcpus, /* domainSetVcpus */ #if HAVE_SCHED_GETAFFINITY qemudDomainPinVcpu, /* domainPinVcpu */ -- 1.6.2.5

4 10

[libvirt] [PATCH] add cd and pwd commands to virsh
by Paolo Bonzini 16 Jul '09

16 Jul '09

This patch adds cd and pwd commands to virsh. These can be useful together with commands that refer to files in the local file systems, especially save and restore. I explicitly decided not to provide any other command, e.g. mkdir, to avoid going down a slippery slope (now you want mkdir, tomorrow ls and rm and so on...). If anything, it would be possible to add a generic shell command---but cd cannot be implemented that way, so pwd shall remain the sole "exception". I did not implement "cd -". Ok? 2009-07-02 Paolo Bonzini <pbonzini(a)redhat.com> * bootstrap: Add getcwd module. * docs/virsh.pod: Document cd and pwd commands. * src/virsh.c (info_cd, opts_cd, cmdCd, info_pwd, cmdPwd): New. (commands): Add cd and pwd commands. * virsh.1: Regenerate. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com> --- bootstrap | 1 + docs/virsh.pod | 12 ++++++++ src/virsh.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ virsh.1 | 12 ++++++++- 4 files changed, 101 insertions(+), 1 deletions(-) diff --git a/bootstrap b/bootstrap index baf52e7..b14d8cc 100755 --- a/bootstrap +++ b/bootstrap @@ -69,6 +69,7 @@ c-ctype close connect getaddrinfo +getcwd gethostname getpass gettext diff --git a/docs/virsh.pod b/docs/virsh.pod index 6434d78..9530ba6 100644 --- a/docs/virsh.pod +++ b/docs/virsh.pod @@ -82,6 +82,18 @@ Running hypervisor: Xen 3.0.0 =back +=item B<cd> I<directory> optional + +Will change current directory to I<directory>. The default directory +for the B<cd> command is the home directory or, if there is no I<HOME> +variable in the environment, the root directory. + +This command is only available in interactive mode. + +=item B<pwd> + +Will print the current directory. + =item B<connect> I<URI> optional I<--readonly> (Re)-Connect to the hypervisor. This is a build-in command after shell diff --git a/src/virsh.c b/src/virsh.c index 5623499..856e5a0 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -6047,6 +6047,81 @@ editReadBackFile (vshControl *ctl, const char *filename) } /* + * "cd" command + */ +static const vshCmdInfo info_cd[] = { + {"help", gettext_noop("change the current directory")}, + {"desc", gettext_noop("Change the current directory.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_cd[] = { + {"dir", VSH_OT_DATA, 0, gettext_noop("directory to switch to (default: home or else root)")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdCd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + const char *dir; + int found; + + if (!ctl->imode) { + vshError(ctl, FALSE, _("cd: command valid only in interactive mode")); + return -1; + } + + dir = vshCommandOptString(cmd, "dir", &found); + if (!found) + dir = getenv ("HOME"); + if (!dir) + dir = "/"; + + if (chdir (dir) == -1) { + vshError(ctl, FALSE, _("cd: %s: %s"), strerror (errno), dir); + return -1; + } + + return 0; +} + +/* + * "pwd" command + */ +static const vshCmdInfo info_pwd[] = { + {"help", gettext_noop("print the current directory")}, + {"desc", gettext_noop("Print the current directory.")}, + {NULL, NULL} +}; + +static int +cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + char *cwd; + size_t path_max; + int err = TRUE; + + path_max = (size_t) PATH_MAX + 2; + cwd = vshMalloc (ctl, path_max); + while (cwd) { + err = getcwd (cwd, path_max) == NULL; + if (!err || errno != ERANGE) + break; + + path_max *= 2; + cwd = vshRealloc (ctl, cwd, path_max); + } + + if (err) + vshError(ctl, FALSE, _("pwd: cannot get current directory: %s"), strerror (errno)); + else + vshPrint (ctl, _("%s\n"), cwd); + + free (cwd); + return !err; +} + +/* * "edit" command */ static const vshCmdInfo info_edit[] = { @@ -6209,6 +6284,7 @@ static const vshCmdDef commands[] = { {"attach-interface", cmdAttachInterface, opts_attach_interface, info_attach_interface}, {"autostart", cmdAutostart, opts_autostart, info_autostart}, {"capabilities", cmdCapabilities, NULL, info_capabilities}, + {"cd", cmdCd, opts_cd, info_cd}, {"connect", cmdConnect, opts_connect, info_connect}, {"console", cmdConsole, opts_console, info_console}, {"create", cmdCreate, opts_create, info_create}, @@ -6277,6 +6353,7 @@ static const vshCmdDef commands[] = { {"pool-undefine", cmdPoolUndefine, opts_pool_undefine, info_pool_undefine}, {"pool-uuid", cmdPoolUuid, opts_pool_uuid, info_pool_uuid}, + {"pwd", cmdPwd, NULL, info_pwd}, {"quit", cmdQuit, NULL, info_quit}, {"reboot", cmdReboot, opts_reboot, info_reboot}, {"restore", cmdRestore, opts_restore, info_restore}, diff --git a/virsh.1 b/virsh.1 index 45ea614..ca03cc1 100644 --- a/virsh.1 +++ b/virsh.1 @@ -132,7 +132,7 @@ .\" ======================================================================== .\" .IX Title "VIRSH 1" -.TH VIRSH 1 "2009-04-16" "libvirt-0.6.2" "Virtualization Support" +.TH VIRSH 1 "2009-07-02" "libvirt-0.6.4" "Virtualization Support" .\" For nroff, turn off justification. Always turn off hyphenation; it makes .\" way too many mistakes in technical documents. .if n .ad l @@ -214,6 +214,16 @@ Running hypervisor: Xen 3.0.0 .RE .RS 4 .RE +.IP "\fBcd\fR \fIdirectory\fR optional" 4 +.IX Item "cd directory optional" +Will change current directory to \fIdirectory\fR. The default directory +for the \fBcd\fR command is the home directory or, if there is no \fI\s-1HOME\s0\fR +variable in the environment, the root directory. +.Sp +This command is only available in interactive mode. +.IP "\fBpwd\fR" 4 +.IX Item "pwd" +Will print the current directory. .IP "\fBconnect\fR \fI\s-1URI\s0\fR optional \fI\-\-readonly\fR" 4 .IX Item "connect URI optional --readonly" (Re)\-Connect to the hypervisor. This is a build-in command after shell -- 1.6.2.5

3 7

[libvirt] consistency: push "update" hook vs. "make syntax-check"
by Jim Meyering 16 Jul '09

16 Jul '09

Currently the server side hook that runs "git diff --check" to prevent pushing a change that adds trailing blanks is more strict than our "make syntax-check" hook, since the former rejects any change that adds blank lines at the end of a file, while "make syntax-check" doesn't complain about that. The two should be consistent. One way is to make "make syntax-check" more strict. If we were to do that, we'd have to choose between cleaning existing files and exempting them from the new test. Cleaning is easy and doesn't impact tests at all, so I prefer it. Here's what would be involved: - remove 121 trailing newlines from 109 files by running this command: git ls-files -z | xargs -0 perl -pi -0777 -e 's/\n\n+$/\n/' Add a rule to cfg.mk so that "make syntax-check" warns about any new violations. It might run something like this: git ls-files -z \ | xargs -0 perl -ln -0777 -e '/\n(\n+)$/ and print "$ARGV: ".length $1' That command prints the name of each offending file with its trailing blank line count. While it takes well under a second on my system, (admittedly, with a hot cache), it's not well optimized, reading each file into memory and processing it. If it matters, we can come up with a more efficient (yet still portable) way to compare the last two bytes of each file to "\n\n". I went ahead and wrote a nearly-minimal script to do that. Rather than reading/processing all 27MB of sources, this reads just the last 2 bytes of each of the 1048 files, comparing those bytes to "\n\n" and printing the name when there's a match: git ls-files -z \ | xargs -0 perl -le ' foreach my $f (@ARGV) { open F,"<",$f or (warn "failed to open $f: $!\n"), next; my $p = sysseek(F, -2, 2); # seek failure probably means file has < 2 bytes; ignore my $two; defined $p and $p = sysread F,$two,2; close F; # ignore read failure $p && $two eq "\n\n" and (print $f),$fail=1; } END {exit defined $fail ? 1 : 0}' However, counting minor page faults, there's little difference (2193 before, 1976 after), but maximum memory consumption is probably way down. I didn't measure that. With a hot cache, the latter takes .02elapsed, and the former takes .09 seconds. I'm leaning towards the simplicity of the former, in spite of its cost. I'll bet someone can come up with a simple *and* efficient script (probably using sed) to list files with one or more trailing blank line.

3 9

[libvirt] [PATCH 0/4] Prepare remote driver client for data streams
by Daniel P. Berrange 16 Jul '09

16 Jul '09

This patch series does a small amount of refactoring to the remote driver client to aid in supporting data streams Daniel P. Berrange (4): Refactor incoming message handling to prepare for data stream support Refactor message sending to allow code reuse for data streams Simplify remote driver error reporting Rename a bunch of internal methods to clarify their meaning src/remote_internal.c | 366 ++++++++++++++++++++++++------------------------- 1 files changed, 181 insertions(+), 185 deletions(-) -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 8

[libvirt] PATCH: Allow QEMU VMs to be run unprivileged
by Daniel P. Berrange 16 Jul '09

16 Jul '09

This patch makes it such that the privileges libvirtd daemon can run unprivileged QEMU guests. The default remains unchanged with QEMU running as root:root, but the package maintainer can request an alternative default user at build time, and the sysadmin can also override this at install time with /etc/libvirt/qemu.conf. As well as making QEMU setuid/gid to the non-root user, this patch takes care of chown'ing all resources it needs to access. This currently includes - /dev/bus/usb/$BUS/$DEVICE for any assigned USB devices - /sys/bus/pci/$ADDR/{config,resource*,rom} for PCI devs - All disk paths Upon shutdown it will restore ownership to root for all of thesem, except shared/readonly disk images NB one minor problem is that USB devices attached based on vendor/product ID aren't handled. Need to figure out a way to deal with this.... Daniel commit 5283a949f4f5cc75e471ddd624362261280378d9 Author: Daniel P. Berrange <berrange(a)redhat.com> Date: Wed Jul 15 22:25:01 2009 +0100 Run QEMU guests as an unprivileged user * configure.in: Add --with-qemu-user and --with-qemu-group args * libvirt.spec.in: use 'qemu' for user/group for Fedora >= 12 * qemud/libvirtd_qemu.arg, qemud/test_libvirtd_qemu.aug, src/qemu.conf: Add 'user' and 'group' args for configuration * src/Makefile.am: Create %localstatedir/cache/libvirt/qemu * src/qemu_conf.c, src/qemu_conf.h: Load user/group from config * src/qemu_driver.c: Change user ID/group ID when launching QEMU guests. Change user/group ownership on disks/usb/pci devs. Put memory dumps in %localstatedir/cache/libvirt/qemu * src/util.c, src/util.h: Add convenient APIs for converting username/groupname to user ID / group ID diff --git a/configure.in b/configure.in index 552089a..634e812 100644 --- a/configure.in +++ b/configure.in @@ -1437,6 +1437,19 @@ AM_CONDITIONAL([WITH_NODE_DEVICES], [test "$with_nodedev" = "yes"]) AM_CONDITIONAL([WITH_LINUX], [test `uname -s` = "Linux"]) + +AC_ARG_WITH([qemu-user], + [ --with-qemu-user username to run QEMU system instance as], + [QEMU_USER=${withval}], + [QEMU_USER=root]) +AC_ARG_WITH([qemu-group], + [ --with-qemu-group groupname to run QEMU system instance as], + [QEMU_GROUP=${withval}], + [QEMU_GROUP=root]) +AC_DEFINE_UNQUOTED([QEMU_USER], ["$QEMU_USER"], [QEMU user account]) +AC_DEFINE_UNQUOTED([QEMU_GROUP], ["$QEMU_GROUP"], [QEMU group account]) + + # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. # Copy one to the other, but only if this is a srcdir-build. @@ -1578,3 +1591,7 @@ AC_MSG_NOTICE([ Debug: $enable_debug]) AC_MSG_NOTICE([ Warnings: $enable_compile_warnings]) AC_MSG_NOTICE([ Readline: $lv_use_readline]) AC_MSG_NOTICE([]) +AC_MSG_NOTICE([Privileges]) +AC_MSG_NOTICE([]) +AC_MSG_NOTICE([ QEMU: $QEMU_USER:$QEMU_GROUP]) +AC_MSG_NOTICE([]) \ No newline at end of file diff --git a/libvirt.spec.in b/libvirt.spec.in index 6321eaa..1bde6c5 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -46,6 +46,14 @@ %define with_capng 0%{!?_without_capng:1} %endif +%if 0%{?fedora} >= 12 +%define qemu_user qemu +%define qemu_group qemu +%else +%define qemu_user root +%define qemu_group root +%endif + # # If building on RHEL switch on the specific support # @@ -309,6 +317,8 @@ of recent versions of Linux (and other OSes). %{?_without_storage_iscsi} \ %{?_without_storage_disk} \ %{?_without_numactl} \ + --with-qemu-user=%{qemu_user} \ + --with-qemu-group=%{qemu_group} \ --with-init-script=redhat \ --with-qemud-pid-file=%{_localstatedir}/run/libvirt_qemud.pid \ --with-remote-file=%{_localstatedir}/run/libvirtd.pid @@ -446,8 +456,9 @@ fi %dir %attr(0700, root, root) %{_localstatedir}/cache/libvirt/ %if %{with_qemu} -%dir %{_localstatedir}/run/libvirt/qemu/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/qemu/ +%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/run/libvirt/qemu/ +%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ +%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %endif %if %{with_lxc} %dir %{_localstatedir}/run/libvirt/lxc/ diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug index 02699d6..abd6b62 100644 --- a/qemud/libvirtd_qemu.aug +++ b/qemud/libvirtd_qemu.aug @@ -29,6 +29,8 @@ module Libvirtd_qemu = | str_entry "vnc_password" | bool_entry "vnc_sasl" | str_entry "vnc_sasl_dir" + | str_entry "user" + | str_entry "group" (* Each enty in the config is one of the following three ... *) let entry = vnc_entry diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug index 6f38e47..f62da01 100644 --- a/qemud/test_libvirtd_qemu.aug +++ b/qemud/test_libvirtd_qemu.aug @@ -79,6 +79,10 @@ vnc_sasl = 1 # point to the directory, and create a qemu.conf in that location # vnc_sasl_dir = \"/some/directory/sasl2\" + +user = \"root\" + +group = \"root\" " test Libvirtd_qemu.lns get conf = @@ -161,3 +165,7 @@ vnc_sasl_dir = \"/some/directory/sasl2\" { "#comment" = "point to the directory, and create a qemu.conf in that location" } { "#comment" = "" } { "vnc_sasl_dir" = "/some/directory/sasl2" } +{ "#comment" = "" } +{ "user"= "root" } +{ "#comment" = "" } +{ "group" = "root" } \ No newline at end of file diff --git a/src/Makefile.am b/src/Makefile.am index 889ede4..9b662ae 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -684,6 +684,7 @@ install-exec-local: if WITH_QEMU $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" endif if WITH_LXC $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lxc" diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0534d53..59c78d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -378,6 +378,8 @@ virRun; virSkipSpaces; virKillProcess; virGetUserDirectory; +virGetUserID; +virGetGroupID; # uuid.h diff --git a/src/qemu.conf b/src/qemu.conf index c2a53c5..3009725 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -88,3 +88,10 @@ # to 'none' instead # # security_driver = "selinux" + + +# The user ID for QEMU processes run by the system instance +#user = "root" + +# The group ID for QEMU processes run by the system instance +#group = "root" diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 414b71b..0718f39 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -92,6 +92,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, const char *filename) { virConfPtr conf; virConfValuePtr p; + char *user; + char *group; /* Setup 2 critical defaults */ if (!(driver->vncListen = strdup("127.0.0.1"))) { @@ -186,6 +188,36 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "user"); + CHECK_TYPE ("user", VIR_CONF_STRING); + if (!(user = strdup(p && p->str ? p->str : QEMU_USER))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + if (virGetUserID(NULL, user, &driver->user) < 0) { + VIR_FREE(user); + virConfFree(conf); + return -1; + } + VIR_FREE(user); + + p = virConfGetValue (conf, "group"); + CHECK_TYPE ("group", VIR_CONF_STRING); + if (!(group = strdup(p && p->str ? p->str : QEMU_GROUP))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + + + if (virGetGroupID(NULL, group, &driver->group) < 0) { + VIR_FREE(group); + virConfFree(conf); + return -1; + } + VIR_FREE(group); + virConfFree (conf); return 0; } diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 175173d..fbf2ab9 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -66,6 +66,9 @@ struct qemud_driver { int privileged; + uid_t user; + gid_t group; + unsigned int qemuVersion; int nextvmid; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 342ba01..f2d021c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -71,12 +71,11 @@ #define VIR_FROM_THIS VIR_FROM_QEMU /* For storing short-lived temporary files. */ -#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt" +#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt/qemu" #define QEMU_CMD_PROMPT "\n(qemu) " #define QEMU_PASSWD_PROMPT "Password: " - static int qemudShutdown(void); static void qemuDriverLock(struct qemud_driver *driver) @@ -1367,6 +1366,205 @@ static int qemudDomainSetSecurityLabel(virConnectPtr conn, struct qemud_driver * return 0; } + +#ifdef __linux__ +static int qemuDomainSetHostdevUSBOwnership(virConnectPtr conn, + virDomainHostdevDefPtr def, + uid_t uid, gid_t gid) +{ + char *usbpath = NULL; + + /* XXX what todo for USB devs assigned based on product/vendor ? Doom :-( */ + if (!def->source.subsys.u.usb.bus || + !def->source.subsys.u.usb.device) + return 0; + + if (virAsprintf(&usbpath, "/dev/bus/usb/%03d/%03d", + def->source.subsys.u.usb.bus, + def->source.subsys.u.usb.device) < 0) { + virReportOOMError(conn); + return -1; + } + + VIR_DEBUG("Setting ownership on %s to %d:%d", usbpath, uid, gid); + if (chown(usbpath, uid, gid) < 0) { + virReportSystemError(conn, errno, _("cannot set ownership on %s"), usbpath); + VIR_FREE(usbpath); + return -1; + } + VIR_FREE(usbpath); + + return 0; +} + +static int qemuDomainSetHostdevPCIOwnership(virConnectPtr conn, + virDomainHostdevDefPtr def, + uid_t uid, gid_t gid) +{ + char *pcidir = NULL; + char *file = NULL; + DIR *dir = NULL; + int ret = -1; + struct dirent *ent; + + if (virAsprintf(&pcidir, "/sys/bus/pci/devices/%04x:%02x:%02x.%x", + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + if (!(dir = opendir(pcidir))) { + virReportSystemError(conn, errno, + _("cannot open %s"), pcidir); + goto cleanup; + } + + while ((ent = readdir(dir)) != NULL) { + /* QEMU device assignment requires: + * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, $PCIDIR/rom + */ + if (STREQ(ent->d_name, "config") || + STRPREFIX(ent->d_name, "resource") || + STREQ(ent->d_name, "rom")) { + if (virAsprintf(&file, "%s/%s", pcidir, ent->d_name) < 0) { + virReportOOMError(conn); + goto cleanup; + } + VIR_DEBUG("Setting ownership on %s to %d:%d", file, uid, gid); + if (chown(file, uid, gid) < 0) { + virReportSystemError(conn, errno, _("cannot set ownership on %s"), file); + goto cleanup; + } + VIR_FREE(file); + } + } + + ret = 0; + +cleanup: + if (dir) + closedir(dir); + VIR_FREE(file); + VIR_FREE(pcidir); + return ret; +} +#endif + + +static int qemuDomainSetHostdevOwnership(virConnectPtr conn, + virDomainHostdevDefPtr def, + uid_t uid, gid_t gid) +{ + if (def->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + +#ifdef __linux__ + switch (def->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + return qemuDomainSetHostdevUSBOwnership(conn, def, uid, gid); + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + return qemuDomainSetHostdevPCIOwnership(conn, def, uid, gid); + + } + return 0; +#else + qemudReportError(conn, NULL, NULL, "%s", + _("unable to set host device ownership on this platform")); + return -1; +#endif + +} + +static int qemuDomainSetDiskOwnership(virConnectPtr conn, + virDomainDiskDefPtr def, + uid_t uid, gid_t gid) +{ + + if (!def->src) + return 0; + + VIR_DEBUG("Setting ownership on %s to %d:%d", def->src, uid, gid); + if (chown(def->src, uid, gid) < 0) { + virReportSystemError(conn, errno, _("cannot set ownership on %s"), + def->src); + return -1; + } + return 0; +} + +static int qemuDomainSetDeviceOwnership(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDeviceDefPtr def, + int restore) +{ + uid_t uid; + gid_t gid; + + if (!driver->privileged) + return 0; + + /* short circuit case of root:root */ + if (!driver->user && !driver->group) + return 0; + + uid = restore ? 0 : driver->user; + gid = restore ? 0 : driver->group; + + switch (def->type) { + case VIR_DOMAIN_DEVICE_DISK: + if (restore && + (def->data.disk->readonly || def->data.disk->shared)) + return 0; + + return qemuDomainSetDiskOwnership(conn, def->data.disk, uid, gid); + + case VIR_DOMAIN_DEVICE_HOSTDEV: + return qemuDomainSetHostdevOwnership(conn, def->data.hostdev, uid, gid); + } + + return 0; +} + +static int qemuDomainSetAllDeviceOwnership(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + int restore) +{ + int i; + uid_t uid; + gid_t gid; + + if (!driver->privileged) + return 0; + + /* short circuit case of root:root */ + if (!driver->user && !driver->group) + return 0; + + uid = restore ? 0 : driver->user; + gid = restore ? 0 : driver->group; + + for (i = 0 ; i < def->ndisks ; i++) { + if (restore && + (def->disks[i]->readonly || def->disks[i]->shared)) + continue; + + if (qemuDomainSetDiskOwnership(conn, def->disks[i], uid, gid) < 0) + return -1; + } + + for (i = 0 ; i < def->nhostdevs ; i++) { + if (qemuDomainSetHostdevOwnership(conn, def->hostdevs[i], uid, gid) < 0) + return -1; + } + + return 0; +} + static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, const char *name); @@ -1377,14 +1575,39 @@ struct gemudHookData { }; static int qemudSecurityHook(void *data) { - struct gemudHookData *h = (struct gemudHookData *) data; + struct gemudHookData *h = (struct gemudHookData *) data; + + if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) { + qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to set security label")); + return -1; + } + + if (h->driver->privileged) { + DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group); + + if (qemuDomainSetAllDeviceOwnership(h->conn, h->driver, h->vm->def, 0) < 0) + return -1; - if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) { - qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Failed to set security label")); + if (h->driver->group) { + if (setregid(h->driver->group, h->driver->group) < 0) { + virReportSystemError(NULL, errno, + _("cannot change to '%d' group"), + h->driver->group); return -1; + } } - return 0; + if (h->driver->user) { + if (setreuid(h->driver->user, h->driver->user) < 0) { + virReportSystemError(NULL, errno, + _("cannot change to '%d' user"), + h->driver->user); + return -1; + } + } + } + + return 0; } static int @@ -1662,6 +1885,10 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_FREE(vm->def->seclabel.imagelabel); } + if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0) + VIR_WARN("Failed to restore all device ownership for %s", + vm->def->name); + if (qemudRemoveDomainStatus(conn, driver, vm) < 0) { VIR_WARN(_("Failed to remove domain status for %s"), vm->def->name); @@ -4227,12 +4454,20 @@ static int qemudDomainAttachDevice(virDomainPtr dom, case VIR_DOMAIN_DISK_DEVICE_FLOPPY: if (driver->securityDriver) driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); + + if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0) + goto cleanup; + ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev); break; case VIR_DOMAIN_DISK_DEVICE_DISK: if (driver->securityDriver) driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); + + if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0) + goto cleanup; + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev); } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || @@ -4255,6 +4490,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom, } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0) + goto cleanup; + ret = qemudDomainAttachHostDevice(dom->conn, vm, dev); } else { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, @@ -4267,8 +4505,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom, ret = -1; cleanup: - if (ret < 0) + if (ret < 0) { + if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) + VIR_WARN0("Fail to restore disk device ownership"); virDomainDeviceDefFree(dev); + } if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -4396,6 +4637,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev); if (driver->securityDriver) driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk); + if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) + VIR_WARN0("Fail to restore disk device ownership"); } else qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, diff --git a/src/util.c b/src/util.c index 178ff0c..2420f8b 100644 --- a/src/util.c +++ b/src/util.c @@ -55,6 +55,7 @@ #include <netdb.h> #ifdef HAVE_GETPWUID_R #include <pwd.h> +#include <grp.h> #endif #if HAVE_CAPNG #include <cap-ng.h> @@ -1862,4 +1863,78 @@ char *virGetUserDirectory(virConnectPtr conn, return ret; } + + +int virGetUserID(virConnectPtr conn, + const char *name, + uid_t *uid) +{ + char *strbuf; + struct passwd pwbuf; + struct passwd *pw = NULL; + size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); + + if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { + virReportOOMError(conn); + return -1; + } + + /* + * From the manpage (terrifying but true): + * + * ERRORS + * 0 or ENOENT or ESRCH or EBADF or EPERM or ... + * The given name or uid was not found. + */ + if (getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) { + virReportSystemError(conn, errno, + _("Failed to find user record for name '%s'"), + name); + VIR_FREE(strbuf); + return -1; + } + + *uid = pw->pw_uid; + + VIR_FREE(strbuf); + + return 0; +} + + +int virGetGroupID(virConnectPtr conn, + const char *name, + gid_t *gid) +{ + char *strbuf; + struct group grbuf; + struct group *gr = NULL; + size_t strbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); + + if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { + virReportOOMError(conn); + return -1; + } + + /* + * From the manpage (terrifying but true): + * + * ERRORS + * 0 or ENOENT or ESRCH or EBADF or EPERM or ... + * The given name or uid was not found. + */ + if (getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr) != 0 || gr == NULL) { + virReportSystemError(conn, errno, + _("Failed to find group record for name '%s'"), + name); + VIR_FREE(strbuf); + return -1; + } + + *gid = gr->gr_gid; + + VIR_FREE(strbuf); + + return 0; +} #endif diff --git a/src/util.h b/src/util.h index 6dd005f..1a7286c 100644 --- a/src/util.h +++ b/src/util.h @@ -217,6 +217,12 @@ int virKillProcess(pid_t pid, int sig); #ifdef HAVE_GETPWUID_R char *virGetUserDirectory(virConnectPtr conn, uid_t uid); +int virGetUserID(virConnectPtr conn, + const char *name, + uid_t *uid); +int virGetGroupID(virConnectPtr conn, + const char *name, + gid_t *gid); #endif int virRandomInitialize(unsigned int seed); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 2

[libvirt] [PATCH 0/9] Refactor remote protocol ready for data streams
by Daniel P. Berrange 16 Jul '09

16 Jul '09

The current libvirtd remote protocol dispatch code is written in such a way that assumes the only incoming messages from clients are method calls. This makes it very hard to support data streams. This patch series does an incrmental refactoring of alot of code to allow data streams to be easily wired in. Daniel P. Berrange (9): Split generic RPC message dispatch code out from remote protocol API handlers Decode incoming request header before invoking dispatch code Separate code for encoding outgoing remote message headers Change code generator to give async event messages their own postfix Move queuing of RPC replies into dispatch code Change the way client event loop watches are managed Split out code for handling incoming method call messages Define an API for registering incoming message dispatch filters Rename 'direction' to 'type' in remote_message_header qemud/Makefile.am | 3 +- qemud/dispatch.c | 533 ++++++++++++++++++++++++++++++++++++ qemud/dispatch.h | 63 +++++ qemud/qemud.c | 136 ++++++---- qemud/qemud.h | 35 ++- qemud/remote.c | 442 ++++-------------------------- qemud/remote.h | 72 +++++ qemud/remote_dispatch_prototypes.h | 7 - qemud/remote_dispatch_ret.h | 1 - qemud/remote_dispatch_table.h | 6 +- qemud/remote_generate_stubs.pl | 23 ++- qemud/remote_protocol.c | 6 +- qemud/remote_protocol.h | 18 +- qemud/remote_protocol.x | 66 ++++-- src/remote_internal.c | 18 +- 15 files changed, 926 insertions(+), 503 deletions(-) create mode 100644 qemud/dispatch.c create mode 100644 qemud/dispatch.h create mode 100644 qemud/remote.h -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 20

[libvirt] PATCH: Fix free of uninitialized data on PCI device failure
by Daniel P. Berrange 16 Jul '09

16 Jul '09

The pci.c file was freeing a variable that was not initialized in its failure path. This causes a crash if the user specifies a PCI device for a guest which does not exist commit 87f584fdd688865b4ea7f54d1b631f546535d8c2 Author: Daniel P. Berrange <berrange(a)redhat.com> Date: Thu Jul 16 13:23:32 2009 +0100 Fix free of unitialized data upon PCI open fail diff --git a/src/pci.c b/src/pci.c index 3ffa0aa..4030a14 100644 --- a/src/pci.c +++ b/src/pci.c @@ -834,10 +834,8 @@ pciReadDeviceID(pciDevice *dev, const char *id_name) dev->name, id_name); /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ - if (virFileReadAll(path, 7, &id_str) < 7) { - VIR_FREE(id_str); + if (virFileReadAll(path, 7, &id_str) < 0) return NULL; - } /* Check for 0x suffix */ if (id_str[0] != '0' || id_str[1] != 'x') { -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] Can not get domain's cpu time
by Zhang Qian 16 Jul '09

16 Jul '09

Hi, I have a domain running in my KVM box, and try to get its vcpu info by calling virDomainGetVcpus(), but it seems the cpu time returned to me is always 0. And I also found virsh can not get the CPU time too: $ virsh vcpuinfo aaa VCPU: 0 CPU: 0 State: running CPU Affinity: yy I tried the same virsh command in my Xen box for a running domain, the output is: $ virsh vcpuinfo test1 VCPU: 0 CPU: 1 State: running CPU time: 322.1s <----- I need this CPU Affinity: yy As you see, for KVM domain, there is no "CPU time". But it's very strange that virt-manager can show the right CPU usage for my running domain, I do not know where virt-manger gets it. Can anyone tell me which libvirt API should I call to get the CPU time? Thanks! Regards, Qian

3 3

[libvirt] PATCH: Handle changed monitor syntax for PCI hotplug/unplug
by Daniel P. Berrange 16 Jul '09

16 Jul '09

Annoyingly when PCI hotplug support was merged from KVM into QEMU the monitor syntax changed :-( This patch tries to deal with this by detecting the error message, and then re-trying with the alternative syntax. It also ensures the monitor error mesages is include with any libvirt error raised, so that we can more easily detect problems in the future FYI, I have a test case for this now in the TCK http://libvirt.org/git/?p=libvirt-tck.git;a=commit;h=0ea2a3f3c426b325f80981… Regards, Daniel >From fca1582a7406d9eef7ff1a5e108986df76aeb6a1 Mon Sep 17 00:00:00 2001 From: Daniel P. Berrange <berrange(a)redhat.com> Date: Mon, 6 Jul 2009 15:58:55 +0100 Subject: [PATCH 2/3] Fix PCI device hotplug/unplug with newer QEMU * src/qemu_driver.c: If hotplug/unplug fails, try with alternative monitor command syntax --- src/qemu_driver.c | 56 +++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index fdbbb56..25d446d 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3915,6 +3915,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, char *cmd, *reply, *s; char *safe_path; const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus); + int tryAltSyntax = 0; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { @@ -3929,14 +3930,15 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, return -1; } +try_command: safe_path = qemudEscapeMonitorArg(dev->data.disk->src); if (!safe_path) { virReportOOMError(conn); return -1; } - ret = virAsprintf(&cmd, "pci_add 0 storage file=%s,if=%s", - safe_path, type); + ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s", + (tryAltSyntax ? "pci_addr=auto" : "0"), safe_path, type); VIR_FREE(safe_path); if (ret == -1) { virReportOOMError(conn); @@ -3952,17 +3954,27 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, DEBUG ("%s: pci_add reply: %s", vm->def->name, reply); /* If the command succeeds qemu prints: - * OK bus 0... */ -#define PCI_ATTACH_OK_MSG "OK bus 0, slot " - if ((s=strstr(reply, PCI_ATTACH_OK_MSG))) { - char* dummy = s; - s += strlen(PCI_ATTACH_OK_MSG); + * OK bus 0, slot XXX... + * or + * OK domain 0, bus 0, slot XXX + */ + if ((s = strstr(reply, "OK ")) && + (s = strstr(s, "slot "))) { + char *dummy = s; + s += strlen("slot "); if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1) VIR_WARN("%s", _("Unable to parse slot number\n")); + /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */ + /* XXX this slotnum is not persistant across restarts :-( */ + } else if (!tryAltSyntax && strstr(reply, "Invalid pci address")) { + VIR_FREE(reply); + VIR_FREE(cmd); + tryAltSyntax = 1; + goto try_command; } else { qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - _("adding %s disk failed"), type); + _("adding %s disk failed: %s"), type, reply); VIR_FREE(reply); VIR_FREE(cmd); return -1; @@ -4179,6 +4191,7 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, char *cmd = NULL; char *reply = NULL; virDomainDiskDefPtr detach = NULL; + int tryAltSyntax = 0; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { @@ -4200,9 +4213,17 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, goto cleanup; } - if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { - virReportOOMError(conn); - goto cleanup; +try_command: + if (tryAltSyntax) { + if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) { + virReportOOMError(conn); + goto cleanup; + } + } else { + if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { + virReportOOMError(conn); + goto cleanup; + } } if (qemudMonitorCommand(vm, cmd, &reply) < 0) { @@ -4212,12 +4233,19 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, } DEBUG ("%s: pci_del reply: %s",vm->def->name, reply); + + if (!tryAltSyntax && + strstr(reply, "extraneous characters")) { + tryAltSyntax = 1; + goto try_command; + } /* If the command fails due to a wrong slot qemu prints: invalid slot, * nothing is printed on success */ - if (strstr(reply, "invalid slot")) { + if (strstr(reply, "invalid slot") || + strstr(reply, "Invalid pci address")) { qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("failed to detach disk %s: invalid slot %d"), - detach->dst, detach->slotnum); + _("failed to detach disk %s: invalid slot %d: %s"), + detach->dst, detach->slotnum, reply); goto cleanup; } -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 3

[libvirt] [PATCH] Fix the configure flags in spec file
by Daniel Veillard 16 Jul '09

16 Jul '09

Our libvirt spec file was still using old configuration options which had been deprecated. Fix that to use the new option, good thing the spec value matches the default ! Thanks for Chris Lalancette for raising this ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 1

[libvirt] Problem with libvirt and pci passthrough
by Mirko Raasch 16 Jul '09

16 Jul '09

Hi, i?m still having problem with libvirt and pci passthrough, if i use more then one pci card in the guest. If i start qemu 0.10.5 manualy, i can use all of the pci devices in the guest, here is the log: http://nopaste.info/610fa81fb5.html If i use libvirt to start the guest and passthrough three pci device, none of the device is usable in the guest: http://nopaste.info/ccfbfcf9a0.html Is there any limitation in libvirt? Greetings Mirko

1 0

[libvirt] PATCH: Relabel disk images *before* hotplugging
by Daniel P. Berrange 16 Jul '09

16 Jul '09

For some bizarre reason the original code we added for sVirt disk labelling with hotplug did not relabel the disks until after telling QEMU to hotplug. Clearly this is going to fail 99% of the time. It also forgot todo labelling when invoking a CDROM media change src/qemu_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) Daniel diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 25d446d..59c163c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4136,10 +4136,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + if (driver->securityDriver) + driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev); break; case VIR_DOMAIN_DISK_DEVICE_DISK: + if (driver->securityDriver) + driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev); } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || @@ -4151,8 +4155,6 @@ static int qemudDomainAttachDevice(virDomainPtr dom, virDomainDiskBusTypeToString(dev->data.disk->bus)); goto cleanup; } - if (driver->securityDriver) - driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); break; default: -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH Fix QEMU monitor prompt confusion
by Daniel P. Berrange 16 Jul '09

16 Jul '09

When you first connect to the QEMU monitor after a VM starts, it will print out QEMU 0.10.5 monitor - type 'help' for more information We already have code to detect & discard this. Unfortunately it seems that with QEMU >= 0.10.0 it will also print out this prompt when you disconnect and reconnect to the QEMU monitor. Except it doesn't print this out every time. It only appears to happen 50% of the time when libvirtd restarts and reconnects. When it does happen though it totally breaks all future monitor commands libvirt tries to run. This patch takes a fairly simple approach to solving it. Before running any QEMU monitor command, read and discard all pending data. 99% of the time there will be none, but if there is, this saves us from disaster. Daniel diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 95ea882..fdbbb56 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1656,6 +1656,28 @@ cleanup: qemuDriverUnlock(driver); } + +/* Throw away any data available on the monitor + * This is done before executing a command, in order + * to allow re-synchronization if something went badly + * wrong in the past. it also deals with problem of + * QEMU *sometimes* re-printing its initial greeting + * when we reconnect to the monitor after restarts. + */ +static void +qemuMonitorDiscardPendingData(virDomainObjPtr vm) { + char buf[1024]; + int ret = 0; + + /* Monitor is non-blocking, so just loop till we + * get -1 or 0. Don't bother with detecting + * errors, since we'll deal with that better later */ + do { + ret = read(vm->monitor, buf, sizeof (buf)-1); + } while (ret > 0); +} + + static int qemudMonitorCommandExtra(const virDomainObjPtr vm, const char *cmd, @@ -1667,6 +1689,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm, size_t cmdlen = strlen(cmd); size_t extralen = extra ? strlen(extra) : 0; + qemuMonitorDiscardPendingData(vm); + if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen) return -1; if (safewrite(vm->monitor, "\r", 1) != 1) -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Set stdout/err to /dev/null for spawned children
by Daniel P. Berrange 16 Jul '09

16 Jul '09

If passing a 'NULL' to virExec() for the stdout/err file descriptor it is intended that the child process get connected to /dev/null. This behaviour was previously broken when built with DEBUG enabled, and with Amy's recent logging change, it is now permanently broken. The problem this causes, is that the child process ends up still connected to the parent app's stderr/out. This causes things like Perl's test harness to hang waiting for end of file that will never come while libvirtd is running. THis patch puts virExec back to its original documented behaviour Daniel src/util.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/util.c b/src/util.c index f82cddc..178ff0c 100644 --- a/src/util.c +++ b/src/util.c @@ -376,6 +376,8 @@ __virExec(virConnectPtr conn, } else { childout = *outfd; } + } else { + childout = null; } if (errfd != NULL) { @@ -403,6 +405,8 @@ __virExec(virConnectPtr conn, } else { childerr = *errfd; } + } else { + childerr = null; } if ((pid = fork()) < 0) { -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Allow libvirtd autostart to be disabled
by Daniel P. Berrange 16 Jul '09

16 Jul '09

When connecting to a non-root libvirt driver, the remote libvirt will attempt to perform autostart of the libvirtd daemon. This is not always desirable, particularly in test scripts where you don't want the daemon autostarted every time it crashes - you want a clear failure Daniel src/remote_internal.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/remote_internal.c b/src/remote_internal.c index 6df0282..77b4810 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -972,6 +972,7 @@ remoteOpen (virConnectPtr conn, { struct private_data *priv; int ret, rflags = 0; + const char *autostart = getenv("LIBVIRT_AUTOSTART"); if (inside_daemon) return VIR_DRV_OPEN_DECLINED; @@ -998,7 +999,9 @@ remoteOpen (virConnectPtr conn, getuid() > 0) { DEBUG0("Auto-spawn user daemon instance"); rflags |= VIR_DRV_OPEN_REMOTE_USER; - rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART; + if (!autostart || + STRNEQ(autostart, "0")) + rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART; } /* @@ -1013,7 +1016,9 @@ remoteOpen (virConnectPtr conn, if (getuid() > 0) { DEBUG0("Auto-spawn user daemon instance"); rflags |= VIR_DRV_OPEN_REMOTE_USER; - rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART; + if (!autostart || + STRNEQ(autostart, "0")) + rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART; } #endif } -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] [PATCH] 1/3 add support for netcf XML import and export
by Daniel Veillard 15 Jul '09

15 Jul '09

Basically this implement the routines to read an netcf XML definition and build the associated internal data structures. It does the checking of the input XML up to making sure all the needed informations are availble in the right place, but does not check for extra data. It should support the full format as defined by interface.rng as of netcf version 0.1.0, both for input and output. This is rather repetitive and boring code, so I tried to factorize things following patterms similar to the one used in the grammar, the code reflects to a large extent the definition blocks there. The main data structure is virInterfaceDef, it defines the most common and shared attributes of all interfaces, the protocol definition allowed in the schemas is only ipv4 at the moment but with the expected extension to allow ipv6 in parallel I made a separate structure (currently embedded in the main one). There is a enum discriminating the specific structures needed for vlan, bride and bonding, and for the two last ones, a dynamically allocated array of bare interfaces (which can be of vlan or ethernet type, merged into a single structure). This passes valgrind, but since I'm not testing agaisnt misformed XML input I afraid of potential leaks on exit paths, otherwise this should be fine. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 5

[libvirt] [PATCH] 3/3 Add netcf XML validation and input and output tests
by Daniel Veillard 15 Jul '09

15 Jul '09

Basic stuff similar to other tests, first we verify the input tests all conform to the provided schemas, then for each test we parse and reserialize verifying the output is identical. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 2

[libvirt] Fix svirt handling of shared/readonly disks
by Daniel P. Berrange 15 Jul '09

15 Jul '09

The patch committed here commit 547147084d03ebf30d09d242a5a721a4df664ffe Author: Mark McLoughlin <markmc(a)redhat.com> Date: Fri Jul 3 10:26:37 2009 +0000 was not actually the latest version currently used in Fedora. It causes shared disks to be re-labelled upon VM shutdown, breaking any other guests still runing with this disk. The only safe option is to skip relabel for all readonly & shared disks Daniel diff --git a/src/security_selinux.c b/src/security_selinux.c index 80c1c85..0db9f49 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -354,6 +354,17 @@ SELinuxRestoreSecurityImageLabel(virConnectPtr conn, char *newpath = NULL; const char *path = disk->src; + /* Don't restore labels on readoly/shared disks, because + * other VMs may still be accessing these + * Alternatively we could iterate over all running + * domains and try to figure out if it is in use, but + * this would not work for clustered filesystems, since + * we can't see running VMs using the file on other nodes + * Safest bet is thus to skip the restore step. + */ + if (disk->readonly || disk->shared) + return 0; + if ((err = virFileResolveLink(path, &newpath)) < 0) { virReportSystemError(conn, err, _("cannot resolve symlink %s"), path); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

1 0

[libvirt] [PATCH] 2/3 Add netcf XML schemas and test data
by Daniel Veillard 15 Jul '09

15 Jul '09

Directly imported from netcf-0.1.0 git with the exception that example have been changed to use single quote instead of double quote, otherwise unchanged. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 1