Re: [PATCH 3/3] lib: Require path in virDomainRestoreParams()

After seeing previous commit one might think that virDomainRestoreParams() would get the similar treatment. Well, it can't. The problem here is: without any indication what domain to restore we don't really know what domain to restore (shocking, right?). Therefore, we have to require path to restore domain from, at which point, we can save callers couple of lines and let them pass the path as a regular argument instead of requiring it in typed params. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 1 + src/driver-hypervisor.h | 1 + src/libvirt-domain.c | 25 +++++++++++++++++++++---- src/qemu/qemu_driver.c | 9 +++------ src/remote/remote_protocol.x | 1 + src/remote_protocol-structs | 1 + src/rpc/gendispatch.pl | 2 +- 7 files changed, 29 insertions(+), 11 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 24846046aa..08082627e5 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1574,6 +1574,7 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags); int virDomainRestoreParams (virConnectPtr conn, + const char *path, virTypedParameterPtr params, int nparams, unsigned int flags); diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 69516e8fea..81c20a749e 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -258,6 +258,7 @@ typedef int typedef int (*virDrvDomainRestoreParams)(virConnectPtr conn, + const char *path, virTypedParameterPtr params, int nparams, unsigned int flags); diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 208c2438fe..481886eb02 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1186,11 +1186,14 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, /** * virDomainRestoreParams: * @conn: pointer to the hypervisor connection + * @path: path to the input file * @params: restore parameters * @nparams: number of restore parameters * @flags: bitwise-OR of virDomainSaveRestoreFlags * * This method extends virDomainRestoreFlags by adding parameters. + * Please note that VIR_DOMAIN_SAVE_PARAM_FILE is not supported for this API as + * @path serves the same purpose. * * Returns 0 in case of success and -1 in case of failure. * @@ -1198,25 +1201,39 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, */ int virDomainRestoreParams(virConnectPtr conn, - virTypedParameterPtr params, int nparams, + const char *path, + virTypedParameterPtr params, + int nparams, unsigned int flags) { - VIR_DEBUG("conn=%p, params=%p, nparams=%d, flags=0x%x", - conn, params, nparams, flags); + VIR_DEBUG("conn=%p, path=%s, params=%p, nparams=%d, flags=0x%x", + conn, path, params, nparams, flags); VIR_TYPED_PARAMS_DEBUG(params, nparams); virResetLastError(); virCheckConnectReturn(conn, -1); virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNullArgGoto(path, error); VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, VIR_DOMAIN_SAVE_PAUSED, error); if (conn->driver->domainRestoreParams) { - if (conn->driver->domainRestoreParams(conn, params, nparams, flags) < 0) + g_autofree char *absolute_path = NULL; + + /* We must absolutize the file path as the restore is done out of process */ + if (!(absolute_path = g_canonicalize_filename(path, NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not build absolute input file path")); goto error; + } + + if (conn->driver->domainRestoreParams(conn, absolute_path, + params, nparams, flags) < 0) + goto error; + return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0b31c73bb9..debf96db19 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5947,22 +5947,19 @@ qemuDomainRestore(virConnectPtr conn, static int qemuDomainRestoreParams(virConnectPtr conn, - virTypedParameterPtr params, int nparams, + const char *path, + virTypedParameterPtr params, + int nparams, unsigned int flags) { - const char *path = NULL; const char *dxml = NULL; int ret = -1; if (virTypedParamsValidate(params, nparams, - VIR_DOMAIN_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING, VIR_DOMAIN_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING, NULL) < 0) return -1; - if (virTypedParamsGetString(params, nparams, - VIR_DOMAIN_SAVE_PARAM_FILE, &path) < 0) - return -1; if (virTypedParamsGetString(params, nparams, VIR_DOMAIN_SAVE_PARAM_DXML, &dxml) < 0) return -1; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 085631c11b..2c7327c1e4 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1000,6 +1000,7 @@ struct remote_domain_restore_flags_args { }; struct remote_domain_restore_params_args { + remote_nonnull_string path; remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; unsigned int flags; }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4ffdce5679..75e86d48f4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -580,6 +580,7 @@ struct remote_domain_restore_flags_args { u_int flags; }; struct remote_domain_restore_params_args { + remote_nonnull_string path; struct { u_int params_len; remote_typed_param * params_val; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index a64ff3e73f..6acefa6b98 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -640,7 +640,7 @@ elsif ($mode eq "server") { # NB: if your new API starts with remote_typed_params, enter it here if you need # the conn arg to be passed first! - if ($call->{ProcName} eq "NodeSetMemoryParameters" || $call->{ProcName} eq "DomainRestoreParams") { + if ($call->{ProcName} eq "NodeSetMemoryParameters") { push(@args_list, $conn_var); } push(@args_list, "$1");