[libvirt PATCH v2] qemu: fix domain start with corrupted save file
This is to fix https://bugzilla.redhat.com/show_bug.cgi?id=1791522 With this change, if a domain comes across a corrupted save file during boot it removes the save file and logs a warning but continues to boot normally instead of failing to boot (with a subsequent boot attempt succeeding). The regression was introduced by 21ad56e932 and this change effectively reverts the relevant part of that commit. --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8d47a41cd..2579ef3984 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6810,13 +6810,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, *ret_def = def; *ret_data = data; + cleanup: return fd; error: virDomainDefFree(def); virQEMUSaveDataFree(data); VIR_FORCE_CLOSE(fd); - return -1; + goto cleanup; } static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6) -- 2.24.1
On Wed, Apr 22, 2020 at 15:15:31 +0200, Pavel Mores wrote:
This is to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1791522
With this change, if a domain comes across a corrupted save file during boot it removes the save file and logs a warning but continues to boot normally instead of failing to boot (with a subsequent boot attempt succeeding).
The regression was introduced by 21ad56e932 and this change effectively reverts the relevant part of that commit. --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8d47a41cd..2579ef3984 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6810,13 +6810,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, *ret_def = def; *ret_data = data;
+ cleanup: return fd;
error: virDomainDefFree(def); virQEMUSaveDataFree(data); VIR_FORCE_CLOSE(fd); - return -1; + goto cleanup;
As pointed out previously this doesn't really help to make it more obvious that 'fd' is abused to cary the other return codes here as well. I prefer the following fix: https://www.redhat.com/archives/libvir-list/2020-April/msg01101.html along with some cleanups which IMO make the function more obvious. Specifically the non-standard return values.
On Wed, Apr 22, 2020 at 05:07:01PM +0200, Peter Krempa wrote:
On Wed, Apr 22, 2020 at 15:15:31 +0200, Pavel Mores wrote:
This is to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1791522
With this change, if a domain comes across a corrupted save file during boot it removes the save file and logs a warning but continues to boot normally instead of failing to boot (with a subsequent boot attempt succeeding).
The regression was introduced by 21ad56e932 and this change effectively reverts the relevant part of that commit. --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8d47a41cd..2579ef3984 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6810,13 +6810,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, *ret_def = def; *ret_data = data;
+ cleanup: return fd;
error: virDomainDefFree(def); virQEMUSaveDataFree(data); VIR_FORCE_CLOSE(fd); - return -1; + goto cleanup;
As pointed out previously this doesn't really help to make it more obvious that 'fd' is abused to cary the other return codes here as well.
I prefer the following fix:
https://www.redhat.com/archives/libvir-list/2020-April/msg01101.html
Cool, that's incidentally *precisely* the same patch I'd come up with initially, before I dug in git history this morning and decided to be conservative and restore Jiří's original fix instead. :-) pvl
participants (2)
-
Pavel Mores -
Peter Krempa