Guido Günther <agx@xxxxxxxxxxx> wrote:
> we currently don't report errors to save qemu's domain status file back
> to the caller. That was o.k. as long as the code was there for testing
> but now that the XML is being picked up on daemon restart we must
> handle these.
> The attached patch does that, although I'm not confident that it's
> enough. While we return error on device attach/unattach the device
> actually got attached/unattached already. Same is true for
> suspend/resume.
> A better solution would be to write out the new domain status into a
> temp file and simply call rename(2) on it when the attach/unattach
> succeeds and discard it if it fails. This would minimize the possible
> error conditions like disk full, missing permissions to write into that
> directory etc. Does this sound reasonable?
Yes.
> O.k. to apply the attached version for now?
Looks good to me.
Minor formatting nits:
>>From 45feca32cc5ac6e48b2b586771736050202fc119 Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx>
> Date: Sat, 31 Jan 2009 13:10:05 +0100
> Subject: [PATCH] don't ignore errors to save the domain status
>
> ---
> src/qemu_driver.c | 23 ++++++++++++++++-------
> 1 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 09f69bf..3933dfa 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -1280,12 +1280,12 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
> (qemudDetectVcpuPIDs(conn, vm) < 0) ||
> (qemudInitCpus(conn, vm, migrateFrom) < 0) ||
> - (qemudInitPasswords(conn, driver, vm) < 0)) {
> + (qemudInitPasswords(conn, driver, vm) < 0) ||
> + (qemudSaveDomainStatus(conn, qemu_driver, vm) < 0)) {
> qemudShutdownVMDaemon(conn, driver, vm);
> return -1;
> }
> }
> - qemudSaveDomainStatus(conn, qemu_driver, vm);
>
> return ret;
> }
> @@ -1319,7 +1319,10 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
> /* shut it off for sure */
> virKillProcess(vm->pid, SIGKILL);
>
> - qemudRemoveDomainStatus(conn, driver, vm);
> + if (qemudRemoveDomainStatus(conn, driver, vm) < 0) {
> + qemudLog(QEMUD_WARN, _("Failed to remove domain status for %s"),
> + vm->def->name);
> + }
> vm->pid = -1;
> vm->def->id = -1;
> vm->state = VIR_DOMAIN_SHUTOFF;
> @@ -1972,7 +1975,8 @@ static int qemudDomainSuspend(virDomainPtr dom) {
> VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
> VIR_FREE(info);
> }
> - qemudSaveDomainStatus(dom->conn, driver, vm);
> + if (qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
> + goto cleanup;
s/ //
> ret = 0;
>
> cleanup:
> @@ -2022,7 +2026,8 @@ static int qemudDomainResume(virDomainPtr dom) {
> VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
> VIR_FREE(info);
> }
> - qemudSaveDomainStatus(dom->conn, driver, vm);
> + if (qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
> + goto cleanup;
Likewise.
> ret = 0;
>
> cleanup:
> @@ -3461,7 +3466,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
> goto cleanup;
> }
>
> - qemudSaveDomainStatus(dom->conn, driver, vm);
> + if (!ret && qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
> + ret = -1;
> +
> cleanup:
> if (ret < 0)
> virDomainDeviceDefFree(dev);
> @@ -3578,7 +3585,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
> qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
> "%s", _("only SCSI or virtio disk device can be detached dynamically"));
>
> - qemudSaveDomainStatus(dom->conn, driver, vm);
> + if (!ret && qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
> + ret = -1;
> +
> cleanup:
> virDomainDeviceDefFree(dev);
> if (vm)
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list