[libvirt] [PATCH 2/2] qemuBuildMemPathStr: Forbid memoryBacking/access for non-numa case

[John Ferlan]

On 12/18/2017 03:28 AM, Michal Privoznik wrote:
> On 12/15/2017 08:48 PM, John Ferlan wrote:
>>
>>
>> On 12/12/2017 08:36 AM, Michal Privoznik wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1448149
>>>
>>> If a domain has no numa nodes, that means we don't put any
>>> memory-backend-file onto the qemu command line. That in turn
>>> means we can't set access='shared'. Therefore, we should produce
>>> an error instead of ignoring the setting silently.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/qemu/qemu_command.c                         |  9 +++
>>>  tests/qemuxml2argvdata/hugepages-memaccess3.xml | 87 +++++++++++++++++++++++++
>>>  tests/qemuxml2argvtest.c                        |  3 +
>>>  3 files changed, 99 insertions(+)
>>>  create mode 100644 tests/qemuxml2argvdata/hugepages-memaccess3.xml
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 2dd50a214..dfc17ce34 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -7682,6 +7682,15 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
>>>          return -1;
>>>      }
>>>  
>>> +    if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT &&
>>> +        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_PRIVATE) {
>>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> +                       _("memory access mode '%s' not supported "
>>> +                         "without guest numa node"),
>>> +                       virDomainMemoryAccessTypeToString(def->mem.access));
>>> +        return -1;
>>> +    }
>>> +
>>
>> This works; however, why not move this and the virQEMUCapsGet check into
>> a qemu_domain qemuDomainDef*Memory*Validate type function?  Thus
>> removing failure checks from qemu_command...
> 
> I'm not quite sure which function you have in mind.
> 

Create one called from qemuDomainDefValidate, but it's not that
important. Just figured that since there seems to be a more accepted way
to catch these types of domain validation things before we get all the
way to building the command line that we should try to do so when we can.

John

>>
>> I suppose it's OK here, but I think better there.  Still for the concept,
>>
>> Reviewed-by: John Ferlan <jferlan at redhat.com>
> 
> Thanks, I'll hold pushing these until we have clear consensus here.
> 
> Michal
>