On 29.09.2016 10:56, Jaroslav Safka wrote:
This first change introduces xml parsing support for preallocated shared file descriptor based memory backing. It allows vhost-user to be used without hugepages.
New xml elements: <memoryBacking> <source type='file|anonymous' path='/path/to/qemu/' /> <access Mode='shared|private'/> <allocation mode='immediate|ondemand'/> </memoryBacking>
Okay, this is definitely interesting approach (not only because while reviewing this, I've found an old branch in my git where I've started to work on this). Frankly, I don't know if this is a good API or not. Historically, we required Dan's ACK on XML schema :-) btw: s/Mode/mode/ ;-)
--- docs/schemas/domaincommon.rng | 37 +++++ src/conf/domain_conf.c | 149 ++++++++++++++++----- src/conf/domain_conf.h | 34 +++++ .../qemuxml2argv-memorybacking-set.xml | 32 +++++ .../qemuxml2argv-memorybacking-unset.xml | 32 +++++ .../qemuxml2xmlout-memorybacking-set.xml | 40 ++++++ .../qemuxml2xmlout-memorybacking-unset.xml | 40 ++++++ tests/qemuxml2xmltest.c | 3 + 8 files changed, 334 insertions(+), 33 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml
You need to update the docs too. formatdomain.html.in to be more precise.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a06da46..7f73569 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
@@ -16179,48 +16194,101 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(tmp);
- if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt, &nodes)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract hugepages nodes")); - goto error; + tmp = virXPathString("string(./memoryBacking/source/@type)", ctxt); + if (tmp) { + if ((def->mem.source = virDomainMemorySourceTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/source/type '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { + tmp = virXPathString("string(./memoryBacking/source/@path)", ctxt); + if (!tmp && VIR_STRDUP(tmp, VIR_DOMAIN_MEMORY_DEFAULT_PATH) < 0) + goto error; + + def->mem.mem_path = tmp; + tmp = NULL; + } }
- if (n) { - if (VIR_ALLOC_N(def->mem.hugepages, n) < 0) + tmp = virXPathString("string(./memoryBacking/access/@mode)", ctxt); + if (tmp) { + if ((def->mem.access = virDomainMemoryAccessTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/access/mode '%s'"), tmp); goto error; + } + VIR_FREE(tmp); + }
- for (i = 0; i < n; i++) { - if (virDomainHugepagesParseXML(nodes[i], ctxt, - &def->mem.hugepages[i]) < 0) + tmp = virXPathString("string(./memoryBacking/allocation/@mode)", ctxt); + if (tmp) { + if ((def->mem.allocation = virDomainMemoryAllocationTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/allocation/mode '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + } + + if (virXPathNode("./memoryBacking/hugepages", ctxt)) { + /* hugepages will be used */ + + if (def->mem.allocation == VIR_DOMAIN_MEMORY_ALLOCATION_ONDEMAND) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("hugepages are not allowed with memory allocation ondemand")); + goto error; + } + + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("hugepages are not allowed with anonymous memory source")); + goto error; + } + + if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract hugepages nodes")); + goto error; + }
These types of checks would normally go to post parse callback.
+ + if (n) { + if (VIR_ALLOC_N(def->mem.hugepages, n) < 0) goto error; - def->mem.nhugepages++;
- for (j = 0; j < i; j++) { - if (def->mem.hugepages[i].nodemask && - def->mem.hugepages[j].nodemask && - virBitmapOverlaps(def->mem.hugepages[i].nodemask, - def->mem.hugepages[j].nodemask)) { - virReportError(VIR_ERR_XML_DETAIL, - _("nodeset attribute of hugepages " - "of sizes %llu and %llu intersect"), - def->mem.hugepages[i].size, - def->mem.hugepages[j].size); - goto error; - } else if (!def->mem.hugepages[i].nodemask && - !def->mem.hugepages[j].nodemask) { - virReportError(VIR_ERR_XML_DETAIL, - _("two master hugepages detected: " - "%llu and %llu"), - def->mem.hugepages[i].size, - def->mem.hugepages[j].size); + for (i = 0; i < n; i++) { + if (virDomainHugepagesParseXML(nodes[i], ctxt, + &def->mem.hugepages[i]) < 0) goto error; + def->mem.nhugepages++; + + for (j = 0; j < i; j++) { + if (def->mem.hugepages[i].nodemask && + def->mem.hugepages[j].nodemask && + virBitmapOverlaps(def->mem.hugepages[i].nodemask, + def->mem.hugepages[j].nodemask)) { + virReportError(VIR_ERR_XML_DETAIL, + _("nodeset attribute of hugepages " + "of sizes %llu and %llu intersect"), + def->mem.hugepages[i].size, + def->mem.hugepages[j].size); + goto error; + } else if (!def->mem.hugepages[i].nodemask && + !def->mem.hugepages[j].nodemask) { + virReportError(VIR_ERR_XML_DETAIL, + _("two master hugepages detected: " + "%llu and %llu"), + def->mem.hugepages[i].size, + def->mem.hugepages[j].size); + goto error; + } } } - }
- VIR_FREE(nodes); - } else { - if ((node = virXPathNode("./memoryBacking/hugepages", ctxt))) { + VIR_FREE(nodes); + } else { + /* no hugepage pages */ if (VIR_ALLOC(def->mem.hugepages) < 0) goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d2065cf..f1290a3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h
@@ -3073,6 +3104,9 @@ VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) VIR_ENUM_DECL(virDomainMemoryModel) VIR_ENUM_DECL(virDomainMemoryBackingModel) +VIR_ENUM_DECL(virDomainMemorySource) +VIR_ENUM_DECL(virDomainMemoryAccess) +VIR_ENUM_DECL(virDomainMemoryAllocation)
These macros declare vir*TypeFromString() and vir*TypeToString() functions. Because they are in the header file, we should update the libvirt_private.syms too (even though these functions are not used anywere else just yet). The rest of the code looks okay (even in 2/2). Michal