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

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?

> > qemu.conf (the settings from domain XML still take precedence as > > they are more specific ones). > > > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > > --- > > > > 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(a)redhat.com&gt; > 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.