Re: [Qemu-devel] [PATCH] nbd/server: fix bitmap export

[Eric Blake <eblake@redhat.com>]

On 9/14/18 12:24 PM, Eric Blake wrote:
> On 9/14/18 11:51 AM, Vladimir Sementsov-Ogievskiy wrote:
>> bitmap_to_extents function is broken: it switches dirty variable after
>> every iteration, however it can process only part of dirty (or zero)
>> area during one iteration in case when this area is too large for one
>> extent.
>>
>> Fortunately, the bug don't produce wrong extents: it just inserts
>> zero-length extents between sequential extents representing large dirty
>> (or zero) area. However, zero-length extents are abandoned by NBD
> 
> s/abandoned by/forbidden by the/
> 
>> protocol. So, careful client should consider such replay as server
> 
> s/replay/reply/
> 
>> fault and not-careful will likely ignore zero-length extents.
> 
> Which camp is qemu 3.0 as client in? Does it tolerate the zero-length 
> extent, and still manage to see correct information overall, or does it 
> crash?
> 
> Hmm - I think we're "safe" with qemu as client - right now, the only way 
> qemu 3.0 accesses the qemu dirty bitmap over NBD is with my 
> x-dirty-bitmap hack (commit 216ee3657), which uses 
> block/nbd-client.c:nbd_client_co_block_status() to read the bitmap, and 
> that always passes NBD_CMD_FLAG_REQ_ONE.  qemu will assert() if 
> nbd_client_co_block_status() doesn't make any progress, but from what 
> I'm reading of your bug report, qemu as client never permits the server 
> to answer with more than one extent, and the bug of a zero-length extent 
> is triggered only after the first extent has been sent.

A further mitigation - the bug can only occur for a client that requests 
block status with a length in the range (4G-bitmap_granularity, 4G). 
Otherwise, the loop terminates after the extent that got truncated to 
4G-bitmap_granularity, preventing the next iteration of the loop from 
detecting a 0-length extent.

> 
> Thus, the primary reason to accept this patch is not because of qemu 3.0 
> as client, but for interoperability with other clients.  I'm planning on 
> updating the commit message to add these additional details.
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org