[PATCH] NVRAM: check NVRAM file size and recover from template

[Daniel P. Berrangé]

On Sat, Aug 08, 2020 at 06:01:22PM +0800, Hao Wang wrote:
> From: Hao Wang <wanghao232 at huawei.com>
> Subject: [PATCH] NVRAM: check NVRAM file size and recover from template
> 
> A corrupted nvram file (e.g. caused by last unsuccessful creation due to
> insufficient memory) can lead to boot or migration failure.

Or it can be caused by incorrect deployment of new NVRAM file by an
administrator or OS Distro...

Just last week we were debugging an issue on Ubuntu where a person
had deployed incorrect NVRAM files of a different size.

> Check the size of the existed nvram file when qemuPrepareNVRAM, and re-create
> if the existed one is unhealthy.

....this will delete valid NVRAM files, hiding the mistake by the
administrator.

Overall I'm not a fan of performing a destructive action like deleting
files which may or may not be correct.


> 
> Signed-off-by: Hao Wang <wanghao232 at huawei.com>
> ---
>  src/qemu/qemu_process.c | 54 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 126fabf5ef..42060bb36c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4376,6 +4376,48 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver,
>  }
> 
> 
> +static bool
> +qemuIsNvramFileHealthy(virQEMUDriverConfigPtr cfg,
> +                       virDomainLoaderDefPtr loader)
> +{
> +    const char *masterNvramPath;
> +    off_t nvramSize;
> +    off_t masterSize;
> +
> +    masterNvramPath = loader->templt;
> +    if (!loader->templt) {
> +        size_t i;
> +        for (i = 0; i < cfg->nfirmwares; i++) {
> +            if (STREQ(cfg->firmwares[i]->name, loader->path)) {
> +                masterNvramPath = cfg->firmwares[i]->nvram;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (!masterNvramPath) {
> +        VIR_WARN("no nvram template is found; assume the nvram file is healthy");
> +        return true;
> +    }
> +
> +    if ((nvramSize = virFileLength(loader->nvram, -1)) < 0 ||
> +        (masterSize = virFileLength(masterNvramPath, -1)) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to get the size of '%s' or '%s'"),
> +                             loader->nvram, masterNvramPath);
> +        return false;
> +    }
> +
> +    if (nvramSize != masterSize) {
> +        VIR_WARN("the size(%zd) of the nvram file is not equal to that of the template %s",
> +                 nvramSize, masterNvramPath);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +
>  static int
>  qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
>                   virDomainObjPtr vm)
> @@ -4388,9 +4430,19 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
>      const char *master_nvram_path;
>      ssize_t r;
> 
> -    if (!loader || !loader->nvram || virFileExists(loader->nvram))
> +    if (!loader || !loader->nvram)
>          return 0;
> 
> +    if (virFileExists(loader->nvram)) {
> +        if (qemuIsNvramFileHealthy(cfg, loader))
> +            return 0;
> +
> +        ignore_value(virFileRemove(loader->nvram, -1, -1));
> +        VIR_WARN("the nvram file %s exists but may be corrupted! "
> +                 "Remove it and try to copy a new one from template.",
> +                 loader->nvram);
> +    }
> +
>      master_nvram_path = loader->templt;
>      if (!loader->templt) {
>          size_t i;
> --
> 2.23.0
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|