[libvirt] [PATCH 00/11] Add support for memory hot(un)plug

A final version of the memory hotplug series. This version incorporates feedback on the RFC and fixes the few missing bits. This series depends on the three previous refactor series: http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html [libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure http://www.redhat.com/archives/libvir-list/2015-February/msg00557.html [libvirt] [PATCH 0/3] Fix memory ABI stability check issues http://www.redhat.com/archives/libvir-list/2015-February/msg00633.html [libvirt] [PATCH 0/7] Automaticaly fill <memory> element for NUMA enabled guests For convenience, you can fetch the complete series in my public branch: git fetch git://pipo.sk/pipo/libvirt.git memory-hotplug-v1 Peter Krempa (11): qemu: caps: Add capability bit for the "pc-dimm" device conf: Add support for parsing and formatting max memory and slot count qemu: Implement setup of memory hotplug parameters conf: Add device address type for dimm devices conf: Add interface to parse and format memory device information qemu: memdev: Add infrastructure to load memory device information qemu: migration: Forbid migration with memory modules lacking info qemu: add support for memory devices qemu: conf: Add support for memory device cold(un)plug qemu: Implement memory device hotplug qemu: Implement memory device hotunplug docs/formatdomain.html.in | 98 ++++ docs/schemas/domaincommon.rng | 76 +++ src/bhyve/bhyve_domain.c | 9 +- src/conf/domain_conf.c | 566 ++++++++++++++++++++- src/conf/domain_conf.h | 59 +++ src/libvirt_private.syms | 7 + src/libxl/libxl_domain.c | 8 + src/lxc/lxc_domain.c | 8 + src/openvz/openvz_driver.c | 14 +- src/parallels/parallels_driver.c | 6 +- src/phyp/phyp_driver.c | 6 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 166 +++++- src/qemu/qemu_command.h | 15 + src/qemu/qemu_domain.c | 80 +++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_driver.c | 29 ++ src/qemu/qemu_hotplug.c | 177 +++++++ src/qemu/qemu_hotplug.h | 6 + src/qemu/qemu_migration.c | 14 + src/qemu/qemu_monitor.c | 42 ++ src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 122 +++++ src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_process.c | 4 + src/uml/uml_driver.c | 9 +- src/vbox/vbox_common.c | 6 +- src/vmware/vmware_driver.c | 6 +- src/vmx/vmx.c | 6 +- src/xen/xen_driver.c | 7 + src/xenapi/xenapi_driver.c | 9 +- tests/domainschemadata/maxMemory.xml | 19 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + .../qemuxml2argv-memory-hotplug-dimm.args | 11 + .../qemuxml2argv-memory-hotplug-dimm.xml | 50 ++ .../qemuxml2argv-memory-hotplug-nonuma.xml | 22 + .../qemuxml2argv-memory-hotplug.args | 6 + .../qemuxml2argv-memory-hotplug.xml | 34 ++ tests/qemuxml2argvtest.c | 6 + tests/qemuxml2xmltest.c | 4 + 41 files changed, 1712 insertions(+), 23 deletions(-) create mode 100644 tests/domainschemadata/maxMemory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml -- 2.2.2

The pc-dimm device represents a RAM memory module. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4596f6c..dc8533e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -277,6 +277,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vmware-svga.vgamem_mb", "qxl.vgamem_mb", "qxl-vga.vgamem_mb", + "pc-dimm", ); @@ -1530,6 +1531,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO }, { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, + { "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 1c1227a..5d08a7a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -223,6 +223,7 @@ typedef enum { QEMU_CAPS_VMWARE_SVGA_VGAMEM = 181, /* -device vmware-svga.vgamem_mb */ QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ + QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 4e040fc..5637edb 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -165,4 +165,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pc-dimm'/> </qemuCaps> -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:26PM +0100, Peter Krempa wrote:
The pc-dimm device represents a RAM memory module. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + 3 files changed, 4 insertions(+)
ACK
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4596f6c..dc8533e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -277,6 +277,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vmware-svga.vgamem_mb", "qxl.vgamem_mb", "qxl-vga.vgamem_mb", + "pc-dimm", );
@@ -1530,6 +1531,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO }, { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, + { "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 1c1227a..5d08a7a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -223,6 +223,7 @@ typedef enum { QEMU_CAPS_VMWARE_SVGA_VGAMEM = 181, /* -device vmware-svga.vgamem_mb */ QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ + QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 4e040fc..5637edb 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -165,4 +165,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pc-dimm'/> </qemuCaps> -- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Feb 20, 2015 at 08:40:52 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:26PM +0100, Peter Krempa wrote:
The pc-dimm device represents a RAM memory module. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + 3 files changed, 4 insertions(+)
ACK
Pushed; Thanks. Peter

Add a XML element that will allow to specify maximum supportable memory and the count of memory slots to use with memory hotplug. To avoid possible confusion and misuse of the new element this patch also explicitly forbids the use of the maxMemory setting in individual drivers's post parse callbacks. This limitation will be lifted when the support will be implemented. --- docs/formatdomain.html.in | 19 +++++++++++ docs/schemas/domaincommon.rng | 8 +++++ src/bhyve/bhyve_domain.c | 4 +++ src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 5 +++ src/lxc/lxc_domain.c | 4 +++ src/openvz/openvz_driver.c | 11 ++++-- src/parallels/parallels_driver.c | 6 +++- src/phyp/phyp_driver.c | 6 +++- src/qemu/qemu_domain.c | 4 +++ src/uml/uml_driver.c | 6 +++- src/vbox/vbox_common.c | 6 +++- src/vmware/vmware_driver.c | 6 +++- src/vmx/vmx.c | 6 +++- src/xen/xen_driver.c | 4 +++ src/xenapi/xenapi_driver.c | 6 +++- tests/domainschemadata/maxMemory.xml | 19 +++++++++++ 19 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 tests/domainschemadata/maxMemory.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f6477c2..806ae42 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -675,6 +675,7 @@ <pre> <domain> ... + <maxMemory slots='123' unit='KiB'>1524288</maxMemory> <memory unit='KiB'>524288</memory> <currentMemory unit='KiB'>524288</currentMemory> ... @@ -708,6 +709,24 @@ <span class='since'><code>unit</code> since 0.9.11</span>, <span class='since'><code>dumpCore</code> since 0.10.2 (QEMU only)</span></dd> + <dt><code>maxMemory</code></dt> + <dd>The run time maximum memory allocation of the guest. The initial + memory specified by <code><memory></code> can be increased by + hot-plugging of memory to the limit specified by this element. + + The <code>unit</code> attribute behaves the same as for + <code>memory</code>. + + The <code>slots</code> attribute specifies the number of slots + available for adding memory to the guest. The bounds are hypervisor + specific. + + Note that due to alignment of the memory chunks added via memory + hotplug the full size allocation specified by this element may be + impossible to achieve. + <span class='since'>Since 1.2.13</span> + </dd> + <dt><code>currentMemory</code></dt> <dd>The actual allocation of memory for the guest. This value can be less than the maximum allocation, to allow for ballooning diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 27a24b4..acfa16a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -577,6 +577,14 @@ --> <define name="resources"> <interleave> + <optional> + <element name="maxMemory"> + <ref name="scaledInteger"/> + <attribute name="slots"> + <ref name="unsignedInt"/> + </attribute> + </element> + </optional> <element name="memory"> <ref name='scaledInteger'/> <optional> diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index ecb1758..25ef852 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -67,6 +67,10 @@ bhyveDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) return -1; + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a4cb930..fb35880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -987,6 +987,27 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, } +/** + * virDomainDefCheckUnsupportedMemoryHotplug: + * @def: domain definition + * + * Returns -1 if the domain definition would enable memory hotplug via the + * <maxMemory> tunable and reports an error. Otherwise returns 0. + */ +int +virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def) +{ + /* memory hotplug tunables are not supported by this driver */ + if (def->mem.max_memory > 0 || def->mem.memory_slots > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory hotplug tunables <maxMemory> are not " + "supported by this hypervisor driver")); + return -1; + } + + return 0; +} + static void virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) @@ -3194,6 +3215,22 @@ virDomainDefPostParseInternal(virDomainDefPtr def, def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def); } + if ((def->mem.max_memory || def->mem.memory_slots) && + !(def->mem.max_memory && def->mem.memory_slots)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("both maximum memory size and " + "memory slot count must be sepcified")); + return -1; + } + + if (def->mem.max_memory && + def->mem.max_memory < virDomainDefGetMemoryCurrent(def)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size must be equal or greater than " + "the actual memory size")); + return -1; + } + /* * Some really crazy backcompat stuff for consoles * @@ -13100,6 +13137,16 @@ virDomainDefParseXML(xmlDocPtr xml, &def->mem.cur_balloon, false, true) < 0) goto error; + if (virDomainParseMemory("./maxMemory[1]", NULL, ctxt, + &def->mem.max_memory, false, false) < 0) + goto error; + + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Failed to parse memory slot count")); + goto error; + } + /* and info about it */ if ((tmp = virXPathString("string(./memory[1]/@dumpCore)", ctxt)) && (def->mem.dump_core = virTristateSwitchTypeFromString(tmp)) <= 0) { @@ -16020,6 +16067,19 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainNumaCheckABIStability(src->numa, dst->numa)) goto error; + if (src->mem.memory_slots != dst->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain memory slots count '%u' doesn't match source '%u"), + dst->mem.memory_slots, src->mem.memory_slots); + goto error; + } + if (src->mem.max_memory != dst->mem.max_memory) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target maximum memory size '%llu' doesn't match source '%llu'"), + dst->mem.max_memory, src->mem.max_memory); + goto error; + } + if (src->vcpus != dst->vcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vCPU count %d does not match source %d"), @@ -19701,6 +19761,12 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; } + if (def->mem.memory_slots && def->mem.max_memory) { + virBufferAsprintf(buf, + "<maxMemory slots='%u' unit='KiB'>%llu</maxMemory>\n", + def->mem.memory_slots, def->mem.max_memory); + } + virBufferAddLit(buf, "<memory"); if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7f9104e..d24db3a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2045,6 +2045,10 @@ struct _virDomainMemtune { virDomainHugePagePtr hugepages; size_t nhugepages; + /* maximum supported memory for a guest, for hotplugging */ + unsigned long long max_memory; /* in kibibytes */ + unsigned int memory_slots; /* maximum count of RAM memory slots */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ @@ -2340,6 +2344,9 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, bool virDomainObjTaint(virDomainObjPtr obj, virDomainTaintFlags taint); + +int virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def); + void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e2eb40c..d01b884 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -187,6 +187,7 @@ virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; virDomainDefAddImplicitControllers; virDomainDefCheckABIStability; +virDomainDefCheckUnsupportedMemoryHotplug; virDomainDefClearCCWAddresses; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 21c41d7..5d186c3 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -576,6 +576,11 @@ libxlDomainDefPostParse(virDomainDefPtr def, def->nconsoles = 1; def->consoles[0] = chrdef; } + + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 55f5b49..1367b0c 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -94,6 +94,10 @@ virLXCDomainDefPostParse(virDomainDefPtr def, !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) return -1; + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 236d372..fe18acc 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -96,8 +96,15 @@ openvzDomainDefPostParse(virDomainDefPtr def, void *opaque ATTRIBUTE_UNUSED) { /* fill the init path */ - if (STREQ(def->os.type, "exe") && !def->os.init) - return VIR_STRDUP(def->os.init, "/sbin/init") < 0 ? -1 : 0; + if (STREQ(def->os.type, "exe") && !def->os.init) { + if (VIR_STRDUP(def->os.init, "/sbin/init") < 0) + return -1; + } + + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index b73a98d..2cfc827 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -154,10 +154,14 @@ parallelsConnectGetCapabilities(virConnectPtr conn) } static int -parallelsDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +parallelsDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9853bfe..7ac2a62 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1094,10 +1094,14 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, static int -phypDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +phypDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c41b3d3..288804d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1040,6 +1040,10 @@ qemuDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_INPUT_BUS_USB) < 0) return -1; + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 59ff33c..d2a1519 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -444,10 +444,14 @@ umlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, static int -umlDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +umlDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 6697ecc..f3e72b8 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -249,10 +249,14 @@ static char *vboxGenerateMediumName(PRUint32 storageBus, } static int -vboxDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +vboxDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 71589a6..720f5e3 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -83,10 +83,14 @@ vmwareDataFreeFunc(void *data) } static int -vmwareDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +vmwareDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index eba1a68..c2e2371 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -524,10 +524,14 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, * Helpers */ static int -vmxDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +vmxDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 5e6ef68..f603477 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -390,6 +390,10 @@ xenDomainDefPostParse(virDomainDefPtr def, def->memballoon = memballoon; } + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index b3dd852..c6acfc9 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -70,10 +70,14 @@ xenapiDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, static int -xenapiDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, +xenapiDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + /* memory hotplug tunables are not supported by this driver */ + if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) + return -1; + return 0; } diff --git a/tests/domainschemadata/maxMemory.xml b/tests/domainschemadata/maxMemory.xml new file mode 100644 index 0000000..df2e3d8 --- /dev/null +++ b/tests/domainschemadata/maxMemory.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='9' unit='KiB'>1233456789</maxMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:27PM +0100, Peter Krempa wrote:
Add a XML element that will allow to specify maximum supportable memory and the count of memory slots to use with memory hotplug.
To avoid possible confusion and misuse of the new element this patch also explicitly forbids the use of the maxMemory setting in individual drivers's post parse callbacks. This limitation will be lifted when the support will be implemented.
s/will be/is/ ? I'm always unsure when it comes to such "details" :)
--- docs/formatdomain.html.in | 19 +++++++++++ docs/schemas/domaincommon.rng | 8 +++++ src/bhyve/bhyve_domain.c | 4 +++ src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 5 +++ src/lxc/lxc_domain.c | 4 +++ src/openvz/openvz_driver.c | 11 ++++-- src/parallels/parallels_driver.c | 6 +++- src/phyp/phyp_driver.c | 6 +++- src/qemu/qemu_domain.c | 4 +++ src/uml/uml_driver.c | 6 +++- src/vbox/vbox_common.c | 6 +++- src/vmware/vmware_driver.c | 6 +++- src/vmx/vmx.c | 6 +++- src/xen/xen_driver.c | 4 +++ src/xenapi/xenapi_driver.c | 6 +++- tests/domainschemadata/maxMemory.xml | 19 +++++++++++ 19 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 tests/domainschemadata/maxMemory.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a4cb930..fb35880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3194,6 +3215,22 @@ virDomainDefPostParseInternal(virDomainDefPtr def, def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def); }
+ if ((def->mem.max_memory || def->mem.memory_slots) && + !(def->mem.max_memory && def->mem.memory_slots)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("both maximum memory size and " + "memory slot count must be sepcified"));
s/sepcified/specified/
@@ -19701,6 +19761,12 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; }
+ if (def->mem.memory_slots && def->mem.max_memory) {
Another dead code, but I like this one because it makes sure all the coverities etc. will not misunderstand the code. visual-based ACK, Martin

To enable memory hotplug the maximum memory size and slot count need to be specified. As qemu supports now other units than mebibytes when specifying memory, use the new interface in this case. --- docs/formatdomain.html.in | 2 +- src/qemu/qemu_command.c | 34 ++++++++++++++++++---- src/qemu/qemu_domain.c | 8 ++--- .../qemuxml2argv-memory-hotplug-nonuma.xml | 22 ++++++++++++++ .../qemuxml2argv-memory-hotplug.args | 6 ++++ .../qemuxml2argv-memory-hotplug.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 ++ 8 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 806ae42..6320703 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -724,7 +724,7 @@ Note that due to alignment of the memory chunks added via memory hotplug the full size allocation specified by this element may be impossible to achieve. - <span class='since'>Since 1.2.13</span> + <span class='since'>Since 1.2.13 supported by the QEMU driver.</span> </dd> <dt><code>currentMemory</code></dt> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7b83f9f..d9666c3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8313,13 +8313,35 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuDomainAlignMemorySizes(def) < 0) goto error; - /* Set '-m MB' based on maxmem, because the lower 'memory' limit - * is set post-startup using the balloon driver. If balloon driver - * is not supported, then they're out of luck anyway. Update the - * XML to reflect our rounding. - */ virCommandAddArg(cmd, "-m"); - virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); + + if (def->mem.max_memory || def->mem.memory_slots) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory hotplug isn't supported by this QEMU binary")); + goto error; + } + + /* due to guest support, qemu would silently enable NUMA with one node + * once the memory hotplug backend is enabled. To avoid possible + * confusion we will enforce user originated numa configuration along + * with memory hotplug. */ + if (virDomainNumaGetNodeCount(def->numa) == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("At least one numa node has to be configured when " + "enabling memory hotplug")); + goto error; + } + + /* Use the 'k' suffix to let qemu handle the units */ + virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk", + virDomainDefGetMemoryInitial(def), + def->mem.memory_slots, + def->mem.max_memory); + + } else { + virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); + } if (def->mem.nhugepages && !virDomainNumaGetNodeCount(def->numa)) { const long system_page_size = virGetSystemPageSizeKB(); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 288804d..46a7411 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1040,10 +1040,6 @@ qemuDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_INPUT_BUS_USB) < 0) return -1; - /* memory hotplug tunables are not supported by this driver */ - if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0) - return -1; - return 0; } @@ -2855,5 +2851,9 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) mem = virDomainDefGetMemoryInitial(def); virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024)); + /* Align maximum memory size. QEMU requires rounding to next 4KiB block. + * We'll take the "traditional" path and round it to 1MiB*/ + def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, 1024); + return 0; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml new file mode 100644 index 0000000..5c807ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='9' unit='KiB'>1233456789</maxMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args new file mode 100644 index 0000000..6c26586 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc \ +-m size=219136k,slots=16,maxmem=1099511627776k \ +-smp 2 -numa node,nodeid=0,cpus=0-1,mem=214 \ +-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml new file mode 100644 index 0000000..567a662 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39ed66b..92b5fc5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1533,6 +1533,10 @@ mymain(void) DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); DO_TEST("cpu-host-passthrough-features", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST); + DO_TEST_FAILURE("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM); + DO_TEST_FAILURE("memory-hotplug", 0); + DO_TEST("memory-hotplug", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 99d4629..be6a010 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -423,6 +423,9 @@ mymain(void) DO_TEST("shmem"); DO_TEST("smbios"); + DO_TEST("memory-hotplug"); + DO_TEST("memory-hotplug-nonuma"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:28PM +0100, Peter Krempa wrote:
To enable memory hotplug the maximum memory size and slot count need to be specified. As qemu supports now other units than mebibytes when specifying memory, use the new interface in this case. --- docs/formatdomain.html.in | 2 +- src/qemu/qemu_command.c | 34 ++++++++++++++++++---- src/qemu/qemu_domain.c | 8 ++--- .../qemuxml2argv-memory-hotplug-nonuma.xml | 22 ++++++++++++++ .../qemuxml2argv-memory-hotplug.args | 6 ++++ .../qemuxml2argv-memory-hotplug.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 ++ 8 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7b83f9f..d9666c3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8313,13 +8313,35 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuDomainAlignMemorySizes(def) < 0) goto error;
- /* Set '-m MB' based on maxmem, because the lower 'memory' limit - * is set post-startup using the balloon driver. If balloon driver - * is not supported, then they're out of luck anyway. Update the - * XML to reflect our rounding. - */ virCommandAddArg(cmd, "-m"); - virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); + + if (def->mem.max_memory || def->mem.memory_slots) {
Bit more cofusing, having this one one place: (def->mem.memory_slots && def->mem.max_memory) and this one second one: (def->mem.max_memory || def->mem.memory_slots) but that's just a detail (unless we drift off and with the values and end up with only one being set).
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39ed66b..92b5fc5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1533,6 +1533,10 @@ mymain(void) DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); DO_TEST("cpu-host-passthrough-features", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST);
+ DO_TEST_FAILURE("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM); + DO_TEST_FAILURE("memory-hotplug", 0);
Use NONE instead '0', please. ACK with this ^^ one thing change changed.

ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3993,6 +3993,18 @@ </attribute> </optional> </define> + <define name="acpidimmaddress"> + <optional> + <attribute name="slot"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="base"> + <ref name="hexuint"/> + </attribute> + </optional> + </define> <define name="devices"> <element name="devices"> <interleave> @@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fb35880..b5d9c6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -249,7 +249,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "virtio-s390", "ccw", "virtio-mmio", - "isa") + "isa", + "acpi-dimm") VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, "disk", @@ -2833,6 +2834,11 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, if (memcmp(&a->addr.isa, &b->addr.isa, sizeof(a->addr.isa))) return false; break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM: + if (memcmp(&a->addr.acpiDimm, &b->addr.acpiDimm, sizeof(a->addr.acpiDimm))) + return false; + break; } return true; @@ -3655,6 +3661,12 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virBufferAsprintf(buf, " irq='0x%x'", info->addr.isa.irq); break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM: + virBufferAsprintf(buf, " slot='%u'", info->addr.acpiDimm.slot); + virBufferAsprintf(buf, " base='0x%llx'", info->addr.acpiDimm.base); + + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST: @@ -4024,6 +4036,41 @@ virDomainDeviceISAAddressParseXML(xmlNodePtr node, return ret; } + +static int +virDomainDeviceACPIDimmAddressParseXML(xmlNodePtr node, + virDomainDeviceACPIDimmAddressPtr addr) +{ + int ret = -1; + char *tmp = NULL; + + if (!(tmp = virXMLPropString(node, "slot")) || + virStrToLong_uip(tmp, NULL, 10, &addr->slot) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid or missing acpi-dimm slot id '%s'"), + NULLSTR(tmp)); + goto cleanup; + } + VIR_FREE(tmp); + + if (!(tmp = virXMLPropString(node, "base")) || + virStrToLong_ullp(tmp, NULL, 16, &addr->base) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid or missing acpi-dimm base address '%s'"), + NULLSTR(tmp)); + goto cleanup; + } + VIR_FREE(tmp); + + ret = 0; + + cleanup: + VIR_FREE(tmp); + + return ret; +} + + /* Parse the XML definition for a device address * @param node XML nodeset to parse for device address definition */ @@ -4165,6 +4212,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, _("virtio-s390 bus doesn't have an address")); goto cleanup; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM: + if (virDomainDeviceACPIDimmAddressParseXML(address, &info->addr.acpiDimm) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST: break; @@ -15245,6 +15297,26 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, } break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM: + if (src->addr.acpiDimm.slot != dst->addr.acpiDimm.slot) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device acpi dimm slot %d does not match " + "source %d"), + dst->addr.acpiDimm.slot, + src->addr.acpiDimm.slot); + return false; + } + + if (src->addr.acpiDimm.base != dst->addr.acpiDimm.base) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device acpi dimm base addres '%llx' does " + "not match source '%llx'"), + dst->addr.acpiDimm.base, + src->addr.acpiDimm.base); + return false; + } + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB: case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO: case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d24db3a..151c1e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -234,6 +234,7 @@ typedef enum { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST } virDomainDeviceAddressType; @@ -309,6 +310,13 @@ struct _virDomainDeviceISAAddress { unsigned int irq; }; +typedef struct _virDomainDeviceACPIDimmAddress virDomainDeviceACPIDimmAddress; +typedef virDomainDeviceACPIDimmAddress *virDomainDeviceACPIDimmAddressPtr; +struct _virDomainDeviceACPIDimmAddress { + unsigned int slot; + unsigned long long base; +}; + typedef struct _virDomainDeviceInfo virDomainDeviceInfo; typedef virDomainDeviceInfo *virDomainDeviceInfoPtr; struct _virDomainDeviceInfo { @@ -327,6 +335,7 @@ struct _virDomainDeviceInfo { virDomainDeviceSpaprVioAddress spaprvio; virDomainDeviceCCWAddress ccw; virDomainDeviceISAAddress isa; + virDomainDeviceACPIDimmAddress acpiDimm; } addr; int mastertype; union { -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3993,6 +3993,18 @@ </attribute> </optional> </define> + <define name="acpidimmaddress"> + <optional> + <attribute name="slot"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="base"> + <ref name="hexuint"/> + </attribute> + </optional> + </define> <define name="devices"> <element name="devices"> <interleave> @@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here: 1) Why not just "dimm"? I feel like the "acpi" complicates everything. 2) It looks like we won't do any address validation or allocation, is that planned?. I hope this won't end up like other address types where we just wait for qemu to fail. Also, if base[n+1] is just base[n]+size[n], then there should be no problem assigning proper addresses automatically. I think it'd be much less pain to automatically assign them in libvirt then making it mandatory for the management application.

On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3993,6 +3993,18 @@ </attribute> </optional> </define> + <define name="acpidimmaddress"> + <optional> + <attribute name="slot"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="base"> + <ref name="hexuint"/> + </attribute> + </optional> + </define> <define name="devices"> <element name="devices"> <interleave> @@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here:
1) Why not just "dimm"? I feel like the "acpi" complicates everything.
That is okay if upstream agrees.
2) It looks like we won't do any address validation or allocation, is that planned?. I hope this won't end up like other address types where we just wait for qemu to fail. Also, if base[n+1] is just base[n]+size[n], then there should be no problem assigning proper addresses automatically. I think it'd be much less pain to automatically assign them in libvirt then making it mandatory for the management application.a
As I've explained a few times already. The management apps ideally shouldn't pass anything in the address and the data are then recalled from qemu. I want to avoid by all means doing the magic alignment done by qemu here since we can recall the data after the module is used. The only reason the address is required is to allow migrations without moving the modules around. This is the main reason this is stashed under the address field and users shouldn't need to set it ... ever. Peter

On Fri, Feb 20, 2015 at 06:25:40PM +0100, Peter Krempa wrote:
On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3993,6 +3993,18 @@ </attribute> </optional> </define> + <define name="acpidimmaddress"> + <optional> + <attribute name="slot"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="base"> + <ref name="hexuint"/> + </attribute> + </optional> + </define> <define name="devices"> <element name="devices"> <interleave> @@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here:
1) Why not just "dimm"? I feel like the "acpi" complicates everything.
That is okay if upstream agrees.
Just a swift idea, not that it's needed. I'd wonder about others' opinions.
2) It looks like we won't do any address validation or allocation, is that planned?. I hope this won't end up like other address types where we just wait for qemu to fail. Also, if base[n+1] is just base[n]+size[n], then there should be no problem assigning proper addresses automatically. I think it'd be much less pain to automatically assign them in libvirt then making it mandatory for the management application.a
As I've explained a few times already. The management apps ideally shouldn't pass anything in the address and the data are then recalled from qemu. I want to avoid by all means doing the magic alignment done by qemu here since we can recall the data after the module is used.
The only reason the address is required is to allow migrations without moving the modules around. This is the main reason this is stashed under the address field and users shouldn't need to set it ... ever.
That's even better than I meant. Maybe we'll have the same possibility for other devices, too. That would deal with some of our current problems. Thanks for the clarification, Martin

On Fri, Feb 20, 2015 at 07:36:53PM +0100, Martin Kletzander wrote:
On Fri, Feb 20, 2015 at 06:25:40PM +0100, Peter Krempa wrote:
On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3993,6 +3993,18 @@ </attribute> </optional> </define> + <define name="acpidimmaddress"> + <optional> + <attribute name="slot"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="base"> + <ref name="hexuint"/> + </attribute> + </optional> + </define> <define name="devices"> <element name="devices"> <interleave> @@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here:
1) Why not just "dimm"? I feel like the "acpi" complicates everything.
That is okay if upstream agrees.
Just a swift idea, not that it's needed. I'd wonder about others' opinions.
Well, from the vast majority of replies, I think there is not that much of disagreement. Although if there was a thread where this was decided and I missed that, feel free to leave it as-is.
2) It looks like we won't do any address validation or allocation, is that planned?. I hope this won't end up like other address types where we just wait for qemu to fail. Also, if base[n+1] is just base[n]+size[n], then there should be no problem assigning proper addresses automatically. I think it'd be much less pain to automatically assign them in libvirt then making it mandatory for the management application.a
As I've explained a few times already. The management apps ideally shouldn't pass anything in the address and the data are then recalled from qemu. I want to avoid by all means doing the magic alignment done by qemu here since we can recall the data after the module is used.
The only reason the address is required is to allow migrations without moving the modules around. This is the main reason this is stashed under the address field and users shouldn't need to set it ... ever.
That's even better than I meant. Maybe we'll have the same possibility for other devices, too. That would deal with some of our current problems.
And since this is dealt with, ACK to this patch with the aforementioned decision left to rest upon your shoulders.
Thanks for the clarification, Martin
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Feb 24, 2015 at 14:22:11 +0100, Martin Kletzander wrote:
On Fri, Feb 20, 2015 at 07:36:53PM +0100, Martin Kletzander wrote:
On Fri, Feb 20, 2015 at 06:25:40PM +0100, Peter Krempa wrote:
On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng
...
@@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here:
1) Why not just "dimm"? I feel like the "acpi" complicates everything.
That is okay if upstream agrees.
Just a swift idea, not that it's needed. I'd wonder about others' opinions.
Well, from the vast majority of replies, I think there is not that much of disagreement. Although if there was a thread where this was decided and I missed that, feel free to leave it as-is.
Actually it was never discussed anywhere besides here so it's still open for discussion. Peter

On Tue, Feb 24, 2015 at 02:48:32PM +0100, Peter Krempa wrote:
On Tue, Feb 24, 2015 at 14:22:11 +0100, Martin Kletzander wrote:
On Fri, Feb 20, 2015 at 07:36:53PM +0100, Martin Kletzander wrote:
On Fri, Feb 20, 2015 at 06:25:40PM +0100, Peter Krempa wrote:
On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote:
On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote:
ACPI Dimm devices are described by the slot and base address. Add a new address type to be able to describe such address. --- docs/schemas/domaincommon.rng | 18 +++++++++++ src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 ++++++ 3 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index acfa16a..1824741 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng
...
@@ -4407,6 +4419,12 @@ </attribute> <ref name="isaaddress"/> </group> + <group> + <attribute name="type"> + <value>acpi-dimm</value> + </attribute> + <ref name="acpidimmaddress"/> + </group> </choice> </element> </define>
I've got 2 questions here:
1) Why not just "dimm"? I feel like the "acpi" complicates everything.
That is okay if upstream agrees.
Just a swift idea, not that it's needed. I'd wonder about others' opinions.
Well, from the vast majority of replies, I think there is not that much of disagreement. Although if there was a thread where this was decided and I missed that, feel free to leave it as-is.
Actually it was never discussed anywhere besides here so it's still open for discussion.
I think just 'dimm' is probably more appropriate than 'acpidimm'. IIUC, ACPI doesn't influence addressing - it is just a BIOS hardware discovery mechanism. So including acpi in the name is redundant, and probably even wrong if we consider architectures which don't use ACPI (everything that isn't x86 or aarch64) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This patch adds code that parses and formats configuration for memory devices. A simple configuration would be: <memory model='acpi-dimm'> <target> <size unit='KiB'>524287</size> <node>0</node> </target> </memory> A complete configuration of a memory device: <memory model='acpi-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 | 326 ++++++++++++++++++++- src/conf/domain_conf.h | 33 +++ 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, 580 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 6320703..881e941 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5866,6 +5866,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 device allow to add additional + memory for the guest in form of memory modules. These devices also allow + hot-add and hot-remove of guest's memory. + <span class="since">Since 1.2.13</span> + </p> + + <p> + Example: usage of the memory devices + </p> +<pre> + ... + <devices> + <memory model='acpi-dimm'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> + </memory> + <memory model='acpi-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>acpi-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. + </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 of the memory device. + </p> + <p> + <code>nodemask</code> can be used to override the default set of NUMA + where the memory would be allocated. + </p> + </dd> + + <dt><code>target</code></dt> + <dd> + <p> + The mandatory <code>target</code> element configures the placement and + sizing 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 1824741..a8c17a8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4034,6 +4034,7 @@ <ref name="rng"/> <ref name="tpm"/> <ref name="shmem"/> + <ref name="memorydev"/> </choice> </zeroOrMore> <optional> @@ -4445,6 +4446,55 @@ </element> </define> + <define name="memorydev"> + <element name="memory"> + <attribute name="model"> + <choice> + <value>acpi-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"/> + </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 b5d9c6f..3eb1426 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", @@ -786,6 +787,12 @@ 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, + "", "acpi-dimm") + +#define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE +#define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE + static virClassPtr virDomainObjClass; static virClassPtr virDomainObjListClass; static virClassPtr virDomainXMLOptionClass; @@ -1010,6 +1017,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) { @@ -1941,6 +1969,15 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def) VIR_FREE(def); } +void virDomainMemoryDefFree(virDomainMemoryDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + VIR_FREE(def); +} + void virDomainDeviceDefFree(virDomainDeviceDefPtr def) { if (!def) @@ -2010,6 +2047,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; @@ -2207,6 +2247,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); @@ -2744,6 +2788,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: @@ -3064,6 +3110,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 @@ -3096,6 +3149,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 @@ -7020,7 +7074,13 @@ virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long virDomainDefGetMemoryCurrent(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; } @@ -11361,6 +11421,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; + } + + 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, @@ -11495,6 +11668,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; @@ -14846,6 +15023,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; @@ -16085,6 +16279,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; + } + + 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; + } + + 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 @@ -16497,6 +16724,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 @@ -16528,6 +16767,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 @@ -19007,6 +19247,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); + + 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) @@ -20596,6 +20911,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"); @@ -21999,6 +22318,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 151c1e7..80d2c88 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -132,6 +132,9 @@ typedef virDomainIdMapDef *virDomainIdMapDefPtr; typedef struct _virDomainPanicDef virDomainPanicDef; typedef virDomainPanicDef *virDomainPanicDefPtr; +typedef struct _virDomainMemoryDef virDomainMemoryDef; +typedef virDomainMemoryDef *virDomainMemoryDefPtr; + /* forward declarations virDomainChrSourceDef, required by * virDomainNetDef */ @@ -168,6 +171,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SHMEM, VIR_DOMAIN_DEVICE_TPM, VIR_DOMAIN_DEVICE_PANIC, + VIR_DOMAIN_DEVICE_MEMORY, VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; @@ -198,6 +202,7 @@ struct _virDomainDeviceDef { virDomainShmemDefPtr shmem; virDomainTPMDefPtr tpm; virDomainPanicDefPtr panic; + virDomainMemoryDefPtr memory; } data; }; @@ -1973,6 +1978,28 @@ struct _virDomainRNGDef { virDomainDeviceInfo info; }; +typedef enum { + VIR_DOMAIN_MEMORY_MODEL_NONE, + VIR_DOMAIN_MEMORY_MODEL_ACPI_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; + unsigned long long size; /* kibibytes */ + + virDomainDeviceInfo info; +}; + +void virDomainMemoryDefFree(virDomainMemoryDefPtr def); + struct _virDomainIdMapEntry { unsigned int start; unsigned int target; @@ -2193,6 +2220,9 @@ struct _virDomainDef { size_t nshmems; virDomainShmemDefPtr *shmems; + size_t nmems; + virDomainMemoryDefPtr *mems; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2355,6 +2385,7 @@ bool virDomainObjTaint(virDomainObjPtr obj, int virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def); +int virDomainDeviceDefCheckUnsupportedMemoryDevice(virDomainDeviceDefPtr dev); void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); @@ -2900,6 +2931,8 @@ VIR_ENUM_DECL(virDomainRNGBackend) VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) VIR_ENUM_DECL(virDomainThreadSched) +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 d01b884..3b7cfdb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -216,6 +216,7 @@ virDomainDefSetMemoryInitial; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; +virDomainDeviceDefCheckUnsupportedMemoryDevice; virDomainDeviceDefCopy; virDomainDeviceDefFree; virDomainDeviceDefParse; @@ -331,6 +332,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 fe18acc..ddf298b 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 46a7411..55dd6b2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1182,6 +1182,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 fa1bcb2..9d6eab9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7135,6 +7135,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: @@ -7212,6 +7215,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: @@ -7329,6 +7334,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: @@ -7471,6 +7477,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: @@ -7596,6 +7605,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: @@ -7710,6 +7722,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 8d91719..2985416 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3072,6 +3072,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 d2a1519..834e91b 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..ec2cb86 --- /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='acpi-dimm'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> + </memory> + <memory model='acpi-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> -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:30PM +0100, Peter Krempa wrote:
This patch adds code that parses and formats configuration for memory devices.
A simple configuration would be: <memory model='acpi-dimm'> <target> <size unit='KiB'>524287</size> <node>0</node> </target> </memory>
A complete configuration of a memory device: <memory model='acpi-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 | 326 ++++++++++++++++++++- src/conf/domain_conf.h | 33 +++ 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, 580 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 6320703..881e941 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5866,6 +5866,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 device allow to add additional
Either "memory device allows adding additional" or "memory devices allow adding ..."
+ memory for the guest in form of memory modules. These devices also allow + hot-add and hot-remove of guest's memory. + <span class="since">Since 1.2.13</span> + </p> + + <p> + Example: usage of the memory devices + </p> +<pre> + ... + <devices> + <memory model='acpi-dimm'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> + </memory> + <memory model='acpi-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>acpi-dimm</code> model is supported that
Don't forget to s/acpi-dimm/dimm/ all of these (and sorry for the added work).
+ 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. + </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.
Seems like too much of "the", but I'm not native speaker.
+ </p> + <p> + <code>pagesize</code> can be used to override the default host page + size used for backing of the memory device.
s/backing of/backing/ ?
+ </p> + <p> + <code>nodemask</code> can be used to override the default set of NUMA
s/NUMA/NUMA nodes/ ?
+ where the memory would be allocated. + </p> + </dd> + + <dt><code>target</code></dt> + <dd> + <p> + The mandatory <code>target</code> element configures the placement and + sizing the added memory from the perspective of the guest.
s/sizing/sizing of/ ?
+ </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 1824741..a8c17a8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4034,6 +4034,7 @@ <ref name="rng"/> <ref name="tpm"/> <ref name="shmem"/> + <ref name="memorydev"/> </choice> </zeroOrMore> <optional> @@ -4445,6 +4446,55 @@ </element> </define>
+ <define name="memorydev"> + <element name="memory"> + <attribute name="model"> + <choice> + <value>acpi-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"/> + </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 b5d9c6f..3eb1426 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", @@ -786,6 +787,12 @@ 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, + "", "acpi-dimm") + +#define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE +#define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE +
These two shouldn't be here I guess.
static virClassPtr virDomainObjClass; static virClassPtr virDomainObjListClass; static virClassPtr virDomainXMLOptionClass; @@ -1010,6 +1017,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) { @@ -1941,6 +1969,15 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def) VIR_FREE(def); }
+void virDomainMemoryDefFree(virDomainMemoryDefPtr def) +{ + if (!def) + return; +
You're not cleaning def->sourceNodes here. I'll continue the review tomorrow.

When using 'acpi-dimm' memory devices with qemu, some of the information like the slot number and base address need to be reloaded from qemu after process start so that it reflects the actual state. The state then allows to use memory devices across migrations. --- src/qemu/qemu_domain.c | 49 +++++++++++++++++ src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_monitor.c | 42 +++++++++++++++ src/qemu/qemu_monitor.h | 14 +++++ src/qemu/qemu_monitor_json.c | 122 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_process.c | 4 ++ 7 files changed, 240 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 55dd6b2..c433cc5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2773,6 +2773,55 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, return 0; } + +int +qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr meminfo = NULL; + int rc; + size_t i; + + if (vm->def->nmems == 0) + return 0; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorGetMemoryDeviceInfo(priv->mon, &meminfo); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + /* if qemu doesn't support the info request, just carry on */ + if (rc == -2) + rc = 0; + + if (rc < 0) + return -1; + + for (i = 0; i < vm->def->nmems; i++) { + virDomainMemoryDefPtr mem = vm->def->mems[i]; + qemuMonitorMemoryDeviceInfoPtr dimm; + + if (!mem->info.alias) + continue; + + if (!(dimm = virHashLookup(meminfo, mem->info.alias))) + continue; + + mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM; + mem->info.addr.acpiDimm.slot = dimm->slot; + mem->info.addr.acpiDimm.base = dimm->address; + } + + virHashFree(meminfo); + return 0; +} + + bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virDomainDefPtr src, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index faf4ee2..7c8a86e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -391,6 +391,10 @@ extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); +int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob); + bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virDomainDefPtr src, virDomainDefPtr dst); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6882a50..1c6012d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4358,3 +4358,45 @@ void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread) VIR_FREE(iothread->name); VIR_FREE(iothread); } + + +/** + * qemuMonitorGetMemoryDeviceInfo: + * @mon: pointer to the monitor + * @info: Location to return the hash of qemuMonitorMemoryDeviceInfo + * + * Retrieve state and addresses of frontend memory devices present in + * the guest. + * + * Returns 0 on success and fills @info with a newly allocated struct; if the + * data can't be retrieved due to lack of support in qemu, returns -2. On + * other errors returns -1. + */ +int +qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, + virHashTablePtr *info) +{ + VIR_DEBUG("mon=%p info=%p", mon, info); + int ret; + + *info = NULL; + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) + return -2; + + if (!(*info = virHashCreate(10, virHashValueFree))) + return -1; + + if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) { + virHashFree(*info); + *info = NULL; + } + + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 133d42d..811ce49 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -892,6 +892,20 @@ int qemuMonitorGetIOThreads(qemuMonitorPtr mon, void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread); +typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo; +typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr; + +struct _qemuMonitorMemoryDeviceInfo { + unsigned long long address; + unsigned int slot; + bool hotplugged; + bool hotpluggable; +}; + +int qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, + virHashTablePtr *info) + ATTRIBUTE_NONNULL(2); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index da5c14d..236b3fd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6569,3 +6569,125 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int +qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, + virHashTablePtr info) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + qemuMonitorMemoryDeviceInfoPtr meminfo = NULL; + ssize_t n; + size_t i; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = -2; + goto cleanup; + } + + ret = qemuMonitorJSONCheckError(cmd, reply); + } + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-memory-devices reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-memory-devices reply data was not an array")); + goto cleanup; + } + + for (i = 0; i < n; i++) { + virJSONValuePtr elem = virJSONValueArrayGet(data, i); + const char *type; + + if (!(type = virJSONValueObjectGetString(elem, "type"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-memory-devices reply data doesn't contain " + "enum type discriminator")); + goto cleanup; + } + + /* dimm memory devices */ + if (STREQ(type, "dimm")) { + virJSONValuePtr dimminfo; + const char *devalias; + + if (!(dimminfo = virJSONValueObjectGet(elem, "data"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-memory-devices reply data doesn't " + "contain enum data")); + goto cleanup; + } + + if (!(devalias = virJSONValueObjectGetString(dimminfo, "id"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("dimm memory info data is missing 'id'")); + goto cleanup; + } + + if (VIR_ALLOC(meminfo) < 0) + goto cleanup; + + if (virJSONValueObjectGetNumberUlong(dimminfo, "addr", + &meminfo->address) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed/missing addr in dimm memory info")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUint(dimminfo, "slot", + &meminfo->slot) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed/missing slot in dimm memory info")); + goto cleanup; + } + + if (virJSONValueObjectGetBoolean(dimminfo, "hotplugged", + &meminfo->hotplugged) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed/missing hotplugged in dimm memory info")); + goto cleanup; + + } + + if (virJSONValueObjectGetBoolean(dimminfo, "hotpluggable", + &meminfo->hotpluggable) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed/missing hotpluggable in dimm memory info")); + goto cleanup; + + } + + if (virHashAddEntry(info, devalias, meminfo) < 0) + goto cleanup; + + meminfo = NULL; + } + } + + ret = 0; + + cleanup: + VIR_FREE(meminfo); + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 1da1a00..eb51a60 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -475,4 +475,9 @@ int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon); int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, qemuMonitorIOThreadsInfoPtr **iothreads) ATTRIBUTE_NONNULL(2); + +int qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, + virHashTablePtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 31ec8eb..57727cc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4964,6 +4964,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuDomainUpdateDeviceList(driver, vm, asyncJob) < 0) goto cleanup; + VIR_DEBUG("Updating info of memory devices"); + if (qemuDomainUpdateMemoryDeviceInfo(driver, vm, asyncJob) < 0) + goto cleanup; + /* Technically, qemuProcessStart can be called from inside * QEMU_ASYNC_JOB_MIGRATION_IN, but we are okay treating this like * a sync job since no other job can call into the domain until -- 2.2.2

Make sure that libvirt has all vital information needed to reliably represent configuration of guest's memory devices in case of a migration. This patch forbids migration in case the required slot number and module base address are not present (failed to be loaded from qemu via monitor). --- src/qemu/qemu_migration.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4506d87..c562efd 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2019,6 +2019,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, } } + /* Verify that memory device config can be transferred reliably */ + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr mem = def->mems[i]; + + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_ACPI_DIMM && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain's acpi-dimm info lacks slot ID " + "or base address")); + + return false; + } + } + return true; } -- 2.2.2

Add support to start qemu instance with 'pc-dimm' device. Thanks to the refactors we are able to reuse the existing function to determine the parameters. --- src/qemu/qemu_command.c | 130 ++++++++++++++++++++- src/qemu/qemu_domain.c | 26 ++++- src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-memory-hotplug-dimm.args | 11 ++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d9666c3..2568a6d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1173,6 +1173,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0) return -1; } + for (i = 0; i < def->nmems; i++) { + if (virAsprintf(&def->mems[i]->info.alias, "dimm%zu", i) < 0) + return -1; + } return 0; } @@ -4558,8 +4562,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, virDomainHugePagePtr hugepage = NULL; virDomainNumatuneMemMode mode; const long system_page_size = virGetSystemPageSizeKB(); - virNumaMemAccess memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode); - + virNumaMemAccess memAccess = VIR_NUMA_MEM_ACCESS_DEFAULT; size_t i; char *mem_path = NULL; virBitmapPtr nodemask = NULL; @@ -4572,6 +4575,16 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(props = virJSONValueNewObject())) return -1; + /* memory devices could provide a invalid guest node */ + if (guestNode >= virDomainNumaGetNodeCount(def->numa)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("can't add memory backend for guest node '%d' as " + "the guest has only '%zu' NUMA nodes configured"), + guestNode, virDomainNumaGetNodeCount(def->numa)); + return -1; + } + + memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode); mode = virDomainNumatuneGetMode(def->numa, guestNode); if (pagesize == 0 || pagesize != system_page_size) { @@ -4769,6 +4782,95 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, } +static char * +qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg) +{ + virJSONValuePtr props = NULL; + char *alias = NULL; + const char *backendType; + char *ret = NULL; + + if (!mem->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("memory device alias is not assigned")); + return NULL; + } + + if (virAsprintf(&alias, "mem%s", mem->info.alias) < 0) + goto cleanup; + + if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, + mem->targetNode, mem->sourceNodes, NULL, + def, qemuCaps, cfg, + &backendType, &props, true) < 0) + goto cleanup; + + ret = qemuBuildObjectCommandlineFromJSON(backendType, alias, props); + + cleanup: + VIR_FREE(alias); + virJSONValueFree(props); + + return ret; +} + + +static char * +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!mem->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing alias for memory device")); + return NULL; + } + + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_ACPI_DIMM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the pc-dimm device")); + return NULL; + } + + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'acpi-dimm' addresses are supported for the " + "pc-dimm device")); + return NULL; + } + + virBufferAsprintf(&buf, "pc-dimm,node=%d,memdev=mem%s,id=%s", + mem->targetNode, mem->info.alias, mem->info.alias); + + if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM) { + virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.acpiDimm.slot); + virBufferAsprintf(&buf, ",base=%llu", mem->info.addr.acpiDimm.base); + } + + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid memory device type")); + break; + + } + + if (virBufferCheckError(&buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} + + char * qemuBuildNicStr(virDomainNetDefPtr net, const char *prefix, @@ -8423,10 +8525,32 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (virDomainNumaGetNodeCount(def->numa)) + if (virDomainNumaGetNodeCount(def->numa)) { if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) goto error; + /* memory hotplug requires NUMA to be enabled - we already checked + * that memory devices are present only when NUMA is */ + for (i = 0; i < def->nmems; i++) { + char *backStr; + char *dimmStr; + + if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, + qemuCaps, cfg))) + goto error; + + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], qemuCaps))) { + VIR_FREE(backStr); + goto error; + } + + virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL); + + VIR_FREE(backStr); + VIR_FREE(dimmStr); + } + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID)) virCommandAddArgList(cmd, "-uuid", uuid, NULL); if (def->virtType == VIR_DOMAIN_VIRT_XEN || diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c433cc5..1806d5b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1182,8 +1182,13 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } - if (virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) + if (dev->type == VIR_DOMAIN_DEVICE_MEMORY && + def->mem.max_memory == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("maxMemory has to be specified when using memory " + "devices ")); goto cleanup; + } ret = 0; @@ -2907,5 +2912,24 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) * We'll take the "traditional" path and round it to 1MiB*/ def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, 1024); + /* Align memory module sizes */ + for (i = 0; i < def->nmems; i++) + qemuDomainMemoryDeviceAlignSize(def->mems[i]); + return 0; } + + +/** + * qemuDomainMemoryDeviceAlignSize: + * @mem: memory device definition object + * + * Aligns the size of the memory module as qemu enforces it. The size is updated + * inplace. Default rounding is now to 1 MiB (qemu requires rouding to page, + * size so this should be safe). + */ +void +qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem) +{ + mem->size = VIR_ROUND_UP(mem->size, 1024); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7c8a86e..c550c86 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -419,5 +419,6 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, void qemuDomObjEndAPI(virDomainObjPtr *vm); int qemuDomainAlignMemorySizes(virDomainDefPtr def); +void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args new file mode 100644 index 0000000..7fbde33 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args @@ -0,0 +1,11 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m size=219136k,slots=16,maxmem=1099511627776k -smp 2 \ +-numa node,nodeid=0,cpus=0-1,mem=214 \ +-object memory-backend-ram,id=memdimm0,size=536870912 \ +-device pc-dimm,node=0,memdev=memdimm0,id=dimm0 \ +-object memory-backend-ram,id=memdimm1,size=536870912,host-nodes=1-3,\ +policy=bind \ +-device pc-dimm,node=0,memdev=memdimm1,id=dimm1 \ +-nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 92b5fc5..0247945 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1536,6 +1536,8 @@ mymain(void) DO_TEST_FAILURE("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM); DO_TEST_FAILURE("memory-hotplug", 0); DO_TEST("memory-hotplug", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA); + DO_TEST("memory-hotplug-dimm", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA, + QEMU_CAPS_DEVICE, QEMU_CAPS_OBJECT_MEMORY_RAM); virObjectUnref(driver.config); virObjectUnref(driver.caps); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index be6a010..471721f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -425,6 +425,7 @@ mymain(void) DO_TEST("memory-hotplug"); DO_TEST("memory-hotplug-nonuma"); + DO_TEST("memory-hotplug-dimm"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.2.2

Add a few helpers that allow to operate with memory device definitions on the domain config and use them to implement memory device coldplug in the qemu driver. --- src/conf/domain_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++ src/libvirt_private.syms | 4 ++ src/qemu/qemu_driver.c | 15 ++++++- 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3eb1426..f89a7ad 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12640,6 +12640,106 @@ virDomainRNGRemove(virDomainDefPtr def, } +static int +virDomainMemoryFindByDefInternal(virDomainDefPtr def, + virDomainMemoryDefPtr mem, + bool allowAddressFallback) +{ + size_t i; + + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr tmp = def->mems[i]; + + /* address, if present */ + if (allowAddressFallback) { + if (tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + } else { + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&tmp->info, &mem->info)) + continue; + } + + /* alias, if present */ + if (mem->info.alias && + STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias)) + continue; + + /* target info -> always present */ + if (tmp->model != mem->model || + tmp->targetNode != mem->targetNode || + tmp->size != mem->size) + continue; + + /* source stuff -> match with device */ + if (tmp->pagesize != mem->pagesize) + continue; + + if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) + continue; + + break; + } + + if (i == def->nmems) + return -1; + + return i; +} + + +int +virDomainMemoryFindByDef(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + return virDomainMemoryFindByDefInternal(def, mem, false); +} + + +int +virDomainMemoryFindInactiveByDef(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + int ret; + + if ((ret = virDomainMemoryFindByDefInternal(def, mem, false)) < 0) + ret = virDomainMemoryFindByDefInternal(def, mem, true); + + return ret; +} + + +int +virDomainMemoryInsert(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + int id = def->nmems; + + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + virDomainDefHasDeviceAddress(def, &mem->info)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Domain already contains a device with the same " + "address")); + return -1; + } + + if (VIR_APPEND_ELEMENT(def->mems, def->nmems, mem) < 0) + return -1; + + return id; +} + + +virDomainMemoryDefPtr +virDomainMemoryRemove(virDomainDefPtr def, + int idx) +{ + virDomainMemoryDefPtr ret = def->mems[idx]; + VIR_DELETE_ELEMENT(def->mems, idx, def->nmems); + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 80d2c88..7aa7826 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2859,6 +2859,16 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model); typedef const char* (*virEventActionToStringFunc)(int type); typedef int (*virEventActionFromStringFunc)(const char *type); +int virDomainMemoryInsert(virDomainDefPtr def, virDomainMemoryDefPtr mem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virDomainMemoryDefPtr virDomainMemoryRemove(virDomainDefPtr def, int idx) + ATTRIBUTE_NONNULL(1); +int virDomainMemoryFindByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, + virDomainMemoryDefPtr mem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b7cfdb..454eb23 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -333,6 +333,10 @@ virDomainLockFailureTypeToString; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemoryDefFree; +virDomainMemoryFindByDef; +virDomainMemoryFindInactiveByDef; +virDomainMemoryInsert; +virDomainMemoryRemove; virDomainNetAppendIpAddress; virDomainNetDefFormat; virDomainNetDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9d6eab9..229a094 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7478,7 +7478,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, break; case VIR_DOMAIN_DEVICE_MEMORY: - /* TODO: implement later */ + if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) + return -1; + dev->data.memory = NULL; + break; case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: @@ -7606,7 +7609,15 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, break; case VIR_DOMAIN_DEVICE_MEMORY: - /* TODO: implement later */ + if ((idx = virDomainMemoryFindInactiveByDef(vmdef, + dev->data.memory)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching memory device was not found")); + return -1; + } + + virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); + break; case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: -- 2.2.2

Add code to hot-add memory devices to running qemu instances. --- src/qemu/qemu_command.c | 4 +-- src/qemu/qemu_command.h | 15 +++++++++ src/qemu/qemu_driver.c | 5 ++- src/qemu/qemu_hotplug.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 3 ++ 5 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2568a6d..fb6d6ea 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4545,7 +4545,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, * other configuration was used (to detect legacy configurations). Returns * -1 in case of an error. */ -static int +int qemuBuildMemoryBackendStr(unsigned long long size, unsigned long long pagesize, int guestNode, @@ -4818,7 +4818,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, } -static char * +char * qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, virQEMUCapsPtr qemuCaps) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 89e8351..900a9ae 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -162,6 +162,21 @@ char *qemuBuildSoundDevStr(virDomainDefPtr domainDef, virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps); +int qemuBuildMemoryBackendStr(unsigned long long size, + unsigned long long pagesize, + int guestNode, + virBitmapPtr userNodeset, + virBitmapPtr autoNodeset, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg, + const char **backendType, + virJSONValuePtr *backendProps, + bool force); + +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps); + /* Legacy, pre device support */ char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 229a094..4c7817e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7135,8 +7135,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, dev->data.rng = NULL; break; - /*TODO: implement later */ case VIR_DOMAIN_DEVICE_MEMORY: + ret = qemuDomainAttachMemory(driver, vm, + dev->data.memory); + dev->data.memory = NULL; + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2985416..8502092 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1674,6 +1674,91 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, } +int +qemuDomainAttachMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *devstr = NULL; + char *objalias = NULL; + const char *backendType; + virJSONValuePtr props = NULL; + int id; + int ret = -1; + + if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0) + goto cleanup; + + if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) + goto cleanup; + + if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv->qemuCaps))) + goto cleanup; + + qemuDomainMemoryDeviceAlignSize(mem); + + if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, + mem->targetNode, mem->sourceNodes, NULL, + vm->def, priv->qemuCaps, cfg, + &backendType, &props, true) < 0) + goto cleanup; + + if (virDomainMemoryInsert(vm->def, mem) < 0) { + virJSONValueFree(props); + goto cleanup; + } + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0) + goto removedef; + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + virErrorPtr err = virSaveLastError(); + ignore_value(qemuMonitorDelObject(priv->mon, objalias)); + virSetError(err); + virFreeError(err); + goto removedef; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + /* we shouldn't touch mem now, as the def might be freed */ + mem = NULL; + goto cleanup; + } + + /* mem is consumed by vm->def */ + mem = NULL; + + /* this step is best effort, removing the device would be so much trouble */ + ignore_value(qemuDomainUpdateMemoryDeviceInfo(driver, vm, + QEMU_ASYNC_JOB_NONE)); + + ret = 0; + + cleanup: + virObjectUnref(cfg); + VIR_FREE(devstr); + VIR_FREE(objalias); + virDomainMemoryDefFree(mem); + return ret; + + removedef: + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + mem = NULL; + goto cleanup; + } + + if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0) + mem = virDomainMemoryRemove(vm->def, id); + else + mem = NULL; + + goto cleanup; +} + + static int qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 8a0b313..ad4ff38 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -57,6 +57,9 @@ int qemuDomainAttachHostDevice(virConnectPtr conn, virDomainHostdevDefPtr hostdev); int qemuDomainFindGraphicsIndex(virDomainDefPtr def, virDomainGraphicsDefPtr dev); +int qemuDomainAttachMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem); int qemuDomainChangeGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainGraphicsDefPtr dev); -- 2.2.2

Add code to hot-remove memory devices from qemu. Unfortunately QEMU doesn't support this right now, so this is just for completenes. --- src/qemu/qemu_driver.c | 4 ++- src/qemu/qemu_hotplug.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 3 ++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4c7817e..09ee1ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7219,7 +7219,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachRNGDevice(driver, vm, dev->data.rng); break; case VIR_DOMAIN_DEVICE_MEMORY: - /* TODO: Implement later */ + ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory); + break; + case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8502092..9be97c0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2820,6 +2820,44 @@ qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver, } +static int +qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virObjectEventPtr event; + char *backendAlias = NULL; + int rc; + int idx; + + VIR_DEBUG("Removing memory device %s from domain %p %s", + mem->info.alias, vm, vm->def->name); + + if ((event = virDomainEventDeviceRemovedNewFromObj(vm, mem->info.alias))) + qemuDomainEventQueue(driver, event); + + if (virAsprintf(&backendAlias, "mem%s", mem->info.alias) < 0) + goto error; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorDelObject(priv->mon, backendAlias); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto error; + + if ((idx = virDomainMemoryFindByDef(vm->def, mem)) >= 0) + virDomainMemoryRemove(vm->def, idx); + + virDomainMemoryDefFree(mem); + VIR_FREE(backendAlias); + return 0; + + error: + VIR_FREE(backendAlias); + return -1; +} + + static void qemuDomainRemovePCIHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3157,8 +3195,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, qemuDomainRemoveRNGDevice(driver, vm, dev->data.rng); break; - /* TODO: implement later */ case VIR_DOMAIN_DEVICE_MEMORY: + ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory); + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: @@ -4109,3 +4148,53 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, qemuDomainResetDeviceRemoval(vm); return ret; } + + +int +qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr memdef) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainMemoryDefPtr mem; + int idx; + int rc; + int ret = -1; + + if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device not present in domain configuration")); + return -1; + } + + mem = vm->def->mems[idx]; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("qemu does not support -device")); + return -1; + } + + if (!mem->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("alias for the memory device was not found")); + return -1; + } + + qemuDomainMarkDeviceForRemoval(vm, &mem->info); + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorDelDevice(priv->mon, mem->info.alias); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + rc = qemuDomainWaitForDeviceRemoval(vm); + if (rc == 0 || rc == 1) + ret = qemuDomainRemoveMemoryDevice(driver, vm, mem); + else + ret = 0; + + cleanup: + qemuDomainResetDeviceRemoval(vm); + return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index ad4ff38..4140da3 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -60,6 +60,9 @@ int qemuDomainFindGraphicsIndex(virDomainDefPtr def, int qemuDomainAttachMemory(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainMemoryDefPtr mem); +int qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr memdef); int qemuDomainChangeGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainGraphicsDefPtr dev); -- 2.2.2

On Thu, Feb 19, 2015 at 04:38:25PM +0100, Peter Krempa wrote:
A final version of the memory hotplug series. This version incorporates feedback on the RFC and fixes the few missing bits.
This series depends on the three previous refactor series: http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html [libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure
http://www.redhat.com/archives/libvir-list/2015-February/msg00557.html [libvirt] [PATCH 0/3] Fix memory ABI stability check issues
http://www.redhat.com/archives/libvir-list/2015-February/msg00633.html [libvirt] [PATCH 0/7] Automaticaly fill <memory> element for NUMA enabled guests
For convenience, you can fetch the complete series in my public branch: git fetch git://pipo.sk/pipo/libvirt.git memory-hotplug-v1
Unfortunately, in this repository, the branch is not re-based on top of the aforementioned re-factors, which can confuse someone when reviewing (definitely 1 person at least) :)
Peter Krempa (11): qemu: caps: Add capability bit for the "pc-dimm" device conf: Add support for parsing and formatting max memory and slot count qemu: Implement setup of memory hotplug parameters conf: Add device address type for dimm devices conf: Add interface to parse and format memory device information qemu: memdev: Add infrastructure to load memory device information qemu: migration: Forbid migration with memory modules lacking info qemu: add support for memory devices qemu: conf: Add support for memory device cold(un)plug qemu: Implement memory device hotplug qemu: Implement memory device hotunplug
docs/formatdomain.html.in | 98 ++++ docs/schemas/domaincommon.rng | 76 +++ src/bhyve/bhyve_domain.c | 9 +- src/conf/domain_conf.c | 566 ++++++++++++++++++++- src/conf/domain_conf.h | 59 +++ src/libvirt_private.syms | 7 + src/libxl/libxl_domain.c | 8 + src/lxc/lxc_domain.c | 8 + src/openvz/openvz_driver.c | 14 +- src/parallels/parallels_driver.c | 6 +- src/phyp/phyp_driver.c | 6 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 166 +++++- src/qemu/qemu_command.h | 15 + src/qemu/qemu_domain.c | 80 +++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_driver.c | 29 ++ src/qemu/qemu_hotplug.c | 177 +++++++ src/qemu/qemu_hotplug.h | 6 + src/qemu/qemu_migration.c | 14 + src/qemu/qemu_monitor.c | 42 ++ src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 122 +++++ src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_process.c | 4 + src/uml/uml_driver.c | 9 +- src/vbox/vbox_common.c | 6 +- src/vmware/vmware_driver.c | 6 +- src/vmx/vmx.c | 6 +- src/xen/xen_driver.c | 7 + src/xenapi/xenapi_driver.c | 9 +- tests/domainschemadata/maxMemory.xml | 19 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + .../qemuxml2argv-memory-hotplug-dimm.args | 11 + .../qemuxml2argv-memory-hotplug-dimm.xml | 50 ++ .../qemuxml2argv-memory-hotplug-nonuma.xml | 22 + .../qemuxml2argv-memory-hotplug.args | 6 + .../qemuxml2argv-memory-hotplug.xml | 34 ++ tests/qemuxml2argvtest.c | 6 + tests/qemuxml2xmltest.c | 4 + 41 files changed, 1712 insertions(+), 23 deletions(-) create mode 100644 tests/domainschemadata/maxMemory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml
-- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Daniel P. Berrange
-
Martin Kletzander
-
Peter Krempa