On Wed, Mar 19, 2025 at 05:09:01PM -0600, Jim Fehlig wrote:
> On 3/19/25 06:49, Daniel P. Berrangé wrote:
> > On Wed, Mar 05, 2025 at 03:48:17PM -0700, Jim Fehlig via Devel wrote:
> > > Move the code in qemuSaveImageCreate that opens, labels, and wraps the
> > > save image fd to a helper function, providing more flexibility for
> > > upcoming mapped-ram support.
> > >
> > > Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> > > ---
> > > src/qemu/qemu_saveimage.c | 65 +++++++++++++++++++++++++++------------
> > > 1 file changed, 45 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
> > > index 29b4e39879..e3f6a0ad0f 100644
> > > --- a/src/qemu/qemu_saveimage.c
> > > +++ b/src/qemu/qemu_saveimage.c
> > > @@ -425,6 +425,50 @@ qemuSaveImageDecompressionStop(virCommand *cmd,
> > > }
> > > +static int
> > > +qemuSaveImageCreateFd(virQEMUDriver *driver,
> > > + virDomainObj *vm,
> > > + const char *path,
> > > + virFileWrapperFd *wrapperFd,
> >
> > Doesn't this need to be virFileWrapperFd **, otherwise...
> >
> > > + bool *needUnlink,
> > > + unsigned int flags)
> > > +{
> > > + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> > > + int ret = -1;
> > > + VIR_AUTOCLOSE fd = -1;
> > > + int directFlag = 0;
> > > + unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;
> > > +
> > > + if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) {
> > > + wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE;
> > > + directFlag = virFileDirectFdFlag();
> > > + if (directFlag < 0) {
> > > + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> > > + _("bypass cache unsupported by this system"));
> > > + return -1;
> > > + }
> > > + }
> > > +
> > > + fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path,
> > > + O_WRONLY | O_TRUNC | O_CREAT | directFlag,
> > > + needUnlink);
> > > +
> > > + if (fd < 0)
> > > + return -1;
> > > +
> > > + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
> > > + return -1;
> > > +
> > > + if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
> > > + return -1;
> >
> > ....this assignment won't be visible to the caller
>
> Opps, which makes qemuDomainFileWrapperFDClose a NOP in all cases. I've
> applied the below diff locally. Any need to respin this series?
..snip..
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
With 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 :|
