On 11/16/23 14:05, Peter Krempa wrote:
> On Mon, Nov 06, 2023 at 12:38:27 +0100, Michal Privoznik wrote:
>> Since we're iterating over def->mems array, might as well check
>> for dimm slot duplicates.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> src/conf/domain_validate.c | 36 +++++++++++++++++++++++-------------
>> 1 file changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index 5d9602666e..f45ee0a8a5 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -2221,6 +2221,7 @@ static int
>> virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
>> const virDomainDef *def)
>> {
>> + const virDomainDeviceDimmAddress *thisAddr = NULL;
>> unsigned long long thisStart = 0;
>> unsigned long long thisEnd = 0;
>> size_t i;
>> @@ -2235,6 +2236,7 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
>> case VIR_DOMAIN_MEMORY_MODEL_DIMM:
>> case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
>> if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
>> + thisAddr = &mem->info.addr.dimm;
>> thisStart = mem->info.addr.dimm.base;
>> }
>> break;
>> @@ -2244,7 +2246,7 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
>> break;
>> }
>>
>> - if (thisStart == 0) {
>> + if (thisStart == 0 && !thisAddr) {
>> return 0;
>> }
>
> This seems a bit suspicious, because if the start address is 0, where
> qemu will auto-populate it, the code below will still try to validate
> it against existing devices.
>
> Very theoretically if you are adding a big enough device it can possibly
> clash with another one that is somewhere further in memory (start
> address already populated) despite the fact that qemu would put the new
> device into a reasonable location at the end of the address space.
>
> IMO if the start address is 0 we must not attempt to do anything about
> the address validation and the above condition will make that no longer
> the case.
>
Fair enough. Consider this squashed in:
diff --git i/src/conf/domain_validate.c w/src/conf/domain_validate.c
index f45ee0a8a5..c72108886e 100644
--- i/src/conf/domain_validate.c
+++ w/src/conf/domain_validate.c
@@ -2304,7 +2304,7 @@ virDomainMemoryDefCheckConflict(const
virDomainMemoryDef *mem,
break;
}
- if (otherStart == 0)
+ if (thisStart == 0 || otherStart == 0)
continue;
if (thisStart <= otherStart && thisEnd > otherStart) {
Or do you want me to send v2?
Michal
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
