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 <[email protected]> > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]> > 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
