On Tue, Jul 01, 2014 at 06:51:46PM -0700, Nicholas A. Bellinger wrote:
Hi Mike & Co,
---------- Forwarded message ---------- From: Mike Perez <thingee@gmail.com> Date: Fri, Jun 27, 2014 at 7:39 PM Subject: Re: [libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi To: Ján Tomko <jtomko@redhat.com> Cc: Eric Blake <eblake@redhat.com>, libvir-list@redhat.com, berrange@redhat.com, "Nab@daterainc.com" <Nab@daterainc.com>
On Thu, Jun 5, 2014 at 6:57 AM, Ján Tomko <jtomko@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?
Can we please keep the libvirt names consistent for virtio-scsi parameters with what is already defined in qemu..? (Paolo CC'ed)
From qemu/include/hw/virtio/virtio-scsi.h:
#define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field) \ DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \ DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF),\ DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128)
AFAICT, having different names between qemu + libvirt parameters can only add confusion.
QEMU is not the only hypervisor that libvirt targets, so tieing libvirt names to QEMU names is a non-goal. We pick the names that make most sense in the context of libvirt. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|