[libvirt] [PATCH v2] qemu: Expose rx/tx_queue_size in qemu.conf too

[Daniel P. Berrangé]

On Mon, Feb 12, 2018 at 12:44:52PM +0100, Martin Kletzander wrote:
> On Thu, Feb 01, 2018 at 09:58:17AM -0500, John Ferlan wrote:
> > 
> > 
> > On 02/01/2018 09:04 AM, Michal Privoznik wrote:
> > > In 2074ef6cd4a2 and c56cdf259 (and friends) we've added two
> > > attributes to virtio NICs: rx_queue_size and tx_queue_size.
> > > However, sysadmins might want to set these on per-host basis but
> > > don't necessarily have an access to domain XML (e.g. because they
> > > are generated by some other app). So let's expose them under
> 
> Define "sysadmins" here.  If there's a management app running that
> covers some hosts, then the app is the sysadmin.  Who will be setting
> these in the conf file?  Some person?  Manually?  I don't think so.  My
> bet is that it will be the same mgmt app that generates the XML.  Or the
> same person who deploys the app, but still manually.  If that app is
> unable to have a default, then why should libvirt be?

IIUC, this feature came in as a request to solve a limitation that
exists in OpenStack nova not having support for setting this feature
for guest XML.

IMHO the right fix for that request is to address the feature gap
in Nova, not add this to libvirt to workaround fact that Nova has
not supported this yet.

> > > qemu.conf (the settings from domain XML still take precedence as
> > > they are more specific ones).
> > > 
> > > Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> > > ---
> > > 
> > > diff to v1:
> > > - Reworded docs and config file comment
> > > - Simplified logic in qemuBuildNicDevStr a bit
> > > - Make the values require qemu with corresponding capabilities
> > > 
> > >  docs/formatdomain.html.in          | 14 ++++++++--
> > >  src/qemu/libvirtd_qemu.aug         |  4 +++
> > >  src/qemu/qemu.conf                 |  6 +++++
> > >  src/qemu/qemu_command.c            | 55 +++++++++++++++++++++++++++-----------
> > >  src/qemu/qemu_command.h            |  3 ++-
> > >  src/qemu/qemu_conf.c               |  4 +++
> > >  src/qemu/qemu_conf.h               |  3 +++
> > >  src/qemu/qemu_hotplug.c            |  2 +-
> > >  src/qemu/test_libvirtd_qemu.aug.in |  2 ++
> > >  9 files changed, 74 insertions(+), 19 deletions(-)
> > > 
> > 
> > Reviewed-by: John Ferlan <jferlan at redhat.com>
> > 
> 
> Sorry I missed this earlier, I'm going through my e-mail backlog kinda
> too late.  But I don't think this should be released.  Or anything
> similar.  If we make this change, then where do we draw the line for
> setting defaults in qemu.conf?  Will we eventually have defaults for
> first interface, memory size or even whole XML that get's parsed before
> the user-supplied one?  I know it's too late to NACK this, but please
> consider reverting it before next release.

Agreed, I'd like to see this reverted too as it is a bad precedent and
there's no obvious reason why Nova can't simply support this feature
itself. Even if nova doesn't want to make it user configurable, it
could be set as a default in the nova.conf file, or driven via information
it acquires from Neutron.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|