Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On 2/23/22 19:20, John Snow wrote:
On Wed, Feb 23, 2022 at 12:09 PM Damien Hedde
wrote:
On 2/23/22 17:18, John Snow wrote:
On Wed, Feb 23, 2022 at 10:44 AM Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
wrote:
This option makes qmp_shell exit (with error code 1)
as soon as one of the following error occurs:
+ command parsing error
+ disconnection
+ command failure (response is an error)
_execute_cmd() method now returns None or the response
so that read_exec_command() can do the last check.
This is meant to be used in combination with an input file
redirection. It allows to store a list of commands
into a file and try to run them by qmp_shell and easily
see if it failed or not.
Signed-off-by: Damien Hedde
Based on this patch, it looks like you really want something
scriptable, so I think the qemu-send idea that Dan has suggested might
be the best way to go. Are you still hoping to use the interactive
"short" QMP command format? That might be a bad idea, given how flaky
the parsing is -- and how we don't actually have a published standard
for that format. We've *never* liked the bad parsing here, so I have a
reluctance to use it in more places.
I'm having the naive idea that a script file could be as simple as a
list of QMP commands to send:
[
{"execute": "block-dirty-bitmap-add", "arguments": { ... }},
...
]
I'd really recommend against creating a new format for the script
file, especially one needing opening & closing [] like this, as
that isn't so amenable to dynamic usage/creation. ie you can't
just append an extcra command to an existing file.
IMHO, the "file" format should be identical to the result of
capturing the socket data off the wire. ie just a concatenation
of QMP commands, with no extra wrapping / change in format.
Eugh. That's just so hard to parse, because there's no off-the-shelf
tooling for "load a sequence of JSON documents". Nothing in Python
does it. :\
It isn't that hard if you require each JSON doc to be followed by
a newline.
Feed one line at a time to the JSON parser, until you get a complete
JSON doc, process that, then re-init the parser and carry on feeding
it lines until it emits the next JSON doc, and so on.
There's two interfaces in Python:
(1) json.load(), which takes a file pointer and either returns a
single, complete JSON document or it raises an Exception. It's not
useful here at all.
(2) json.JSONDecoder().raw_decode(strbuf), which takes a string buffer
and returns a 2-tuple of a JSON Document and the position at which it
stopped decoding.
The second is what we need here, but it does require buffering the
entire file into a string first, and then iteratively calling it. It
feels like working against the grain a little bit. We also can't use
the QAPI parser, as that parser has intentionally removed support for
constructs we don't use in the qapi schema language. Boo. (Not that I
want more non-standard configuration files like that propagating,
either.)
It would be possible to generate a JSON-Schema document to describe a
script file that used a containing list construct, but impossible for
a concatenation of JSON documents. This is one of the reasons I
instinctively shy away from non-standard file formats, they tend to
cut off support for this sort of thing.
Wanting to keep the script easy to append to is legitimate. I'm keen
to hear a bit more about the use case here before I press extremely
hard in any given direction, but those are my impulses here.
The use case is to be able to feed qemu with a bunch of commands we
expect to succeed and let qemu continue (unlike Daniel's wrap use case,
we don't want to quit qemu after the last command).
Typically it's the use case I present in the following cover-letter:
https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.he...@greensocs.com/
OK (Sorry for blowing this out into a bigger ordeal than you had maybe
hoped for. I want to get you happy and on your way ASAP, I promise)
So, I see comments and simple QMP commands using the short-hand
format. If I understand correctly, you want this script upstream so
that you don't have to re-engineer the hack every time I shift
something around in qmp-shell, and so that examples can be easily
shared and reproduced between parties. Good reasons, so I want to help
you out and get something merged. (An aside: In the past I have just
copy-pasted stuff into my qmp-shell terminal. It's less reproducible
for people who aren't used to using the tool, but it's been just
enough juice for me in the past. I empathize with wanting to give
someone a single-shot command they can copy-paste and have it Just
Work.)
Some observations:
(1) Comments we don't have in JSON, but we could use YAML instead, I'm
fine with that personally.
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 12:09 PM Damien Hedde
wrote:
>
>
>
> On 2/23/22 17:18, John Snow wrote:
> > On Wed, Feb 23, 2022 at 10:44 AM Daniel P. Berrangé
> > wrote:
> >>
> >> On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
> >>> On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé
> >>> wrote:
>
> On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> > On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> > wrote:
> >>
> >> This option makes qmp_shell exit (with error code 1)
> >> as soon as one of the following error occurs:
> >> + command parsing error
> >> + disconnection
> >> + command failure (response is an error)
> >>
> >> _execute_cmd() method now returns None or the response
> >> so that read_exec_command() can do the last check.
> >>
> >> This is meant to be used in combination with an input file
> >> redirection. It allows to store a list of commands
> >> into a file and try to run them by qmp_shell and easily
> >> see if it failed or not.
> >>
> >> Signed-off-by: Damien Hedde
> >
> > Based on this patch, it looks like you really want something
> > scriptable, so I think the qemu-send idea that Dan has suggested might
> > be the best way to go. Are you still hoping to use the interactive
> > "short" QMP command format? That might be a bad idea, given how flaky
> > the parsing is -- and how we don't actually have a published standard
> > for that format. We've *never* liked the bad parsing here, so I have a
> > reluctance to use it in more places.
> >
> > I'm having the naive idea that a script file could be as simple as a
> > list of QMP commands to send:
> >
> > [
> > {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> > ...
> > ]
>
> I'd really recommend against creating a new format for the script
> file, especially one needing opening & closing [] like this, as
> that isn't so amenable to dynamic usage/creation. ie you can't
> just append an extcra command to an existing file.
>
> IMHO, the "file" format should be identical to the result of
> capturing the socket data off the wire. ie just a concatenation
> of QMP commands, with no extra wrapping / change in format.
>
> >>>
> >>> Eugh. That's just so hard to parse, because there's no off-the-shelf
> >>> tooling for "load a sequence of JSON documents". Nothing in Python
> >>> does it. :\
> >>
> >> It isn't that hard if you require each JSON doc to be followed by
> >> a newline.
> >>
> >> Feed one line at a time to the JSON parser, until you get a complete
> >> JSON doc, process that, then re-init the parser and carry on feeding
> >> it lines until it emits the next JSON doc, and so on.
> >>
> >
> > There's two interfaces in Python:
> >
> > (1) json.load(), which takes a file pointer and either returns a
> > single, complete JSON document or it raises an Exception. It's not
> > useful here at all.
> > (2) json.JSONDecoder().raw_decode(strbuf), which takes a string buffer
> > and returns a 2-tuple of a JSON Document and the position at which it
> > stopped decoding.
> >
> > The second is what we need here, but it does require buffering the
> > entire file into a string first, and then iteratively calling it. It
> > feels like working against the grain a little bit. We also can't use
> > the QAPI parser, as that parser has intentionally removed support for
> > constructs we don't use in the qapi schema language. Boo. (Not that I
> > want more non-standard configuration files like that propagating,
> > either.)
> >
> > It would be possible to generate a JSON-Schema document to describe a
> > script file that used a containing list construct, but impossible for
> > a concatenation of JSON documents. This is one of the reasons I
> > instinctively shy away from non-standard file formats, they tend to
> > cut off support for this sort of thing.
> >
> > Wanting to keep the script easy to append to is legitimate. I'm keen
> > to hear a bit more about the use case here before I press extremely
> > hard in any given direction, but those are my impulses here.
> >
>
> The use case is to be able to feed qemu with a bunch of commands we
> expect to succeed and let qemu continue (unlike Daniel's wrap use case,
> we don't want to quit qemu after the last command).
>
> Typically it's the use case I present in the following cover-letter:
> https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.he...@greensocs.com/
>
OK (Sorry for blowing this out into a bigger ordeal than you had maybe
hoped for. I want to get you happy and on your way ASAP, I promise)
So, I see comments and simple QMP commands using the short-hand
format. If I understand correctly, you want this script upstream so
that you don't have to re-engineer the hack every time I shift
something around in qmp-shell, and so that examples can be easily
shared and
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 11:18:26AM -0500, John Snow wrote:
> On Wed, Feb 23, 2022 at 10:44 AM Daniel P. Berrangé
> wrote:
> >
> > On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
> > > On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé
> > > wrote:
> > > >
> > > > On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> > > > > On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> > > > > wrote:
> > > > > >
> > > > > > This option makes qmp_shell exit (with error code 1)
> > > > > > as soon as one of the following error occurs:
> > > > > > + command parsing error
> > > > > > + disconnection
> > > > > > + command failure (response is an error)
> > > > > >
> > > > > > _execute_cmd() method now returns None or the response
> > > > > > so that read_exec_command() can do the last check.
> > > > > >
> > > > > > This is meant to be used in combination with an input file
> > > > > > redirection. It allows to store a list of commands
> > > > > > into a file and try to run them by qmp_shell and easily
> > > > > > see if it failed or not.
> > > > > >
> > > > > > Signed-off-by: Damien Hedde
> > > > >
> > > > > Based on this patch, it looks like you really want something
> > > > > scriptable, so I think the qemu-send idea that Dan has suggested might
> > > > > be the best way to go. Are you still hoping to use the interactive
> > > > > "short" QMP command format? That might be a bad idea, given how flaky
> > > > > the parsing is -- and how we don't actually have a published standard
> > > > > for that format. We've *never* liked the bad parsing here, so I have a
> > > > > reluctance to use it in more places.
> > > > >
> > > > > I'm having the naive idea that a script file could be as simple as a
> > > > > list of QMP commands to send:
> > > > >
> > > > > [
> > > > > {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> > > > > ...
> > > > > ]
> > > >
> > > > I'd really recommend against creating a new format for the script
> > > > file, especially one needing opening & closing [] like this, as
> > > > that isn't so amenable to dynamic usage/creation. ie you can't
> > > > just append an extcra command to an existing file.
> > > >
> > > > IMHO, the "file" format should be identical to the result of
> > > > capturing the socket data off the wire. ie just a concatenation
> > > > of QMP commands, with no extra wrapping / change in format.
> > > >
> > >
> > > Eugh. That's just so hard to parse, because there's no off-the-shelf
> > > tooling for "load a sequence of JSON documents". Nothing in Python
> > > does it. :\
> >
> > It isn't that hard if you require each JSON doc to be followed by
> > a newline.
> >
> > Feed one line at a time to the JSON parser, until you get a complete
> > JSON doc, process that, then re-init the parser and carry on feeding
> > it lines until it emits the next JSON doc, and so on.
> >
>
> There's two interfaces in Python:
>
> (1) json.load(), which takes a file pointer and either returns a
> single, complete JSON document or it raises an Exception. It's not
> useful here at all.
> (2) json.JSONDecoder().raw_decode(strbuf), which takes a string buffer
> and returns a 2-tuple of a JSON Document and the position at which it
> stopped decoding.
Yes, the latter would do it, but you can also be lazy and just
repeatedly call json.loads() until you get a valid parse
$ cat demo.py
import json
cmds = []
bits = []
with open("qmp.txt", "r") as fh:
for line in fh:
bits.append(line)
try:
cmdstr = "".join(bits)
cmds.append(json.loads(cmdstr))
bits = []
except json.JSONDecodeError:
pass
for cmd in cmds:
print("Command: %s" % cmd)
$ cat qmp.txt
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
"node-name": "drive0",
"driver": "file",
"filename": "$TEST_IMG"
}
}
{ "execute": "blockdev-add",
"arguments": {
"driver": "$IMGFMT",
"node-name": "drive0-debug",
"file": {
"driver": "blkdebug",
"image": "drive0",
"inject-error": [{
"event": "l2_load"
}]
}
}
}
{ "execute": "human-monitor-command",
"arguments": {
"command-line": "qemu-io drive0-debug \"read 0 512\""
}
}
{ "execute": "quit" }
$ python demo.py
Command: {'execute': 'qmp_capabilities'}
Command: {'execute': 'blockdev-add', 'arguments': {'node-name': 'drive0',
'driver': 'file', 'filename': '$TEST_IMG'}}
Command: {'execute': 'blockdev-add', 'arguments': {'driver': '$IMGFMT',
'node-name': 'drive0-debug', 'file': {'driver': 'blkdebug', 'image': 'drive0',
'inject-error': [{'event': 'l2_load'}]}}}
Command: {'execute': 'human-monitor-command', 'arguments': {'command-line':
'qemu-io drive0-debug "read 0 512"'}}
Command: {'execute': 'quit'}
> Wanting to keep the script easy to append to is legitimate. I'm keen
> to hear a bit more about the use case here before I press
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On 2/23/22 17:18, John Snow wrote:
On Wed, Feb 23, 2022 at 10:44 AM Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
wrote:
This option makes qmp_shell exit (with error code 1)
as soon as one of the following error occurs:
+ command parsing error
+ disconnection
+ command failure (response is an error)
_execute_cmd() method now returns None or the response
so that read_exec_command() can do the last check.
This is meant to be used in combination with an input file
redirection. It allows to store a list of commands
into a file and try to run them by qmp_shell and easily
see if it failed or not.
Signed-off-by: Damien Hedde
Based on this patch, it looks like you really want something
scriptable, so I think the qemu-send idea that Dan has suggested might
be the best way to go. Are you still hoping to use the interactive
"short" QMP command format? That might be a bad idea, given how flaky
the parsing is -- and how we don't actually have a published standard
for that format. We've *never* liked the bad parsing here, so I have a
reluctance to use it in more places.
I'm having the naive idea that a script file could be as simple as a
list of QMP commands to send:
[
{"execute": "block-dirty-bitmap-add", "arguments": { ... }},
...
]
I'd really recommend against creating a new format for the script
file, especially one needing opening & closing [] like this, as
that isn't so amenable to dynamic usage/creation. ie you can't
just append an extcra command to an existing file.
IMHO, the "file" format should be identical to the result of
capturing the socket data off the wire. ie just a concatenation
of QMP commands, with no extra wrapping / change in format.
Eugh. That's just so hard to parse, because there's no off-the-shelf
tooling for "load a sequence of JSON documents". Nothing in Python
does it. :\
It isn't that hard if you require each JSON doc to be followed by
a newline.
Feed one line at a time to the JSON parser, until you get a complete
JSON doc, process that, then re-init the parser and carry on feeding
it lines until it emits the next JSON doc, and so on.
There's two interfaces in Python:
(1) json.load(), which takes a file pointer and either returns a
single, complete JSON document or it raises an Exception. It's not
useful here at all.
(2) json.JSONDecoder().raw_decode(strbuf), which takes a string buffer
and returns a 2-tuple of a JSON Document and the position at which it
stopped decoding.
The second is what we need here, but it does require buffering the
entire file into a string first, and then iteratively calling it. It
feels like working against the grain a little bit. We also can't use
the QAPI parser, as that parser has intentionally removed support for
constructs we don't use in the qapi schema language. Boo. (Not that I
want more non-standard configuration files like that propagating,
either.)
It would be possible to generate a JSON-Schema document to describe a
script file that used a containing list construct, but impossible for
a concatenation of JSON documents. This is one of the reasons I
instinctively shy away from non-standard file formats, they tend to
cut off support for this sort of thing.
Wanting to keep the script easy to append to is legitimate. I'm keen
to hear a bit more about the use case here before I press extremely
hard in any given direction, but those are my impulses here.
The use case is to be able to feed qemu with a bunch of commands we
expect to succeed and let qemu continue (unlike Daniel's wrap use case,
we don't want to quit qemu after the last command).
Typically it's the use case I present in the following cover-letter:
https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.he...@greensocs.com/
--
Damien
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On 2/23/22 17:43, Damien Hedde wrote:
On 2/23/22 16:44, Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé
wrote:
On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
wrote:
This option makes qmp_shell exit (with error code 1)
as soon as one of the following error occurs:
+ command parsing error
+ disconnection
+ command failure (response is an error)
_execute_cmd() method now returns None or the response
so that read_exec_command() can do the last check.
This is meant to be used in combination with an input file
redirection. It allows to store a list of commands
into a file and try to run them by qmp_shell and easily
see if it failed or not.
Signed-off-by: Damien Hedde
Based on this patch, it looks like you really want something
scriptable, so I think the qemu-send idea that Dan has suggested might
be the best way to go. Are you still hoping to use the interactive
"short" QMP command format? That might be a bad idea, given how flaky
the parsing is -- and how we don't actually have a published standard
for that format. We've *never* liked the bad parsing here, so I have a
reluctance to use it in more places.
I don't really care of the command format. I was just using the one
expected by qmp-shell to avoid providing another script.
I think it's better not to use some standard syntax like json.
I wanted to say the opposite: it's best to use json.
As long as we can store the commands into a file and tests them easily,
it's ok. The crucial feature is the "stop as soon something unexpected
happens" so that we can easily spot an issue.
I'm having the naive idea that a script file could be as simple as a
list of QMP commands to send:
[
{"execute": "block-dirty-bitmap-add", "arguments": { ... }},
...
]
I used this format at some point because it's so trivial to feed into
the QMP tools. Even used a yaml version of that to get the "human
readability" that goes with it.
I'd really recommend against creating a new format for the script
file, especially one needing opening & closing [] like this, as
that isn't so amenable to dynamic usage/creation. ie you can't
just append an extcra command to an existing file.
IMHO, the "file" format should be identical to the result of
capturing the socket data off the wire. ie just a concatenation
of QMP commands, with no extra wrapping / change in format.
>>
Eugh. That's just so hard to parse, because there's no off-the-shelf
tooling for "load a sequence of JSON documents". Nothing in Python
does it. :\
It isn't that hard if you require each JSON doc to be followed by
a newline.
Feed one line at a time to the JSON parser, until you get a complete
JSON doc, process that, then re-init the parser and carry on feeding
it lines until it emits the next JSON doc, and so on.
I agree it's doable. I can look into that.
It makes me think that I've managed to modify the chardev 'file' backend
a few months ago so that it can be used with an input file on the cli.
This allowed to give such raw qmp file directly with the -qmp option
instead of using an intermediate socket and a script issuing the same file.
But I gave up with this approach because then it can't stop if a command
failed without hacking into the receiving side in qemu.
--
Damien
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On 2/23/22 16:44, Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé wrote:
On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
wrote:
This option makes qmp_shell exit (with error code 1)
as soon as one of the following error occurs:
+ command parsing error
+ disconnection
+ command failure (response is an error)
_execute_cmd() method now returns None or the response
so that read_exec_command() can do the last check.
This is meant to be used in combination with an input file
redirection. It allows to store a list of commands
into a file and try to run them by qmp_shell and easily
see if it failed or not.
Signed-off-by: Damien Hedde
Based on this patch, it looks like you really want something
scriptable, so I think the qemu-send idea that Dan has suggested might
be the best way to go. Are you still hoping to use the interactive
"short" QMP command format? That might be a bad idea, given how flaky
the parsing is -- and how we don't actually have a published standard
for that format. We've *never* liked the bad parsing here, so I have a
reluctance to use it in more places.
I don't really care of the command format. I was just using the one
expected by qmp-shell to avoid providing another script.
I think it's better not to use some standard syntax like json.
As long as we can store the commands into a file and tests them easily,
it's ok. The crucial feature is the "stop as soon something unexpected
happens" so that we can easily spot an issue.
I'm having the naive idea that a script file could be as simple as a
list of QMP commands to send:
[
{"execute": "block-dirty-bitmap-add", "arguments": { ... }},
...
]
I used this format at some point because it's so trivial to feed into
the QMP tools. Even used a yaml version of that to get the "human
readability" that goes with it.
I'd really recommend against creating a new format for the script
file, especially one needing opening & closing [] like this, as
that isn't so amenable to dynamic usage/creation. ie you can't
just append an extcra command to an existing file.
IMHO, the "file" format should be identical to the result of
capturing the socket data off the wire. ie just a concatenation
of QMP commands, with no extra wrapping / change in format.
>>
Eugh. That's just so hard to parse, because there's no off-the-shelf
tooling for "load a sequence of JSON documents". Nothing in Python
does it. :\
It isn't that hard if you require each JSON doc to be followed by
a newline.
Feed one line at a time to the JSON parser, until you get a complete
JSON doc, process that, then re-init the parser and carry on feeding
it lines until it emits the next JSON doc, and so on.
I agree it's doable. I can look into that.
It makes me think that I've managed to modify the chardev 'file' backend
a few months ago so that it can be used with an input file on the cli.
This allowed to give such raw qmp file directly with the -qmp option
instead of using an intermediate socket and a script issuing the same file.
But I gave up with this approach because then it can't stop if a command
failed without hacking into the receiving side in qemu.
--
Damien
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 10:44 AM Daniel P. Berrangé wrote:
>
> On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
> > On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé
> > wrote:
> > >
> > > On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> > > > On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> > > > wrote:
> > > > >
> > > > > This option makes qmp_shell exit (with error code 1)
> > > > > as soon as one of the following error occurs:
> > > > > + command parsing error
> > > > > + disconnection
> > > > > + command failure (response is an error)
> > > > >
> > > > > _execute_cmd() method now returns None or the response
> > > > > so that read_exec_command() can do the last check.
> > > > >
> > > > > This is meant to be used in combination with an input file
> > > > > redirection. It allows to store a list of commands
> > > > > into a file and try to run them by qmp_shell and easily
> > > > > see if it failed or not.
> > > > >
> > > > > Signed-off-by: Damien Hedde
> > > >
> > > > Based on this patch, it looks like you really want something
> > > > scriptable, so I think the qemu-send idea that Dan has suggested might
> > > > be the best way to go. Are you still hoping to use the interactive
> > > > "short" QMP command format? That might be a bad idea, given how flaky
> > > > the parsing is -- and how we don't actually have a published standard
> > > > for that format. We've *never* liked the bad parsing here, so I have a
> > > > reluctance to use it in more places.
> > > >
> > > > I'm having the naive idea that a script file could be as simple as a
> > > > list of QMP commands to send:
> > > >
> > > > [
> > > > {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> > > > ...
> > > > ]
> > >
> > > I'd really recommend against creating a new format for the script
> > > file, especially one needing opening & closing [] like this, as
> > > that isn't so amenable to dynamic usage/creation. ie you can't
> > > just append an extcra command to an existing file.
> > >
> > > IMHO, the "file" format should be identical to the result of
> > > capturing the socket data off the wire. ie just a concatenation
> > > of QMP commands, with no extra wrapping / change in format.
> > >
> >
> > Eugh. That's just so hard to parse, because there's no off-the-shelf
> > tooling for "load a sequence of JSON documents". Nothing in Python
> > does it. :\
>
> It isn't that hard if you require each JSON doc to be followed by
> a newline.
>
> Feed one line at a time to the JSON parser, until you get a complete
> JSON doc, process that, then re-init the parser and carry on feeding
> it lines until it emits the next JSON doc, and so on.
>
There's two interfaces in Python:
(1) json.load(), which takes a file pointer and either returns a
single, complete JSON document or it raises an Exception. It's not
useful here at all.
(2) json.JSONDecoder().raw_decode(strbuf), which takes a string buffer
and returns a 2-tuple of a JSON Document and the position at which it
stopped decoding.
The second is what we need here, but it does require buffering the
entire file into a string first, and then iteratively calling it. It
feels like working against the grain a little bit. We also can't use
the QAPI parser, as that parser has intentionally removed support for
constructs we don't use in the qapi schema language. Boo. (Not that I
want more non-standard configuration files like that propagating,
either.)
It would be possible to generate a JSON-Schema document to describe a
script file that used a containing list construct, but impossible for
a concatenation of JSON documents. This is one of the reasons I
instinctively shy away from non-standard file formats, they tend to
cut off support for this sort of thing.
Wanting to keep the script easy to append to is legitimate. I'm keen
to hear a bit more about the use case here before I press extremely
hard in any given direction, but those are my impulses here.
--js
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 10:41:11AM -0500, John Snow wrote:
> On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé
> wrote:
> >
> > On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> > > On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> > > wrote:
> > > >
> > > > This option makes qmp_shell exit (with error code 1)
> > > > as soon as one of the following error occurs:
> > > > + command parsing error
> > > > + disconnection
> > > > + command failure (response is an error)
> > > >
> > > > _execute_cmd() method now returns None or the response
> > > > so that read_exec_command() can do the last check.
> > > >
> > > > This is meant to be used in combination with an input file
> > > > redirection. It allows to store a list of commands
> > > > into a file and try to run them by qmp_shell and easily
> > > > see if it failed or not.
> > > >
> > > > Signed-off-by: Damien Hedde
> > >
> > > Based on this patch, it looks like you really want something
> > > scriptable, so I think the qemu-send idea that Dan has suggested might
> > > be the best way to go. Are you still hoping to use the interactive
> > > "short" QMP command format? That might be a bad idea, given how flaky
> > > the parsing is -- and how we don't actually have a published standard
> > > for that format. We've *never* liked the bad parsing here, so I have a
> > > reluctance to use it in more places.
> > >
> > > I'm having the naive idea that a script file could be as simple as a
> > > list of QMP commands to send:
> > >
> > > [
> > > {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> > > ...
> > > ]
> >
> > I'd really recommend against creating a new format for the script
> > file, especially one needing opening & closing [] like this, as
> > that isn't so amenable to dynamic usage/creation. ie you can't
> > just append an extcra command to an existing file.
> >
> > IMHO, the "file" format should be identical to the result of
> > capturing the socket data off the wire. ie just a concatenation
> > of QMP commands, with no extra wrapping / change in format.
> >
>
> Eugh. That's just so hard to parse, because there's no off-the-shelf
> tooling for "load a sequence of JSON documents". Nothing in Python
> does it. :\
It isn't that hard if you require each JSON doc to be followed by
a newline.
Feed one line at a time to the JSON parser, until you get a complete
JSON doc, process that, then re-init the parser and carry on feeding
it lines until it emits the next JSON doc, and so on.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 10:27 AM Daniel P. Berrangé wrote:
>
> On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> > On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> > wrote:
> > >
> > > This option makes qmp_shell exit (with error code 1)
> > > as soon as one of the following error occurs:
> > > + command parsing error
> > > + disconnection
> > > + command failure (response is an error)
> > >
> > > _execute_cmd() method now returns None or the response
> > > so that read_exec_command() can do the last check.
> > >
> > > This is meant to be used in combination with an input file
> > > redirection. It allows to store a list of commands
> > > into a file and try to run them by qmp_shell and easily
> > > see if it failed or not.
> > >
> > > Signed-off-by: Damien Hedde
> >
> > Based on this patch, it looks like you really want something
> > scriptable, so I think the qemu-send idea that Dan has suggested might
> > be the best way to go. Are you still hoping to use the interactive
> > "short" QMP command format? That might be a bad idea, given how flaky
> > the parsing is -- and how we don't actually have a published standard
> > for that format. We've *never* liked the bad parsing here, so I have a
> > reluctance to use it in more places.
> >
> > I'm having the naive idea that a script file could be as simple as a
> > list of QMP commands to send:
> >
> > [
> > {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> > ...
> > ]
>
> I'd really recommend against creating a new format for the script
> file, especially one needing opening & closing [] like this, as
> that isn't so amenable to dynamic usage/creation. ie you can't
> just append an extcra command to an existing file.
>
> IMHO, the "file" format should be identical to the result of
> capturing the socket data off the wire. ie just a concatenation
> of QMP commands, with no extra wrapping / change in format.
>
Eugh. That's just so hard to parse, because there's no off-the-shelf
tooling for "load a sequence of JSON documents". Nothing in Python
does it. :\
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Wed, Feb 23, 2022 at 10:22:11AM -0500, John Snow wrote:
> On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
> wrote:
> >
> > This option makes qmp_shell exit (with error code 1)
> > as soon as one of the following error occurs:
> > + command parsing error
> > + disconnection
> > + command failure (response is an error)
> >
> > _execute_cmd() method now returns None or the response
> > so that read_exec_command() can do the last check.
> >
> > This is meant to be used in combination with an input file
> > redirection. It allows to store a list of commands
> > into a file and try to run them by qmp_shell and easily
> > see if it failed or not.
> >
> > Signed-off-by: Damien Hedde
>
> Based on this patch, it looks like you really want something
> scriptable, so I think the qemu-send idea that Dan has suggested might
> be the best way to go. Are you still hoping to use the interactive
> "short" QMP command format? That might be a bad idea, given how flaky
> the parsing is -- and how we don't actually have a published standard
> for that format. We've *never* liked the bad parsing here, so I have a
> reluctance to use it in more places.
>
> I'm having the naive idea that a script file could be as simple as a
> list of QMP commands to send:
>
> [
> {"execute": "block-dirty-bitmap-add", "arguments": { ... }},
> ...
> ]
I'd really recommend against creating a new format for the script
file, especially one needing opening & closing [] like this, as
that isn't so amenable to dynamic usage/creation. ie you can't
just append an extcra command to an existing file.
IMHO, the "file" format should be identical to the result of
capturing the socket data off the wire. ie just a concatenation
of QMP commands, with no extra wrapping / change in format.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 4/5] python: qmp_shell: add -e/--exit-on-error option
On Mon, Feb 21, 2022 at 10:55 AM Damien Hedde
wrote:
>
> This option makes qmp_shell exit (with error code 1)
> as soon as one of the following error occurs:
> + command parsing error
> + disconnection
> + command failure (response is an error)
>
> _execute_cmd() method now returns None or the response
> so that read_exec_command() can do the last check.
>
> This is meant to be used in combination with an input file
> redirection. It allows to store a list of commands
> into a file and try to run them by qmp_shell and easily
> see if it failed or not.
>
> Signed-off-by: Damien Hedde
Based on this patch, it looks like you really want something
scriptable, so I think the qemu-send idea that Dan has suggested might
be the best way to go. Are you still hoping to use the interactive
"short" QMP command format? That might be a bad idea, given how flaky
the parsing is -- and how we don't actually have a published standard
for that format. We've *never* liked the bad parsing here, so I have a
reluctance to use it in more places.
I'm having the naive idea that a script file could be as simple as a
list of QMP commands to send:
[
{"execute": "block-dirty-bitmap-add", "arguments": { ... }},
...
]
And a processing script could be something as simple as:
~~~
with open("script.json") as infile:
script = json.load(infile)
for command in script:
await qmp.execute_msg(command)
~~~
It's very simple to do, but the script file format is now a bit more
chatty than it used to be. You could also elide the "execute" and
"arguments" keys, perhaps:
[
{"block-dirty-bitmap-add": {"name": ..., "node": ...},
...
]
And then the script only changes a little bit:
for item in script:
for cmd, args in item.items():
await qmp.execute(cmd, args)
but this might lose the ability to opt into "execute-oob" as one
consequence of a more terse script format.
> ---
> python/qemu/aqmp/qmp_shell.py | 39 +--
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/python/qemu/aqmp/qmp_shell.py b/python/qemu/aqmp/qmp_shell.py
> index cce7732ba2..dd38ef8a13 100644
> --- a/python/qemu/aqmp/qmp_shell.py
> +++ b/python/qemu/aqmp/qmp_shell.py
> @@ -11,7 +11,7 @@
> """
> Low-level QEMU shell on top of QMP.
>
> -usage: qmp-shell [-h] [-H] [-N] [-v] [-p] qmp_server
> +usage: qmp-shell [-h] [-H] [-N] [-v] [-p] [-e] qmp_server
>
> positional arguments:
>qmp_server< UNIX socket path | TCP address:port >
> @@ -23,6 +23,8 @@
> Skip negotiate (for qemu-ga)
>-v, --verbose Verbose (echo commands sent and received)
>-p, --pretty Pretty-print JSON
> + -e, --exit-on-error Exit when an error occurs (command parsing,
> +disconnection and command failure)
>
>
> Start QEMU with:
> @@ -177,9 +179,11 @@ class QMPShell(QEMUMonitorProtocol):
> :param address: Address of the QMP server.
> :param pretty: Pretty-print QMP messages.
> :param verbose: Echo outgoing QMP messages to console.
> +:param raise_error: Don't continue after an error occured
> """
> def __init__(self, address: SocketAddrT,
> - pretty: bool = False, verbose: bool = False):
> + pretty: bool = False, verbose: bool = False,
> + raise_error: bool = False):
> super().__init__(address)
> self._greeting: Optional[QMPMessage] = None
> self._completer = QMPCompleter()
> @@ -189,6 +193,7 @@ def __init__(self, address: SocketAddrT,
>'.qmp-shell_history')
> self.pretty = pretty
> self.verbose = verbose
> +self.raise_error = raise_error
>
> def close(self) -> None:
> # Hook into context manager of parent to save shell history.
> @@ -343,19 +348,19 @@ def _print_parse_error(self, err: QMPShellParseError)
> -> None:
> file=sys.stderr
> )
>
> -def _execute_cmd(self, cmdline: str) -> bool:
> +def _execute_cmd(self, cmdline: str) -> Optional[QMPMessage]:
> qmpcmd = self._build_cmd(cmdline)
>
> # For transaction mode, we may have just cached the action:
> if qmpcmd is None:
> -return True
> +return None
> if self.verbose:
> self._print(qmpcmd)
> resp = self.cmd_obj(qmpcmd)
> if resp is None:
> raise QMPShellConnectError('Disconnected')
> self._print(resp)
> -return True
> +return resp
>
> def connect(self, negotiate: bool = True) -> None:
> self._greeting = super().connect(negotiate)
> @@ -401,8 +406,13 @@ def read_exec_command(self) -> bool:
> print(event)
> return True
>
> +if self.raise_error:
> +resp = self._execute_cmd(cmdline)
> +if resp and 'error' in resp:
> +raise QMPShellError(f"Command failed:
