[libvirt] [PATCH v2 06/14] qemu: Implement @access for <memory/> banks

[Michal Privoznik]

On 03/07/2017 05:31 PM, John Ferlan wrote:
> 
> 
> On 02/27/2017 08:19 AM, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/qemu/qemu_command.c                            | 11 +++++++--
>>  src/qemu/qemu_command.h                            |  1 +
>>  src/qemu/qemu_hotplug.c                            |  2 +-
>>  .../qemuxml2argv-memory-hotplug-nvdimm-access.args | 26 ++++++++++++++++++++++
>>  tests/qemuxml2argvtest.c                           |  2 ++
>>  5 files changed, 39 insertions(+), 3 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
>>
> 
> Umm - even more reason to pass 'mem' to qemuBuildMemoryBackendStr

Yup. It makes patches more clean and readable now that I've switched to
that. Thank you for "forcing" me to overcome my laziness, roll up
sleeves and get the work done.

> 
> 
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 46a6ca9f0..287387055 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3090,6 +3090,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>>   *               for default
>>   * @autoNodeset: fallback nodeset in case of automatic NUMA placement
>>   * @memPathReq: request memory-backend-file with specific mem-path
>> + * @memAccessReq: specifically requested memAccess mode
>>   * @force: forcibly use one of the backends
>>   *
>>   * Creates a configuration object that represents memory backend of given guest
>> @@ -3122,6 +3123,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>>                            virBitmapPtr userNodeset,
>>                            virBitmapPtr autoNodeset,
>>                            const char *memPathReq,
>> +                          virDomainMemoryAccess memAccessReq,
>>                            bool force)
>>  {
>>      virDomainHugePagePtr master_hugepage = NULL;
>> @@ -3154,6 +3156,9 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>>          memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
>>      }
>>  
>> +    if (memAccessReq)
>> +        memAccess = memAccessReq;
>> +
> 
> Does or could this overwrite what we just got if guestNode >= 0? and
> virDomainNumaGetNodeMemoryAccessMode returns something?  Which takes
> precedence?  Does the first check even matter if the memAccessReq is set?
> 

I think memAccessReq is the most specific, thus should have the highest
level of importance. IOW, you can have a numa node in say 'private' mode
but one specific NVDIMM module in it in 'shared' mode.

Michal