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: