[libvirt] [PATCH v2] qemu: Support multiple queue virtio-scsi
Osier Yang
jyang at redhat.com
Sat Apr 6 02:06:46 UTC 2013
On 06/04/13 04:27, John Ferlan wrote:
> On 04/05/2013 12:21 PM, Osier Yang wrote:
>> This introduce a new attribute "num_queues" (same with the good name
>> QEMU uses) for virtio-scsi controller. An example of the XML:
>>
>> <controller type='scsi' index='0' model='virtio-scsi' num_queues='8'/>
>>
>> The corresponding QEMU command line:
>>
>> -device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \
>> ---
>> docs/formatdomain.html.in | 18 +++++++++------
>> docs/schemas/domaincommon.rng | 5 +++++
>> src/conf/domain_conf.c | 13 +++++++++++
>> src/conf/domain_conf.h | 1 +
>> src/qemu/qemu_command.c | 11 +++++++++
>> .../qemuxml2argv-disk-virtio-scsi-num_queues.args | 8 +++++++
>> .../qemuxml2argv-disk-virtio-scsi-num_queues.xml | 26 ++++++++++++++++++++++
>> tests/qemuxml2argvtest.c | 3 +++
>> tests/qemuxml2xmltest.c | 1 +
>> 9 files changed, 79 insertions(+), 7 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index cf382e8..2973ce2 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -2092,13 +2092,17 @@
>> controller. A "scsi" controller has an optional
>> attribute <code>model</code>, which is one of "auto", "buslogic",
>> "ibmvscsi", "lsilogic", "lsisas1068", "lsisas1078", "virtio-scsi" or
>> - "vmpvscsi". A "usb" controller has an optional attribute
>> - <code>model</code>, which is one of "piix3-uhci", "piix4-uhci", "ehci",
>> - "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3",
>> - "vt82c686b-uhci", "pci-ohci" or "nec-xhci". Additionally,
>> - <span class="since">since 0.10.0</span>, if the USB bus needs to be
>> - explicitly disabled for the guest, <code>model='none'</code> may be used.
>> - The PowerPC64 "spapr-vio" addresses do not have an associated controller.
>> + "vmpvscsi". The attribute <code>num_queues</code>
>> + (<span class="since">1.0.5 (QEMU and KVM only)</span>) is to specify
> s/is to specify/specifies/
>
>> + the number of queues for the controller, it's recommended to specify
> s/, it's/. For best performance, it's/
>
>> + the value matching the number of vCPUs for best performance. A "usb"
> s/the/a
>
> s/ for best performance//
>
>> + controller has an optional attribute <code>model</code>, which is one
>> + of "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1",
>> + "ich9-uhci2", "ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci".
>> + Additionally, <span class="since">since 0.10.0</span>, if the USB bus
>> + needs to be explicitly disabled for the guest, <code>model='none'</code>
>> + may be used. The PowerPC64 "spapr-vio" addresses do not have an
>> + associated controller.
>> </p>
>>
>> <p>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 63ba7d1..d486ae8 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1427,6 +1427,11 @@
>> </attribute>
>> </optional>
>> <optional>
>> + <attribute name="num_queues">
>> + <ref name="unsignedInt"/>
>> + </attribute>
>> + </optional>
>> + <optional>
>> <ref name="usbmaster"/>
>> </optional>
>> <optional>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 5ad3080..f50a964 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -5034,6 +5034,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>> char *type = NULL;
>> char *idx = NULL;
>> char *model = NULL;
>> + char *num_queues = NULL;
>>
>> if (VIR_ALLOC(def) < 0) {
>> virReportOOMError();
>> @@ -5069,6 +5070,14 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>> def->model = -1;
>> }
>>
>> + if ((num_queues = virXMLPropString(node, "num_queues"))) {
>> + if (virStrToLong_ui(num_queues, NULL, 10, &def->num_queues) < 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Malformed 'num_queues' value '%s'"), num_queues);
>> + goto error;
>> + }
>> + }
>> +
>> if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
>> goto error;
>>
>> @@ -5146,6 +5155,7 @@ cleanup:
>> VIR_FREE(type);
>> VIR_FREE(idx);
>> VIR_FREE(model);
>> + VIR_FREE(num_queues);
>>
>> return def;
>>
>> @@ -13195,6 +13205,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
>> virBufferEscapeString(buf, " model='%s'", model);
>> }
>>
>> + if (def->num_queues)
>> + virBufferAsprintf(buf, " num_queues='%u'", def->num_queues);
>> +
>> switch (def->type) {
>> case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
>> if (def->opts.vioserial.ports != -1) {
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 700f03f..75e3f15 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -728,6 +728,7 @@ struct _virDomainControllerDef {
>> int type;
>> int idx;
>> int model; /* -1 == undef */
>> + unsigned int num_queues;
>> union {
>> virDomainVirtioSerialOpts vioserial;
>> } opts;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 493e5f8..53caba5 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3533,6 +3533,14 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>> int model;
>>
>> + if (def->num_queues &&
>> + !(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
>> + def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("'num_queues' is only supported by virtio-scsi controller"));
>> + return NULL;
>> + }
>> +
>> switch (def->type) {
>> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
>> model = def->model;
>> @@ -3616,6 +3624,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>> goto error;
>> }
>>
>> + if (def->num_queues)
>> + virBufferAsprintf(&buf, ",num_queues=%u", def->num_queues);
>> +
>> if (qemuBuildDeviceAddressStr(&buf, &def->info, qemuCaps) < 0)
>> goto error;
>>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.args
>> new file mode 100644
>> index 0000000..8d46799
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.args
>> @@ -0,0 +1,8 @@
>> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
>> +/usr/bin/qemu -S -M pc -m 214 -smp 8 -nographic -nodefconfig -nodefaults \
>> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
>> +-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \
>> +-usb \
>> +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \
>> +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
>> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
>> new file mode 100644
>> index 0000000..dfa9cf1
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
>> @@ -0,0 +1,26 @@
>> +<domain type='qemu'>
>> + <name>QEMUGuest1</name>
>> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>> + <memory unit='KiB'>219136</memory>
>> + <currentMemory unit='KiB'>219136</currentMemory>
>> + <vcpu placement='static'>8</vcpu>
>> + <os>
>> + <type arch='i686' machine='pc'>hvm</type>
>> + <boot dev='hd'/>
>> + </os>
>> + <clock offset='utc'/>
>> + <on_poweroff>destroy</on_poweroff>
>> + <on_reboot>restart</on_reboot>
>> + <on_crash>destroy</on_crash>
>> + <devices>
>> + <emulator>/usr/bin/qemu</emulator>
>> + <disk type='block' device='disk'>
>> + <source dev='/dev/HostVG/QEMUGuest1'/>
>> + <target dev='sdb' bus='scsi'/>
>> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>> + </disk>
>> + <controller type='usb' index='0'/>
>> + <controller type='scsi' index='0' model='virtio-scsi' num_queues='8'/>
>> + <memballoon model='virtio'/>
>> + </devices>
>> +</domain>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 099fb36..d6575e7 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -553,6 +553,9 @@ mymain(void)
>> DO_TEST("disk-scsi-virtio-scsi",
>> QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
>> QEMU_CAPS_VIRTIO_SCSI);
>> + DO_TEST("disk-virtio-scsi-num_queues",
>> + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
>> + QEMU_CAPS_VIRTIO_SCSI);
>> DO_TEST("disk-scsi-megasas",
>> QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
>> QEMU_CAPS_SCSI_MEGASAS);
>> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>> index aa058bd..10a6ea2 100644
>> --- a/tests/qemuxml2xmltest.c
>> +++ b/tests/qemuxml2xmltest.c
>> @@ -179,6 +179,7 @@ mymain(void)
>> DO_TEST("disk-scsi-device");
>> DO_TEST("disk-scsi-vscsi");
>> DO_TEST("disk-scsi-virtio-scsi");
>> + DO_TEST("disk-virtio-scsi-num_queues");
>> DO_TEST("disk-scsi-megasas");
>> DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
>> DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
>>
> ACK - mechanically it seems fine.
>
> Does there need to be any capability checks done that the QEMU supports
> the option? Or does QEMU happily ignore the option if passed and it
> doesn't support it?
It showed up in same qemu version with virtio-scsi-pci, which we
already supported.
>
> What is the implication of someone supplying 100 to num_queues with a
> domain with a vCPU count of 1 or 2? I know, if they want to shoot
> themselves in the foot, well go ahead. However, if this is something
> that could be prevented or perhaps would be rejected by QEMU, then one
> would think some sort of range validation would exist. The only reason I
> mention it is the trigger that went off in my head when it was stated
> "For performance reasons" this value should match the vCPU count.
See paolo's feedback on v1. Currently qemu uses 1 queue for any number
not matching the number of vCPU. I don't think we should prevent it,
as it's very possible qemu will do some improvements.
Thanks for the reviewing, I pushed this.
Osier
More information about the libvir-list
mailing list