9:39 a.m.
On 03/04/2015 11:24 AM, Peter Krempa wrote:
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 4f20aa6..0f6058b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12687,6 +12687,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;
So this path says - we want address fallback and this device has either
DIMM or LAST (oy!) as a type, so go to the next one and ignore this one
+ } else {
+ if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virDomainDeviceInfoAddressIsEqual(&tmp->info,
&mem->info))
+ continue;
This path says - we don't want address fallback and as long as we're
DIMM or LAST (oy!), then compare
What happens when we add a new address type? It would seem the more
straightforward way would be
if (type == DIMM && !Equal)
if (!allowAddressFallback)
continue'
Otherwise we're falling through to alias, target, etc. checks
+ }
+
+ /* alias, if present */
+ if (mem->info.alias &&
+ STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias))
+ continue;
I thought the NULLABLE checks both elems for NULL...
+
+ /* 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);
I would seem Inactive would probably not have the dimm address set, so
we're incurring a 2X penalty for perhaps no reason... Unless perhaps the
incoming mem def being checked has an address...
That is if my incoming has an address, then don't allow fallback;
otherwise, well best match.
+
+ 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)) {
Hmm... so if our incoming mem has an address defined - it could be
anything - we're just failing declaring that the domain already contains
a device with the same address? Doesn't seem right.
And again - we have this problem of TYPE_NONE, TYPE_DIMM, TYPE_LAST and
who knows what in the future.
+ 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 706040d..c38614d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2853,6 +2853,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 2491d2d..1504c9a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -334,6 +334,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 d6935d9..6b10e96 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7438,7 +7438,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:
@@ -7566,7 +7569,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));
Ug - tricked my eyes on calling the Remove inside the DefFree
ACK with some cleanups which I think are more or less simple
John
I have to run - I will finish 9 & 10 a bit later.
+ break;
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND: