Re: [libvirt] [PATCH 3/4] domain_conf: Add support for iothreads in disk definition
On 08/26/2014 01:51 PM, Ján Tomko wrote:
On 08/26/2014 06:42 PM, John Ferlan wrote:
>
>
> On 08/26/2014 12:04 PM, Ján Tomko wrote:
>> On 08/26/2014 02:38 AM, John Ferlan wrote:
>>> Add a new disk "driver" attribute "iothread" to be parsed
as the thread
>>> number for the disk to use. In order to more easily facilitate the usage
>>> and configuration of the iothread, a "zero" for the attribute
indicates
>>> iothreads are not supported for the device and a positive value indicates
>>> the specific thread to try and use.
>>>
>>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>>> ---
>>> docs/formatdomain.html.in | 8 ++++++++
>>> docs/schemas/domaincommon.rng | 13 +++++++++++++
>>> src/conf/domain_conf.c | 24 +++++++++++++++++++++++-
>>> src/conf/domain_conf.h | 2 ++
>>> 4 files changed, 46 insertions(+), 1 deletion(-)
>>>
>>
>>> @@ -11968,6 +11980,14 @@ virDomainDefParseXML(xmlDocPtr xml,
>>> goto error;
>>> }
>>> def->iothreads = count;
>>> +
>>> + /* Create a bitmap for inuse threads - noting that entries are
>>> + * numbered 1..def->iothreads since 0 (zero) iothreads means
>>> + * nothing and assigning a disk to an IOThread requires at least a
>>> + * thread# > 0 since a zero would indicate no IOThread for the
disk
>>> + */
>>> + if (!(def->iothreadmap = virBitmapNew(def->iothreads+1)))
>>> + goto error;
>>> }
>>
>> Do you plan to do multiple disks on a single iothread in the followup patches?
>> Or is that a configuration that doesn't make sense?
>>
>
> I would say it doesn't make sense and from the bz I have the comment is
> "2. 1 IOThread per device" (bz 1101574 if you care to read although
it's
> mostly private comments though which is why I didn't put it in the patches).
>
I was wondering what's up with all the private comments.
Hopefully I can share the bit you're quoting:
There are basically two ways to use IOThreads:
1. 1 or maybe 2 IOThreads per host CPU
2. 1 IOThread per device
For usage 1), can you just add some <iothreads> to the domain XML and they
will automatically be used, or do you need to 'bind' the specific disk devices
to them?
Binding of disks would be "specific" to "pick" which thread to use.
Although I do see a perhaps veiled point = "iothread=yes|no" would work
too where the "next" available could/would be picked. I like the 0
better because if it's 0, then it's not written out to the config file.
The one part that still doesn't quite make sense how it will be linked
is the desired feature to bind an iothread to a specific (set of)
cpu(s). There's nothing in how iothreads are created/sent to qemu that
would allow that yet as far as I can tell.
The other part that makes me a bit nervous on this is there's no set
upper bound for iothreads to create. So if by mistake I fat finger 100
to 1000 or 10000, I'm in for a world of hurt when the domain starts.
There's only an advisory 1 or 2 iothreads per host CPU.
Trying not to overthink or overengineer this feature, but I've been
burned by similar a feature in a previous project.
John