- Devel - libvirt List Archives

[libvirt] [PATCH] Explicitly set VM state to paused after migration completes
by Daniel P. Berrange 02 Jun '11

02 Jun '11

In v3 migration, once migration is completed, the VM needs to be left in a paused state until after Finish3 has been executed on the target. Only then will the VM be killed off. When using non-JSON QEMU monitor though, we don't receive any 'STOP' event from QEMU, so we need to manually set our state offline & thus release lock manager leases. It doesn't hurt to run this on the JSON case too, just in case the event gets lost somehow * src/qemu/qemu_migration.c: Explicitly set VM state to paused when migration completes --- src/qemu/qemu_migration.c | 28 +++++++++++++++++++++++++++- 1 files changed, 27 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f4eda92..aa74d86 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -718,7 +718,7 @@ qemuMigrationSetOffline(struct qemud_driver *driver, virDomainObjPtr vm) { int ret; - + VIR_DEBUG("driver=%p vm=%p", driver, vm); ret = qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION); if (ret == 0) { virDomainEventPtr event; @@ -1521,6 +1521,19 @@ static int doNativeMigrate(struct qemud_driver *driver, if (qemuMigrationWaitForCompletion(driver, vm) < 0) goto cleanup; + /* When migration completed, QEMU will have paused the + * CPUs for us, but unless we're using the JSON monitor + * we won't have been notified of this, so might still + * think we're running. For v2 protocol this doesn't + * matter because we'll kill the VM soon, but for v3 + * this is important because we stay paused until the + * confirm3 step, but need to release the lock state + */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (qemuMigrationSetOffline(driver, vm) < 0) + goto cleanup; + } + if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) VIR_WARN("Unable to encode migration cookie"); @@ -1816,6 +1829,19 @@ static int doTunnelMigrate(struct qemud_driver *driver, ret = qemuMigrationWaitForCompletion(driver, vm); + /* When migration completed, QEMU will have paused the + * CPUs for us, but unless we're using the JSON monitor + * we won't have been notified of this, so might still + * think we're running. For v2 protocol this doesn't + * matter because we'll kill the VM soon, but for v3 + * this is important because we stay paused until the + * confirm3 step, but need to release the lock state + */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (qemuMigrationSetOffline(driver, vm) < 0) + goto cleanup; + } + /* Close now to ensure the IO thread quits & is joinable in next method */ VIR_FORCE_CLOSE(client_sock); -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix error reporting with virDomainBlockStats on inactive guest
by Daniel P. Berrange 02 Jun '11

02 Jun '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Pull the check on virDomainIsActive upto top of the QEMU impl of virDomainBlockStats so that is is reported promptly * src/qemu/qemu_driver.c: Check if guest is active immediately --- src/qemu/qemu_driver.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5632d62..3f8f814 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5531,6 +5531,12 @@ qemudDomainBlockStats (virDomainPtr dom, goto cleanup; } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(path, vm->def->disks[i]->dst)) { disk = vm->def->disks[i]; -- 1.7.5.2

2 2

[libvirt] [PATCH] Fix auditing of disk hotunplug operations
by Daniel P. Berrange 02 Jun '11

02 Jun '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The qemuAuditDisk calls in disk hotunplug operations were being passed 'ret >= 0', but the code which sets ret to 0 was not yet executed, and the error path had already jumped to the 'cleanup' label. This meant hotunplug failures were never audited, and hotunplug success was audited as a failure * src/qemu/qemu_hotplug.c: Fix auditing of hotunplug --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c9e2d08..fe47896 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1191,12 +1191,14 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); + qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { qemuDomainObjExitMonitor(vm); + qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } } @@ -1206,7 +1208,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); + qemuAuditDisk(vm, detach, NULL, "detach", true); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) @@ -1283,6 +1285,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); + qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } @@ -1291,7 +1294,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); + qemuAuditDisk(vm, detach, NULL, "detach", true); virDomainDiskRemove(vm->def, i); -- 1.7.5.2

2 1

[libvirt] [PATCH] Avoid crash on NULL pointer in lock driver impls during hotplug
by Daniel P. Berrange 02 Jun '11

02 Jun '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> When virLockDriverAcquire is invoked during hotplug the state parameter will be left as NULL. * src/locking/lock_driver_nop.c, src/locking/lock_driver_sanlock.c: Don't reference NULL state parameter --- src/locking/lock_driver_nop.c | 7 ++++--- src/locking/lock_driver_sanlock.c | 29 ++++++++++++++++++----------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/locking/lock_driver_nop.c b/src/locking/lock_driver_nop.c index 5ebbd8d..36a9083 100644 --- a/src/locking/lock_driver_nop.c +++ b/src/locking/lock_driver_nop.c @@ -76,7 +76,8 @@ static int virLockManagerNopRelease(virLockManagerPtr lock ATTRIBUTE_UNUSED, char **state, unsigned int flags ATTRIBUTE_UNUSED) { - *state = NULL; + if (state) + *state = NULL; return 0; } @@ -85,8 +86,8 @@ static int virLockManagerNopInquire(virLockManagerPtr lock ATTRIBUTE_UNUSED, char **state, unsigned int flags ATTRIBUTE_UNUSED) { - - *state = NULL; + if (state) + *state = NULL; return 0; } diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index a60d7ce..adead76 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -374,18 +374,20 @@ static int virLockManagerSanlockRelease(virLockManagerPtr lock, virCheckFlags(0, -1); - if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) { - if (rv <= -200) - virLockError(VIR_ERR_INTERNAL_ERROR, - _("Failed to inquire lock: error %d"), rv); - else - virReportSystemError(-rv, "%s", - _("Failed to inquire lock")); - return -1; - } + if (state) { + if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) { + if (rv <= -200) + virLockError(VIR_ERR_INTERNAL_ERROR, + _("Failed to inquire lock: error %d"), rv); + else + virReportSystemError(-rv, "%s", + _("Failed to inquire lock")); + return -1; + } - if (STREQ(*state, "")) - VIR_FREE(*state); + if (STREQ(*state, "")) + VIR_FREE(*state); + } if ((rv = sanlock_release(-1, priv->vm_pid, SANLK_REL_ALL, 0, NULL)) < 0) { if (rv <= -200) @@ -409,6 +411,11 @@ static int virLockManagerSanlockInquire(virLockManagerPtr lock, virCheckFlags(0, -1); + if (!state) { + virLockError(VIR_ERR_INVALID_ARG, "state"); + return -1; + } + VIR_DEBUG("pid=%d", priv->vm_pid); if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) { -- 1.7.5.2

2 1

[libvirt] [PATCH] Fix return value in lock manager hotplug methods
by Daniel P. Berrange 02 Jun '11

02 Jun '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Refactoring of the lock manager hotplug methods lost the ret = 0 assignment for successful return path * src/locking/domain_lock.c: Add missing ret = 0 assignments --- src/locking/domain_lock.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index f0a11b7..771ed53 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -221,6 +221,8 @@ int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, if (virLockManagerAcquire(lock, NULL, 0) < 0) goto cleanup; + ret = 0; + cleanup: virLockManagerFree(lock); @@ -240,6 +242,8 @@ int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, if (virLockManagerRelease(lock, NULL, 0) < 0) goto cleanup; + ret = 0; + cleanup: virLockManagerFree(lock); @@ -260,6 +264,8 @@ int virDomainLockLeaseAttach(virLockManagerPluginPtr plugin, if (virLockManagerAcquire(lock, NULL, 0) < 0) goto cleanup; + ret = 0; + cleanup: virLockManagerFree(lock); @@ -279,6 +285,8 @@ int virDomainLockLeaseDetach(virLockManagerPluginPtr plugin, if (virLockManagerRelease(lock, NULL, 0) < 0) goto cleanup; + ret = 0; + cleanup: virLockManagerFree(lock); -- 1.7.5.2

2 1

[libvirt] [PATCH] Make sure virDomainSave/virDomainManagedSave reset id to -1
by Daniel P. Berrange 02 Jun '11

02 Jun '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> After successfull virDomainSave/virDomainManagedSave calls the guest will no longer be active, so the domain ID must be reset to -1 * daemon/remote_generator.pl: Special case virDomainSave & virDomainManagedSave for same reason as virDomainDestroy --- daemon/remote_generator.pl | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl index e204b76..6bddfa9 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -1363,8 +1363,11 @@ elsif ($opt_k) { print "\n"; } - if ($call->{ProcName} eq "DomainDestroy") { - # SPECIAL: virDomainDestroy needs to reset the domain id explicitly + if ($call->{ProcName} eq "DomainDestroy" || + $call->{ProcName} eq "DomainSave" || + $call->{ProcName} eq "DomainManagedSave") { + # SPECIAL: virDomain{Destroy|Save|ManagedSave} need to reset + # the domain id explicitly on success print " dom->id = -1;\n"; } -- 1.7.5.2

2 1

[libvirt] [PATCH] build: avoid corrupting / in RHEL 5
by Eric Blake 02 Jun '11

02 Jun '11

I noticed this while building from libvirt.git on RHEL 5.6: Generating internals/command.html.tmp mkdir: cannot create directory `/internals': Permission denied If I had been building as root instead, this pollutes /. Older autoconf lacks $(builddir), but it is rigorously equal to '.' in newer autoconf, so we could use '$(MKDIR_P) internals' instead. However, since internals/command.html is part of the tarball, we _already_ build it in $(srcdir), not $(builddir) during VPATH builds, so the mkdir is wasted effort! * docs/Makefile.am (internals/%.html.tmp): Drop unused mkdir. --- docs/Makefile.am | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index a8024b3..a98ced0 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -127,7 +127,6 @@ hvsupport.html.in: $(srcdir)/hvsupport.pl $(srcdir)/../src/libvirt_public.syms \ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in @if [ -x $(XSLTPROC) ] ; then \ echo "Generating $@"; \ - $(MKDIR_P) "$(builddir)/internals"; \ name=`echo $@ | sed -e 's/.tmp//'`; \ $(XSLTPROC) --stringparam pagename $$name --nonet --html \ $(top_srcdir)/docs/subsite.xsl $< > $@ \ -- 1.7.4.4

3 6

[libvirt] [PATCH v3] screenshot: Expose the new API in virsh
by Michal Privoznik 02 Jun '11

02 Jun '11

* tools/virsh.c: Add screenshot command * tools/virsh.pod: Document new command * src/libvirt.c: Fix off-be-one error --- diff to v1: - make filename optional and generate filename when missing diff to v2: - Eric's review suggestions included src/libvirt.c | 2 +- tools/virsh.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++--- tools/virsh.pod | 9 +++ 3 files changed, 186 insertions(+), 12 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ee5c7cd..eaae0ec 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2464,7 +2464,7 @@ error: * The screen ID is the sequential number of screen. In case of multiple * graphics cards, heads are enumerated before devices, e.g. having * two graphics cards, both with four heads, screen ID 5 addresses - * the first head on the second card. + * the second head on the second card. * * Returns a string representing the mime-type of the image format, or * NULL upon error. The caller must free() the returned value. diff --git a/tools/virsh.c b/tools/virsh.c index dfd5bd2..da10a0b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -264,6 +264,9 @@ static bool vshCmdGrpHelp(vshControl *ctl, const char *name); static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name); static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +static int vshCommandOptUInt(const vshCmd *cmd, const char *name, + unsigned int *value) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; static int vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; @@ -1938,6 +1941,153 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) return ret; } +static const vshCmdInfo info_screenshot[] = { + {"help", N_("take a screenshot of a current domain console and store it " + "into a file")}, + {"desc", N_("screenshot of a current domain console")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_screenshot[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_NONE, N_("where to store the screenshot")}, + {"screen", VSH_OT_INT, VSH_OFLAG_NONE, N_("ID of a screen to take screenshot of")}, + {NULL, 0, 0, NULL} +}; + +static int vshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED, + const char *bytes, size_t nbytes, void *opaque) +{ + int *fd = opaque; + + return safewrite(*fd, bytes, nbytes); +} + +/** + * Generate string: '<domain name>-<timestamp>[<extension>]' + */ +static char * +vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) +{ + char timestr[100]; + struct timeval cur_time; + struct tm time_info; + const char *ext = NULL; + char *ret = NULL; + + /* We should be already connected, but doesn't + * hurt to check */ + if (!vshConnectionUsability(ctl, ctl->conn)) + return NULL; + + if (!dom) { + vshError(ctl, "%s", _("Invalid domain supplied")); + return NULL; + } + + if (STREQ(mime, "image/x-portable-pixmap")) + ext = ".ppm"; + else if (STREQ(mime, "image/png")) + ext = ".png"; + /* add mime type here */ + + gettimeofday(&cur_time, NULL); + localtime_r(&cur_time.tv_sec, &time_info); + strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); + + if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom), + timestr, ext ? ext : "") < 0) { + vshError(ctl, "%s", _("Out of memory")); + return NULL; + } + + return ret; +} + +static bool +cmdScreenshot(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name = NULL; + char *file = NULL; + int fd = -1; + virStreamPtr st = NULL; + unsigned int screen = 0; + unsigned int flags = 0; /* currently unused */ + int ret = false; + bool created = true; + bool generated = false; + char *mime = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (vshCommandOptString(cmd, "file", (const char **) &file) < 0) { + vshError(ctl, "%s", _("file must not be empty")); + return false; + } + + if (vshCommandOptUInt(cmd, "screen", &screen) < 0) { + vshError(ctl, "%s", _("invalid screen ID")); + return false; + } + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + st = virStreamNew(ctl->conn, 0); + + mime = virDomainScreenshot(dom, st, screen, flags); + if (!mime) { + vshError(ctl, _("could not take a screenshot of %s"), name); + goto cleanup; + } + + if (!file) { + if (!(file=vshGenFileName(ctl, dom, mime))) + return false; + generated = true; + } + + if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) { + created = false; + if (errno != EEXIST || + (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) { + vshError(ctl, _("cannot create file %s"), file); + goto cleanup; + } + } + + if (virStreamRecvAll(st, vshStreamSink, &fd) < 0) { + vshError(ctl, _("could not receive data from domain %s"), name); + goto cleanup; + } + + if (VIR_CLOSE(fd) < 0) { + vshError(ctl, _("cannot close file %s"), file); + goto cleanup; + } + + if (virStreamFinish(st) < 0) { + vshError(ctl, _("cannot close stream on domain %s"), name); + goto cleanup; + } + + vshPrint(ctl, _("Screenshot saved to %s, with type of %s"), file, mime); + ret = true; + +cleanup: + if (!ret && created) + unlink(file); + if (generated) + VIR_FREE(file); + virDomainFree(dom); + if (st) + virStreamFree(st); + VIR_FORCE_CLOSE(fd); + return ret; +} + /* * "resume" command */ @@ -7451,16 +7601,6 @@ static const vshCmdOptDef opts_vol_download[] = { {NULL, 0, 0, NULL} }; - -static int -cmdVolDownloadSink(virStreamPtr st ATTRIBUTE_UNUSED, - const char *bytes, size_t nbytes, void *opaque) -{ - int *fd = opaque; - - return safewrite(*fd, bytes, nbytes); -} - static bool cmdVolDownload (vshControl *ctl, const vshCmd *cmd) { @@ -7510,7 +7650,7 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (virStreamRecvAll(st, cmdVolDownloadSink, &fd) < 0) { + if (virStreamRecvAll(st, vshStreamSink, &fd) < 0) { vshError(ctl, _("cannot receive data from volume %s"), name); goto cleanup; } @@ -10945,6 +11085,7 @@ static const vshCmdDef domManagementCmds[] = { {"resume", cmdResume, opts_resume, info_resume, 0}, {"save", cmdSave, opts_save, info_save, 0}, {"schedinfo", cmdSchedinfo, opts_schedinfo, info_schedinfo, 0}, + {"screenshot", cmdScreenshot, opts_screenshot, info_screenshot, 0}, {"setmaxmem", cmdSetmaxmem, opts_setmaxmem, info_setmaxmem, 0}, {"setmem", cmdSetmem, opts_setmem, info_setmem, 0}, {"setvcpus", cmdSetvcpus, opts_setvcpus, info_setvcpus, 0}, @@ -11527,6 +11668,30 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) return ret; } + +/* + * Convert option to unsigned int + * See vshCommandOptInt() + */ +static int +vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value) +{ + vshCmdOpt *arg = vshCommandOpt(cmd, name); + unsigned int ret = 0, num; + char *end_p = NULL; + + if ((arg != NULL) && (arg->data != NULL)) { + num = strtoul(arg->data, &end_p, 10); + ret = -1; + if ((arg->data != end_p) && (*end_p == 0)) { + *value = num; + ret = 1; + } + } + return ret; +} + + /* * Convert option to unsigned long * See vshCommandOptInt() diff --git a/tools/virsh.pod b/tools/virsh.pod index 9251db6..e4a11d5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -596,6 +596,15 @@ Therefore, -1 is a useful shorthand for 262144. B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>. +=item B<screenshot> I<domain-id> optional I<imagefilepath> I<--screen> B<screenID> + +Takes a screenshot of a current domain console and stores it into a file. +Optionally, if hypervisor supports more displays for a domain, I<screenID> +allows to specify which screen will be captured. It is the sequential number +of screen. In case of multiple graphics cards, heads are enumerated before +devices, e.g. having two graphics cards, both with four heads, screen ID 5 +addresses the second head on the second card. + =item B<setmem> I<domain-id> B<kilobytes> optional I<--config> I<--live> I<--current> -- 1.7.5.rc3

3 2

[libvirt] [PATCH] Add call to sanlock_restrict() in QEMU lock driver
by Daniel P. Berrange 02 Jun '11

02 Jun '11

In between fork and exec, a connection to sanlock is acquired and the socket file descriptor is intionally leaked to the child process. sanlock watches this FD for POLL_HANGUP to detect when QEMU has exited. We don't want a rogus/compromised QEMU from issuing sanlock RPC calls on the leaked FD though, since that could be used to DOS other guests. By calling sanlock_restrict() on the socket before exec() we can lock it down. * configure.ac: Check for sanlock_restrict API * src/locking/domain_lock.c: Restrict lock acquired in process startup phase * src/locking/lock_driver.h: Add VIR_LOCK_MANAGER_ACQUIRE_RESTRICT * src/locking/lock_driver_sanlock.c: Add call to sanlock_restrict when requested by VIR_LOCK_MANAGER_ACQUIRE_RESTRICT flag --- configure.ac | 2 +- src/locking/domain_lock.c | 8 +++++--- src/locking/lock_driver.h | 4 +++- src/locking/lock_driver_sanlock.c | 15 ++++++++++++++- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index a1bd64d..25669cf 100644 --- a/configure.ac +++ b/configure.ac @@ -972,7 +972,7 @@ if test "x$with_sanlock" != "xno"; then fail=1 fi]) if test "x$with_sanlock" != "xno" ; then - AC_CHECK_LIB([sanlock], [sanlock_acquire],[ + AC_CHECK_LIB([sanlock], [sanlock_restrict],[ SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock" with_sanlock=yes ],[ diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 85352e2..f0a11b7 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -159,10 +159,12 @@ int virDomainLockProcessStart(virLockManagerPluginPtr plugin, { virLockManagerPtr lock = virDomainLockManagerNew(plugin, dom, true); int ret; + int flags = VIR_LOCK_MANAGER_ACQUIRE_RESTRICT; + if (paused) - ret = virLockManagerAcquire(lock, NULL, VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY); - else - ret = virLockManagerAcquire(lock, NULL, 0); + flags |= VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY; + + ret = virLockManagerAcquire(lock, NULL, flags); virLockManagerFree(lock); diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index 40a55f6..2e71113 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -59,7 +59,9 @@ typedef enum { typedef enum { /* Don't acquire the resources, just register the object PID */ - VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0) + VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0), + /* Prevent further lock/unlock calls from this process */ + VIR_LOCK_MANAGER_ACQUIRE_RESTRICT = (1 << 1), } virLockManagerAcquireFlags; enum { diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 7e0610d..a60d7ce 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -240,7 +240,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, int rv; int i; - virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1); + virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT | + VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1); if (priv->res_count == 0 && priv->hasRWDisks) { @@ -327,6 +328,18 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, virSetInherit(sock, true) < 0) goto error; + if (flags & VIR_LOCK_MANAGER_ACQUIRE_RESTRICT) { + if ((rv = sanlock_restrict(sock, SANLK_RESTRICT_ALL)) < 0) { + if (rv <= -200) + virLockError(VIR_ERR_INTERNAL_ERROR, + _("Failed to restrict process: error %d"), rv); + else + virReportSystemError(-rv, "%s", + _("Failed to restrict process")); + goto error; + } + } + VIR_DEBUG("Acquire completed fd=%d", sock); if (res_free) { -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix handling of VIR_EVENT_HANDLE_ERROR in QEMU monitor
by Daniel P. Berrange 02 Jun '11

02 Jun '11

Commit 4454a9efc728b91e791b1f14c26ea23a19d57f48 introduced bad behaviour on the VIR_EVENT_HANDLE_ERROR condition. This condition is only hit when an invalid FD is used in poll() (typically due to a double-close bug). The QEMU monitor code was treating this condition as non-fatal, and thus libvirt would poll() in a fast loop forever burning 100% CPU. VIR_EVENT_HANDLE_ERROR must be handled in the same way as VIR_EVENT_HANDLE_HANGUP, killing the QEMU instance. * src/qemu/qemu_monitor.c: Treat VIR_EVENT_HANDLE_ERROR as EOF --- src/qemu/qemu_monitor.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 09a1b8e..26bb814 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -535,14 +535,14 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { #endif if (mon->fd != fd || mon->watch != watch) { - if (events & VIR_EVENT_HANDLE_HANGUP) + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) eof = true; qemuReportError(VIR_ERR_INTERNAL_ERROR, _("event from unexpected fd %d!=%d / watch %d!=%d"), mon->fd, fd, mon->watch, watch); error = true; } else if (mon->lastError.code != VIR_ERR_OK) { - if (events & VIR_EVENT_HANDLE_HANGUP) + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) eof = true; error = true; } else { @@ -581,8 +581,9 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { if (!error && !eof && events & VIR_EVENT_HANDLE_ERROR) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Error while waiting for monitor")); - error = 1; + _("Invalid file descriptor while waiting for monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_ERROR; } if (!error && events) { qemuReportError(VIR_ERR_INTERNAL_ERROR, -- 1.7.4.4

2 1