Re: [libvirt] [PATCH v4 RESEND 4/4] Using threadpool API to manage qemud worker

> - > -/* Caller must hold server lock */ > -static struct qemud_client *qemudPendingJob(struct qemud_server *server) > +static void qemudWorker(void *data, void *opaque ATTRIBUTE_UNUSED) > { > - int i; > - for (i = 0 ; i < server->nclients ; i++) { > - virMutexLock(&server->clients[i]->lock); > - if (server->clients[i]->dx) { > - /* Delibrately don't unlock client - caller wants the lock */ > - return server->clients[i]; > - } > - virMutexUnlock(&server->clients[i]->lock); > - } > - return NULL; > -} > + struct qemud_client *client = data; > + struct qemud_client_message *msg; > > -static void *qemudWorker(void *data) > -{ > - struct qemud_worker *worker = data; > - struct qemud_server *server = worker->server; > + virMutexLock(&client->lock); It is neccessary to hold the lock on 'server' before obtaining a lock on 'client'. The server lock can be released again immediately if no longer needed.

> + if (!server->workerPool) { > + VIR_ERROR0(_("Failed to create thread pool")); > + virMutexUnlock(&server->lock); > + return NULL; > } > > for (;!server->quitEventThread;) { A small change in that we no longer kill off idle worker threads,

but the improved simplicity of libvirtd code makes this a worthwhile tradeoff. So looks good to me aside from the minor locking bug. Regards, Daniel