On Wed, Mar 05, 2025 at 03:48:10PM -0700, Jim Fehlig via Devel wrote:
When invoking virDomainSaveParams with a relative path, the image is
saved to the daemon's CWD. Similarly, when providing virDomainRestoreParams
with a relative path, it attempts to restore from the daemon's CWD. In most
configurations, the daemon's CWD is set to '/'. Ensure a relative path is
converted to absolute before invoking the driver domain{Save,Restore}Params
functions.
Are you aware of any common usage of these APIs with a relative path ?
Although we've not documented it, my general view is that all paths given
to libvirt are expected to be absolute, everywhere - whether API parameters
like these, or config file parmeters, or XML elements/attributes.
In the perfect world we would canonicalize all relative paths on the
client side, but doing that is such an enourmous & complex job it is
not likely to be practical. We'd be better off just documenting use
of relative paths as "out of scope" and / or "undefined behaviour"
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libvirt-domain.c | 89 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 73 insertions(+), 16 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 18451ebdb9..0ee7c6f45b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1023,6 +1023,11 @@ virDomainSaveParams(virDomainPtr domain,
unsigned int flags)
{
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);
@@ -1033,23 +1038,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(¶ms_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(¶ms_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;
}
@@ -1206,6 +1234,12 @@ virDomainRestoreParams(virConnectPtr conn,
virTypedParameterPtr params, int nparams,
unsigned int flags)
{
+ virTypedParameterPtr params_copy = NULL;
+ int nparams_copy = 0;
+ const char *from = NULL;
+ g_autofree char *absolute_from = NULL;
+ int ret = -1;
+
VIR_DEBUG("conn=%p, params=%p, nparams=%d, flags=0x%x",
conn, params, nparams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams);
@@ -1213,23 +1247,46 @@ virDomainRestoreParams(virConnectPtr conn,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckReadOnlyGoto(conn->flags, error);
+ virCheckReadOnlyGoto(conn->flags, done);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
VIR_DOMAIN_SAVE_PAUSED,
- error);
+ done);
if (conn->driver->domainRestoreParams) {
+ /* We must absolutize the file path as the save is done out of process */
+ virTypedParamsCopy(¶ms_copy, params, nparams);
+ nparams_copy = nparams;
+ if (virTypedParamsGetString(params_copy, nparams_copy,
+ VIR_DOMAIN_SAVE_PARAM_FILE, &from) < 0)
+ goto done;
+
+ if (from) {
+ if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not build absolute output file
path"));
+ goto done;
+ }
+
+ if (virTypedParamsReplaceString(¶ms_copy, &nparams_copy,
+ VIR_DOMAIN_SAVE_PARAM_FILE,
+ absolute_from) < 0)
+ goto done;
+ }
+
if (conn->driver->domainRestoreParams(conn, params, nparams, flags) <
0)
- goto error;
- return 0;
+ goto done;
+ ret = 0;
}
virReportUnsupportedError();
- error:
- virDispatchError(conn);
- return -1;
+ done:
+ if (ret < 0)
+ virDispatchError(conn);
+ virTypedParamsFree(params_copy, nparams_copy);
+
+ return ret;
}
--
2.43.0
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 :|