Re: [libvirt] [PATCH v10 1/5] nodeinfo: Fix output on PPC64 KVM hosts

> + online_cpus = nodeGetOnlineCPUBitmap(sysfs_prefix); > + if (online_cpus == NULL) > + goto cleanup; > + > + nonline_cpus = virBitmapSize(online_cpus); > + > + for (cpu = 0; cpu < nonline_cpus; cpu++) { > + > + /* Skip primary threads */ > + if (cpu % threads_per_subcore == 0) > + continue; > + > + /* A single online subthread is enough to make the > + * configuration invalid */ > + if (virBitmapIsBitSet(online_cpus, cpu)) > + goto cleanup; > + } Could virBitmapNextSetBit be used? Where if the returned pos (cpu) % threads_per_subcore != 0, then jump to cleanup? I think both work, I just didn't know how large nonline_cpus could be and since the bitmap code has a way to return 'next' bit set, we could use it.

> +int > +nodeGetThreadsPerSubcore(virArch arch) > +{ > + const char *kvmpath = "/dev/kvm"; > + int kvmfd; These could move inside the if below. I'm thinking of one particular architecture where we consistently get compilation errors when there's variables that aren't used (which would be the case if HAVE_LINUX_KVM_H or KVM_CAP_PPC_SMT were not true

> + if ((kvmfd = open(kvmpath, O_RDONLY)) < 0) { > + /* This can happen when running as a regular user if > + * permissions are tight enough, in which case > erroring out > + * is better than silently falling back and reporting > + * different nodeinfo depending on the user */ > + virReportSystemError(errno, > + _("Failed to open '%s'"), > + kvmpath); > + threads_per_subcore = kvmfd; I would just set to -1 here directly rather than the return failure from open

Beyond those - things look OK to me.

Let me know if you want to make changes... Probably should wait until DV generates the release before pushing though...