[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]