Re: [libvirt] Remove OPTION section in output of 'virsh help command' if no option exists.
于 2013年05月23日 06:19, Eric Blake 写道:
On 05/21/2013 09:15 PM, Zhang Xiaohe wrote:
Don't print 'OPTION' if there's no options. Just behaves as DESCRIPTION
does.
This mostly affects 'interface' command group.
Signed-off-by: Zhang Xiaohezhan...@cn.fujitsu.com
Reported-by: Li Yangliyang.f...@cn.fujitsu.com
---
tools/virsh.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
For some reason, the patch didn't apply for me with 'git am', so I had
to do it by hand; in the process, I simplified slightly.
diff --git a/tools/virsh.c b/tools/virsh.c
index ecb7bd4..7c60800 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1270,7 +1270,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
if (def-opts) {
const vshCmdOptDef *opt;
-fputs(_(\n OPTIONS\n), stdout);
+/* Print the option only if there are options */
+if (def-opts-name)
+fputs(_(\n OPTIONS\n), stdout);
Hmm, I wonder why we even bother to create 1-element arrays with a NULL
terminator instead of passing NULL when registering option-less
functions, on commands like 'iface-commit'. But your idea is fine. ACK
and here's what I pushed, after tweaking the subject line to be shorter:
diff --git a/tools/virsh.c b/tools/virsh.c
index ecb7bd4..6f0c1ef 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1268,7 +1268,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
fprintf(stdout, %s\n, _(desc));
}
-if (def-opts) {
+if (def-opts def-opts-name) {
const vshCmdOptDef *opt;
fputs(_(\n OPTIONS\n), stdout);
for (opt = def-opts; opt-name; opt++) {
Thanks, this looks more pretty~
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Remove OPTION section in output of 'virsh help command' if no option exists.
Don't print 'OPTION' if there's no options. Just behaves as DESCRIPTION
does.
This mostly affects 'interface' command group.
Signed-off-by: Zhang Xiaohe zhan...@cn.fujitsu.com
Reported-by: Li Yang liyang.f...@cn.fujitsu.com
---
tools/virsh.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ecb7bd4..7c60800 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1270,7 +1270,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
if (def-opts) {
const vshCmdOptDef *opt;
-fputs(_(\n OPTIONS\n), stdout);
+/* Print the option only if there are options */
+if (def-opts-name)
+fputs(_(\n OPTIONS\n), stdout);
for (opt = def-opts; opt-name; opt++) {
switch (opt-type) {
case VSH_OT_BOOL:
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [question] Why doesn't virsh command support an optional parameter
hi guys, It's hard for me to understand why doesn't virsh command support an optional parameter because I think omitting a parameter and offering a default value sometimes is quite convenient. For example: $ virsh shutdown guest --mode acpi The option --mode only accept string acpi, agent, initctl and signal, assuming that acpi is the most frequently used parameter when --mode is specified, why not just using $ virsh shutdown guest --mode acpi instead. Maybe this example is not very precise, but what I mean is if there is an option which only accepts several candidate values and one of the value is always used when it is specified, then we can make it the default parameter of this option. This would be convenient and save some typing. Of course, this can also result in ambiguity to people who is not familiar with the command. But with the help, it won't trap one very long, will it? So, why doesn't virsh support this and what do you think of adding this feature? Thanks Zhang Xiaohe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [question] Why doesn't virsh command support an optional parameter
于 2013年04月26日 11:35, Eric Blake 写道: On 04/25/2013 09:19 PM, Zhang Xiaohe wrote: hi guys, It's hard for me to understand why doesn't virsh command support an optional parameter because I think omitting a parameter and offering a default value sometimes is quite convenient. For example: $ virsh shutdown guest --mode acpi Did you mean 'virsh shutdown guest --mode' here, with a missing trailing argument to the --mode option? Sorry, I intended to say that use 'virsh shutdown guest --mode' instead of 'virsh shutdown guest --mode acpi' The option --mode only accept string acpi, agent, initctl and signal, assuming that acpi is the most frequently used parameter when --mode is specified, why not just using $ virsh shutdown guest --mode acpi instead. The default is to not specify --mode at all; if you are worried about a mode, then there is no way for virsh to know what mode is best for you. In this particular example, while acpi works for qemu, it does not work for lxc; there, the best default is more likely to be initctl. Since we can't make a different default per hypervisor without repeating all the logic of a hypervisor in virsh, this seems like a change that's not worth making. I use this example to describe only the format, so just ignore what mode really means. Also, consider what it would take to implement optional option parsing. Existing scripts that used --mode=acpi are immune, but scripts that used '--mode acpi' as two arguments now have a problem - how do we know that the user didn't intend '--mode' with its default setting, and 'acpi' as a separate argument? This does be a problem. Maybe this example is not very precise, but what I mean is if there is an option which only accepts several candidate values and one of the value is always used when it is specified, then we can make it the default parameter of this option. This would be convenient and save some typing. Of course, this can also result in ambiguity to people who is not familiar with the command. But with the help, it won't trap one very long, will it? I'm unwilling to make any current option that requires an argument switch to becoming an option with an optional argument (because that in itself introduces parser ambiguities that may break scrips written against an older virsh). OK, I see. However, there IS a request to add user-defined alias support to virsh. If you find yourself frequently typing a given command, it would make sense to have virsh have a way to let you define an alias that shortens the amount of typing you have to do to get that alias. It wouldn't be default out of the box, but may be a compromise that could help the situation you are worried about. For virsh aliases to work, someone would have to submit patches - is that something you are interested in writing? My members and I currently are writing a new feature for qemu and this needs to add an option to one virsh command. We are discussing whether a default value can be offered with an optional parameter because one of the values is always valid while the others are not. But now we'll try another way. Thanks, Zhang Xiaohe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
This patch makes '~' and '$HOME' can be recognized by virsh in
interactive mode. These two variables are replaced with real
path.
eg:
virsh # pwd
/home/libvirt
virsh # cd ~/rpmbuild
virsh # pwd
/root/rpmbuild
see https://bugzilla.redhat.com/show_bug.cgi?id=806793
Signed-off-by: Zhang Xiaohe zhan...@cn.fujitsu.com
---
tools/virsh.c | 25 +
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index b574d7e..5c8df6b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1232,6 +1232,27 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
* ---
*/
static void
+vshExpandPath(vshControl *ctl, char **tkdata)
+{
+char *argstr = NULL;
+char *buf = NULL;
+char *p = NULL;
+const char *home = getenv(HOME);
+size_t len = strlen(home) + strlen(*tkdata);
+
+buf = vshMalloc(ctl, len);
+p = buf;
+buf = virStrcpy(buf, home, len);
+argstr = strchr(*tkdata, '/');
+if (argstr) {
+buf += strlen(home);
+buf = virStrcpy(buf, argstr, strlen(*tkdata));
+}
+VIR_FREE(*tkdata);
+*tkdata = p;
+}
+
+static void
vshCommandOptFree(vshCmdOpt * arg)
{
vshCmdOpt *a = arg;
@@ -1855,6 +1876,10 @@ get_data:
/* save option */
vshCmdOpt *arg = vshMalloc(ctl, sizeof(vshCmdOpt));
+/* replace the ~ or $HOME with real path */
+if (tkdata[0] == '~' || STRPREFIX(tkdata, $HOME))
+vshExpandPath(ctl, tkdata);
+
arg-def = opt;
arg-data = tkdata;
arg-next = NULL;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
于 2013年03月21日 21:08, Eric Blake 写道: On 03/21/2013 04:20 AM, Daniel P. Berrange wrote: In other words, if we're going to do this, go all the way and use wordexp() to get shell-like expansion, instead of reinventing it ourselves. Except that wordexp() is not portable to mingw, and not provided in gnulib. Also, we'll need a way to escape the special meaning of '~' and '$' to get them treated as literal characters instead of special characters. We already have the ability to quote characters, so that we can embed spaces; our quoting rules are (intentionally) copied on shell rules, so they would still work with a wordexp() approach. This seems better than just expanding $HOME, i will try this wordexp(). One question, is variable can be accepted in the position of command and option? That is, is this form virsh # $VAR --$OPT=~/rpmbuild could be valid? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
于 2013年03月22日 10:17, Eric Blake 写道: On 03/21/2013 07:33 PM, Zhang Xiaohe wrote: 于 2013年03月21日 21:08, Eric Blake 写道: On 03/21/2013 04:20 AM, Daniel P. Berrange wrote: In other words, if we're going to do this, go all the way and use wordexp() to get shell-like expansion, instead of reinventing it ourselves. Except that wordexp() is not portable to mingw, and not provided in gnulib. Also, we'll need a way to escape the special meaning of '~' and '$' to get them treated as literal characters instead of special characters. We already have the ability to quote characters, so that we can embed spaces; our quoting rules are (intentionally) copied on shell rules, so they would still work with a wordexp() approach. This seems better than just expanding $HOME, i will try this wordexp(). One question, is variable can be accepted in the position of command and option? That is, is this form virsh # $VAR --$OPT=~/rpmbuild could be valid? wordexp() is not portable to mingw, and not provided by gnulib. If you try to use wordexp(), you will basically be re-writing a big chunk of /bin/sh. At this point, I'm not sure it's worth the complexity. Interactive virsh does not need to be a full-blown shell. From the command line, you already have the shell parsing things before handing it to virsh. Originally, I think '~' and '$HOME' is most commonly used, so it should be acceptable to just expand these. But now I'm confused. You said if we're going to do this, go all the way and Interactive virsh does not need to be a full-blown shell. I'm not sure but are you suggesting that no need to add this expansion ? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
于 2013年03月22日 11:43, Eric Blake 写道: Expanding everything means re-implementing what the shell does. wordexp() would be ideal for this, except that wordexp() is not portable enough. By the time we end up rewriting enough code to do what wordexp() already could do, we are adding lots of bloat into virsh to make it mimic what /bin/sh can already do. Ergo, I think that this patch idea is not worth it. If a user wants shell expansions, they should let the shell do it, when building up the virsh command line. That is, instead of trying to do expansions in a virsh interactive session: $ virsh virsh# echo $HOME you should just let the shell do it beforehand: $ virsh echo $HOME Thanks for explanation, I get it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
