On 03/04/2015 11:24 AM, Peter Krempa wrote:
> This patch adds code that parses and formats configuration for memory
> devices.
>
> A simple configuration would be:
> <memory model='dimm'>
> <target>
> <size unit='KiB'>524287</size>
> <node>0</node>
> </target>
> </memory>
>
> A complete configuration of a memory device:
> <memory model='dimm'>
> <source>
> <pagesize unit='KiB'>4096</pagesize>
> <nodemask>1-3</nodemask>
> </source>
> <target>
> <size unit='KiB'>524287</size>
> <node>1</node>
> </target>
> </memory>
>
> This patch preemptively forbids use of the <memory> device in individual
> drivers so the users are warned right away that the device is not
> supported.
> ---
> docs/formatdomain.html.in | 79 +++++
> docs/schemas/domaincommon.rng | 50 ++++
> src/bhyve/bhyve_domain.c | 5 +-
> src/conf/domain_conf.c | 324 ++++++++++++++++++++-
> src/conf/domain_conf.h | 34 +++
> src/libvirt_private.syms | 2 +
> src/libxl/libxl_domain.c | 3 +
> src/lxc/lxc_domain.c | 4 +
> src/openvz/openvz_driver.c | 3 +
> src/qemu/qemu_domain.c | 3 +
> src/qemu/qemu_driver.c | 13 +
> src/qemu/qemu_hotplug.c | 3 +
> src/uml/uml_driver.c | 3 +
> src/xen/xen_driver.c | 3 +
> src/xenapi/xenapi_driver.c | 3 +
> .../qemuxml2argv-memory-hotplug-dimm.xml | 50 ++++
> 16 files changed, 579 insertions(+), 3 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fcb4ca2..005f6f6 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5901,6 +5901,85 @@ qemu-kvm -net nic,model=? /dev/null
> </dd>
> </dl>
>
> + <h4><a name="elementsMemory">Memory devices</a></h4>
> +
> + <p>
> + Apart from initial memory the memory devices allow adding additional
> + memory for the guest in form of memory modules. These devices also allow
> + hot-add and hot-remove of guest's memory.
In addition to the initial memory assigned to the guest, memory devices
allow additional memory to be assigned to the guest in the form of
memory modules. There can be 1 or many memory devices configured;
however, NUMA must also be configured for the guest. A memory device can
be hot-plugged or hot-unplugged depending on the guests' memory resource
needs.
> + <span class="since">Since 1.2.14</span>
> + </p>
> +
> + <p>
> + Example: usage of the memory devices
> + </p>
> +<pre>
> + ...
> + <devices>
> + <memory model='dimm'>
> + <target>
> + <size unit='KiB'>524287</size>
> + <node>0</node>
> + </target>
> + </memory>
> + <memory model='dimm'>
> + <source>
> + <pagesize unit='KiB'>4096</pagesize>
> + <nodemask>1-3</nodemask>
> + </source>
> + <target>
> + <size unit='KiB'>524287</size>
> + <node>1</node>
> + </target>
> + </memory>
> + </devices>
> + ...
> +</pre>
> + <dl>
> + <dt><code>model</code></dt>
> + <dd>
> + <p>
> + Currently only the <code>dimm</code> model is supported that
> + will result in adding a virtual DIMM module to the guest. Note that
> + hypervisors require having NUMA enabled for the guest for the memory
> + modules to work.
Reads strangely - the "that will result in adding a virtual DIMM module
to the guest" almost seems redundant, but I also see it's importance.
Perhaps if the "that will result in adding" is replace by "in order to add".
Whether it's worth repeating the NUMA here or not is up to you, I just
figured it was better to call it out earlier rather...
> + </p>
> + </dd>
> +
> + <dt><code>source</code></dt>
> + <dd>
> + <p>
> + The optional source element allows to fine tune the source of the
> + memory used for the given memory device. If the element is not
> + provided defaults configured via <code>numatune</code> are used.
> + </p>
> + <p>
> + <code>pagesize</code> can be used to override the default host page
> + size used for backing the memory device.
Is pagesize something hypervisor specific or should it be "suggested" as
a power of 2 at least?
> + </p>
> + <p>
> + <code>nodemask</code> can be used to override the default set of NUMA
> + nodes where the memory would be allocated.
> + </p>
> + </dd>
> +
Is it worth nothing that either or both can be used (eg, both are optional).
> + <dt><code>target</code></dt>
> + <dd>
> + <p>
> + The mandatory <code>target</code> element configures the placement and
> + sizing of the added memory from the perspective of the guest.
> + </p>
> + <p>
> + The mandatory <code>size</code> subelement configures the size of the
> + added memory as a scaled integer.
> + </p>
> + <p>
> + The mandatory <code>node</code> subelement configures the guest NUMA
> + node to attach the memory to.
> + </p>
> + </dd>
> + </dl>
> +
> <h3><a name="seclabel">Security label</a></h3>
>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index c1c02e4..1b40c2e 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4069,6 +4069,7 @@
> <ref name="rng"/>
> <ref name="tpm"/>
> <ref name="shmem"/>
> + <ref name="memorydev"/>
> </choice>
> </zeroOrMore>
> <optional>
> @@ -4485,6 +4486,55 @@
> </element>
> </define>
>
> + <define name="memorydev">
> + <element name="memory">
> + <attribute name="model">
> + <choice>
> + <value>dimm</value>
> + </choice>
> + </attribute>
> + <interleave>
> + <optional>
> + <ref name="memorydev-source"/>
> + </optional>
> + <ref name="memorydev-target"/>
> + <optional>
> + <ref name="address"/>
> + </optional>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name="memorydev-source">
> + <element name="source">
> + <interleave>
> + <optional>
> + <element name="pagesize">
> + <ref name="scaledInteger"/>
> + </element>
> + </optional>
> + <optional>
> + <element name="nodemask">
> + <ref name="cpuset"/>
> + </element>
> + </optional>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name="memorydev-target">
> + <element name="target">
> + <interleave>
> + <element name="size">
> + <ref name="scaledInteger"/>
> + </element>
> + <element name="node">
> + <ref name="unsignedInt"/>
Listed as unsigned here, but read/managed as an 'int' later. A -1 value
doesn't quite make sense I believe...
> + </element>
> + </interleave>
> + </element>
> + </define>
> +
> <define name="rng">
> <element name="rng">
> <attribute name="model">
> diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
> index 25ef852..890963e 100644
> --- a/src/bhyve/bhyve_domain.c
> +++ b/src/bhyve/bhyve_domain.c
> @@ -75,11 +75,14 @@ bhyveDomainDefPostParse(virDomainDefPtr def,
> }
>
> static int
> -bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
> +bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> const virDomainDef *def ATTRIBUTE_UNUSED,
> virCapsPtr caps ATTRIBUTE_UNUSED,
> void *opaque ATTRIBUTE_UNUSED)
> {
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3be52a7..4f20aa6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -236,7 +236,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
> "rng",
> "shmem",
> "tpm",
> - "panic")
> + "panic",
> + "memory")
>
> VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
> "none",
> @@ -779,6 +780,9 @@ VIR_ENUM_DECL(virDomainBlockJob)
> VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
> "", "", "copy", "", "active-commit")
>
> +VIR_ENUM_IMPL(virDomainMemoryModel, VIR_DOMAIN_MEMORY_MODEL_LAST,
> + "", "dimm")
> +
> static virClassPtr virDomainObjClass;
> static virClassPtr virDomainObjListClass;
> static virClassPtr virDomainXMLOptionClass;
> @@ -1003,6 +1007,27 @@ virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def)
> }
>
>
> +/**
> + * virDomainDeviceDefCheckUnsupportedMemoryDevice:
> + * @dev: device definition
> + *
> + * Returns -1 if the device definition describes a memory device and reports an
> + * error. Otherwise returns 0.
> + */
> +int
> +virDomainDeviceDefCheckUnsupportedMemoryDevice(virDomainDeviceDefPtr dev)
> +{
> + /* This driver doesn't yet know how to handle memory devices */
> + if (dev->type == VIR_DOMAIN_DEVICE_MEMORY) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("memory devices are not supported by this driver"));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> static void
> virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
> {
> @@ -1936,6 +1961,16 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def)
> VIR_FREE(def);
> }
>
> +void virDomainMemoryDefFree(virDomainMemoryDefPtr def)
> +{
> + if (!def)
> + return;
> +
> + virBitmapFree(def->sourceNodes);
> + virDomainDeviceInfoClear(&def->info);
> + VIR_FREE(def);
> +}
> +
> void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
> {
> if (!def)
> @@ -2005,6 +2040,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
> case VIR_DOMAIN_DEVICE_PANIC:
> virDomainPanicDefFree(def->data.panic);
> break;
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + virDomainMemoryDefFree(def->data.memory);
> + break;
> case VIR_DOMAIN_DEVICE_LAST:
> case VIR_DOMAIN_DEVICE_NONE:
> break;
> @@ -2202,6 +2240,10 @@ void virDomainDefFree(virDomainDefPtr def)
> virDomainRNGDefFree(def->rngs[i]);
> VIR_FREE(def->rngs);
>
> + for (i = 0; i < def->nmems; i++)
> + virDomainMemoryDefFree(def->mems[i]);
> + VIR_FREE(def->mems);
> +
> virDomainTPMDefFree(def->tpm);
>
> virDomainPanicDefFree(def->panic);
> @@ -2739,6 +2781,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
> return &device->data.tpm->info;
> case VIR_DOMAIN_DEVICE_PANIC:
> return &device->data.panic->info;
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + return &device->data.memory->info;
>
> /* The following devices do not contain virDomainDeviceInfo */
> case VIR_DOMAIN_DEVICE_LEASE:
> @@ -3059,6 +3103,13 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
> return -1;
> }
>
> + device.type = VIR_DOMAIN_DEVICE_MEMORY;
> + for (i = 0; i < def->nmems; i++) {
> + device.data.memory = def->mems[i];
> + if (cb(def, &device, &def->mems[i]->info, opaque) < 0)
> + return -1;
> + }
> +
> /* Coverity is not very happy with this - all dead_error_condition */
> #if !STATIC_ANALYSIS
> /* This switch statement is here to trigger compiler warning when adding
> @@ -3091,6 +3142,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
> case VIR_DOMAIN_DEVICE_PANIC:
> case VIR_DOMAIN_DEVICE_LAST:
> case VIR_DOMAIN_DEVICE_RNG:
> + case VIR_DOMAIN_DEVICE_MEMORY:
> break;
> }
> #endif
> @@ -7029,7 +7081,13 @@ virDomainDefSetMemoryInitial(virDomainDefPtr def,
> unsigned long long
> virDomainDefGetMemoryActual(virDomainDefPtr def)
> {
> - return virDomainDefGetMemoryInitial(def);
> + unsigned long long ret = virDomainDefGetMemoryInitial(def);
> + size_t i;
> +
> + for (i = 0; i < def->nmems; i++)
> + ret += def->mems[i]->size;
> +
> + return ret;
> }
>
>
> @@ -11383,6 +11441,119 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,
> return ret;
> }
>
> +
> +static int
> +virDomainMemorySourceDefParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> + virDomainMemoryDefPtr def)
> +{
> + int ret = -1;
> + char *nodemask = NULL;
> + xmlNodePtr save = ctxt->node;
> + ctxt->node = node;
> +
> + if (virDomainParseMemory("./pagesize", "./pagesize/@unit", ctxt,
> + &def->pagesize, false, false) < 0)
> + goto cleanup;
> +
> + if ((nodemask = virXPathString("string(./nodemask)", ctxt))) {
> + if (virBitmapParse(nodemask, 0, &def->sourceNodes,
> + VIR_DOMAIN_CPUMASK_LEN) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(nodemask);
> + ctxt->node = save;
> + return ret;
> +}
> +
> +
> +static int
> +virDomainMemoryTargetDefParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> + virDomainMemoryDefPtr def)
> +{
> + int ret = -1;
> + xmlNodePtr save = ctxt->node;
> + ctxt->node = node;
> +
> + if (virXPathInt("string(./node)", ctxt, &def->targetNode) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("invalid or missing value of memory device node"));
> + goto cleanup;
> + }
Listed as unsigned in rng file... We should follow that here.
Also, perhaps using the virStrToLong_uip would be better...
> +
> + if (virDomainParseMemory("./size", "./size/@unit", ctxt,
> + &def->size, true, false) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + ctxt->node = save;
> + return ret;
> +}
> +
> +
> +static virDomainMemoryDefPtr
> +virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
> + xmlXPathContextPtr ctxt,
> + unsigned int flags)
> +{
> + char *tmp = NULL;
> + xmlNodePtr save = ctxt->node;
> + xmlNodePtr node;
> + virDomainMemoryDefPtr def;
> +
> + ctxt->node = memdevNode;
> +
> + if (VIR_ALLOC(def) < 0)
> + return NULL;
> +
> + if (!(tmp = virXMLPropString(memdevNode, "model"))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing memory model"));
> + goto error;
> + }
> +
> + if ((def->model = virDomainMemoryModelTypeFromString(tmp)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("invalid memory model '%s'"), tmp);
> + goto error;
> + }
> + VIR_FREE(tmp);
> +
> + /* source */
> + if ((node = virXPathNode("./source", ctxt)) &&
> + virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
> + goto error;
> +
> + /* target */
> + if (!(node = virXPathNode("./target", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing <target> element for <memory> device"));
> + goto error;
> + }
> +
> + if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
> + goto error;
> +
> + if (virDomainDeviceInfoParseXML(memdevNode, NULL, &def->info, flags) < 0)
> + goto error;
> +
> + return def;
> +
> + error:
> + VIR_FREE(tmp);
> + virDomainMemoryDefFree(def);
> + ctxt->node = save;
> + return NULL;
> +}
> +
> +
> virDomainDeviceDefPtr
> virDomainDeviceDefParse(const char *xmlStr,
> const virDomainDef *def,
> @@ -11517,6 +11688,10 @@ virDomainDeviceDefParse(const char *xmlStr,
> if (!(dev->data.panic = virDomainPanicDefParseXML(node)))
> goto error;
> break;
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + if (!(dev->data.memory = virDomainMemoryDefParseXML(node, ctxt, flags)))
> + goto error;
> + break;
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_LAST:
> break;
> @@ -14896,6 +15071,23 @@ virDomainDefParseXML(xmlDocPtr xml,
> ctxt->node = node;
> VIR_FREE(nodes);
>
> + /* analysis of memory devices */
> + if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0)
> + goto error;
> + if (n && VIR_ALLOC_N(def->mems, n) < 0)
> + goto error;
> +
> + for (i = 0; i < n; i++) {
> + virDomainMemoryDefPtr mem = virDomainMemoryDefParseXML(nodes[i],
> + ctxt,
> + flags);
> + if (!mem)
> + goto error;
> +
> + def->mems[def->nmems++] = mem;
> + }
> + VIR_FREE(nodes);
> +
> /* analysis of the user namespace mapping */
> if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0)
> goto error;
> @@ -16135,6 +16327,39 @@ virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
> return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
> }
>
> +static bool
> +virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src,
> + virDomainMemoryDefPtr dst)
> +{
> + if (src->model != dst->model) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target memory device model '%s' "
> + "doesn't match source model '%s'"),
> + virDomainMemoryModelTypeToString(dst->model),
> + virDomainMemoryModelTypeToString(src->model));
> + return false;
> + }
> +
> + if (src->targetNode != dst->targetNode) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target memory device targetNode '%d' "
> + "doesn't match source targetNode '%d'"),
> + dst->targetNode, src->targetNode);
> + return false;
> + }
These could be %u if we match types w/ rng
> +
> + if (src->size != dst->size) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target memory device size '%llu' doesn't match "
> + "source memory device size '%llu'"),
> + dst->size, src->size);
> + return false;
> + }
No need to compare the sourceNodes bitmaps and pagesizes?
> +
> + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
> +}
> +
> +
> /* This compares two configurations and looks for any differences
> * which will affect the guest ABI. This is primarily to allow
> * validation of custom XML config passed in during migration
> @@ -16553,6 +16778,18 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> goto error;
> }
>
> + if (src->nmems != dst->nmems) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target domain memory device count %zu "
> + "does not match source %zu"), dst->nmems, src->nmems);
> + goto error;
> + }
> +
> + for (i = 0; i < src->nmems; i++) {
> + if (!virDomainMemoryDefCheckABIStability(src->mems[i], dst->mems[i]))
> + goto error;
> + }
> +
> /* Coverity is not very happy with this - all dead_error_condition */
> #if !STATIC_ANALYSIS
> /* This switch statement is here to trigger compiler warning when adding
> @@ -16584,6 +16821,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> case VIR_DOMAIN_DEVICE_TPM:
> case VIR_DOMAIN_DEVICE_PANIC:
> case VIR_DOMAIN_DEVICE_SHMEM:
> + case VIR_DOMAIN_DEVICE_MEMORY:
> break;
> }
> #endif
> @@ -19066,6 +19304,81 @@ virDomainRNGDefFree(virDomainRNGDefPtr def)
> VIR_FREE(def);
> }
>
> +
> +static int
> +virDomainMemorySourceDefFormat(virBufferPtr buf,
> + virDomainMemoryDefPtr def)
> +{
> + char *bitmap = NULL;
> + int ret = -1;
> +
> + if (!def->pagesize && !def->sourceNodes)
> + return 0;
> +
> + virBufferAddLit(buf, "<source>\n");
> + virBufferAdjustIndent(buf, 2);
> +
> + if (def->sourceNodes) {
> + if (!(bitmap = virBitmapFormat(def->sourceNodes)))
> + goto cleanup;
> +
> + virBufferAsprintf(buf, "<nodemask>%s</nodemask>\n", bitmap);
> + }
> +
> + if (def->pagesize)
> + virBufferAsprintf(buf, "<pagesize unit='KiB'>%llu</pagesize>\n",
> + def->pagesize);
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</source>\n");
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(bitmap);
> + return ret;
> +}
> +
> +
> +static void
> +virDomainMemoryTargetDefFormat(virBufferPtr buf,
> + virDomainMemoryDefPtr def)
> +{
> + virBufferAddLit(buf, "<target>\n");
> + virBufferAdjustIndent(buf, 2);
> +
> + virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->size);
> + virBufferAsprintf(buf, "<node>%d</node>\n", def->targetNode);
Would be a %u if we match rng type...
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</target>\n");
> +}
> +
> +static int
> +virDomainMemoryDefFormat(virBufferPtr buf,
> + virDomainMemoryDefPtr def,
> + unsigned int flags)
> +{
> + const char *model = virDomainMemoryModelTypeToString(def->model);
> +
> + virBufferAsprintf(buf, "<memory model='%s'>\n", model);
> + virBufferAdjustIndent(buf, 2);
> +
> + if (virDomainMemorySourceDefFormat(buf, def) < 0)
> + return -1;
> +
> + virDomainMemoryTargetDefFormat(buf, def);
> +
> + if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
> + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> + return -1;
> + }
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</memory>\n");
> + return 0;
> +}
> +
> static void
> virDomainVideoAccelDefFormat(virBufferPtr buf,
> virDomainVideoAccelDefPtr def)
> @@ -20655,6 +20968,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> if (virDomainShmemDefFormat(buf, def->shmems[n], flags) < 0)
> goto error;
>
> + for (n = 0; n < def->nmems; n++)
> + if (virDomainMemoryDefFormat(buf, def->mems[n], flags) < 0)
> + goto error;
> +
> virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</devices>\n");
>
> @@ -22058,6 +22375,9 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
> case VIR_DOMAIN_DEVICE_PANIC:
> rc = virDomainPanicDefFormat(&buf, src->data.panic);
> break;
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + rc = virDomainMemoryDefFormat(&buf, src->data.memory, flags);
> + break;
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index da13bf6..706040d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -133,6 +133,9 @@ typedef virDomainIdMapDef *virDomainIdMapDefPtr;
> typedef struct _virDomainPanicDef virDomainPanicDef;
> typedef virDomainPanicDef *virDomainPanicDefPtr;
>
> +typedef struct _virDomainMemoryDef virDomainMemoryDef;
> +typedef virDomainMemoryDef *virDomainMemoryDefPtr;
> +
> /* forward declarations virDomainChrSourceDef, required by
> * virDomainNetDef
> */
> @@ -169,6 +172,7 @@ typedef enum {
> VIR_DOMAIN_DEVICE_SHMEM,
> VIR_DOMAIN_DEVICE_TPM,
> VIR_DOMAIN_DEVICE_PANIC,
> + VIR_DOMAIN_DEVICE_MEMORY,
>
> VIR_DOMAIN_DEVICE_LAST
> } virDomainDeviceType;
> @@ -199,6 +203,7 @@ struct _virDomainDeviceDef {
> virDomainShmemDefPtr shmem;
> virDomainTPMDefPtr tpm;
> virDomainPanicDefPtr panic;
> + virDomainMemoryDefPtr memory;
> } data;
> };
>
> @@ -1966,6 +1971,28 @@ struct _virDomainRNGDef {
> virDomainDeviceInfo info;
> };
>
> +typedef enum {
> + VIR_DOMAIN_MEMORY_MODEL_NONE,
> + VIR_DOMAIN_MEMORY_MODEL_DIMM, /* dimm hotpluggable memory device */
> +
> + VIR_DOMAIN_MEMORY_MODEL_LAST
> +} virDomainMemoryModel;
> +
> +struct _virDomainMemoryDef {
> + /* source */
> + virBitmapPtr sourceNodes;
> + unsigned long long pagesize; /* kibibytes */
> +
> + /* target */
> + int model; /* virDomainMemoryModel */
> + int targetNode;
Listed as unsigned in rng...
> + unsigned long long size; /* kibibytes */
> +
> + virDomainDeviceInfo info;
> +};
> +
> +void virDomainMemoryDefFree(virDomainMemoryDefPtr def);
> +
> struct _virDomainIdMapEntry {
> unsigned int start;
> unsigned int target;
> @@ -2186,6 +2213,9 @@ struct _virDomainDef {
> size_t nshmems;
> virDomainShmemDefPtr *shmems;
>
> + size_t nmems;
> + virDomainMemoryDefPtr *mems;
> +
> /* Only 1 */
> virDomainWatchdogDefPtr watchdog;
> virDomainMemballoonDefPtr memballoon;
> @@ -2348,6 +2378,7 @@ bool virDomainObjTaint(virDomainObjPtr obj,
>
>
> int virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def);
> +int virDomainDeviceDefCheckUnsupportedMemoryDevice(virDomainDeviceDefPtr dev);
>
> void virDomainPanicDefFree(virDomainPanicDefPtr panic);
> void virDomainResourceDefFree(virDomainResourceDefPtr resource);
> @@ -2893,6 +2924,9 @@ VIR_ENUM_DECL(virDomainRNGModel)
> VIR_ENUM_DECL(virDomainRNGBackend)
> VIR_ENUM_DECL(virDomainTPMModel)
> VIR_ENUM_DECL(virDomainTPMBackend)
> +VIR_ENUM_DECL(virDomainThreadSched)
^^^ This seems out of place
Rest seems fine - couple of nits, ACK with the adjustments.
John
NOTE: It's late for me - I'll pick up the rest of these when I get up in
the morning...
> +VIR_ENUM_DECL(virDomainMemoryModel)
> +VIR_ENUM_DECL(virDomainMemoryBackingModel)
> /* from libvirt.h */
> VIR_ENUM_DECL(virDomainState)
> VIR_ENUM_DECL(virDomainNostateReason)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1289884..2491d2d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -216,6 +216,7 @@ virDomainDefSetMemoryInitial;
> virDomainDeleteConfig;
> virDomainDeviceAddressIsValid;
> virDomainDeviceAddressTypeToString;
> +virDomainDeviceDefCheckUnsupportedMemoryDevice;
> virDomainDeviceDefCopy;
> virDomainDeviceDefFree;
> virDomainDeviceDefParse;
> @@ -332,6 +333,7 @@ virDomainLockFailureTypeFromString;
> virDomainLockFailureTypeToString;
> virDomainMemballoonModelTypeFromString;
> virDomainMemballoonModelTypeToString;
> +virDomainMemoryDefFree;
> virDomainNetAppendIpAddress;
> virDomainNetDefFormat;
> virDomainNetDefFree;
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 5d186c3..2317434 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -546,6 +546,9 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> }
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
> index 1367b0c..c2180cb 100644
> --- a/src/lxc/lxc_domain.c
> +++ b/src/lxc/lxc_domain.c
> @@ -113,6 +113,10 @@ virLXCDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)
> dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC;
>
> +
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 8cc392e..8b15402 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -130,6 +130,9 @@ openvzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> return -1;
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index a654d2e..4225f38 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1193,6 +1193,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> }
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + goto cleanup;
> +
> ret = 0;
>
> cleanup:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7c8c06b..d6935d9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7095,6 +7095,9 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> dev->data.rng = NULL;
> break;
>
> + /*TODO: implement later */
> + case VIR_DOMAIN_DEVICE_MEMORY:
> +
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_FS:
> case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7172,6 +7175,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> case VIR_DOMAIN_DEVICE_RNG:
> ret = qemuDomainDetachRNGDevice(driver, vm, dev->data.rng);
> break;
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + /* TODO: Implement later */
> case VIR_DOMAIN_DEVICE_FS:
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> @@ -7289,6 +7294,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
> case VIR_DOMAIN_DEVICE_HOSTDEV:
> case VIR_DOMAIN_DEVICE_CONTROLLER:
> case VIR_DOMAIN_DEVICE_REDIRDEV:
> + case VIR_DOMAIN_DEVICE_MEMORY:
> case VIR_DOMAIN_DEVICE_CHR:
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_TPM:
> @@ -7431,6 +7437,9 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> dev->data.rng = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + /* TODO: implement later */
> +
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -7556,6 +7565,9 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> virDomainRNGDefFree(virDomainRNGRemove(vmdef, idx));
> break;
>
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + /* TODO: implement later */
> +
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -7670,6 +7682,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
> case VIR_DOMAIN_DEVICE_CONTROLLER:
> case VIR_DOMAIN_DEVICE_REDIRDEV:
> case VIR_DOMAIN_DEVICE_CHR:
> + case VIR_DOMAIN_DEVICE_MEMORY:
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_TPM:
> case VIR_DOMAIN_DEVICE_PANIC:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 2b5c45f..cb2270e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3070,6 +3070,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
> qemuDomainRemoveRNGDevice(driver, vm, dev->data.rng);
> break;
>
> + /* TODO: implement later */
> + case VIR_DOMAIN_DEVICE_MEMORY:
> +
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_LEASE:
> case VIR_DOMAIN_DEVICE_FS:
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index bdfc12e..2d59126 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -439,6 +439,9 @@ umlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> return -1;
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index f603477..1cceacb 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -372,6 +372,9 @@ xenDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> }
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
> index c6acfc9..439f767 100644
> --- a/src/xenapi/xenapi_driver.c
> +++ b/src/xenapi/xenapi_driver.c
> @@ -65,6 +65,9 @@ xenapiDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> return -1;
> }
>
> + if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
> new file mode 100644
> index 0000000..78088e2
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
> @@ -0,0 +1,50 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
> + <memory unit='KiB'>1267710</memory>
> + <currentMemory unit='KiB'>1267710</currentMemory>
> + <vcpu placement='static' cpuset='0-1'>2</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <cpu>
> + <topology sockets='2' cores='1' threads='1'/>
> + <numa>
> + <cell id='0' cpus='0-1' memory='219136' unit='KiB'/>
> + </numa>
> + </cpu>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='block' device='disk'>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='ide' index='0'/>
> + <controller type='usb' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <memballoon model='virtio'/>
> + <memory model='dimm'>
> + <target>
> + <size unit='KiB'>524287</size>
> + <node>0</node>
> + </target>
> + </memory>
> + <memory model='dimm'>
> + <source>
> + <nodemask>1-3</nodemask>
> + <pagesize unit='KiB'>4096</pagesize>
> + </source>
> + <target>
> + <size unit='KiB'>524287</size>
> + <node>0</node>
> + </target>
> + </memory>
> + </devices>
> +</domain>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list