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