Re: [Qemu-devel] [RFC PATCH] net: introduce monitor command to query mactables

[Amos Kong]

Hi Eric,

On Thu, Apr 11, 2013 at 09:36:40AM -0600, Eric Blake wrote:
> On 04/11/2013 09:11 AM, Amos Kong wrote:
> > We want to do macvtap programming by libvirt, this patch added
> > a monitor command to query rx mode information. I will also
> > work on another patch to add QMP event notification for rx-mode
> > changing. Libvirt will sync the rx-mode change to macvtap
> > devices, there maybe exist an un-controlled delay, guests
> > normally expect rx mode updates immediately, it's another
> > separate issue.
> > 
> 
> > +++ b/qapi-schema.json
> > @@ -3513,3 +3513,40 @@
> >      '*asl_compiler_rev':  'uint32',
> >      '*file':              'str',
> >      '*data':              'str' }}
> > +
> > +# @MacTableInfo:
> > +#
> > +# Mac table information.
> > +#
> > +# @name: the name of nic device
> > +#
> > +# @unicast: #optional a list of unicast mac strings
> > +#
> > +# @multicast: #optional a list of multicast mac strings
> 
> Quite a few undocumented fields in relation to...
> 
> > +#
> > +# Since 1.5
> > +##
> > +{ 'type': 'MacTableInfo',
> > +  'data': {
> > +    'name':            'str',
> > +    '*promisc':        'bool',
> > +    '*allmulti':       'bool',
> > +    '*alluni':         'bool',
> > +    '*nomulti':        'bool',
> > +    '*nouni':          'bool',
> > +    '*nobcast':        'bool',
> > +    '*multi_overflow': 'bool',
> > +    '*uni_overflow':   'bool',
> > +    '*unicast':        ['String'],
> > +    '*multicast':      ['String'] }}
> 
> ...the size of the struct itself.

Ok.
 
> Why ['String'] instead of the simpler ['str']?  That's just adding
> additional JSON structure for no discernible gain (unless it is a
> workaround for a weakness in the code generation tool not allowing an
> array of a native JSON type - in which case, let's fix that).

Yes, 'str' is the native JSON type, but strList is not defined
by default, I will fix it in next version.
 
> s/_overflow/-overflow/2 - we prefer '-' over '_' in new QMP interfaces.

got it.
 
> > +
> > +##
> > +# @query-mac-table:
> > +#
> > +# Return mac tables information of nic devices
> > +#
> > +# Returns: @MacTableInfo on success
> > +#
> > +# Since: 1.5
> > +##
> > +{ 'command': 'query-mac-table', 'returns': ['MacTableInfo'] }
> 
> Seems useful.
> 
> > +-> { "execute": "query-mac-table"}
> > +<- {"return": [
> > +        {
> > +            "name": "virtio-net-pci.0",
> > +            "uni_overflow": false,
> > +            "nobcast": false,
> > +            "promisc": ,
> 
> Invalid JSON.
 
I wanted to edit it to true manually ;)

> > +            "nouni": false,
> > +            "nomulti": false,
> > +            "allmulti": false,
> > +            "multi_overflow": false,
> > +            "alluni": false
> > +            "unicast": [
> > +                {
> > +                    "str": "33:33:ff:00:00:00"
> > +                }
> 
> If you use 'str' instead of 'String', this would be:
> 
> "unicast": [ "33:33:ff:00:00:00" ]

Yeah, the expected format should be(after fixed the native type 'str'):
  "unicast": [ "33:33:ff:00:00:00",
               "33:33:ff:00:00:01",
               "33:33:ff:00:00:02" ]
 
Thanks.

-- 
                        Amos.