Re: [PATCH 4/5] virDomainMemoryDefCheckConflict: Validate dimm slot too

On 11/16/23 14:05, Peter Krempa wrote: > On Mon, Nov 06, 2023 at 12:38:27 +0100, Michal Privoznik wrote:

>> @@ -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?