[libvirt] [PATCH 0/4] Few unrelated fixes in save/dump code
I spent some time in the save/dump code today and found a few nits to fix. Peter Krempa (4): util: process: Don't report OOM errors in helper virsh: domain: Clean up handling of "dom" in "save" command qemu: dump: Fix formatting of function headers and code inline qemu: dump: Resume CPUs only when the VM is still alive src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++------------------------ src/util/virprocess.c | 10 +++++----- tools/virsh-domain.c | 6 +++--- 3 files changed, 35 insertions(+), 32 deletions(-) -- 2.0.2
virProcessTranslateStatus is used on error paths that should not spoil the returned error. As the errors are ignored, use the quiet versions of virAsprintf to create the message. --- src/util/virprocess.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 97cce4f..5bb2298 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -73,13 +73,13 @@ virProcessTranslateStatus(int status) { char *buf; if (WIFEXITED(status)) { - ignore_value(virAsprintf(&buf, _("exit status %d"), - WEXITSTATUS(status))); + ignore_value(virAsprintfQuiet(&buf, _("exit status %d"), + WEXITSTATUS(status))); } else if (WIFSIGNALED(status)) { - ignore_value(virAsprintf(&buf, _("fatal signal %d"), - WTERMSIG(status))); + ignore_value(virAsprintfQuiet(&buf, _("fatal signal %d"), + WTERMSIG(status))); } else { - ignore_value(virAsprintf(&buf, _("invalid value %d"), status)); + ignore_value(virAsprintfQuiet(&buf, _("invalid value %d"), status)); } return buf; } -- 2.0.2
--- tools/virsh-domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ed2e3ea..f04dd17 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3887,7 +3887,8 @@ doSave(void *opaque) out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: - if (dom) virDomainFree(dom); + if (dom) + virDomainFree(dom); VIR_FREE(xml); ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); } @@ -4051,8 +4052,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, _("\nDomain %s saved to %s\n"), name, to); cleanup: - if (dom) - virDomainFree(dom); + virDomainFree(dom); return ret; } -- 2.0.2
Also drop a comment with obvious content. --- src/qemu/qemu_driver.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d724eeb..f0e8994 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3607,10 +3607,12 @@ getCompressionType(virQEMUDriverPtr driver) return ret; } -static int qemuDomainCoreDumpWithFormat(virDomainPtr dom, - const char *path, - unsigned int dumpformat, - unsigned int flags) + +static int +qemuDomainCoreDumpWithFormat(virDomainPtr dom, + const char *path, + unsigned int dumpformat, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3672,13 +3674,8 @@ static int qemuDomainCoreDumpWithFormat(virDomainPtr dom, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); - } - - /* 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. */ - else if (((resume && paused) || (flags & VIR_DUMP_RESET)) && - virDomainObjIsActive(vm)) { + } else if (((resume && paused) || (flags & VIR_DUMP_RESET)) && + virDomainObjIsActive(vm)) { if ((ret == 0) && (flags & VIR_DUMP_RESET)) { priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); @@ -3713,14 +3710,18 @@ static int qemuDomainCoreDumpWithFormat(virDomainPtr dom, return ret; } -static int qemuDomainCoreDump(virDomainPtr dom, - const char *path, - unsigned int flags) + +static int +qemuDomainCoreDump(virDomainPtr dom, + const char *path, + unsigned int flags) { return qemuDomainCoreDumpWithFormat(dom, path, - VIR_DOMAIN_CORE_DUMP_FORMAT_RAW, flags); + VIR_DOMAIN_CORE_DUMP_FORMAT_RAW, + flags); } + static char * qemuDomainScreenshot(virDomainPtr dom, virStreamPtr st, -- 2.0.2
Check if the VM is alive after we possibly called into monitor to reset the guest. --- src/qemu/qemu_driver.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f0e8994..9765af5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3683,15 +3683,17 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, qemuDomainObjExitMonitor(driver, vm); } - if (resume && qemuProcessStartCPUs(driver, vm, dom->conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_DUMP) < 0) { - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); - if (virGetLastError() == NULL) - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("resuming after dump failed")); + if (resume && virDomainObjIsActive(vm)) { + if (qemuProcessStartCPUs(driver, vm, dom->conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_DUMP) < 0) { + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); + if (virGetLastError() == NULL) + virReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("resuming after dump failed")); + } } } -- 2.0.2
On Tue, Sep 09, 2014 at 05:25:31PM +0200, Peter Krempa wrote:
I spent some time in the save/dump code today and found a few nits to fix.
Peter Krempa (4): util: process: Don't report OOM errors in helper virsh: domain: Clean up handling of "dom" in "save" command qemu: dump: Fix formatting of function headers and code inline qemu: dump: Resume CPUs only when the VM is still alive
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++------------------------ src/util/virprocess.c | 10 +++++----- tools/virsh-domain.c | 6 +++--- 3 files changed, 35 insertions(+), 32 deletions(-)
ACK series, Martin
On 09/09/14 19:58, Martin Kletzander wrote:
On Tue, Sep 09, 2014 at 05:25:31PM +0200, Peter Krempa wrote:
I spent some time in the save/dump code today and found a few nits to fix.
Peter Krempa (4): util: process: Don't report OOM errors in helper virsh: domain: Clean up handling of "dom" in "save" command qemu: dump: Fix formatting of function headers and code inline qemu: dump: Resume CPUs only when the VM is still alive
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++------------------------ src/util/virprocess.c | 10 +++++----- tools/virsh-domain.c | 6 +++--- 3 files changed, 35 insertions(+), 32 deletions(-)
ACK series,
Martin
Pushed; Thanks. Peter
participants (2)
-
Martin Kletzander -
Peter Krempa