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 <mpriv...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to