Re: [PATCH] net: Complete qapi-fication of netdev_add

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

[...]
> We better check all other uses of option group "netdev".

To the best of my grepping abilities:

= Definition =

* qemu_netdev_opts

  Option group definition, with empty .desc[].

= CLI =

Keep in mind: we finish processing CLI netdev before we start monitors
(we call net_init_clients() is mon_init_func()).  Monitor commands can
mess with CLI netdevs, but CLI cannot mess with monitor netdevs.

* main()

  - case QEMU_OPTION_netdev

    Straighforward parse of option argument; record in
    @qemu_netdev_opts.

  - case QEMU_OPTION_readconfig

    Similar.

  - case QEMU_OPTION_writeconfig

    "Unparse" of @qemu_netdev_opts.  Can contains only CLI netdevs.

* net_init_clients()

  Pass the CLI netdevs to net_client_init().

= Monitor =

* hmp_netdev_add()

  Straighforward parse of command argument (record in
  @qemu_netdev_opts), pass to net_client_init() via netdev_add().

* qmp_netdev_add()

  Before your patch: convert from JSON to QemuOpts (record in
  @qemu_netdev_opts), pass to net_client_init() via netdev_add().

  Note: net_client_init() converts QemuOpts to QAPI type Netdev, and
  passes that to net_client_init1().

  Afterwards: convert JSON straight to Netdev, pass to
  net_client_init1().  Also create in @qemu_netdev_opts, but empty apart
  from ID.

* qmp_netdev_del()
* netdev_del_completion()

  Both implement a "NetClientState is a netdev" predicate by checking
  "@qemu_netdev_opts has an entry with this name".

Looks like the only thing we do with the QemuOpts recorded in
@qemu_netdev_opts is checking existence.  Your patch preserves that.  I
doesn't preserve the option parameters, but they are not used.  Okay.

The correctness argument becomes simpler if we implement the "is a
netdev" predicate without @qemu_netdev_opts.  Could be done either
before or after this patch.