On Tue, Jan 23, 2024 at 04:47:44PM +0100, Michal Prívozník wrote:
> On 1/23/24 10:45, Andrea Bolognani wrote:
> > On Fri, Jan 19, 2024 at 04:25:11PM +0100, Michal Privoznik wrote:
> >> +++ b/src/conf/domain_validate.c
> >> @@ -2315,7 +2316,10 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
> >> if (thisStart == 0 || otherStart == 0)
> >> continue;
> >>
> >> - if (thisStart <= otherStart && thisEnd > otherStart) {
> >> + otherEnd = otherStart + other->size;
> >
> > Shouldn't you multiply other->size by 1024? That's what happens
> > earlier with mem->size.
>
> D'oh! Of course. Nice catch.
>
> > I'm also curious about the zero check on thisStart and otherStart
> > right above that. It looks like it would allow two overlapping memory
> > devices to exists as long as either of them starts at zero, but I've
> > certainly missed something in related code that makes that scenario
> > impossible.
>
> There are two ways in which thisStart/otherStart can be zero:
>
> 1) It's initialized to 0 and never changed => we're dealing with a
> memory device model that doesn't support setting addresses (e.g. SGX), or
>
> 2) it was set to zero => it's basically a not-user-set default, i.e.
> it's formatted into XML iff != zero (see virDomainDeviceInfoFormat(),
> virDomainMemoryTargetDefFormat()).
>
> IOW, this == 0 comparison checks if an address was set by user and only
> then check is performed. But truth to be told - we don't deny those
> values during parsing. They are silently ignored (and it's probably too
> late to change that). But I guess nobody want's to map a memory onto 0x0
> anyway.
That's convincing enough for me.
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
with the unit conversion issue fixed.
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
