>>> On 24.04.19 at 16:46, <roger....@citrix.com> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
