[Qemu-devel] [PATCH for-4.0] nbd-client: Work around server BLOCK_STATUS misalignment at EOF

[Eric Blake]

On 3/27/19 11:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 26.03.2019 20:13, Eric Blake wrote:
>> The NBD spec is clear that a server that advertises a minimum block
>> size should reply to NBD_CMD_BLOCK_STATUS with extents aligned
>> accordingly. However, we know that the qemu NBD server implementation
>> has had a corner-case bug where it is not compliant with the spec,
>> present since the introduction of NBD_CMD_BLOCK_STATUS in qemu 2.12
>> (and unlikely to be patched in time for 4.0). Namely, when qemu is
>> serving a file that is not a multiple of 512 bytes, it rounds the size
>> advertised over NBD up to the next sector boundary (someday, I'd like
>> to fix that to be byte-accurate, but it's a much bigger audit not
>> appropriate for this release); yet if the final sector contains data
>> prior to EOF, lseek(SEEK_HOLE) will point to the implicit hole
>> mid-sector which qemu then reported over NBD.
>>

>>
>> Easy reproduction:
>> $ printf %1000d 1 > file
>> $ qemu-nbd -f raw -t file & pid=$!
>> $ qemu-img map --output=json -f raw nbd://localhost:10809
>> qemu-img: Could not read file metadata: Invalid argument
>> $ kill $pid
> 
> would be good to add it to iotests
> 

Will do in a followup; probably by reviving a v2 of this series:
https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00305.html

>>
>> where the patched version instead succeeds with:
>> [{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
>>
>> Signed-off-by: Eric Blake <address@hidden>
> 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> 

Thanks; will queue through my NBD tree for -rc2.


>> +++ b/block/nbd-client.c
>> @@ -269,14 +269,36 @@ static int 
>> nbd_parse_blockstatus_payload(NBDClientSession *client,
>>       extent->length = payload_advance32(&payload);
>>       extent->flags = payload_advance32(&payload);
>>
>> -    if (extent->length == 0 ||
>> -        (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,
>> -                                                    
>> client->info.min_block))) {
>> +    if (extent->length == 0) {
>>           error_setg(errp, "Protocol error: server sent status chunk with "
>>                      "invalid length");
> 
> may be improved to s/invalid/zero/

Will do.

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

signature.asc
Description: OpenPGP digital signature