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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux