diff --git a/net/net.c b/net/net.c
index c337d3d753fe..440957b272ee 100644
--- a/net/net.c
+++ b/net/net.c
...
@@ -1612,7 +1617,19 @@ void net_init_clients(void)
*/
static bool netdev_is_modern(const char *optarg)
{
- return false;
+ QDict *args;
+ const char *type;
+ bool is_modern;
+
+ args = keyval_parse(optarg, "type", NULL, NULL);
+ if (!args) {
+ return false;
+ }
+ type = qdict_get_try_str(args, "type");
+ is_modern = !g_strcmp0(type, "stream") || !g_strcmp0(type, "dgram");
+ qobject_unref(args);
+
+ return is_modern;
}
You could use g_autoptr here:
g_autoptr(QDict) args = NULL;
const char *type;
bool is_modern;
args = keyval_parse(optarg, "type", NULL, NULL);
if (!args) {
return false;
}
type = qdict_get_try_str(args, "type");
return !g_strcmp0(type, "stream") || !g_strcmp0(type, "dgram");
Matter of taste; you decide.
Looks good. We already had some series to convert existing code to g_autoptr(),
so it
seems the way to do.
Now recall how this function is used: it decides whether to parse the
modern way (with qobject_input_visitor_new_str()) or the traditional way
(with qemu_opts_parse_noisily()).
qemu_opts_parse_noisily() parses into a QemuOpts, for later use with the
opts visitor.
qobject_input_visitor_new_str() supports both dotted keys and JSON. The
former is parsed with keyval_parse(), the latter with
qobject_from_json(). It returns the resulting parse tree wrapped in a
suitable QAPI input visitor.
Issue 1: since we get there only when keyval_parse() succeeds, JSON is
unreachable. Reproducer:
$ qemu-system-x86_64 -netdev '{"id":"foo"}'
upstream-qemu: -netdev {"id":"foo"}: Parameter 'id' is missing
This is parsed with qemu_opts_parse_noisily(), resulting in a QemuOpts
with a single option 'type' with value '{"id":"foo"}'. The error
message comes from the opts visitor.
To fix this, make netdev_is_modern() return true when optarg[0] == '{'.
This matches how qobject_input_visitor_new_str() recognizes JSON.
OK
Issue 2: when keyval_parse() detects an error, we throw it away and fall
back to QemuOpts. This is commonly what we want. But not always. For
instance:
$ qemu-system-x86_64 -netdev
'type=stream,id=foo,addr.type=inet,addr.host=localhost,addr.port=1234,addr.ipv4-off'
Note the typo "ipv4-off" instead of ipv4=off. The error reporting is crap:
qemu-system-x86_64: -netdev
type=stream,id=foo,addr.type=inet,addr.host=localhost,addr.port=1234,addr.ipv4-off:
warning: short-form boolean option 'addr.ipv4-off' deprecated
Please use addr.ipv4-off=on instead
qemu-system-x86_64: -netdev
type=stream,id=foo,addr.type=inet,addr.host=localhost,addr.port=1234,addr.ipv4-off:
Parameter 'type' is missing
We get this because netdev_is_modern() guesses wrongly: keyval_parse()
fails with the perfectly reasonable error message "Expected '=' after
parameter 'addr.ipv4-off'", but netdev_is_modern() ignores the error,
and fails. We fall back to QemuOpts, and confusion ensues.
I'm not sure we can do much better with reasonable effort. If we decide
to accept this behavior, it should be documented at least in the source
code.
What about using modern syntax by default?
args = keyval_parse(optarg, "type", NULL, NULL);
if (!args) {
/* cannot detect the syntax, use new style syntax */
return true;
}