[PATCH] xl: rename "list-vm" command to "vm-list"

[PATCH] xl: rename "list-vm" command to "vm-list"

[Matt Wilson]

All of the other "list" verbs are of the form "$noun-list". For
example: "pci-list", "vcpu-list", "network-list", "block-list", etc.

Additionally, many people have well trained muscle memory from years
of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
in "command not implemented".

Finally, this command was missing from the xl man page.

Signed-off-by: Matt Wilson <msw@amazon.com>

diff -r 32034d1914a6 -r 5b1ed71c74d6 docs/man/xl.pod.1
--- a/docs/man/xl.pod.1	Thu Jun 07 19:46:57 2012 +0100
+++ b/docs/man/xl.pod.1	Thu Jun 28 06:34:26 2012 +0000
@@ -617,6 +617,18 @@ different run state is appropriate.  Pin
 this, by ensuring certain VCPUs can only run on certain physical
 CPUs.
 
+=item B<vm-list>
+
+Prints information about all domains except for dom0.
+
+B<EXAMPLE>
+
+An example format for the list is as follows:
+
+UUID                                  ID    name
+59e1cf6c-6ab9-4879-90e7-adc8d1c63bf5  2    win
+50bc8f75-81d0-4d53-b2e6-95cb44e2682e  3    linux
+
 =item B<vncviewer> [I<OPTIONS>] I<domain-id>
 
 Attach to domain's VNC server, forking a vncviewer process.
diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl.h
--- a/tools/libxl/xl.h	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl.h	Thu Jun 28 06:34:26 2012 +0000
@@ -54,7 +54,7 @@ int main_destroy(int argc, char **argv);
 int main_shutdown(int argc, char **argv);
 int main_reboot(int argc, char **argv);
 int main_list(int argc, char **argv);
-int main_list_vm(int argc, char **argv);
+int main_vm_list(int argc, char **argv);
 int main_create(int argc, char **argv);
 int main_config_update(int argc, char **argv);
 int main_button_press(int argc, char **argv);
diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Thu Jun 28 06:34:26 2012 +0000
@@ -3623,11 +3623,11 @@ int main_list(int argc, char **argv)
     return 0;
 }
 
-int main_list_vm(int argc, char **argv)
+int main_vm_list(int argc, char **argv)
 {
     int opt;
 
-    if ((opt = def_getopt(argc, argv, "", "list-vm", 0)) != -1)
+    if ((opt = def_getopt(argc, argv, "", "vm-list", 0)) != -1)
         return opt;
 
     list_vm();
diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c	Thu Jun 28 06:34:26 2012 +0000
@@ -214,9 +214,9 @@ struct cmd_spec cmd_table[] = {
       "Set the number of active VCPUs allowed for the domain",
       "<Domain> <vCPUs>",
     },
-    { "list-vm",
-      &main_list_vm, 0, 0,
-      "List the VMs,without DOM0",
+    { "vm-list",
+      &main_vm_list, 0, 0,
+      "List the VMs, without DOM0",
       "",
     },
     { "info",

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Campbell]

On Thu, 2012-06-28 at 08:59 +0100, Matt Wilson wrote:
> All of the other "list" verbs are of the form "$noun-list". For
> example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> 
> Additionally, many people have well trained muscle memory from years
> of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> in "command not implemented".
> 
> Finally, this command was missing from the xl man page.
> 
> Signed-off-by: Matt Wilson <msw@amazon.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think this appropriate for 4.2.

> 
> diff -r 32034d1914a6 -r 5b1ed71c74d6 docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1	Thu Jun 07 19:46:57 2012 +0100
> +++ b/docs/man/xl.pod.1	Thu Jun 28 06:34:26 2012 +0000
> @@ -617,6 +617,18 @@ different run state is appropriate.  Pin
>  this, by ensuring certain VCPUs can only run on certain physical
>  CPUs.
>  
> +=item B<vm-list>
> +
> +Prints information about all domains except for dom0.
> +
> +B<EXAMPLE>
> +
> +An example format for the list is as follows:
> +
> +UUID                                  ID    name
> +59e1cf6c-6ab9-4879-90e7-adc8d1c63bf5  2    win
> +50bc8f75-81d0-4d53-b2e6-95cb44e2682e  3    linux
> +
>  =item B<vncviewer> [I<OPTIONS>] I<domain-id>
>  
>  Attach to domain's VNC server, forking a vncviewer process.
> diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl.h
> --- a/tools/libxl/xl.h	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl.h	Thu Jun 28 06:34:26 2012 +0000
> @@ -54,7 +54,7 @@ int main_destroy(int argc, char **argv);
>  int main_shutdown(int argc, char **argv);
>  int main_reboot(int argc, char **argv);
>  int main_list(int argc, char **argv);
> -int main_list_vm(int argc, char **argv);
> +int main_vm_list(int argc, char **argv);
>  int main_create(int argc, char **argv);
>  int main_config_update(int argc, char **argv);
>  int main_button_press(int argc, char **argv);
> diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Jun 28 06:34:26 2012 +0000
> @@ -3623,11 +3623,11 @@ int main_list(int argc, char **argv)
>      return 0;
>  }
>  
> -int main_list_vm(int argc, char **argv)
> +int main_vm_list(int argc, char **argv)
>  {
>      int opt;
>  
> -    if ((opt = def_getopt(argc, argv, "", "list-vm", 0)) != -1)
> +    if ((opt = def_getopt(argc, argv, "", "vm-list", 0)) != -1)
>          return opt;
>  
>      list_vm();
> diff -r 32034d1914a6 -r 5b1ed71c74d6 tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl_cmdtable.c	Thu Jun 28 06:34:26 2012 +0000
> @@ -214,9 +214,9 @@ struct cmd_spec cmd_table[] = {
>        "Set the number of active VCPUs allowed for the domain",
>        "<Domain> <vCPUs>",
>      },
> -    { "list-vm",
> -      &main_list_vm, 0, 0,
> -      "List the VMs,without DOM0",
> +    { "vm-list",
> +      &main_vm_list, 0, 0,
> +      "List the VMs, without DOM0",
>        "",
>      },
>      { "info",

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Jackson]

Matt Wilson writes ("[Xen-devel] [PATCH] xl: rename "list-vm" command to "vm-list""):
> diff -r 32034d1914a6 -r 5b1ed71c74d6 docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1	Thu Jun 07 19:46:57 2012 +0100
> +++ b/docs/man/xl.pod.1	Thu Jun 28 06:34:26 2012 +0000
> @@ -617,6 +617,18 @@ different run state is appropriate.  Pin
>  this, by ensuring certain VCPUs can only run on certain physical
>  CPUs.
>  
> +=item B<vm-list>
> +
> +Prints information about all domains except for dom0.

Doesn't it also exclude dm stubdoms, service domains (stub xenstored),
etc. ?  IMO it should, and the docs should say so.

If it doesn't then that's IMO a bug but stubdoms are a bit buggy
anyway so I don't regard fixing that as a blocker for this patch.  But
I think we should introduce docs that are correct.

If you and Ian agree, perhaps you'd like to clarify that (and perhaps
change the usage message too).

Thanks,
Ian.

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Campbell]

On Thu, 2012-06-28 at 16:08 +0100, Ian Jackson wrote:
> Matt Wilson writes ("[Xen-devel] [PATCH] xl: rename "list-vm" command to "vm-list""):
> > diff -r 32034d1914a6 -r 5b1ed71c74d6 docs/man/xl.pod.1
> > --- a/docs/man/xl.pod.1	Thu Jun 07 19:46:57 2012 +0100
> > +++ b/docs/man/xl.pod.1	Thu Jun 28 06:34:26 2012 +0000
> > @@ -617,6 +617,18 @@ different run state is appropriate.  Pin
> >  this, by ensuring certain VCPUs can only run on certain physical
> >  CPUs.
> >  
> > +=item B<vm-list>
> > +
> > +Prints information about all domains except for dom0.
> 
> Doesn't it also exclude dm stubdoms, service domains (stub xenstored),
> etc. ?  IMO it should, and the docs should say so.

Usually we would say "guest" rather than "domain" to convey this (a
guest might consist of multiple domains).

> If it doesn't then that's IMO a bug but stubdoms are a bit buggy
> anyway so I don't regard fixing that as a blocker for this patch.  But
> I think we should introduce docs that are correct.
> 
> If you and Ian agree, perhaps you'd like to clarify that (and perhaps
> change the usage message too).

I'm happy for the docs to change to reflect what we want but I don't
want to delay this patch actually making the implementation match.

Ian.

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: rename "list-vm" command to "vm-list""):
> On Thu, 2012-06-28 at 16:08 +0100, Ian Jackson wrote:
> > Doesn't it also exclude dm stubdoms, service domains (stub xenstored),
> > etc. ?  IMO it should, and the docs should say so.
> 
> Usually we would say "guest" rather than "domain" to convey this (a
> guest might consist of multiple domains).

Yes, that's the right terminology.

> > If it doesn't then that's IMO a bug but stubdoms are a bit buggy
> > anyway so I don't regard fixing that as a blocker for this patch.  But
> > I think we should introduce docs that are correct.
> > 
> > If you and Ian agree, perhaps you'd like to clarify that (and perhaps
> > change the usage message too).
> 
> I'm happy for the docs to change to reflect what we want but I don't
> want to delay this patch actually making the implementation match.

Exactly.

Ian.

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Matt Wilson]

On Thu, Jun 28, 2012 at 08:08:11AM -0700, Ian Jackson wrote:
> Matt Wilson writes ("[Xen-devel] [PATCH] xl: rename "list-vm" command to "vm-list""):
> > diff -r 32034d1914a6 -r 5b1ed71c74d6 docs/man/xl.pod.1
> > --- a/docs/man/xl.pod.1	Thu Jun 07 19:46:57 2012 +0100
> > +++ b/docs/man/xl.pod.1	Thu Jun 28 06:34:26 2012 +0000
> > @@ -617,6 +617,18 @@ different run state is appropriate.  Pin
> >  this, by ensuring certain VCPUs can only run on certain physical
> >  CPUs.
> >  
> > +=item B<vm-list>
> > +
> > +Prints information about all domains except for dom0.
> 
> Doesn't it also exclude dm stubdoms, service domains (stub xenstored),
> etc. ?  IMO it should, and the docs should say so.

The command uses libxl_list_vm(), which does skip stubdoms by checking
libxl_is_stubdom()

> If it doesn't then that's IMO a bug but stubdoms are a bit buggy
> anyway so I don't regard fixing that as a blocker for this patch.  But
> I think we should introduce docs that are correct.
> 
> If you and Ian agree, perhaps you'd like to clarify that (and perhaps
> change the usage message too).

v3, coming up.

Matt

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Campbell]

On Thu, 2012-06-28 at 08:37 +0100, Matt Wilson wrote:
> On Thu, Jun 28, 2012 at 12:36:17AM -0700, Ian Campbell wrote:
> > > > On Thu, 2012-06-28 at 07:46 +0100, Matt Wilson wrote:
> > > > > All of the other "list" verbs are of the form "$noun-list". For
> > > > > example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> > > > > Additionally, many people have well trained muscle memory from years
> > > > > of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> > > > > in "command not implemented".
> > > > 
> > > > Which did xm have list-vm or vm-list (or neither?) 
> > > 
> > > Neither.
> > 
> > In which case I would ack the patch except you forgot to update
> > docs/man/xl*.pod.? (hrm, somehow list-vm isn't there -- can you add
> > vm-list anyway?)
> 
> I grepped for list-vm in the whole xen source tree, trying to make
> sure that I updated any corresponding docs. I guess that means I
> didn't notice it not being documented at all. :-P

I only double checked because I'm pretty sure xl.pod.1 was started with
a grep over xl_cmdtable.c so I can't see how we missed it...

> v2 in a moment.

Thanks!

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Matt Wilson]

On Thu, Jun 28, 2012 at 12:36:17AM -0700, Ian Campbell wrote:
> > > On Thu, 2012-06-28 at 07:46 +0100, Matt Wilson wrote:
> > > > All of the other "list" verbs are of the form "$noun-list". For
> > > > example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> > > > Additionally, many people have well trained muscle memory from years
> > > > of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> > > > in "command not implemented".
> > > 
> > > Which did xm have list-vm or vm-list (or neither?) 
> > 
> > Neither.
> 
> In which case I would ack the patch except you forgot to update
> docs/man/xl*.pod.? (hrm, somehow list-vm isn't there -- can you add
> vm-list anyway?)

I grepped for list-vm in the whole xen source tree, trying to make
sure that I updated any corresponding docs. I guess that means I
didn't notice it not being documented at all. :-P

v2 in a moment.

Matt

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Campbell]

On Thu, 2012-06-28 at 08:28 +0100, Matt Wilson wrote:
> On Wed, Jun 27, 2012 at 11:59:16PM -0700, Ian Campbell wrote:
> > On Thu, 2012-06-28 at 07:46 +0100, Matt Wilson wrote:
> > > All of the other "list" verbs are of the form "$noun-list". For
> > > example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> > > Additionally, many people have well trained muscle memory from years
> > > of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> > > in "command not implemented".
> > 
> > Which did xm have list-vm or vm-list (or neither?) 
> 
> Neither.

In which case I would ack the patch except you forgot to update
docs/man/xl*.pod.? (hrm, somehow list-vm isn't there -- can you add
vm-list anyway?)

Ian.

> 
> > Aside: I'd love for someone to implement (for 4.3) a better error
> > message than "command not implemented". At the least printing out the
> > list of clashing options, or even git style "Did you mean" when you make
> > a typo.
> 
> That would definitely be nice to have.
> 
> Matt

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Matt Wilson]

On Wed, Jun 27, 2012 at 11:59:16PM -0700, Ian Campbell wrote:
> On Thu, 2012-06-28 at 07:46 +0100, Matt Wilson wrote:
> > All of the other "list" verbs are of the form "$noun-list". For
> > example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> > Additionally, many people have well trained muscle memory from years
> > of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> > in "command not implemented".
> 
> Which did xm have list-vm or vm-list (or neither?) 

Neither.

> Aside: I'd love for someone to implement (for 4.3) a better error
> message than "command not implemented". At the least printing out the
> list of clashing options, or even git style "Did you mean" when you make
> a typo.

That would definitely be nice to have.

Matt

Re: [PATCH] xl: rename "list-vm" command to "vm-list"

[Ian Campbell]

On Thu, 2012-06-28 at 07:46 +0100, Matt Wilson wrote:
> All of the other "list" verbs are of the form "$noun-list". For
> example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
> Additionally, many people have well trained muscle memory from years
> of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
> in "command not implemented".

Which did xm have list-vm or vm-list (or neither?) 

Aside: I'd love for someone to implement (for 4.3) a better error
message than "command not implemented". At the least printing out the
list of clashing options, or even git style "Did you mean" when you make
a typo.

> 
> Signed-off-by: Matt Wilson <msw@amazon.com>
> 
> diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl.h
> --- a/tools/libxl/xl.h	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl.h	Thu Jun 28 06:34:26 2012 +0000
> @@ -54,7 +54,7 @@ int main_destroy(int argc, char **argv);
>  int main_shutdown(int argc, char **argv);
>  int main_reboot(int argc, char **argv);
>  int main_list(int argc, char **argv);
> -int main_list_vm(int argc, char **argv);
> +int main_vm_list(int argc, char **argv);
>  int main_create(int argc, char **argv);
>  int main_config_update(int argc, char **argv);
>  int main_button_press(int argc, char **argv);
> diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Jun 28 06:34:26 2012 +0000
> @@ -3623,11 +3623,11 @@ int main_list(int argc, char **argv)
>      return 0;
>  }
>  
> -int main_list_vm(int argc, char **argv)
> +int main_vm_list(int argc, char **argv)
>  {
>      int opt;
>  
> -    if ((opt = def_getopt(argc, argv, "", "list-vm", 0)) != -1)
> +    if ((opt = def_getopt(argc, argv, "", "vm-list", 0)) != -1)
>          return opt;
>  
>      list_vm();
> diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c	Thu Jun 07 19:46:57 2012 +0100
> +++ b/tools/libxl/xl_cmdtable.c	Thu Jun 28 06:34:26 2012 +0000
> @@ -214,9 +214,9 @@ struct cmd_spec cmd_table[] = {
>        "Set the number of active VCPUs allowed for the domain",
>        "<Domain> <vCPUs>",
>      },
> -    { "list-vm",
> -      &main_list_vm, 0, 0,
> -      "List the VMs,without DOM0",
> +    { "vm-list",
> +      &main_vm_list, 0, 0,
> +      "List the VMs, without DOM0",
>        "",
>      },
>      { "info",

[PATCH] xl: rename "list-vm" command to "vm-list"

[Matt Wilson]

All of the other "list" verbs are of the form "$noun-list". For
example: "pci-list", "vcpu-list", "network-list", "block-list", etc.
Additionally, many people have well trained muscle memory from years
of typing "xm li". "xl li" was ambiguous due to "xl list-vm" resulted
in "command not implemented".

Signed-off-by: Matt Wilson <msw@amazon.com>

diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl.h
--- a/tools/libxl/xl.h	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl.h	Thu Jun 28 06:34:26 2012 +0000
@@ -54,7 +54,7 @@ int main_destroy(int argc, char **argv);
 int main_shutdown(int argc, char **argv);
 int main_reboot(int argc, char **argv);
 int main_list(int argc, char **argv);
-int main_list_vm(int argc, char **argv);
+int main_vm_list(int argc, char **argv);
 int main_create(int argc, char **argv);
 int main_config_update(int argc, char **argv);
 int main_button_press(int argc, char **argv);
diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Thu Jun 28 06:34:26 2012 +0000
@@ -3623,11 +3623,11 @@ int main_list(int argc, char **argv)
     return 0;
 }
 
-int main_list_vm(int argc, char **argv)
+int main_vm_list(int argc, char **argv)
 {
     int opt;
 
-    if ((opt = def_getopt(argc, argv, "", "list-vm", 0)) != -1)
+    if ((opt = def_getopt(argc, argv, "", "vm-list", 0)) != -1)
         return opt;
 
     list_vm();
diff -r 32034d1914a6 -r 34e47ba2612e tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c	Thu Jun 28 06:34:26 2012 +0000
@@ -214,9 +214,9 @@ struct cmd_spec cmd_table[] = {
       "Set the number of active VCPUs allowed for the domain",
       "<Domain> <vCPUs>",
     },
-    { "list-vm",
-      &main_list_vm, 0, 0,
-      "List the VMs,without DOM0",
+    { "vm-list",
+      &main_vm_list, 0, 0,
+      "List the VMs, without DOM0",
       "",
     },
     { "info",