On 03/18/2013 11:01 AM, John Ferlan wrote:
> On 03/18/2013 05:57 AM, Li Zhang wrote:
>> From: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
>>
>> Now that VCPU number are removed from qemu_monitor_text.c.
Mentioning commit cc78d7ba would have been helpful.
>> VCPU string checking also should be removed.
>>
>> Report-by: John Ferlan <jferlan@xxxxxxxxxx>
>> Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
>> ---
>> src/qemu/qemu_monitor_text.c | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>
>>From the view point of this fix resolves the Coverity complaint/error,
> this seems fine. However, one nit I saw when looking at the code...
>
> The line:
>
> VIR_DEBUG("pid=%d", tid);
>
>
> probably should be
>
> VIR_DEBUG("tid=%d", tid);
On Linux, thread ids share the same namespace as process ids, so either
works, but I like the tweak to 'tid'. It's a debug message after all,
so no real impact.
>> +++ b/src/qemu/qemu_monitor_text.c
>> @@ -527,17 +527,10 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
>> */
>> line = qemucpus;
>> do {
>> - char *offset = strchr(line, '#');
>> + char *offset = NULL;
>> char *end = NULL;
>> int tid = 0;
>>
>> - /* See if we're all done */
>> - if (offset == NULL)
>> - break;
>
> I'm not familiar with the output, but if a line didn't have the '#' in
> it is there any reason not to break? Especially since now if it doesn't
> have thread_id in it, we're going to go to error?
I think we're okay here; we use QMP for new qemu, and older qemu isn't
going to change its text output (which we documented a few lines above
in a comment:
/*
* This is the gross format we're about to parse :-{
*
* (qemu) info cpus
* * CPU #0: pc=0x00000000000f0c4a thread_id=30019
* CPU #1: pc=0x00000000fffffff0 thread_id=30020
* CPU #2: pc=0x00000000fffffff0 thread_id=30021
*
*/
ACK. I went ahead and pushed this.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list