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(-)
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
qemu.conf (the settings from domain XML still take precedence as
they are more specific ones).
Signed-off-by: Michal Privoznik <mprivozn@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(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3ec1173c6..6707744bd 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5456,7 +5456,12 @@ qemu-kvm -net nic,model=? /dev/null
some restrictions on actual value. For instance, latest
QEMU (as of 2016-09-01) requires value to be a power of two
from [256, 1024] range.
- <span class="since">Since 2.3.0 (QEMU and KVM only)</span><br/><br/>
+ <span class="since">Since 2.3.0 (QEMU and KVM only)</span>
+ Additionally, <span class="since">since 4.1.0</span> the
+ value can be set in the <code>qemu.conf</code> file in order
+ to override the hypervisor default value. Note that XML has
+ higher precedence because it's more specific.
+ <br/><br/>
<b>In general you should leave this option alone, unless you
are very certain you know what you are doing.</b>
@@ -5472,7 +5477,12 @@ qemu-kvm -net nic,model=? /dev/null
range. In addition to that, this may work only for a subset of
interface types, e.g. aforementioned QEMU enables this option
only for <code>vhostuser</code> type.
- <span class="since">Since 3.7.0 (QEMU and KVM only)</span><br/><br/>
+ <span class="since">Since 3.7.0 (QEMU and KVM only)</span>
+ Additionally, <span class="since">since 4.1.0</span> the
+ value can be set in the <code>qemu.conf</code> file in order
+ to override the hypervisor default value. Note that XML has
+ higher precedence because it's more specific.
+ <br/><br/>
<b>In general you should leave this option alone, unless you
are very certain you know what you are doing.</b>
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index c19bf3a43..084290296 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -118,6 +118,9 @@ module Libvirtd_qemu =
let vxhs_entry = bool_entry "vxhs_tls"
| str_entry "vxhs_tls_x509_cert_dir"
+ let virtio_entry = int_entry "rx_queue_size"
+ | int_entry "tx_queue_size"
+
(* Each entry in the config is one of the following ... *)
let entry = default_tls_entry
| vnc_entry
@@ -137,6 +140,7 @@ module Libvirtd_qemu =
| gluster_debug_level_entry
| memory_entry
| vxhs_entry
+ | virtio_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 43dd561cc..62c4265ea 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -775,3 +775,9 @@
# This directory is used for memoryBacking source if configured as file.
# NOTE: big files will be stored here
#memory_backing_dir = "/var/lib/libvirt/qemu/ram"
+
+# The following two values set the default RX/TX ring buffer size for virtio
+# interfaces. These values are taken unless overridden in domain XML. For more
+# info consult docs to corresponding attributes from domain XML.
+#rx_queue_size = 1024
+#tx_queue_size = 1024
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 899f0cbbb..543270028 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3751,7 +3751,8 @@ qemuBuildNicStr(virDomainNetDefPtr net,
char *
-qemuBuildNicDevStr(virDomainDefPtr def,
+qemuBuildNicDevStr(virQEMUDriverConfigPtr cfg,
+ virDomainDefPtr def,
virDomainNetDefPtr net,
int vlan,
unsigned int bootindex,
@@ -3871,21 +3872,41 @@ qemuBuildNicDevStr(virDomainDefPtr def,
virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2);
}
}
- if (usingVirtio && net->driver.virtio.rx_queue_size) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio rx_queue_size option is not supported with this QEMU binary"));
- goto error;
+ if (usingVirtio) {
+ unsigned int rx_queue_size = net->driver.virtio.rx_queue_size;
+
+ if (rx_queue_size == 0)
+ rx_queue_size = cfg->rx_queue_size;
+
+ if (rx_queue_size) {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio rx_queue_size option is "
+ "not supported with this QEMU binary"));
+ goto error;
+ }
+
+ net->driver.virtio.rx_queue_size = rx_queue_size;
+ virBufferAsprintf(&buf, ",rx_queue_size=%u", rx_queue_size);
}
- virBufferAsprintf(&buf, ",rx_queue_size=%u", net->driver.virtio.rx_queue_size);
}
- if (usingVirtio && net->driver.virtio.tx_queue_size) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio tx_queue_size option is not supported with this QEMU binary"));
- goto error;
+ if (usingVirtio) {
+ unsigned int tx_queue_size = net->driver.virtio.tx_queue_size;
+
+ if (tx_queue_size == 0)
+ tx_queue_size = cfg->tx_queue_size;
+
+ if (tx_queue_size) {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio tx_queue_size option is "
+ "not supported with this QEMU binary"));
+ goto error;
+ }
+
+ net->driver.virtio.tx_queue_size = tx_queue_size;
+ virBufferAsprintf(&buf, ",tx_queue_size=%u", tx_queue_size);
}
- virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size);
}
if (usingVirtio && net->mtu) {
@@ -8571,7 +8592,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virCommandAddArg(cmd, netdev);
VIR_FREE(netdev);
- if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex,
+ if (!(nic = qemuBuildNicDevStr(cfg, def, net, -1, bootindex,
queues, qemuCaps))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Error generating NIC -device string"));
@@ -8608,6 +8629,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
int **nicindexes,
bool chardevStdioLogd)
{
+ virQEMUDriverConfigPtr cfg = NULL;
int ret = -1;
char *nic = NULL, *host = NULL;
int *tapfd = NULL;
@@ -8669,6 +8691,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
return -1;
}
+ cfg = virQEMUDriverGetConfig(driver);
+
switch (actualType) {
case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_BRIDGE:
@@ -8864,7 +8888,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virCommandAddArgList(cmd, "-netdev", host, NULL);
}
if (qemuDomainSupportsNicdev(def, net)) {
- if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex,
+ if (!(nic = qemuBuildNicDevStr(cfg, def, net, vlan, bootindex,
vhostfdSize, qemuCaps)))
goto cleanup;
virCommandAddArgList(cmd, "-device", nic, NULL);
@@ -8908,6 +8932,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
VIR_FREE(host);
VIR_FREE(tapfdName);
VIR_FREE(vhostfdName);
+ virObjectUnref(cfg);
return ret;
}
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 31c9da673..644988329 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -90,7 +90,8 @@ char *qemuBuildNicStr(virDomainNetDefPtr net,
int vlan);
/* Current, best practice */
-char *qemuBuildNicDevStr(virDomainDefPtr def,
+char *qemuBuildNicDevStr(virQEMUDriverConfigPtr cfg,
+ virDomainDefPtr def,
virDomainNetDefPtr net,
int vlan,
unsigned int bootindex,
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index af503d31c..2fa96431f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -912,6 +912,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueString(conf, "memory_backing_dir", &cfg->memoryBackingDir) < 0)
goto cleanup;
+ if (virConfGetValueUInt(conf, "rx_queue_size", &cfg->rx_queue_size) < 0 ||
+ virConfGetValueUInt(conf, "tx_queue_size", &cfg->tx_queue_size) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a553e30e2..3f38a76c2 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -206,6 +206,9 @@ struct _virQEMUDriverConfig {
bool vxhsTLS;
char *vxhsTLSx509certdir;
+
+ unsigned int rx_queue_size;
+ unsigned int tx_queue_size;
};
/* Main driver state */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2aa927e62..0672ed130 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1120,7 +1120,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
for (i = 0; i < vhostfdSize; i++)
VIR_FORCE_CLOSE(vhostfd[i]);
- if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0,
+ if (!(nicstr = qemuBuildNicDevStr(cfg, vm->def, net, vlan, 0,
queueSize, priv->qemuCaps)))
goto try_remove;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 688e5b9fd..4fc4e2f4e 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -100,3 +100,5 @@ module Test_libvirtd_qemu =
{ "1" = "mount" }
}
{ "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
+{ "rx_queue_size" = "1024" }
+{ "tx_queue_size" = "1024" }
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 > qemu.conf (the settings from domain XML still take precedence as > they are more specific ones). > > Signed-off-by: Michal Privoznik <mprivozn@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@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@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. Also feel free to discuss if you have arguments. Thank you. >John > >-- >libvir-list mailing list >libvir-list@redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/12/2018 01:05 PM, Daniel P. Berrangé wrote: > 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. Agreed. Lets revert the commit. Sorry for the noise. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.