1. Patchew
  2. Libvirt
  3. [libvirt RFCv7 00/40] multifd save restore prototype
  4. View patch

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

Claudio Fontana posted 40 patches 3 years, 9 months ago
There is a newer version of this series
[libvirt RFCv7 12/40] tools: make use of params in doSave
Posted by Claudio Fontana 3 years, 9 months ago 
Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 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);
     }
@@ -4252,6 +4262,7 @@ doSave(void *opaque)
     data->ret = 0;
 
  out:
+    virTypedParamsFree(params, nparams);
 #ifndef WIN32
     pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL);
  out_sig:
-- 
2.35.3
Re: [libvirt RFCv7 12/40] tools: make use of params in doSave
Posted by Claudio Fontana 3 years, 9 months ago 
whops this is pretty wrong.

On 5/6/22 3:10 PM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  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 ...?


>      }
> @@ -4252,6 +4262,7 @@ doSave(void *opaque)
>      data->ret = 0;
>  
>   out:
> +    virTypedParamsFree(params, nparams);
>  #ifndef WIN32
>      pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL);
>   out_sig:
>
Re: [libvirt RFCv7 12/40] tools: make use of params in doSave
Posted by Daniel P. Berrangé 3 years, 9 months ago 
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@suse.de>
> > ---
> >  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
-- 
|: 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 :|
Re: [libvirt RFCv7 12/40] tools: make use of params in doSave
Posted by Claudio Fontana 3 years, 9 months ago 
On 5/6/22 3:24 PM, Daniel P. Berrangé wrote:
> 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@suse.de>
>>> ---
>>>  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
> 

Very helpful, thanks.

Claudio

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.