Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
On Thu, Mar 21, 2013 at 09:43:02PM -0600, Eric Blake wrote: Expanding everything means re-implementing what the shell does. wordexp() would be ideal for this, except that wordexp() is not portable enough. Maybe wordexp() could be used on platform which support it, and on platforms that do not have it, it would not be used, and env var would not be expanded? Or do we want to have identical behaviour for virsh on all platforms to avoid confusion? Christophe pgpYZlfTGRBhh.pgp Description: PGP signature -- 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
On 03/22/2013 03:34 AM, Christophe Fergeau wrote: On Thu, Mar 21, 2013 at 09:43:02PM -0600, Eric Blake wrote: Expanding everything means re-implementing what the shell does. wordexp() would be ideal for this, except that wordexp() is not portable enough. Maybe wordexp() could be used on platform which support it, and on platforms that do not have it, it would not be used, and env var would not be expanded? Or do we want to have identical behaviour for virsh on all platforms to avoid confusion? I think the confusion aspect must not be underestimated. If I write a virsh script on Linux, and then port it to a Windows machine, I would still want it to have the same effect on my guests. But if I relied on expansions, which happened to work on Linux but are missing on mingw, my script would change semantics. Also, we strive hard to avoid backward-incompatible changes; a script that was written before expansions, but which used unquoted '$', should still work after expansions are added. Maybe we could make expansions be off by default, and provide a new virsh command to opt-in; but at that point, we are back to the question of why not script the shell to do it in the first place. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- 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
On Thu, Mar 21, 2013 at 05:00:58PM +0800, Zhang Xiaohe wrote: This patch makes '~' and '$HOME' can be recognized by virsh in interactive mode. These two variables are replaced with real path. If we're going to the trouble of expanding $HOME, then we might as well just make it expand arbitrary environment variables rather than hardcoding $HOME. Also, we'll need a way to escape the special meaning of '~' and '$' to get them treated as literal characters instead of special characters. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- 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
On 03/21/2013 04:20 AM, Daniel P. Berrange wrote: On Thu, Mar 21, 2013 at 05:00:58PM +0800, Zhang Xiaohe wrote: This patch makes '~' and '$HOME' can be recognized by virsh in interactive mode. These two variables are replaced with real path. If we're going to the trouble of expanding $HOME, then we might as well just make it expand arbitrary environment variables rather than hardcoding $HOME. 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. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- 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
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. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- 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
On 03/21/2013 09:18 PM, Zhang Xiaohe wrote: 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 Expanding just '~' and '$HOME' but nothing else is a disservice to the user. If we're going to expand anything, then we should be consistent and allow for the expansion of all variables. 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 ? 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 -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- 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