Re: [Xen-devel] [PATCH RFC v2 03/12] xen/mem_event: Relax error condition on debug builds

[Andres Lagar Cavilla <andres@xxxxxxxxxxxxxxxx>]

On Wed, Aug 27, 2014 at 11:36 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 27.08.14 at 19:02, <andres@xxxxxxxxxxxxxxxx> wrote:
> On Wed, Aug 27, 2014 at 7:06 AM, Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>
> wrote:
>
>> A faulty tool stack can brick a debug hypervisor. Unpleasant while
>> dev/test.
>>
>> Suggested-by: Andres Lagar Cavilla <andres@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>
>> ---
>>Â xen/common/mem_event.c | 6 +++++-
>>Â 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
>> index e22b78e..8be32e1 100644
>> --- a/xen/common/mem_event.c
>> +++ b/xen/common/mem_event.c
>> @@ -279,7 +279,11 @@ void mem_event_put_request(struct domain *d,
>>Â Â Â if ( current->domain != d )
>>Â Â Â {
>>Â Â Â Â Â req->flags |= MEM_EVENT_FLAG_FOREIGN;
>> -Â Â Â Â ASSERT( !(req->flags & MEM_EVENT_FLAG_VCPU_PAUSED) );
>> +#ifndef NDEBUG
>> +Â Â Â Â if ( !(req->flags & MEM_EVENT_FLAG_VCPU_PAUSED) )
>> +Â Â Â Â Â Â printk(XENLOG_G_WARNING
>> +Â Â Â Â Â Â Â Â Â Â"VCPU was not paused.\n");
>>
>
> 1. use gdprintk

Maybe, but then the message (which now will include current
domain/vcpu) should also include subject domain and vcpu; perhaps
it should have done so from the beginning to make it half way useful.

Yes.

Â

> 2. enclose only the gdprintk in #ifdef
> 3. if the flags contain the improper VCPU_PAUSED value, also clear that
> value from flags (regardless from NDEBUG)

How would fixing (actually setting rather than clearing) that flag
help (as this wouldn't put the vcpu in the intended state)?

It wouldn't (setting the flag, indeed). I got it wrong.

AndresÂ

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel