Re: [Xen-devel] [PATCH v1] x86/mm: Make change_type_range return error

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.04.19 at 16:46, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Apr 24, 2019 at 02:27:32PM +0000, Alexandru Stefan ISAILA wrote:
>> @@ -1053,15 +1053,11 @@ static void change_type_range(struct p2m_domain *p2m,
>>       * This should be revisited later, but for now post a warning.
>>       */
>>      if ( unlikely(end > host_max_pfn) )
>> -    {
>> -        printk(XENLOG_G_WARNING "Dom%d logdirty rangeset clipped to 
>> max_mapped_pfn\n",
>> -               d->domain_id);
>> -        end = invalidate_end = host_max_pfn;
>> -    }
>> +        return -EINVAL;
>>  
>>      /* If the requested range is out of scope, return doing nothing. */
>>      if ( start > end )
>> -        return;
>> +        return 0;
> 
> Since you are already changing the behavior of the function above this
> should also return EINVAL IMO.

I don't think I agree. Quite the other way around: In the latter
case it's simply an empty range that gets requested, which is a
no-op (and hence no reason to fail). Avoiding empty ranges in
the callers may result in less readable code there.

Even in the former case I don't think returning an error is
appropriate, the more that the comment there says this is
probably not the right behavior. I think it would be better to
leave this alone until we have settled on what the right
behavior here is. It is an issue anyway that a change is
made without saying why the new behavior preferable over
the current one.

In any event the comment there would become stale with the
removal of the printk().

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel