[libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address

[lhuang]

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);
>>           }
>
>