1. Patchew
  2. Libvirt
  3. [PATCH 00/14] virsh: Completion improvements and checking tool
  4. View patch

[PATCH 10/14] virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh

Peter Krempa posted 14 patches 4 years, 4 months ago
[PATCH 10/14] virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh
Posted by Peter Krempa 4 years, 4 months ago 
In cases such as the APIs for managed save management, the file path
provided via the '--file' option is passed to the API.

We'll need to make them distinct from cases for when virsh is using the
file so that different completers can be used.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------
 tools/virsh-volume.c |  6 +++++-
 tools/virsh.h        |  1 +
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 05fa5c07f6..f45ab5b9d1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = {

 static const vshCmdOptDef opts_save[] = {
     VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
-    VIRSH_COMMON_OPT_FILE(N_("where to save the data")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("where to save the data")
+    },
     {.name = "bypass-cache",
      .type = VSH_OT_BOOL,
      .help = N_("avoid file system cache when saving")
@@ -4474,7 +4478,11 @@ static const vshCmdInfo info_save_image_dumpxml[] = {
 };

 static const vshCmdOptDef opts_save_image_dumpxml[] = {
-    VIRSH_COMMON_OPT_FILE(N_("saved state file to read")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("saved state file to read")
+    },
     {.name = "security-info",
      .type = VSH_OT_BOOL,
      .help = N_("include security sensitive information in XML dump")
@@ -4518,7 +4526,11 @@ static const vshCmdInfo info_save_image_define[] = {
 };

 static const vshCmdOptDef opts_save_image_define[] = {
-    VIRSH_COMMON_OPT_FILE(N_("saved state file to modify")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("saved state file to modify")
+    },
     {.name = "xml",
      .type = VSH_OT_DATA,
      .flags = VSH_OFLAG_REQ,
@@ -4581,7 +4593,11 @@ static const vshCmdInfo info_save_image_edit[] = {
 };

 static const vshCmdOptDef opts_save_image_edit[] = {
-    VIRSH_COMMON_OPT_FILE(N_("saved state file to edit")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("saved state file to edit")
+    },
     {.name = "running",
      .type = VSH_OT_BOOL,
      .help = N_("set domain to be running on restore")
@@ -5221,7 +5237,11 @@ static const vshCmdInfo info_restore[] = {
 };

 static const vshCmdOptDef opts_restore[] = {
-    VIRSH_COMMON_OPT_FILE(N_("the state to restore")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("the state to restore")
+    },
     {.name = "bypass-cache",
      .type = VSH_OT_BOOL,
      .help = N_("avoid file system cache when restoring")
@@ -5293,7 +5313,11 @@ static const vshCmdInfo info_dump[] = {

 static const vshCmdOptDef opts_dump[] = {
     VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
-    VIRSH_COMMON_OPT_FILE(N_("where to dump the core")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("where to dump the core")
+    },
     VIRSH_COMMON_OPT_LIVE(N_("perform a live core dump if supported")),
     {.name = "crash",
      .type = VSH_OT_BOOL,
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 197ed2489c..b73837f4c8 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -775,7 +775,11 @@ static const vshCmdInfo info_vol_download[] = {

 static const vshCmdOptDef opts_vol_download[] = {
     VIRSH_COMMON_OPT_VOL_FULL,
-    VIRSH_COMMON_OPT_FILE(N_("file")),
+    {.name = "file",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("file")
+    },
     VIRSH_COMMON_OPT_POOL_OPTIONAL,
     {.name = "offset",
      .type = VSH_OT_INT,
diff --git a/tools/virsh.h b/tools/virsh.h
index 4d777545ff..8e1b8ced90 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -95,6 +95,7 @@
      .help = _helpstr \
     }

+/* Use this only for files which are existing and used locally by virsh */
 #define VIRSH_COMMON_OPT_FILE(_helpstr) \
     {.name = "file", \
      .type = VSH_OT_DATA, \
-- 
2.31.1
Re: [PATCH 10/14] virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh
Posted by Michal Prívozník 4 years, 4 months ago 
On 9/16/21 7:10 PM, Peter Krempa wrote:
> In cases such as the APIs for managed save management, the file path
> provided via the '--file' option is passed to the API.
> 
> We'll need to make them distinct from cases for when virsh is using the
> file so that different completers can be used.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------
>  tools/virsh-volume.c |  6 +++++-
>  tools/virsh.h        |  1 +
>  3 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 05fa5c07f6..f45ab5b9d1 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = {
> 
>  static const vshCmdOptDef opts_save[] = {
>      VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
> -    VIRSH_COMMON_OPT_FILE(N_("where to save the data")),
> +    {.name = "file",
> +     .type = VSH_OT_DATA,
> +     .flags = VSH_OFLAG_REQ,
> +     .help = N_("where to save the data")
> +    },
>      {.name = "bypass-cache",
>       .type = VSH_OT_BOOL,
>       .help = N_("avoid file system cache when saving")

Maybe have new macro VIRSH_COMMON_OPT_FILE_REMOTE? If we ever come with
a completer for remote paths we have just one place to put .completer = XXX?

Michal
Re: [PATCH 10/14] virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh
Posted by Peter Krempa 4 years, 4 months ago 
On Fri, Sep 17, 2021 at 09:31:11 +0200, Michal Prívozník wrote:
> On 9/16/21 7:10 PM, Peter Krempa wrote:
> > In cases such as the APIs for managed save management, the file path
> > provided via the '--file' option is passed to the API.
> > 
> > We'll need to make them distinct from cases for when virsh is using the
> > file so that different completers can be used.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------
> >  tools/virsh-volume.c |  6 +++++-
> >  tools/virsh.h        |  1 +
> >  3 files changed, 36 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 05fa5c07f6..f45ab5b9d1 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = {
> > 
> >  static const vshCmdOptDef opts_save[] = {
> >      VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
> > -    VIRSH_COMMON_OPT_FILE(N_("where to save the data")),
> > +    {.name = "file",
> > +     .type = VSH_OT_DATA,
> > +     .flags = VSH_OFLAG_REQ,
> > +     .help = N_("where to save the data")
> > +    },
> >      {.name = "bypass-cache",
> >       .type = VSH_OT_BOOL,
> >       .help = N_("avoid file system cache when saving")
> 
> Maybe have new macro VIRSH_COMMON_OPT_FILE_REMOTE? If we ever come with
> a completer for remote paths we have just one place to put .completer = XXX?

I'm not a fan of this kind of macros as they aren't very flexible and
the number of lines saved is IMO not worth it in this case.

On the other hand I've briefly thought about a dummy 'Remote' version of
the file completer to annotate the rest of the file input fields even
without implementing it, but that's a fairly trivial task once the
completer is ready.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.