[libvirt] [PATCH] virsh: Treat \n like ; in batch mode
I wanted to do a demonstration with virsh batch mode, which takes multiple commands all packed into a single argument: $ virsh -c test:///default 'echo a; echo b;' a b but that produced a really long line, so I tried to make it more legible: $ virsh -c test:///default ' echo a; echo b; ' error: unknown command: ' ' Let's be more like the shell, and treat unquoted newline as a command separator just as we do for semicolon. In fact, with that, I can even now mix styles: $ virsh -c test:///default ' echo a; echo b echo c ' a b c Fix the grammer in a nearby comment while at it. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh.pod | 4 ++-- tools/virt-admin.pod | 4 ++-- tools/vsh.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 67edb57b14..2bf1ee77bb 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -40,8 +40,8 @@ as a name. The B<virsh> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions -and their arguments joined with whitespace, and separated by semicolons -between commands. Within I<COMMAND_STRING>, virsh understands the +and their arguments joined with whitespace, and separated by semicolons or +newlines between commands. Within I<COMMAND_STRING>, virsh understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virsh> will then start a minimal diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 3ddbbff934..b2c9a1db6d 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -23,8 +23,8 @@ Where I<command> is one of the commands listed below. The B<virt-admin> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions -and their arguments joined with whitespace, and separated by semicolons -between commands. Within I<COMMAND_STRING>, virt-admin understands the +and their arguments joined with whitespace, and separated by semicolons or +newlines between commands. Within I<COMMAND_STRING>, virt-admin understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virt-admin> will then start a minimal diff --git a/tools/vsh.c b/tools/vsh.c index 610de4495b..3d9bec896b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1664,7 +1664,7 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, if (*p == '\0') return VSH_TK_END; - if (*p == ';') { + if (*p == ';' || *p == '\n') { parser->pos = ++p; /* = \0 or begin of next command */ return VSH_TK_SUBCMD_END; } @@ -1672,7 +1672,7 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, while (*p) { /* end of token is blank space or ';' */ if (!double_quote && !single_quote && - (*p == ' ' || *p == '\t' || *p == ';')) + (*p == ' ' || *p == '\t' || *p == ';' || *p == '\n')) break; if (!double_quote && *p == '\'') { /* single quote */ @@ -1681,7 +1681,7 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, continue; } else if (!single_quote && *p == '\\') { /* escape */ /* - * The same as the bash, a \ in "" is an escaper, + * The same as in shell, a \ in "" is an escaper, * but a \ in '' is not an escaper. */ p++; -- 2.20.1
On 2/21/19 12:55 PM, Eric Blake wrote:
I wanted to do a demonstration with virsh batch mode, which takes multiple commands all packed into a single argument:
Let's be more like the shell, and treat unquoted newline as a command separator just as we do for semicolon. In fact, with that, I can even now mix styles:
$ virsh -c test:///default ' echo a; echo b echo c ' a b c
Hmm - if we REALLY want to be more like the shell, then we should also fix the parser to elide backslash-newline pairs, to allow splitting long commands without turning them into multiple commands. Right now, we are treating them as quoted newlines (here, the second echo is called with four arguments, 'b', $'\n', 'echo', and 'c'), rather than as command separators (three echos) or elided (the second echo would have just three arguments, 'b', 'echo', 'c'): $ tools/virsh -c test:///default ' echo a; echo b \ echo c ' a b echo c I guess that can be a followup patch.
Fix the grammer in a nearby comment while at it.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh.pod | 4 ++-- tools/virt-admin.pod | 4 ++-- tools/vsh.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The previous patch made it possible to split multiple commands by adding newline, but not to split a long single command. The sequence backslash-newline was being used as if it were a quoted newline character, rather than completely elided the way the shell does. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh.pod | 4 +++- tools/virt-admin.pod | 3 ++- tools/vsh.c | 8 ++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 2bf1ee77bb..4057edf3a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -41,7 +41,9 @@ The B<virsh> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions and their arguments joined with whitespace, and separated by semicolons or -newlines between commands. Within I<COMMAND_STRING>, virsh understands the +and their arguments joined with whitespace, and separated by semicolons or +newlines between commands, and where unquoted backslash-newline pairs are +elided. Within I<COMMAND_STRING>, virsh understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virsh> will then start a minimal diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index b2c9a1db6d..e1513e27e5 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -24,7 +24,8 @@ The B<virt-admin> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions and their arguments joined with whitespace, and separated by semicolons or -newlines between commands. Within I<COMMAND_STRING>, virt-admin understands the +newlines between commands, and where unquoted backslash-newline pairs are +elided. Within I<COMMAND_STRING>, virt-admin understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virt-admin> will then start a minimal diff --git a/tools/vsh.c b/tools/vsh.c index 3d9bec896b..9f29bbc777 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1659,8 +1659,8 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, *res = q; - while (*p && (*p == ' ' || *p == '\t')) - p++; + while (*p && (*p == ' ' || *p == '\t' || (*p == '\\' && p[1] == '\n'))) + p += 1 + (*p == '\\'); if (*p == '\0') return VSH_TK_END; @@ -1689,6 +1689,10 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, if (report) vshError(ctl, "%s", _("dangling \\")); return VSH_TK_ERROR; + } else if (*p == '\n') { + /* Elide backslash-newline entirely */ + p++; + continue; } } else if (!single_quote && *p == '"') { /* double quote */ double_quote = !double_quote; -- 2.20.1
On 2/21/19 3:20 PM, Eric Blake wrote:
The previous patch made it possible to split multiple commands by adding newline, but not to split a long single command. The sequence backslash-newline was being used as if it were a quoted newline character, rather than completely elided the way the shell does.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh.pod | 4 +++- tools/virt-admin.pod | 3 ++- tools/vsh.c | 8 ++++++-- 3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 2bf1ee77bb..4057edf3a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -41,7 +41,9 @@ The B<virsh> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions and their arguments joined with whitespace, and separated by semicolons or
I think you meant to chop the above line too...
-newlines between commands. Within I<COMMAND_STRING>, virsh understands the +and their arguments joined with whitespace, and separated by semicolons or +newlines between commands, and where unquoted backslash-newline pairs are
s/, and where/, where/
+elided. Within I<COMMAND_STRING>, virsh understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virsh> will then start a minimal diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index b2c9a1db6d..e1513e27e5 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -24,7 +24,8 @@ The B<virt-admin> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions and their arguments joined with whitespace, and separated by semicolons or
Same here.
-newlines between commands. Within I<COMMAND_STRING>, virt-admin understands the +newlines between commands, and where unquoted backslash-newline pairs are
s/, and where/, where/ Reviewed-by: John Ferlan <jferlan@redhat.com> John
+elided. Within I<COMMAND_STRING>, virt-admin understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virt-admin> will then start a minimal
[...]
No good feature is complete without tests ;) Signed-off-by: Eric Blake <eblake@redhat.com> --- Maybe I should squash all three patches into one? tests/virshtest.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/virshtest.c b/tests/virshtest.c index 7fb701d580..d77d3d81d4 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -411,6 +411,13 @@ mymain(void) DO_TEST(34, "hello\n", "echo --str hello"); DO_TEST(35, "hello\n", "echo --hi"); + /* Tests of multiple commands. */ + DO_TEST(36, "a\nb\n", " echo a; echo b;"); + DO_TEST(37, "a\nb\n", "\necho a\n echo b\n"); + DO_TEST(38, "a\nb\n", "ec\\\nho a\n echo \\\n b;"); + DO_TEST(39, "a\n b\n", "\"ec\\\nho\" a\n echo \"\\\n b\";"); + DO_TEST(40, "a\n\\\n b\n", "ec\\\nho a\n echo '\\\n b';"); + # undef DO_TEST VIR_FREE(custom_uri); -- 2.20.1
On 2/21/19 3:20 PM, Eric Blake wrote:
No good feature is complete without tests ;)
Signed-off-by: Eric Blake <eblake@redhat.com> ---
Maybe I should squash all three patches into one?
I like splitting the first two. Obviously parts of this could be split into the first one with the second one obtaining the \\\ eyesore. I leave that fun up to you though ;-)... End result of 2 patches I think.
tests/virshtest.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
On 2/25/19 10:52 AM, John Ferlan wrote:
On 2/21/19 3:20 PM, Eric Blake wrote:
No good feature is complete without tests ;)
Signed-off-by: Eric Blake <eblake@redhat.com> ---
Maybe I should squash all three patches into one?
I like splitting the first two. Obviously parts of this could be split into the first one with the second one obtaining the \\\ eyesore. I leave that fun up to you though ;-)... End result of 2 patches I think.
tests/virshtest.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com>
I redid this into the two patches, and pushed. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Tue, 2019-02-26 at 15:48 -0600, Eric Blake wrote:
I redid this into the two patches, and pushed.
I don't think that was appropriate, considering that we're during the pre-release freeze and this looks like a new feature rather than an urgent fix. -- Andrea Bolognani / Red Hat / Virtualization
On 2/27/19 2:53 AM, Andrea Bolognani wrote:
On Tue, 2019-02-26 at 15:48 -0600, Eric Blake wrote:
I redid this into the two patches, and pushed.
I don't think that was appropriate, considering that we're during the pre-release freeze and this looks like a new feature rather than an urgent fix.
The ack was given before the freeze. But I'm also okay with reverting them back out of 5.1 if you think that is more appropriate. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Wed, 2019-02-27 at 09:22 -0600, Eric Blake wrote:
On 2/27/19 2:53 AM, Andrea Bolognani wrote:
On Tue, 2019-02-26 at 15:48 -0600, Eric Blake wrote:
I redid this into the two patches, and pushed.
I don't think that was appropriate, considering that we're during the pre-release freeze and this looks like a new feature rather than an urgent fix.
The ack was given before the freeze. But I'm also okay with reverting them back out of 5.1 if you think that is more appropriate.
I didn't realize that, sorry. I'm not really sure whether we have an established policy for this kind of situation, but in my opinion the freeze applies regardless of when the ACK was granted, since it's intended to be a period during which the code that's going to end up in the next release is tested and validated, which makes minimizing changes and limit them to bugfixes only highly desiderable. That said, the feature is small enough and was merged early enough in the freeze period that I would not have advocated for reverting it either way. -- Andrea Bolognani / Red Hat / Virtualization
On 2/21/19 1:55 PM, Eric Blake wrote:
I wanted to do a demonstration with virsh batch mode, which takes multiple commands all packed into a single argument:
$ virsh -c test:///default 'echo a; echo b;' a b
but that produced a really long line, so I tried to make it more legible:
$ virsh -c test:///default ' echo a; echo b; ' error: unknown command: ' '
Let's be more like the shell, and treat unquoted newline as a command separator just as we do for semicolon. In fact, with that, I can even now mix styles:
$ virsh -c test:///default ' echo a; echo b echo c ' a b c
Fix the grammer in a nearby comment while at it.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh.pod | 4 ++-- tools/virt-admin.pod | 4 ++-- tools/vsh.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 67edb57b14..2bf1ee77bb 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -40,8 +40,8 @@ as a name. The B<virsh> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions -and their arguments joined with whitespace, and separated by semicolons -between commands. Within I<COMMAND_STRING>, virsh understands the +and their arguments joined with whitespace, and separated by semicolons or
s/whitespace, and/whitespace and/
+newlines between commands. Within I<COMMAND_STRING>, virsh understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virsh> will then start a minimal diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 3ddbbff934..b2c9a1db6d 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -23,8 +23,8 @@ Where I<command> is one of the commands listed below. The B<virt-admin> program can be used either to run one I<COMMAND> by giving the command and its arguments on the shell command line, or a I<COMMAND_STRING> which is a single shell argument consisting of multiple I<COMMAND> actions -and their arguments joined with whitespace, and separated by semicolons -between commands. Within I<COMMAND_STRING>, virt-admin understands the +and their arguments joined with whitespace, and separated by semicolons or
s/whitespace, and/whitespace and/ Reviewed-by: John Ferlan <jferlan@redhat.com> John
+newlines between commands. Within I<COMMAND_STRING>, virt-admin understands the same single, double, and backslash escapes as the shell, although you must add another layer of shell escaping in creating the single shell argument. If no command is given in the command line, B<virt-admin> will then start a minimal
[...]
participants (3)
-
Andrea Bolognani -
Eric Blake -
John Ferlan