Re: [libvirt RFCv7 12/40] tools: make use of params in doSave

On Fri, May 06, 2022 at 03:19:31PM +0200, Claudio Fontana wrote: > whops this is pretty wrong. > > On 5/6/22 3:10 PM, Claudio Fontana wrote: >> Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; >> --- >> tools/virsh-domain.c | 25 ++++++++++++++++++------- >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c >> index ba492e807e..be5679df0f 100644 >> --- a/tools/virsh-domain.c >> +++ b/tools/virsh-domain.c >> @@ -4203,6 +4203,9 @@ doSave(void *opaque) >> g_autoptr(virshDomain) dom = NULL; >> const char *name = NULL; >> const char *to = NULL; >> + virTypedParameterPtr params = NULL; >> + int nparams = 0; >> + int maxparams = 0; >> unsigned int flags = 0; >> const char *xmlfile = NULL; >> g_autofree char *xml = NULL; >> @@ -4216,9 +4219,13 @@ doSave(void *opaque) >> goto out_sig; >> #endif /* !WIN32 */ >> >> - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) >> + if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) { >> goto out; >> - >> + } else { >> + if (virTypedParamsAddString(&params, &nparams, &maxparams, >> + VIR_SAVE_PARAM_FILE, to) < 0) >> + goto out; >> + } >> if (vshCommandOptBool(cmd, "bypass-cache")) >> flags |= VIR_DOMAIN_SAVE_BYPASS_CACHE; >> if (vshCommandOptBool(cmd, "running")) >> @@ -4232,14 +4239,17 @@ doSave(void *opaque) >> if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) >> goto out; >> >> - if (xmlfile && >> - virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) { >> - vshReportError(ctl); >> - goto out; >> + if (xmlfile) { >> + if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) { >> + vshReportError(ctl); >> + goto out; >> + } else if (virTypedParamsAddString(&params, &nparams, &maxparams, >> + VIR_SAVE_PARAM_DXML, xml) < 0) >> + goto out; >> } >> >> if (flags || xml) { >> - rc = virDomainSaveFlags(dom, to, xml, flags); >> + rc = virDomainSaveParams(dom, params, nparams, flags); >> } else { >> rc = virDomainSave(dom, to); > > Could you help me understand what is the right check to use here? > > Should I do like before, > > if (flags || xml) { > rc = virDomainSaveFlags(dom, to, xml, flags); > } else { > rc = virDomainSave(dom, to); > } > > and only involve virDomainSaveParams when introduced later? > > How should we decide whether to trigger virDomainSaveFlags > or virDomainSaveParams ...? virsh should always use the oldest API possible to achieve the task. IOW, only invoke virDomainSaveParams for a command line argument requires use of a parameter that the old API doesn't support. With regards, Daniel