Re: [BUGFIX][PATCH v2 5/5] xg_main: If XEN_DOMCTL_gdbsx_guestmemio fails then force error.

[Don Slutz <dslutz@verizon.com>]

On 01/08/14 05:35, Ian Campbell wrote:
> On Tue, 2014-01-07 at 19:25 -0500, Don Slutz wrote:
>> Without this gdb does not report an error.
>>
>> With this patch and using a 1G hvm domU:
>>
>> (gdb) x/1xh 0x6ae9168b
>> 0x6ae9168b:     Cannot access memory at address 0x6ae9168b
>>
>> Drop output of iop->remain because it most likely will be zero.
>> This leads to a strange message:
>>
>> ERROR: failed to read 0 bytes. errno:14 rc:-1
>>
>> Add address to write error because it may be the only message
>> displayed.
>>
>> Note: currently XEN_DOMCTL_gdbsx_guestmemio does not change 'iop' on
>> error and so iop->remain will be zero.
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>>   tools/debugger/gdbsx/xg/xg_main.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
>> index 3b2a285..0fc3f82 100644
>> --- a/tools/debugger/gdbsx/xg/xg_main.c
>> +++ b/tools/debugger/gdbsx/xg/xg_main.c
>> @@ -787,8 +787,10 @@ xg_read_mem(uint64_t guestva, char *tobuf, int tobuf_len, uint64_t pgd3val)
>>       iop->gwr = 0;       /* not writing to guest */
>>   
>>       if ( (rc = _domctl_hcall(XEN_DOMCTL_gdbsx_guestmemio, tobuf, tobuf_len)) )
>> -        XGTRC("ERROR: failed to read %d bytes. errno:%d rc:%d\n",
>> -              iop->remain, errno, rc);
>> +    {
>> +        XGTRC("ERROR: failed to read bytes. errno:%d rc:%d\n", errno, rc);
> Is it worth printing the expect number (i.e. the input) of bytes? Is
> that buflen here?

Not in my testing.  If I turn on debug output (-d), I get:


...
process_remote_request:E:m8957006a,1 curvcpu:0
xg_read_mem:E:gva:8957006a tobuf:1c81020 len:1
ERROR:xg_read_mem:ERROR: failed to read bytes. errno:14 rc:-1
xg_read_mem:X:remain:0 buf8:0x24
process_m_request:Failed read mem. addr:0x8957006a len:1 remn:1 errno:14
process_remote_request:X:E01 curvcpu:0
...


Which outputs the length (1) a few times already.   The expected number here is tobuf_len.


>> +        return tobuf_len;
>> +    }
>>   
>>       for(i=0; i < XGMIN(8, tobuf_len); u.buf8[i]=tobuf[i], i++);
>>       XGTRC("X:remain:%d buf8:0x%llx\n", iop->remain, u.llbuf8);
>> @@ -818,8 +820,11 @@ xg_write_mem(uint64_t guestva, char *frombuf, int buflen, uint64_t pgd3val)
>>       iop->gwr = 1;       /* writing to guest */
>>   
>>       if ((rc=_domctl_hcall(XEN_DOMCTL_gdbsx_guestmemio, frombuf, buflen)))
>> -        XGERR("ERROR: failed to write %d bytes. errno:%d rc:%d\n",
>> -              iop->remain, errno, rc);
>> +    {
>> +        XGERR("ERROR: failed to write bytes to %llx. errno:%d rc:%d\n",
>> +              guestva, errno, rc);
> Same here.

Since this error message is output without other context, it might help.   Most of the time, the user knows the write size requested. Since this code has an ACK from Mukesh, I lean to not making a change, but if you want it, I will.

    -Don Slutz

>> +        return buflen;
>> +    }
>>       return iop->remain;
>>   }
>>   
>