[libvirt] [PATCH v2 4/4] qemu: Allow use of iothreads for disk definitions

[John Ferlan]

On 08/27/2014 03:00 AM, Martin Kletzander wrote:
> On Tue, Aug 26, 2014 at 06:15:49PM -0400, John Ferlan wrote:
>> For virtio-blk-pci disks with the disk iothread attribute that are
>> running the correct emulator, add the "iothread=iothread#" to the
>> -device command line in order to enable iothreads for the disk.
>>
>> This code will check both the "start" and "hotplug" paths for the capability,
>> whether the iothreadsmap has been defined, and whether there's an available
>> iothread to be used for the disk.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/qemu/qemu_command.c                            | 51 ++++++++++++++++++++++
>> src/qemu/qemu_hotplug.c                            |  6 +++
>> .../qemuxml2argv-iothreads-disk.args               | 17 ++++++++
>> .../qemuxml2argv-iothreads-disk.xml                | 40 +++++++++++++++++
>> tests/qemuxml2argvtest.c                           |  2 +
>> tests/qemuxml2xmltest.c                            |  1 +
>> 6 files changed, 117 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index ba0b977..763bb74 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3673,6 +3673,49 @@ qemuBuildDriveStr(virConnectPtr conn,
>>     return NULL;
>> }
>>
>> +
>> +static bool
>> +qemuCheckIothreads(virDomainDefPtr def,
>> +                   virQEMUCapsPtr qemuCaps,
>> +                   virDomainDiskDefPtr disk)
>> +{
>> +    bool inuse;
>> +
>> +    /* Have capability */
>> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                       _("IOThreads not supported for this QEMU"));
>> +        return false;
>> +    }
>> +
>> +    /* Right "type" of disk" */
>> +    if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
>> +        disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                       _("IOThreads not available for this disk"));
> 
> Sorry to say that, but this is like another extreme, it feels too
> generic.  If there are multiple disks, the user might not be sure
> which one you mean.  Listing the allowed ones is fine, I'd say
> something like "IOThreads only available for virtio pci disk" is
> enough.
> 
> ACK with that changed.
> 
> 

OK - I left it at "virtio pci", but can change it to "virtio-pci" or
"virtio-blk-pci" based on the answer to the following question...

Is this safe enough to be pushed with the freeze now in effect.  I know
the review started before and all that, but I figured I'd ask before
pushing in any case!

Tks,

John