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

[Martin Kletzander]

On Wed, Aug 27, 2014 at 10:41:45AM -0400, John Ferlan wrote:
>
>
>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...
>

Whatever you like, all of them are understandable.

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

Since the patches (both versions) were sent before the freeze started,
and reviewed as well, if I'm counting correctly, it is OK for 1.2.8.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140827/99016edc/attachment-0001.sig>