[libvirt] [PATCH 1/1] Clean redundant code about VCPU string checking

[Eric Blake]

On 03/18/2013 11:01 AM, John Ferlan wrote:
> On 03/18/2013 05:57 AM, Li Zhang wrote:
>> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>
>> 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 at redhat.com>
>> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>> ---
>>  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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130320/a9faadaa/attachment-0001.sig>