On Fri, 24 May 2013 06:55:44 -0600 Eric Blake <ebl...@redhat.com> wrote:
> On 05/24/2013 06:23 AM, Luiz Capitulino wrote: > >>> I don't think we need this argument. This command is quite simple in its > >>> response, let's do this filtering in HMP only. > >> > >> Event message contains the net client name, management might only want > >> to query the single net client. > > > > The client can do the filtering itself. > > If we're arguing that we want this to be as responsive as possible, then > the less data we send over the wire, the faster management can react to > the guest's request for a particular NIC. That is, if libvirt is > listening to events that says NIC2 wants to change rx-filter, libvirt > would rather do a filtered query where it knows the JSON array of 1 > element matches NIC2 data, rather than do a global query and search > through the returned array until it finds NIC2. This sounds like premature optimization to me, but I wonder if instead of cluttering commands with arguments to do the filtering we could add some standard way of doing this in the QAPI. It was you who suggested a filter command? > Filtering is relatively easy to add, whether you do it in QMP or make > every client add it. Libvirt will survive if you don't have filtering, > but I don't see why we can't have it in QMP. Also, if you DO decide to > rip filtering out of QMP, you STILL need to keep a per-NIC flag. Since > the events say which NIC is requesting a change, even if the query reads > all nics, libvirt will only change the macvtap settings of the nic(s) > for which it has received an event (it doesn't make sense to waste time > requesting a (no-op) change to macvtap settings on a nic that hasn't > requested a change). But if you argue that having no filtering in the > QMP command means that you can get away with a single flag instead of a > per-nic flag, then you will fail to emit an event for NIC2 if it changes > in between the time that NIC1 fired an event and libvirt finally does > the query, and libvirt wouldn't realize that NIC2 also needs a macvtap > change. >