Re: [libvirt PATCH] qemu: fix domain start with corrupted save file

On Wed, Apr 22, 2020 at 12:54:26 +0200, Pavel Mores wrote: > This is to fix > > https://bugzilla.redhat.com/show_bug.cgi?id=1791522 Please describe the bug in the commit message rather than have users open and try understand external links.

> > 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. Please break commit messages at 72 colums as it's common. > --- > 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; I think this should be fixed properly. 'fd' is serving double duty of also reporting the erorrs here and it's not really obvious from this hunk. The code above sets 'fd' to a variety of values e.g. -3 on some errors. This is really tricky and hard to follow. In this case we want a 'ret' variable which is set to the proper value and returned on error and fd returned on success or alternatively assigned to 'ret' right before returning it. This should make it possible to do it even without adding new not really useful labels.