On Tue, Oct 30, 2018 at 07:30:52AM -0400, John Ferlan wrote:
>
>
> On 10/30/18 5:25 AM, Daniel P. Berrangé wrote:
> > On Mon, Oct 29, 2018 at 05:55:36PM -0400, John Ferlan wrote:
> >>
> >>
> >> On 10/17/18 10:15 AM, Daniel P. Berrangé wrote:
> >>> MTTCG is the new multi-threaded impl of TCG which follows
> >>> KVM in having one host OS thread per vCPU. Historically
> >>> we have discarded all PIDs reported for TCG guests, but
> >>> we must now selectively honour this data.
> >>>
> >>> We don't have anything in the domain XML that indicates
> >>> whether a guest is using TCG or MTTCG. While QEMU does
> >>> have an option (-accel tcg,thread=single|multi), it is
> >>> not desirable to expose this in libvirt. QEMU will
> >>> automatically use MTTCG when the host/guest architecture
> >>> pairing is known to be safe. Only developers of QEMU TCG
> >>> have a strong reason to override this logic.
> >>>
> >>> Thus we use two sanity checks to decide if the vCPU
> >>> PID information is usable. First we see if the PID
> >>> duplicates the main emulator PID, and second we see
> >>> if the PID duplicates any other vCPUs.
> >>>
> >>> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> >>> ---
> >>> src/qemu/qemu_domain.c | 81 ++++++++++++++++++++++++++----------------
> >>> 1 file changed, 51 insertions(+), 30 deletions(-)
> >>>
> >>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> >>> index f00f1b3fdb..c7a0c03e3f 100644
> >>> --- a/src/qemu/qemu_domain.c
> >>> +++ b/src/qemu/qemu_domain.c
> >>> @@ -10326,9 +10326,10 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> >>> qemuDomainVcpuPrivatePtr vcpupriv;
> >>> qemuMonitorCPUInfoPtr info = NULL;
> >>> size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
> >>> - size_t i;
> >>> + size_t i, j;
> >>> bool hotplug;
> >>> bool fast;
> >>> + bool validTIDs = true;
> >>> int rc;
> >>> int ret = -1;
> >>>
> >>> @@ -10336,6 +10337,8 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> >>> fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
> >>> QEMU_CAPS_QUERY_CPUS_FAST);
> >>>
> >>> + VIR_DEBUG("Maxvcpus %zu hotplug %d fast query %d", maxvcpus, hotplug, fast);
> >>> +
> >>> if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> >>> return -1;
> >>>
> >>> @@ -10348,39 +10351,57 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> >>> if (rc < 0)
> >>> goto cleanup;
> >>>
> >>> + /*
> >>> + * The query-cpus[-fast] commands return information
> >>> + * about the vCPUs, including the OS level PID that
> >>> + * is executing the vCPU.
> >>> + *
> >>> + * For KVM there is always a 1-1 mapping between
> >>> + * vCPUs and host OS PIDs.
> >>> + *
> >>> + * For TCG things are a little more complicated.
> >>> + *
> >>> + * - In some cases the vCPUs will all have the same
> >>> + * PID as the main emulator thread.
> >>> + * - In some cases the first vCPU will have a distinct
> >>> + * PID, but other vCPUs will share the emulator thread
> >>> + *
> >>> + * For MTTCG, things work the same as KVM, with each
> >>> + * vCPU getting its own PID.
> >>> + *
> >>> + * We use the Host OS PIDs for doing vCPU pinning
> >>> + * and reporting. The TCG data reporting will result
> >>> + * in bad behaviour such as pinning the wrong PID.
> >>> + * We must thus detect and discard bogus PID info
> >>> + * from TCG, while still honouring the modern MTTCG
> >>> + * impl which we can support.
> >>> + */
> >>> + for (i = 0; i < maxvcpus && validTIDs; i++) {
> >>> + if (info[i].tid == vm->pid) {
> >>> + VIR_DEBUG("vCPU[%zu] PID %llu duplicates process",
> >>> + i, (unsigned long long)info[i].tid);
> >>> + validTIDs = false;
> >>> + }
> >>> +
> >>
> >> If !validTIDs does the next loop matter? IOW:
> >>
> >> Should the above section add a "continue;" since the loop exit would
> >> force the exit?
> >
> > Mostly I wanted to ensure that we logged all the vCPU pids and it
> > won't impose an unreasonable performance impact by doing so.
> >
>
> Those are only VIR_DEBUG's though, so who really sees them? Perhaps
> using VIR_WARN would be different.
We has developers / maintainers see them. It is the kind of information
I like to have captured in logs to make it easier to diagnose bug
reports from users.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
