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