Bug 1161617

As soon as bz 1135491 is complete to add the API's, virsh commands, etc. in order to manage IOThreads & iothreadpin in the XML or display the current configuration, API's added in commit id '6908f8cab' will provide the plumbing to add/delete objects.

Now that I've added IOThreads output and pin manipulation to libvirt upstream - I'm going to focus on this one.

Couple of questions... I couldn't figure out a way to have qemu/qmp tell me whether a disk resource was using an IOThread - so I'll have to go by what libvirt has in it's XML file for disks with an iothread.  Not a problem; however, it does lead me to consider what would happen if an IOThread object was removed and there was a disk resource assigned to that IOThread.  Would qemu/qmp object at 'object-del' time if some disk resource was assigned?  IOW: Can I use the failure as a way to avoid the search through the device list for a thread that matches?

Can you "foresee" a reason to add/remove more than one IOThread at a time? Is there a limit to how many at a time?  Adding is of course the easier of the two operations. I would think deletion would require allowing someone to choose a thread to delete; however, that's a bit tricky given the implementation.

I could mimic the add vcpus api where one would then be allowed to change the number of IOThreads for the domain, but unlike vcpus, there isn't a "maximum" number of IOThreads allowed. So perhaps either a magic random number is determined or I only allow adding some number at a time. Thus a domain with 3 IOThreads could add 1 or 'n' more at a time. But without a maximum someone could keep adding until there are no resources.  Of course in order to add they have to be privileged, so hopefully a priv'd user isn't malicious...

As for deletion - that's where things get tricky. I could allow the user to decide which thread to delete, but since display of the IOThreads that have disks assigned to them was rejected it may not be obvious to the user which one to remove.  So my thought was allow them to remove 'n' IOThreads, then search the XML for "enough" available threads without resources assigned and then remove those. If an incorrect number is provided, then it's a failure from the libvirt side. 

Just checking that this seems reasonable..

I posted some patches today upstream:

http://www.redhat.com/archives/libvir-list/2015-March/msg01014.html

I took a mostly simplistic approach. There is no upper limit to the number of IOThreads... One can add/del more than one at a time in a command, but the code handles adding/deleting each object and updating the cgroup/pin data one at a time - it just certain things easier.

(In reply to John Ferlan from comment #4)
> Now that I've added IOThreads output and pin manipulation to libvirt
> upstream - I'm going to focus on this one.
> 
> Couple of questions... I couldn't figure out a way to have qemu/qmp tell me
> whether a disk resource was using an IOThread - so I'll have to go by what
> libvirt has in it's XML file for disks with an iothread.  Not a problem;
> however, it does lead me to consider what would happen if an IOThread object
> was removed and there was a disk resource assigned to that IOThread.  Would
> qemu/qmp object at 'object-del' time if some disk resource was assigned? 
> IOW: Can I use the failure as a way to avoid the search through the device
> list for a thread that matches?

Sorry for the slow response.

IOThread objects have a refcount so the actual thread stays alive although it continues to run and be used until the virtio-blk-pci devices release it.  This isn't a great design on the QEMU side.

If libvirt remembers which drives are using the IOThread object then it can manage this itself.

> Can you "foresee" a reason to add/remove more than one IOThread at a time?
> Is there a limit to how many at a time?  Adding is of course the easier of
> the two operations. I would think deletion would require allowing someone to
> choose a thread to delete; however, that's a bit tricky given the
> implementation.

Yes, deleting IOThreads probably requires choosing a particular thread since each thread may be bound to a particular host CPU and has a set of devices that are using it.

From the QEMU perspective there is no need to add/remove multiple IOThreads in a single operation.  It's fine if several QMP commands are sent to add/remove a set of IOThreads.

> I could mimic the add vcpus api where one would then be allowed to change
> the number of IOThreads for the domain, but unlike vcpus, there isn't a
> "maximum" number of IOThreads allowed. So perhaps either a magic random
> number is determined or I only allow adding some number at a time. Thus a
> domain with 3 IOThreads could add 1 or 'n' more at a time. But without a
> maximum someone could keep adding until there are no resources.  Of course
> in order to add they have to be privileged, so hopefully a priv'd user isn't
> malicious...

I'm not sure an upper limit enforced by libvirt is useful.

Do you set limits on the number of disks, network interfaces, etc?  They consume resources too.

> As for deletion - that's where things get tricky. I could allow the user to
> decide which thread to delete, but since display of the IOThreads that have
> disks assigned to them was rejected it may not be obvious to the user which
> one to remove.  So my thought was allow them to remove 'n' IOThreads, then
> search the XML for "enough" available threads without resources assigned and
> then remove those. If an incorrect number is provided, then it's a failure
> from the libvirt side. 
> 
> Just checking that this seems reasonable..

The cost of an unused IOThread is minimal - it's just a userspace thread blocked in a poll(2) call.  I'm not sure there is even a need to delete IOThreads.

If they can be deleted then it would be good to specify the exact thread for reasons mentioned above in my reply.

(In reply to John Ferlan from comment #5)
> I posted some patches today upstream:
> 
> http://www.redhat.com/archives/libvir-list/2015-March/msg01014.html
> 
> I took a mostly simplistic approach. There is no upper limit to the number
> of IOThreads... One can add/del more than one at a time in a command, but
> the code handles adding/deleting each object and updating the cgroup/pin
> data one at a time - it just certain things easier.

Great.

I'm not sure whether removing IOThreads is useful without being able to specify a specific thread.

Understood; however, the only "visible" way to know which disk is using a thread is to look at the XML and see which thread_id a disk is assigned to. Determining which thread isn't being used means you have to have to figure it out on your own. The code follows how Vcpu's are handled with respect to adding and removing to/from the end and internally the threads are kept in an ordered array. There are "optimizations" that were used to keep things mostly simple - no sense building the rocket ship if all it's bells and whistles aren't going to be used.

I have reworked the patches - now it's possible to add/delete IOThreads by ID... New set of patches is here:

http://www.redhat.com/archives/libvir-list/2015-April/msg00426.html

After quite a few reworks, the patches have been pushed upstream:

git describe 1f7e811249588f9418e58ca9a23e2b94433b50e2
v1.2.14-337-g1f7e811

commit 1f7e811249588f9418e58ca9a23e2b94433b50e2
Author: John Ferlan <jferlan>
Date:   Wed Mar 18 09:01:50 2015 -0400

    virsh: Add iothreadadd and iothreaddel commands
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1161617
    
    Add command to allow adding and removing IOThreads from the domain including
    the configuration and live domain.
    
    $ virsh iothreadadd --help
      NAME
        iothreadadd - add an IOThread to the guest domain
    
      SYNOPSIS
        iothreadadd <domain> <id> [--config] [--live] [--current]
    
      DESCRIPTION
        Add an IOThread to the guest domain.
    
      OPTIONS
        [--domain] <string>  domain name, id or uuid
        [--id] <number>  iothread for the new IOThread
        --config         affect next boot
        --live           affect running domain
        --current        affect current domain
    
    $ virsh iothreaddel --help
      NAME
        iothreaddel - delete an IOThread from the guest domain
    
      SYNOPSIS
        iothreaddel <domain> <id> [--config] [--live] [--current]
    
      DESCRIPTION
        Delete an IOThread from the guest domain.
    
      OPTIONS
        [--domain] <string>  domain name, id or uuid
        [--id] <number>  iothread_id for the IOThread to delete
        --config         affect next boot
        --live           affect running domain
        --current        affect current domain
    
...

Verify this bug with build




1. prepare a guest with XML
#virsh dumpxml vm1
...
 <iothreads>4</iothreads>
  <iothreadids>
    <iothread id='2'/>
    <iothread id='1'/>
    <iothread id='3'/>
    <iothread id='6'/>
  </iothreadids>
  <cputune>
    <iothreadpin iothread='2' cpuset='3'/>
    <iothreadpin iothread='1' cpuset='0-4'/>
    <iothreadpin iothread='3' cpuset='3-4'/>
  </cputune>
...
 <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' iothread='1'/>
      <source file='/mnt/wzhang/rhel7.0-3.qcow2'/>
      <backingStore/>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </disk>
    <disk type='file' device='disk'>
      <driver name='qemu' type='raw' iothread='3'/>
      <source file='/mnt/wzhang/rhel7.raw'/>
      <backingStore/>
      <target dev='vdb' bus='virtio'/>
      <alias name='virtio-disk1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
    </disk>

...

2. start guest, and check iothreadinfo
# virsh list
 Id    Name                           State
----------------------------------------------------
 12    vm1                            running

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 6               0-7


3. check iothreadadd with --live --config --current

# virsh iothreadadd vm1 1
error: invalid argument: an IOThread is already using iothread_id '1'

# virsh iothreadadd vm1 1 --config
error: internal error: cannot duplicate iothread_id '1' in iothreadids


# virsh iothreadadd vm1 4 --live

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 6               0-7
 4               0-7

# virsh iothreadadd vm1 5 --config

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 6               0-7
 4               0-7

# virsh iothreadinfo vm1 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 6               0-7
 5               0-7

# virsh iothreadadd vm1 abc
error: Numeric value 'abc' for <id> option is malformed or out of range

[root@server libvirt]# virsh iothreadadd vm1 0
error: Invalid IOThread id value: '0'

[root@server libvirt]# virsh iothreadadd vm1 -1
error: Invalid IOThread id value: '-1'

[root@server libvirt]# virsh iothreadadd vm1 9999999999999999 --current
error: Numeric value '9999999999999999' for <id> option is malformed or out of range

# virsh iothreadadd vm1 99999999 --current
error: invalid argument: an IOThread is already using iothread_id '99999999'

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 6               0-7
 4               0-7
 999999999       0-7

4. check iothreaddel with --live, --config, --current

# virsh iothreaddel vm1 1
error: invalid argument: cannot remove IOThread 1 since it is being used by disk 'vda'

# virsh iothreaddel vm1 3 --live
error: invalid argument: cannot remove IOThread 3 since it is being used by disk 'vdb'


# virsh iothreaddel vm1 2 --live

[root@server libvirt]# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-4
 3               3-4
 6               0-7
 4               0-7
 999999999       0-7

# virsh iothreaddel vm1 6 --config

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-4
 3               3-4
 6               0-7
 4               0-7
 999999999       0-7

# virsh iothreaddel vm1 999999999 --current

# virsh iothreadinfo vm1
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-4
 3               3-4
 6               0-7
 4               0-7

# virsh iothreadinfo vm1 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 2               3
 1               0-4
 3               3-4
 5               0-7


test some negative parameter

# virsh iothreaddel vm1 7
error: invalid argument: cannot find IOThread '7' in iothreadids list

[root@server libvirt]# virsh iothreaddel vm1 abc
error: Numeric value 'abc' for <id> option is malformed or out of range

[root@server libvirt]# virsh iothreaddel vm1 0
error: Invalid IOThread id value: '0'

[root@server libvirt]# virsh iothreaddel vm1 -232
error: Invalid IOThread id value: '-232'

[root@server libvirt]# virsh iothreaddel vm1 -99999999
error: Invalid IOThread id value: '-99999999'

[root@server libvirt]# virsh iothreaddel vm1 9999999999999999999999999999999
error: Numeric value '9999999999999999999999999999999' for <id> option is malformed or out of range


and John, I find that during the verification steps, the report error in step2 are different with the patch descirbed, is it acceptable?

the actual results:

# virsh iothreadadd vm1 1
error: invalid argument: an IOThread is already using iothread_id '1'

# virsh iothreadadd vm1 1 --config
error: internal error: cannot duplicate iothread_id '1' in iothreadids


the patch show us:
$ virsh iothreadadd $dom 1
    error: invalid argument: an IOThread is already using iothread_id '1' in iothreadpids
    
    $ virsh iothreadadd $dom 1 --config
    error: invalid argument: an IOThread is already using iothread_id '1' in persistent iothreadids

As comment 10 notes, a few reworks and adjustments were done - I just forgot to update this bz with the "most recent" link... So here it is:

http://www.redhat.com/archives/libvir-list/2015-April/msg01264.html

for the series

and in particular patch 9:

http://www.redhat.com/archives/libvir-list/2015-April/msg01273.html

where the error message changed.

The exact text not matching is not that important as long as the information is conveyed correctly.

sorry to miss the information of verified build libvirt-1.2.17-2.el7.x86_64

and according to comment 13, move this bug to verified

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-2202.html