Re: [libvirt] [PATCH 06/10] qemu: command: Move dimm device checks from formatter to checker

Aggregate the checks of the dimm device into the verification function rather than having them in the formatter. --- src/qemu/qemu_command.c | 65 ++------------------------------------ src/qemu/qemu_command.h | 4 +-- src/qemu/qemu_domain.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 2 +- 4 files changed, 86 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a709db..5e7b052 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5318,44 +5318,8 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, } -static bool -qemuCheckMemoryDimmConflict(virDomainDefPtr def, - virDomainMemoryDefPtr mem) -{ - size_t i; - - for (i = 0; i < def->nmems; i++) { - virDomainMemoryDefPtr tmp = def->mems[i]; - - if (tmp == mem || - tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) - continue; - - if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device slot '%u' is already being " - "used by another memory device"), - mem->info.addr.dimm.slot); - return true; - } - - if (mem->info.addr.dimm.base != 0 && - mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device base '0x%llx' is already being " - "used by another memory device"), - mem->info.addr.dimm.base); - return true; - } - } - - return false; -} - char * -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - virDomainDefPtr def, - virQEMUCapsPtr qemuCaps) +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -5367,35 +5331,10 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, switch ((virDomainMemoryModel) mem->model) { case VIR_DOMAIN_MEMORY_MODEL_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_DIMM && - mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only 'dimm' addresses are supported for the " - "pc-dimm device")); - return NULL; - } - - if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && - mem->info.addr.dimm.slot >= def->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device slot '%u' exceeds slots count '%u'"), - mem->info.addr.dimm.slot, def->mem.memory_slots); - 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_DIMM) { - if (qemuCheckMemoryDimmConflict(def, mem)) - return NULL; - virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.dimm.slot); virBufferAsprintf(&buf, ",addr=%llu", mem->info.addr.dimm.base); } @@ -9486,7 +9425,7 @@ qemuBuildCommandLine(virConnectPtr conn, qemuCaps, cfg))) goto error; - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, qemuCaps))) { + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) { VIR_FREE(backStr); goto error; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4aa7f2d..c7ed300 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -173,9 +173,7 @@ int qemuBuildMemoryBackendStr(unsigned long long size, virJSONValuePtr *backendProps, bool force); -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - virDomainDefPtr def, - virQEMUCapsPtr qemuCaps); +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); /* Legacy, pre device support */ char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cb50754..1474a5b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3552,6 +3552,79 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def) } +static bool +qemuCheckMemoryDimmConflict(const virDomainDef *def, + const virDomainMemoryDef *mem) +{ + size_t i; + + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr tmp = def->mems[i]; + + if (tmp == mem || + tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) + continue; + + if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device slot '%u' is already being " + "used by another memory device"), + mem->info.addr.dimm.slot); + return true; + } + + if (mem->info.addr.dimm.base != 0 && + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device base '0x%llx' is already being " + "used by another memory device"), + mem->info.addr.dimm.base); + return true; + } + } + + return false; +} +static int +qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, + const virDomainDef *def) +{ + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'dimm' addresses are supported for the " + "pc-dimm device")); + return -1; + } + + if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { + if (mem->info.addr.dimm.slot >= def->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device slot '%u' exceeds slots " + "count '%u'"), + mem->info.addr.dimm.slot, def->mem.memory_slots); + return -1; + } + + + if (qemuCheckMemoryDimmConflict(def, mem)) + return -1; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid memory device type")); + return -1; + } + + return 0; +} + + /** * qemuDomainDefValidateMemoryHotplug: * @def: domain definition @@ -3616,9 +3689,17 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, return -1; } - for (i = 0; i < def->nmems; i++) + for (i = 0; i < def->nmems; i++) { hotplugMemory += def->mems[i]->size; + if (qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0) + return -1;

+ } + + if (mem && + qemuDomainDefValidateMemoryHotplugDevice(mem, def) < 0) + return -1; +

if (hotplugMemory > hotplugSpace) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("memory device total size exceeds hotplug space")); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2c7f165..d3630d7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1796,7 +1796,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (vm->def->mem.cur_balloon == virDomainDefGetMemoryActual(vm->def)) fix_balloon = true; - if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps))) + if (!(devstr = qemuBuildMemoryDeviceStr(mem))) goto cleanup; qemuDomainMemoryDeviceAlignSize(vm->def, mem);