[libvirt] [PATCH 3/3] qemu: Return true pining info when using numad

[John Ferlan]

On 08/07/2015 06:44 AM, Martin Kletzander wrote:
> On Tue, Aug 04, 2015 at 09:27:13PM -0400, John Ferlan wrote:
>> $SUBJ
>>
>> s/pining/pinning
>>
>> Or perhaps - "qemu: Use numad information when getting pin information""
>>
> 
> OK, I won't mention anything pine-cone-related then ;)
> 
>> On 07/26/2015 12:57 PM, Martin Kletzander wrote:
>>> Pinning information returned for emulatorpin and vcpupin calls is being
>>> returned from our data without querying cgroups for some time.  However,
>>> not all the data were utilized.  When automatic placement is used the
>>> information is not returned for the calls mentioned above.  Since the
>>> numad hint in private data is properly saved/restored, we can safely use
>>> it to return true information.
>>>
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1162947
>>>
>>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>>> ---
>>>  src/qemu/qemu_driver.c | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>
>> Should qemuDomainGetIOThreadsConfig be adjusted as well?  In the for
>> loop that's fetching/filling in the iothreadid there's a filling of the
>> cpumask as well.
>>
> 
> From the name I think that rather qemuDomainGetIOThreadsLive() should
> be adjusted, not Config(), unless I misunderstood what they do.
> 

The Live function takes whatever bitmap was used to set using
GetAffinity. It can be initially set from autoCpuset in hotplug or
qemuProcessStart (return from qemuSetupCgroupForIOThreads).  So I think
Live is fine.  The "Config" though would need adjustment.

For the Emulator/Vcpu code LIVE/CONFIG is a bit different since it
combines live/config...

>> Patches seem reasonable otherwise, although patch2 could have a wee bit
>> more information in the commit log to explain what's being done...
> 
> OK, I'll add some info in there.
> 
>> Beyond that does that value matter if placement_mode !=
>> VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO? or if
>> !virDomainDefNeedsPlacementAdvice (from qemuProcessStart)?  Was checking
>> where it was set and if it's set to something reasonable...
>>
> 
> No it doesn't.  Is there a problem with that?  I haven't found any.
> 

IIRC - when I was looking at how/where autoCpuset was initialized - it
was only done so in qemuProcessStart, but I think my thought was around
whether there was a chance if placement_mode could be AUTO and
autoCpuset not be properly initialized. In hindsight and after more
looking it doesn't seem possible; however, I do note that the
NeedsPlacementAdvice can return true for more than one reason.  So now
I'm wondering - should any of those checks that only check
placement_mode == AUTO - should they change to using
virDomainDefNeedsPlacementAdvice?


John