[PATCH] qemu: fix missing cleanup on error in qemuSaveImageStartVM
Commit 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus") replaced a call to virDomainObjSave() with qemuDomainSaveStatus() as a part of cleanup. Since qemuDomainSaveStatus() does not indicate any failure through its return code, the error handling cleanup code got eliminated in the process. Thus upon failure, we will no longer killing the started qemu process. This commit fixes this by reverting the change that was introduced with the above commit. Fixes: 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus") Signed-off-by: Ani Sinha <ani@anisinha.ca> --- src/qemu/qemu_saveimage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 28d6098dd8..557ee2cd21 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -586,6 +586,7 @@ qemuSaveImageStartVM(virConnectPtr conn, VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeader *header = &data->header; g_autoptr(qemuDomainSaveCookie) cookie = NULL; int rc = 0; @@ -679,7 +680,10 @@ qemuSaveImageStartVM(virConnectPtr conn, "%s", _("failed to resume domain")); goto cleanup; } - qemuDomainSaveStatus(vm); + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { + VIR_WARN("Failed to save status on vm %s", vm->def->name); + goto cleanup; + } } else { int detail = (start_paused ? VIR_DOMAIN_EVENT_SUSPENDED_PAUSED : VIR_DOMAIN_EVENT_SUSPENDED_RESTORED); -- 2.25.1
On Wed, Dec 15, 2021 at 18:37:21 +0530, Ani Sinha wrote:
Commit 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus") replaced a call to virDomainObjSave() with qemuDomainSaveStatus() as a part of cleanup. Since qemuDomainSaveStatus() does not indicate any failure through its return code, the error handling cleanup code got eliminated in the process. Thus upon failure, we will no longer killing the started qemu process. This commit fixes this by reverting the change that was introduced with the above commit.
Fixes: 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus")
Signed-off-by: Ani Sinha <ani@anisinha.ca> --- src/qemu/qemu_saveimage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 28d6098dd8..557ee2cd21 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -586,6 +586,7 @@ qemuSaveImageStartVM(virConnectPtr conn, VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeader *header = &data->header; g_autoptr(qemuDomainSaveCookie) cookie = NULL; int rc = 0; @@ -679,7 +680,10 @@ qemuSaveImageStartVM(virConnectPtr conn, "%s", _("failed to resume domain")); goto cleanup; } - qemuDomainSaveStatus(vm); + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { + VIR_WARN("Failed to save status on vm %s", vm->def->name); + goto cleanup; + } } else { int detail = (start_paused ? VIR_DOMAIN_EVENT_SUSPENDED_PAUSED : VIR_DOMAIN_EVENT_SUSPENDED_RESTORED);
Oops, I apparently overlooked the goto statement here. Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
On Wed, Dec 15, 2021 at 16:14:34 +0100, Jiri Denemark wrote:
On Wed, Dec 15, 2021 at 18:37:21 +0530, Ani Sinha wrote:
Commit 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus") replaced a call to virDomainObjSave() with qemuDomainSaveStatus() as a part of cleanup. Since qemuDomainSaveStatus() does not indicate any failure through its return code, the error handling cleanup code got eliminated in the process. Thus upon failure, we will no longer killing the started qemu process. This commit fixes this by reverting the change that was introduced with the above commit.
Fixes: 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus")
Signed-off-by: Ani Sinha <ani@anisinha.ca> --- src/qemu/qemu_saveimage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 28d6098dd8..557ee2cd21 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -586,6 +586,7 @@ qemuSaveImageStartVM(virConnectPtr conn, VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeader *header = &data->header; g_autoptr(qemuDomainSaveCookie) cookie = NULL; int rc = 0; @@ -679,7 +680,10 @@ qemuSaveImageStartVM(virConnectPtr conn, "%s", _("failed to resume domain")); goto cleanup; } - qemuDomainSaveStatus(vm); + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { + VIR_WARN("Failed to save status on vm %s", vm->def->name); + goto cleanup; + } } else { int detail = (start_paused ? VIR_DOMAIN_EVENT_SUSPENDED_PAUSED : VIR_DOMAIN_EVENT_SUSPENDED_RESTORED);
Oops, I apparently overlooked the goto statement here.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Heh, bad key pressed, what I really wanted to say is: Reviewed-by: Jiri Denemark <jdenemar@redhat.com> and pushed.
participants (2)
-
Ani Sinha -
Jiri Denemark