Re: [libvirt] [PATCH v5 02/10] qemu: Use domain iothreadids to IOThread's 'thread_id'

On 04/27/2015 11:46 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 11:25:15 -0400, John Ferlan wrote: >> ... >> >>>>>> --- a/src/qemu/qemu_process.c >>>>>> +++ b/src/qemu/qemu_process.c >>>>>> @@ -2267,12 +2267,16 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, >>>>>> goto cleanup; >>>>>> } >>>>> >>>>> A few lines prior here is the check that the expected thread count >>>>> equals to the actual thread count. For some reason a few lines before >>>>> returns success if 0 threads are returned by the monitor. The two checks >>>>> should be inverted so that it makes sense. >>>>> >>>> >>>> If there are no threads, then it's not a failure, thus change ret to be >>>> 0. Again, this is something that's not within the scope of this set of >>>> changes and I believe if necessary could be a followup patch. >>>> >>>> I'm not clear on the value of changing the order of the checks. >>> >>> The problem is that if there are no iothreads reported by qemu, but we >>> did request some then it IS failure. >>> >> >> But that's an issue not related to iothreadid's per se - it's a more >> common general issue that should be a follow-up patch then I think. >> That is not introduced by this set of changes. > > Agreed, this should be done separately. > >> >> Other issues were addressed changed - do you need to see the diffs or an >> updated patch with the diffs already squashed in? > > I'd like to see the fixed hunk of qemuProcessDetectIOThreadPIDs that > parses the reply from the monitor. > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 66c9321..3def84f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2249,13 +2249,18 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, for (i = 0; i < vm->def->niothreadids; i++) {

unsigned int iothread_id; + virDomainIOThreadIDDefPtr iothrid; if (qemuDomainParseIOThreadAlias(iothreads[i]->name, &iothread_id) < 0) goto cleanup; - vm->def->iothreadids[i]->thread_id = iothreads[i]->thread_id; - vm->def->iothreadids[i]->iothread_id = iothread_id; + if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("iothread %d not found"), iothread_id); + goto cleanup; + } + iothrid->thread_id = iothreads[i]->thread_id; } ret = 0;