Re: [libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address
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);
> }