On Wed, Apr 22, 2020 at 01:07:30PM +0200, Peter Krempa wrote: > 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. It's described in the next paragraph. > > > > > > 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. >