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

[John Ferlan <jferlan@xxxxxxxxxx>]

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;

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list