[PATCH 5/5] qemuPrepareNVRAM: Us virFileRewrite() to write NVRAM

Daniel P. Berrangé berrange at redhat.com
Thu Feb 10 16:16:12 UTC 2022


On Thu, Feb 10, 2022 at 12:13:26PM +0100, Michal Privoznik wrote:
> After previous commits there is no need for qemuPrepareNVRAM() to
> open code virFileRewrite(). Deduplicate the code by calling the
> function.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_process.c | 118 +++++++++++++---------------------------
>  1 file changed, 39 insertions(+), 79 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8fccf6b760..500a91f153 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4421,6 +4421,40 @@ qemuProcessUpdateCPU(virQEMUDriver *driver,
>  }
>  
>  
> +static int
> +qemuPrepareNVRAMHelper(int dstFD,
> +                       const void *opaque)
> +{
> +    const char *master_nvram_path = opaque;
> +    VIR_AUTOCLOSE srcFD = -1;
> +    ssize_t r;
> +
> +    if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY,
> +                               0, -1, -1, 0)) < 0) {
> +        virReportSystemError(-srcFD,
> +                             _("Failed to open file '%s'"),
> +                             master_nvram_path);
> +        return -2;
> +    }

Can't we keep this in the caller and pass 'srcFD' in via opaque ?

> +
> +    do {
> +        char buf[1024];
> +
> +        if ((r = saferead(srcFD, buf, sizeof(buf))) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Unable to read from file '%s'"),
> +                                 master_nvram_path);
> +            return -2;
> +        }
> +
> +        if (safewrite(dstFD, buf, r) < 0)
> +            return -1;

This just  strengthens my view that we should only have 1 return
value for errors and be consistent in usage.

> +    } while (r);
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuPrepareNVRAM(virQEMUDriver *driver,
>                   virDomainObj *vm,
> @@ -4428,13 +4462,8 @@ qemuPrepareNVRAM(virQEMUDriver *driver,
>  {
>      g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>      int ret = -1;
> -    int srcFD = -1;
> -    int dstFD = -1;
>      virDomainLoaderDef *loader = vm->def->os.loader;
> -    bool created = false;
>      const char *master_nvram_path;
> -    ssize_t r;
> -    g_autofree char *tmp_dst_path = NULL;
>  
>      if (!loader || !loader->nvram ||
>          (virFileExists(loader->nvram) && !reset_nvram))
> @@ -4458,84 +4487,15 @@ qemuPrepareNVRAM(virQEMUDriver *driver,
>          goto cleanup;
>      }
>  
> -    if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY,
> -                               0, -1, -1, 0)) < 0) {
> -        virReportSystemError(-srcFD,
> -                             _("Failed to open file '%s'"),
> -                             master_nvram_path);

We don't need to remove this surely, if we pass...

> +    if (virFileRewrite(loader->nvram,
> +                       S_IRUSR | S_IWUSR,
> +                       cfg->user, cfg->group,
> +                       qemuPrepareNVRAMHelper,
> +                       master_nvram_path) < 0)

... &srcFD instead of master_nvram_path   here  ?


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 :|




More information about the libvir-list mailing list