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@xxxxxxxxxx> >>> --- >>> 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@xxxxxxxxxx> > > Thanks, I'll hold pushing these until we have clear consensus here. > > Michal > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
