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