Re: [libvirt] [PATCH 2/4] conf: Move hugepage XML validation check out of qemu_command

On 07/11/2018 10:22 AM, Pavel Hrdina wrote: > We can safely validate the hugepage nodeset attribute at a define time. > This validation is not done for already existing domains when the daemon > is restarted. > > All the changes to the tests are necessary because we move the error > from domain start into XML parse. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 32 +++++++++++++++++ > src/qemu/qemu_command.c | 34 ------------------- > .../seclabel-dynamic-none-relabel.xml | 2 +- > tests/qemuxml2argvtest.c | 16 +++++---- > .../qemuxml2xmloutdata/hugepages-pages10.xml | 30 ---------------- > tests/qemuxml2xmloutdata/hugepages-pages4.xml | 1 - > tests/qemuxml2xmloutdata/hugepages-pages9.xml | 31 ----------------- > .../seclabel-dynamic-none-relabel.xml | 2 +- > tests/qemuxml2xmltest.c | 3 -- > 9 files changed, 43 insertions(+), 108 deletions(-) > delete mode 100644 tests/qemuxml2xmloutdata/hugepages-pages10.xml > delete mode 120000 tests/qemuxml2xmloutdata/hugepages-pages4.xml > delete mode 100644 tests/qemuxml2xmloutdata/hugepages-pages9.xml > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 7396616eda..20d67e7854 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -6104,6 +6104,35 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def) > } > > > +static int > +virDomainDefMemtuneValidate(const virDomainDef *def) > +{ > + const virDomainMemtune *mem = &(def->mem); > + size_t i; > + ssize_t pos = virDomainNumaGetNodeCount(def->numa) - 1; > + > + for (i = 0; i < mem->nhugepages; i++) { > + ssize_t nextBit; > + > + if (!mem->hugepages[i].nodemask) { > + /* This is the master hugepage to use. Skip it as it has no > + * nodemask anyway. */ > + continue; > + } > + > + nextBit = virBitmapNextSetBit(mem->hugepages[i].nodemask, pos); > + if (nextBit >= 0) { I think its fair to enable hugepages for node #0 which is always there (even if not configured in domain XML). Just try to run 'numactl -H' from a domain that has no <numa/> in its XML.

> + virReportError(VIR_ERR_XML_DETAIL, > + _("hugepages: node %zd not found"), > + nextBit); > + return -1; > + } > + } Also, I see that you're removing hugepages-pages9 test from xml2xml test. But that is needed only because you disallowed nodeset='0' for nonnuma domain. The real problem there is that the default page size has

no numa node to apply to, not nodeset='0'. I guess we need to check for that too (or do we want to?)