[libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi

[Eric Blake]

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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140522/a2882ae4/attachment-0001.sig>