Re: [PULL V2 16/20] qapi: net: Add query-netdev command

[Jason Wang]

在 2021/3/17 上午5:27, Peter Maydell 写道:
> On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
> > From: Alexey Kirillov <lekiravi@yandex-team.ru>
> >
> > The query-netdev command is used to get the configuration of the current
> > network device backends (netdevs).
> > This is the QMP analog of the HMP command "info network" but only for
> > netdevs (i.e. excluding NIC and hubports).
> >
> > The query-netdev command returns an array of objects of the NetdevInfo
> > type, which are an extension of Netdev type. It means that response can
> > be used for netdev-add after small modification. This can be useful for
> > recreate the same netdev configuration.
> >
> > Information about the network device is filled in when it is created or
> > modified and is available through the NetClientState->stored_config.
> >
> > Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> Hi; Coverity is doubtful (CID 1450841) about this code:
>
> > @@ -668,12 +686,65 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
> > NetClientState *peer,
> > +        if (!stored->has_fds) {
> > +            stored->has_fds = true;
> > +            stored->fds = g_strdup_printf("%d", fd);
> > +        } else {
> > +            char *tmp_s = stored->fds;
> > +            stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
> > +            g_free(tmp_s);
> > +        }
> Here we have a bit of code which maintains stored->fds as a
> colon-separated string of integers, by tacking the new fd onto
> the end of the old string if it's already present.
>
> > @@ -731,6 +813,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
> > NetClientState *peer,
> >           }
> >           options.opaque = (void *)(uintptr_t)vhostfd;
> >
> > +        if (!stored->has_vhostfds) {
> > +            stored->has_vhostfds = true;
> > +            stored->vhostfds = g_strdup_printf("%d", vhostfd);
> > +        } else {
> > +            char *tmp_s = stored->vhostfds;
> > +            stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, vhostfd);
> > +            g_free(tmp_s);
> > +        }
> Here we have a bit of code that's kind of similar, except that
> the first argument to g_strdup_printf() is 'stored->fds', not
> 'stored->vhostfds'.
>
> Coverity suspects cut-n-paste error -- is it right ?


Yes, stored->vhostfds should be used here.

Alexey, please send patch to fix this.

Thanks


> thanks
> -- PMM