Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 06/28/2014 04:39 AM, Mike Perez wrote: On Thu, Jun 5, 2014 at 6:57 AM, Ján Tomko jto...@redhat.com wrote: On 05/23/2014 12:06 AM, Eric Blake wrote: On 05/22/2014 12:22 PM, Mike Perez wrote: This introduces two new attributes cmd_per_lun and max_sectors same with the names QEMU uses for virtio-scsi. An example of the XML: controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50' max_sectors='512'/ I'm not a fan of underscore (why type the shift key, when a dash will do). The libvirt xml name does not have to exactly match the qemu option name, so maybe there's some room for bikeshedding what names we should actually present to the user. IMO using underscores here would be consistent with other disk-related options like read_iops_sec; dashes are mostly used for network-related options. We could also use camelCase [1], or just roll a dice. Well underscores are originally what I had in my first patch [1]. Eric what do you think? [1] - http://www.redhat.com/archives/libvir-list/2014-May/msg00798.html I have pushed the underscore version (with tabs and release numbers fixed) now that we're out of the freeze. Sorry it took so long. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On Thu, Jun 5, 2014 at 6:57 AM, Ján Tomko jto...@redhat.com wrote: On 05/23/2014 12:06 AM, Eric Blake wrote: On 05/22/2014 12:22 PM, Mike Perez wrote: This introduces two new attributes cmd_per_lun and max_sectors same with the names QEMU uses for virtio-scsi. An example of the XML: controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50' max_sectors='512'/ I'm not a fan of underscore (why type the shift key, when a dash will do). The libvirt xml name does not have to exactly match the qemu option name, so maybe there's some room for bikeshedding what names we should actually present to the user. IMO using underscores here would be consistent with other disk-related options like read_iops_sec; dashes are mostly used for network-related options. We could also use camelCase [1], or just roll a dice. Well underscores are originally what I had in my first patch [1]. Eric what do you think? [1] - http://www.redhat.com/archives/libvir-list/2014-May/msg00798.html -- Mike Perez -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 05/23/2014 12:06 AM, Eric Blake wrote: On 05/22/2014 12:22 PM, Mike Perez wrote: This introduces two new attributes cmd_per_lun and max_sectors same with the names QEMU uses for virtio-scsi. An example of the XML: controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50' max_sectors='512'/ I'm not a fan of underscore (why type the shift key, when a dash will do). The libvirt xml name does not have to exactly match the qemu option name, so maybe there's some room for bikeshedding what names we should actually present to the user. IMO using underscores here would be consistent with other disk-related options like read_iops_sec; dashes are mostly used for network-related options. We could also use camelCase [1], or just roll a dice. Jan [1] http://www.redhat.com/archives/libvir-list/2013-April/msg01340.html signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
This introduces two new attributes cmd_per_lun and max_sectors same
with the names QEMU uses for virtio-scsi. An example of the XML:
controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/
The corresponding QEMU command line:
-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: Mike Perez thin...@gmail.com
---
docs/formatdomain.html.in | 29 +++---
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c | 27 ++--
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_command.c| 27
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml | 29 ++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.xml | 29 ++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c| 2 ++
11 files changed, 168 insertions(+), 11 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max_sectors.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max_sectors.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 691a451..8ffb247 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2559,11 +2559,32 @@
p
An optional sub-element codedriver/code can specify the driver
- specific options. Currently it only supports attribute
codequeues/code
- (span class=since1.0.5/span, QEMU and KVM only), which specifies
the
- number of queues for the controller. For best performance, it's
recommended
- to specify a value matching the number of vCPUs.
+ specific options:
/p
+dl
+ dtcodequeues/code/dt
+ dd
+The optional codequeues/code attribute specifies the number of
+queues for the controller. For best performance, it's recommended to
+specify a value matching the number of vCPUs.
+span class=sinceSince 1.0.5 (QEMU and KVM only)/span
+ /dd
+ dtcodecmd_per_lun/code/dt
+ dd
+The optional codecmd_per_lun/code attribute specifies the maximum
+number of commands that can be queued on devices controlled by the
+host.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+ dtcodemax_sectors/code/dt
+ dd
+The optional codemax_sectors/code attribute specifies the maximum
+amount of data in bytes that will be transferred to or from the device
+in a single command. The transfer length is measured in sectors, where
+a sector is 512 bytes.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+/dl
p
USB companion controllers have an optional
sub-element codelt;mastergt;/code to specify the exact
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..4bf4699 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1735,6 +1735,16 @@
ref name=unsignedInt/
/attribute
/optional
+ optional
+ attribute name=cmd_per_lun
+ref name=unsignedInt/
+ /attribute
+/optional
+optional
+ attribute name=max_sectors
+ref name=unsignedInt/
+ /attribute
+/optional
/element
/optional
/interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40c385e..4388cb8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6041,6 +6041,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
char *idx = NULL;
char *model = NULL;
char *queues = NULL;
+char *cmd_per_lun = NULL;
+char *max_sectors = NULL;
xmlNodePtr saved = ctxt-node;
int rc;
@@ -6084,6 +6086,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (cur-type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur-name, BAD_CAST driver))
queues = virXMLPropString(cur, queues);
+cmd_per_lun = virXMLPropString(cur, cmd_per_lun);
+max_sectors = virXMLPropString(cur, max_sectors);
}
cur = cur-next;
}
@@ -6094,6 +6098,17 @@ virDomainControllerDefParseXML(xmlNodePtr node,
goto error;
}
+if (cmd_per_lun virStrToLong_ui(cmd_per_lun, NULL, 10,
def-cmd_per_lun) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+
Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 05/22/2014 12:22 PM, Mike Perez wrote:
This introduces two new attributes cmd_per_lun and max_sectors same
with the names QEMU uses for virtio-scsi. An example of the XML:
controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/
I'm not a fan of underscore (why type the shift key, when a dash will
do). The libvirt xml name does not have to exactly match the qemu
option name, so maybe there's some room for bikeshedding what names we
should actually present to the user.
The corresponding QEMU command line:
-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: Mike Perez thin...@gmail.com
---
docs/formatdomain.html.in | 29
+++---
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c | 27 ++--
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_command.c| 27
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml | 29
++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.xml | 29
++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c| 2 ++
Good job covering docs and testsuite additions alongside the code changes.
+++ b/docs/schemas/domaincommon.rng
@@ -1735,6 +1735,16 @@
ref name=unsignedInt/
/attribute
/optional
+ optional
TAB damage.
@@ -15279,13 +15296,19 @@ virDomainControllerDefFormat(virBufferPtr buf,
break;
}
-if (def-queues || virDomainDeviceInfoIsSet(def-info, flags) ||
-pcihole64) {
+if (def-queues || def-cmd_per_lun || def-max_sectors ||
+ virDomainDeviceInfoIsSet(def-info, flags) || pcihole64) {
More TAB damage. Please make sure 'make syntax-check' passes.
@@ -4369,6 +4380,12 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
if (def-queues)
virBufferAsprintf(buf, ,num_queues=%u, def-queues);
+if (def-cmd_per_lun)
+ virBufferAsprintf(buf, ,cmd_per_lun=%u, def-cmd_per_lun);
and again
But once we settle on the naming, the patch looks pretty good.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 16:06 Thu 22 May , Eric Blake wrote: On 05/22/2014 12:22 PM, Mike Perez wrote: This introduces two new attributes cmd_per_lun and max_sectors same with the names QEMU uses for virtio-scsi. An example of the XML: controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50' max_sectors='512'/ I'm not a fan of underscore (why type the shift key, when a dash will do). The libvirt xml name does not have to exactly match the qemu option name, so maybe there's some room for bikeshedding what names we should actually present to the user. Thanks Eric. I posted v2 patch up which fixes the tab issues and switches the option names from underscores to dashes. -- Mike Perez -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
