[libvirt] [PATCH 0/7] Add more options for IOThreads

[John Ferlan]

On 09/15/2014 07:06 PM, Eric Blake wrote:
> On 09/03/2014 10:15 AM, John Ferlan wrote:
>> The following patches will add more support for IOThreads not completed
>> from the initial patches. These changes support the remaining elements of
>> bz https://bugzilla.redhat.com/show_bug.cgi?id=1101574 (working through
>> unsetting the private bits - as there's nothing in there that should
>> necessarily be private).
>>
>> Changes:
>>
>>  1. Add "--iothread" option to virsh attach-disk to allow using a specific
>>     iothread when attaching a disk
>>
>>  2. Add the ability to set CPU affinity for an IOThread.  This involves
>>     multiple steps (patches 2-6) of adding the infrastructure to support
>>     setting scheduler affinity for the IOThread including cgroup setup.
>>     For the most part it's a "copy" of the vCPU pinning code, but without
>>     the external interfaces - those will come at a later time after RHEL7.1.
>>
>>  3. Add to <cpuset/> a new element <iothreadpin iothread='#" cpuset="string"/>
>>
>>   NOTE: I can combine any/all of patches 2-6 - I just kept them separate
>>         so it wasn't a larger single patch.
>>
>> Although future changes will support API's and virsh commands to modify
>> the iothreadpin, this set of changes at least is backportable to RHEL7.1
>> since there are no external API changes.
>>
>> John Ferlan (7):
>>   virsh: Add iothread to 'attach-disk'
>>   qemu: Issue query-iothreads and to get list of active IOThreads
>>   vircgroup: Introduce virCgroupNewIOThread
>>   qemu_domain: Add niothreadpids and iothreadpids
> 
> Up to this patch, things worked for me,
> 

Hmm... not sure what happened because I usually do go thru these one at
a time before sending them out - of course that was Sep 3 before my
brain was flooded with Coverity issues, so I don't quite remember
everthing I did.  I think adding the cgroup code was something I did
last though since and figured I could rearrange things, but I guess I
failed to check if the rearranging worked.

In any case, the domain_conf.h from patch 7 should have been added
before/with "qemu_cgroup: Introduce cgroup functions for IOThreads"

Doing that resolves the build issue... and if I build/install from there
I can run the code below for the first patch, but not the second one.

The second one fails because of :

    if (niothreads <= 0)
         goto cleanup;

However, changing it to

+    if (niothreads < 0)
         goto cleanup;
+    if (niothreads == 0)
+        return 0;

Now this is even more perplexing because I'm quite certain I tested the
path of starting a guest without iothreads configured, but apparently
that doesn't seem to be the case.

Patches coming...

<sigh>

John

>>   qemu_cgroup: Introduce cgroup functions for IOThreads
>>   qemu: Allow pinning specific IOThreads to a CPU
> 
> but with these two patches, I get a compilation failure:
> 
> qemu/qemu_cgroup.c: In function 'qemuSetupCgroupForIOThreads':
> qemu/qemu_cgroup.c:1160:41: error: 'struct <anonymous>' has no member
> named 'niothreadspin'
>              for (j = 0; j < def->cputune.niothreadspin; j++) {
>                                          ^
>>   domain_conf: Add iothreadpin to cputune
> 
> and with this patch, I'm now unable to start a transient domain, with a
> less-than-stellar message:
> 
> $ qemu-img create -f qcow2 -o compat=0.10 base.img 10M
> $ virsh create /dev/stdin <<EOF
> <domain type='kvm'>
>  <name>testvm1</name>
>  <memory unit='MiB'>256</memory>
>  <vcpu>1</vcpu>
>  <os>
>    <type arch='x86_64'>hvm</type>
>  </os>
>  <devices>
>    <disk type='file' device='disk'>
>      <driver name='qemu' type='qcow2'/>
>      <source file='$PWD/base.img'/>
>      <target dev='vda' bus='virtio'/>
>    </disk>
>    <graphics type='vnc'/>
>  </devices>
> </domain>
> EOF
> 
> error: Failed to create domain from /dev/stdin
> error: An error occurred, but the cause is unknown
> 
> Normally, that command starts up a guest (yeah, the guest is doing
> nothing but wasting CPU cycles, because it has a blank disk, but it's a
> great little smoke test for my recent work in block jobs)
> 

Sounds like this smoke test needs to be added to the list of make check
tests somehow...