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(a)suse.com>
> ---
> 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?
Regards,
Jim
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index e3f6a0ad0f..7ab44edcdc 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -429,7 +429,7 @@ static int
qemuSaveImageCreateFd(virQEMUDriver *driver,
virDomainObj *vm,
const char *path,
- virFileWrapperFd *wrapperFd,
+ virFileWrapperFd **wrapperFd,
bool *needUnlink,
unsigned int flags)
{
@@ -459,7 +459,7 @@ qemuSaveImageCreateFd(virQEMUDriver *driver,
if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
return -1;
- if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
+ if (!(*wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
return -1;
ret = fd;
@@ -488,7 +488,7 @@ qemuSaveImageCreate(virQEMUDriver *driver,
virFileWrapperFd *wrapperFd = NULL;
/* Obtain the file handle. */
- fd = qemuSaveImageCreateFd(driver, vm, path, wrapperFd, &needUnlink, flags);
+ fd = qemuSaveImageCreateFd(driver, vm, path, &wrapperFd, &needUnlink,
flags);
if (fd < 0)
goto cleanup;