[libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr). Introduce a address check when build memory device qemu command line. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- v3: rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and remove the refactor. src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0a6d92f..d3f0a23 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4952,6 +4952,40 @@ 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 && tmp->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, @@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, 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); } -- 1.8.3.1
On 2015/6/15 20:33, Luyao Huang wrote:
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr).
+ 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 && tmp->info.addr.dimm.base != 0 && + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) {
The logic here equals: if (mem->info.addr.dimm.base != 0 && mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { will it be better like this?
+ 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, @@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, 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); }
-- Oscar oscar.zhangbo@huawei.com
On 06/16/2015 08:27 AM, zhang bo wrote:
On 2015/6/15 20:33, Luyao Huang wrote:
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr).
+ 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 && tmp->info.addr.dimm.base != 0 && + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) {
Equal the memory device base with 0 means: make sure the 2 memory device base is specified (not let qemu auto assign the base). So the logic here is : 1. make sure memory device A and B base is not auto assign mode. 2. then equal the base address
The logic here equals: if (mem->info.addr.dimm.base != 0 && mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { will it be better like this?
Indeed, your code looks better. Thanks a lot for your review.
+ 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, @@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, 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); }
On 06/15/2015 08:33 AM, Luyao Huang wrote:
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr).
Introduce a address check when build memory device qemu command line.
Signed-off-by: Luyao Huang <lhuang@redhat.com> ---
NOTE: Used the following for commit message: qemu: Add a check for slot and base dimm address conflicts When hotplugging a memory device, there wasn't a check to determine if there is a conflict with the address space being used by the to be added memory device and any existing device which is disallowed by qemu. This patch adds a check to ensure the new device address doesn't conflict with any existing device.
v3: rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and remove the refactor.
src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
ACK Adjusted patch as shown below and pushed. John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0a6d92f..d3f0a23 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4952,6 +4952,40 @@ 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"),
Placed space at end of first line rather than at the beginning of the appended string (eg "...being " and "used by...")
+ mem->info.addr.dimm.slot); + return true; + } + + if (mem->info.addr.dimm.base != 0 && tmp->info.addr.dimm.base != 0 && + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) {
Used: 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"),
Similar space adjustment here
+ mem->info.addr.dimm.base); + return true; + } + } + + return false; +} + char * qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, virDomainDefPtr def, @@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, 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); }
On 06/18/2015 08:12 PM, John Ferlan wrote:
On 06/15/2015 08:33 AM, Luyao Huang wrote:
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr).
Introduce a address check when build memory device qemu command line.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- NOTE: Used the following for commit message:
qemu: Add a check for slot and base dimm address conflicts
When hotplugging a memory device, there wasn't a check to determine if there is a conflict with the address space being used by the to be added memory device and any existing device which is disallowed by qemu.
This patch adds a check to ensure the new device address doesn't conflict with any existing device.
Thanks for the message improvement
v3: rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and remove the refactor.
src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
ACK
Adjusted patch as shown below and pushed.
Thanks a lot for your review and help
John
Luyao
participants (4)
-
John Ferlan -
lhuang -
Luyao Huang
-
zhang bo