Re: [Qemu-devel] [PATCH 2/7] nbd/server: Trace server noncompliance on unaligned requests

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

On 4/8/19 7:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> 05.04.2019 23:04, Eric Blake wrote:
>> On 4/5/19 9:39 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 03.04.2019 6:05, Eric Blake wrote:
>>>> We've recently added traces for clients to flag server non-compliance;
>>>> let's do the same for servers to flag client non-compliance. According
>>
>> Thus, s/Trace server/Trace client/ in the subject line.
>>

>>> Patch seems correct anyway, so if you are in a hurry, it's OK as is:
>>>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>
>> Here's what I'll squash in; I think it is obvious enough to still keep
>> your R-b, if I send the pull request before you reply back.
>>

>> @@ -2136,7 +2136,10 @@ static int nbd_co_receive_request(NBDRequestData
>> *req, NBDRequest *request,
>>            * The block layer gracefully handles unaligned requests, but
>>            * it's still worth tracing client non-compliance
>>            */
>> -
>> trace_nbd_co_receive_align_compliance(nbd_cmd_lookup(request->type));
>> +        trace_nbd_co_receive_align_compliance(nbd_cmd_lookup(request->type,
>> +                                                             request->from,
>> +                                                             request->len,
>> +
>> client->check_align));
> 
> something strange here with brackets.

Yeah, I grabbed the diff before compile-testing, but fixed it before
pushing to my branch.


>> +++ w/nbd/trace-events
>> @@ -71,5 +71,5 @@ nbd_co_send_extents(uint64_t handle, unsigned int
>> extents, uint32_t id, uint64_t
>>   nbd_co_send_structured_error(uint64_t handle, int err, const char
>> *errname, const char *msg) "Send structured error reply: handle = %"
>> PRIu64 ", error = %d (%s), msg = '%s'"
>>   nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type,
>> const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16
>> " (%s)"
>>   nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len)
>> "Payload received: handle = %" PRIu64 ", len = %" PRIu32
>> -nbd_co_receive_align_compliance(const char *op) "client sent
>> non-compliant unaligned %s request"
>> +nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t
>> len, uint32_t align) "client sent non-compliant unaligned %s request:
>> from=0x%" PRIx64 ", len=0x%x, align=0x%x"
> 
> %x or % PRIx32 - doesn't matter?

Will use PRIx32 for consistency.

> 
>>   nbd_trip(void) "Reading request"
>>
>>
>>
> 
> OK for me, thanks.

Thanks for a second look. Pull request will be out later today.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]