Re: [PATCH 01/20] lib: virDomainSaveParams: Ensure absolute save path

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Thu, Aug 08, 2024 at 05:37:54PM -0600, Jim Fehlig via Devel wrote:
> When invoking virDomainSaveParams with a relative path, the image
> is saved to the daemon's CWD, which in most cases is '/'. Ensure
> a relative path is converted to absolute before invoking the
> driver 'domainSaveParams' function.
> 
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  src/libvirt-domain.c | 46 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 7c6b93963c..5aedae1b49 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -1020,6 +1020,11 @@ virDomainSaveParams(virDomainPtr domain,
>                      unsigned int flags)

The same change appears to be needed in virDomainRestoreParams ?

>  {
>      virConnectPtr conn;
> +    virTypedParameterPtr params_copy = NULL;
> +    int nparams_copy = 0;
> +    const char *to = NULL;
> +    g_autofree char *absolute_to = NULL;
> +    int ret = -1;
>  
>      VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x",
>                       params, nparams, flags);
> @@ -1030,23 +1035,46 @@ virDomainSaveParams(virDomainPtr domain,
>      virCheckDomainReturn(domain, -1);
>      conn = domain->conn;
>  
> -    virCheckReadOnlyGoto(conn->flags, error);
> +    virCheckReadOnlyGoto(conn->flags, done);
>  
>      VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
>                               VIR_DOMAIN_SAVE_PAUSED,
> -                             error);
> +                             done);
> +
> +    /* We must absolutize the file path as the save is done out of process */
> +    virTypedParamsCopy(&params_copy, params, nparams);
> +    nparams_copy = nparams;
> +    if (virTypedParamsGetString(params_copy, nparams_copy,
> +                                VIR_DOMAIN_SAVE_PARAM_FILE, &to) < 0)
> +        goto done;
> +
> +    if (to) {
> +        if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("could not build absolute output file path"));
> +            goto done;
> +        }
> +
> +        if (virTypedParamsReplaceString(&params_copy, &nparams_copy,
> +                                        VIR_DOMAIN_SAVE_PARAM_FILE,
> +                                        absolute_to) < 0)
> +            goto done;
> +    }
>  
>      if (conn->driver->domainSaveParams) {
> -        if (conn->driver->domainSaveParams(domain, params, nparams, flags) < 0)
> -            goto error;
> -        return 0;
> +        if (conn->driver->domainSaveParams(domain, params_copy, nparams_copy, flags) < 0)
> +            goto done;
> +        ret = 0;
> +    } else {
> +        virReportUnsupportedError();
>      }
>  
> -    virReportUnsupportedError();
> + done:
> +    if (ret < 0)
> +        virDispatchError(domain->conn);
> +    virTypedParamsFree(params_copy, nparams_copy);
>  
> - error:
> -    virDispatchError(domain->conn);
> -    return -1;
> +    return ret;
>  }
>  
>  
> -- 
> 2.35.3
> 

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