Re: [libvirt] [PATCH 4/4] conf: Optimize the iothreadid initialization

On 10/13/2015 12:29 PM, Peter Krempa wrote: > On Tue, Oct 13, 2015 at 11:47:10 -0400, John Ferlan wrote:

>> @@ -2341,15 +2343,49 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def) >> if (def->iothreads == def->niothreadids) >> return 0; > > The code below seems too complex. I'd suggest something along the > following pseudo-code: > > assert(def->iothreads >= def->niothreads); > > virBitmapPtr *threads = virBitmapNew(def->iothreads); > ssize_t nxt = 0; > > virBitmapSetAll(threads); > > /* mark which are already provided by the user */ > for (i = 0; i < def->niothreads; i++) > virBitmapClearBit(threads, def->iothreadids[i]->id); > > /* resize array */ > VIR_REALLOC_N(def->iothreadids....); > > while ((nxt = virBitmapNextSetBit(bitmap, nxt)) >= 0) {

> /* add the stuff */ > } > This would work if we required thread_id values to be <= def->iothreads, but we don't, yet... I thought of using a bitmap, but it would no cover the following case :

<iothreads>4</iothreads> <iothreadids> <iothread id='100'/> <iothreadids> This would/should create thread ids 100, 1, 2, 3

We never placed a limit on the thread_id value other than it cannot be zero. Although we do indicate the default algorithm is 1 to the number of iothreads. Would a 'real world' user do something like this - perhaps not. Much less make "<iothreads>1000000</iothreads>" (that'd be one long command line - say nothing of the resources required in order to really start it. I'd be all for restricting iothread id values to be <= def->iothreads - that'd solve the unnecessarily complex algorithm. My other thought was to restrict the number of iothreads to be the number of disk devices or even 2 * ndisks. It's possible to assign multiple disks to 1 iothread, but impossible to assign 1 disk to multiple threads.