On 08/26/2014 01:03 AM, Martin Kletzander wrote:
> On Mon, Aug 25, 2014 at 08:38:07PM -0400, John Ferlan wrote:
>> Add a new capability to ensure the iothreads feature exists for the qemu
>> emulator being run - requires the "query-iothreads" QMP command. Using the
>> domain XML add correspoding command argument in order to generate the
>> threads. The iothreads will use a name space "libvirtIothread#" where, the
>> future patch to add support for using an iothread to a disk definition to
>> merely define which of the available threads to use.
>>
>> Add tests to ensure the xml/argv processing is correct. Note that no
>> change was made to qemuargv2xmltest.c as processing the -object element
>> would require knowing more than just iothreads.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_capabilities.c | 2 ++
>> src/qemu/qemu_capabilities.h | 1 +
>> src/qemu/qemu_command.c | 13 ++++++++++
>> tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 ++++++
>> tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++++++++++++++
>> tests/qemuxml2argvtest.c | 2 ++
>> tests/qemuxml2xmltest.c | 1 +
>> 7 files changed, 56 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 410086b..b331be7 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -268,6 +268,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>> "rtc-reset-reinjection",
>>
>> "splash-timeout", /* 175 */
>> + "query-iothreads",
>
> Why "query-" when the capability is _OBJECT_? Or is this just a typo?
> </bikeshed>
>
hmm... so how is this different then say perhaps
QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_MEMORY_RAM or
QEMU_CAPS_OBJECT_MEMORY_FILE which each eventually uses "-object" to
build it's part of the command line
>> );
>>
>>
>> @@ -1430,6 +1431,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
>> { "nbd-server-start", QEMU_CAPS_NBD_SERVER },
>> { "change-backing-file", QEMU_CAPS_CHANGE_BACKING_FILE },
>> { "rtc-reset-reinjection", QEMU_CAPS_RTC_RESET_REINJECTION },
>> + { "query-iothreads", QEMU_CAPS_OBJECT_IOTHREAD},
>
> We have virQEMUCapsObjectTypes[] where you can just stick the name of
> the object you want to test, and not rely on a related command to
> probe for it.
>
Hmm.. I wasn't completely sure where to put this - it's not overly
clear which of the various capabilities structures should be used for
what reason. Although I guess since the above named flags are in
ObjectTypes I suppose I should have used it. I'll move it.
>> };
>>
>> struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
>> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>> index 48a4eaa..0980c00 100644
>> --- a/src/qemu/qemu_capabilities.h
>> +++ b/src/qemu/qemu_capabilities.h
>> @@ -215,6 +215,7 @@ typedef enum {
>> QEMU_CAPS_OBJECT_USB_AUDIO = 173, /* usb-audio device support */
>> QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor command */
>> QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
>> + QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */
>>
>> QEMU_CAPS_LAST, /* this must always be the last item */
>> } virQEMUCapsFlags;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 6dac9d3..287a3f3 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7510,6 +7510,19 @@ qemuBuildCommandLine(virConnectPtr conn,
>> virCommandAddArg(cmd, smp);
>> VIR_FREE(smp);
>>
>> + if (def->iothreads > 0 &&
>> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
>> + /* Create named iothread objects starting with 1. These may be used
>> + * by a disk definition which will associate to an iothread by
>> + * supplying a value of 1 up to the number of iothreads available
>> + * (since 0 would indicate to not use the feature).
>> + */
>> + for (i = 1; i <= def->iothreads; i++) {
>> + virCommandAddArg(cmd, "-object");
>> + virCommandAddArgFormat(cmd, "iothread,id=libvirtIothread%zu", i);
>
> I don't see we would use 'libvirt*' naming for any other IDs,
> 'iothread%zu' would be enough, I guess (and the command-line wouldn't
> be so long as well).
Using 'libvirt' as the prefix for "iothread" was more of a convention to
make sure it was clear what created it. I can go with the shorter name
and initially had done so.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list