[libvirt] [PATCH] fix various breakages in qemu "virsh dump"

1) qemuMigrateToCommand uses ">>" so we have to truncate the file before starting the migration; 2) the command wasn't updated to chown the driver and set/restore the security lavels; 3) the VM does not have to be resumed if migration fails; 4) the file is not removed when migration fails. * src/qemu/qemu_driver.c (qemuDomainCoreDump): Truncate file before dumping, set/restore ownership and security labels for the file. --- src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9b5ac2..92d4629 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3542,7 +3542,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int resume = 0, paused = 0; - int ret = -1; + int ret = -1, fd = -1; const char *args[] = { "cat", NULL, @@ -3569,6 +3569,33 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } + /* Create an empty file with appropriate ownership. */ + if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to create '%s'"), path); + goto endjob; + } + + if (close(fd) < 0) { + virReportSystemError(dom->conn, errno, + _("unable to save file %s"), + path); + goto endjob; + } + + if (driver->privileged && + chown(path, driver->user, driver->group) < 0) { + virReportSystemError(NULL, errno, + _("unable to set ownership of '%s' to user %d:%d"), + path, driver->user, driver->group); + goto endjob; + } + + if (driver->securityDriver && + driver->securityDriver->domainSetSavedStateLabel && + driver->securityDriver->domainSetSavedStateLabel(dom->conn, vm, path) == -1) + goto endjob; + /* Migrate will always stop the VM, so once we support live dumping the resume condition will stay the same, independent of whether the stop command is issued. */ @@ -3590,8 +3617,22 @@ static int qemudDomainCoreDump(virDomainPtr dom, qemuDomainObjEnterMonitor(vm); ret = qemuMonitorMigrateToCommand(priv->mon, 0, args, path); qemuDomainObjExitMonitor(vm); - paused = 1; + paused |= (ret == 0); + + if (driver->privileged && + chown(path, 0, 0) < 0) { + virReportSystemError(NULL, errno, + _("unable to set ownership of '%s' to user %d:%d"), + path, 0, 0); + goto endjob; + } + if (driver->securityDriver && + driver->securityDriver->domainRestoreSavedStateLabel && + driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, path) == -1) + goto endjob; + +endjob: /* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here after the migration is complete. */ @@ -3605,10 +3646,11 @@ static int qemudDomainCoreDump(virDomainPtr dom, qemuDomainObjExitMonitor(vm); } -endjob: qemuDomainObjEndJob(vm); cleanup: + if (ret != 0) + unlink(path); if (vm) virDomainObjUnlock(vm); return ret; -- 1.6.5.2

On Fri, Nov 27, 2009 at 06:33:13PM +0100, Paolo Bonzini wrote:
1) qemuMigrateToCommand uses ">>" so we have to truncate the file before starting the migration;
2) the command wasn't updated to chown the driver and set/restore the security lavels;
3) the VM does not have to be resumed if migration fails;
4) the file is not removed when migration fails.
* src/qemu/qemu_driver.c (qemuDomainCoreDump): Truncate file before dumping, set/restore ownership and security labels for the file. --- src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 45 insertions(+), 3 deletions(-)
ACK, looks good. Daniel -- |: 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 :|

On Mon, Nov 30, 2009 at 10:13:34AM +0000, Daniel P. Berrange wrote:
On Fri, Nov 27, 2009 at 06:33:13PM +0100, Paolo Bonzini wrote:
1) qemuMigrateToCommand uses ">>" so we have to truncate the file before starting the migration;
2) the command wasn't updated to chown the driver and set/restore the security lavels;
3) the VM does not have to be resumed if migration fails;
4) the file is not removed when migration fails.
* src/qemu/qemu_driver.c (qemuDomainCoreDump): Truncate file before dumping, set/restore ownership and security labels for the file. --- src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 45 insertions(+), 3 deletions(-)
ACK, looks good.
Yup, pushed, I just had to slightly rebase the patch, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Paolo Bonzini