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 <thingee@xxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list