[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpf
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-trivial] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently |
Date: |
Wed, 13 Feb 2013 17:20:55 -0200 |
On Fri, 08 Feb 2013 20:34:20 +0100
Markus Armbruster <address@hidden> wrote:
> > The real problem here is that the k, M, G suffixes, for example, are not
> > good to be reported by QMP. So maybe we should refactor the code in a way
> > that we separate what's done in QMP from what is done in HMP/command-line.
>
> Isn't it separated already? parse_option_size() is used when parsing
> key=value,... Such strings should not exist in QMP. If they do, it's a
> design bug.
No and no. Such strings don't exist in QMP as far as can tell (see bugs
below though), but parse_option_size() is theoretically "present" in a
possible QMP call stack:
qemu_opts_from_dict_1()
qemu_opt_set_err()
opt_set()
qemu_opt_paser()
parse_option_size()
I can't tell if this will ever happen because qemu_opts_from_dict_1()
restricts the call to qemu_opt_set_err() to certain values, but the
fact that it's not clear is an indication that a better separation is
necessary.
Now, I think I've found at least two bugs. The first one is the
netdev_add doc in the schema, which states that we do accept key=value
strings. The problem is here is that that's about the C API, on the
wire we act as before (ie. accepting each key as a separate argument).
The qapi-schame.json is more or less format-independent, so I'm not
exactly sure what's the best way to describe commands using QemuOpts
the way QMP uses it.
The second bug is that I entirely ignored how set_option_paramater()
handles errors when doing parse_option_size() conversion to Error **
and also when converting bdrv_img_create(). The end result is that
we can report an error twice, once with error_set() and later with
qerror_report() (or vice-versa). Shouldn't hurt on QMP as it knows
how to deal with this, on HMP and command-line we get complementary
error messages if we're lucky.
I'm very surprised with my mistakes on the second bug (although some
of the mess with fprintf() was already there), but I honestly think we
should defer this to 1.5 (and I can do it myself next week).
- Re: [Qemu-trivial] [PATCH for-1.4 v2 1/6] error: Clean up error strings with embedded newlines, (continued)
[Qemu-trivial] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently, Markus Armbruster, 2013/02/08
[Qemu-trivial] [PATCH for-1.4 v2 5/6] vl: Drop redundant "parse error" reports, Markus Armbruster, 2013/02/08
[Qemu-trivial] [PATCH for-1.4 v2 6/6] vl: Exit unsuccessfully on option argument syntax error, Markus Armbruster, 2013/02/08
[Qemu-trivial] [PATCH for-1.4 v2 3/6] error: Strip trailing '\n' from error string arguments (again), Markus Armbruster, 2013/02/08
Re: [Qemu-trivial] [PATCH for-1.4 v2 0/6] Error reporting fixes, Markus Armbruster, 2013/02/08