Re: [Qemu-devel] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add

[Eric Blake]

On 1/15/19 3:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.01.2019 20:57, Eric Blake wrote:
>> We only had two callers to nbd_export_new; qemu-nbd.c always
>> passed a valid offset/length pair (because it already checked
>> the file length, to ensure that offset was in bounds), while
>> blockdev-nbd always passed 0/-1.  Then nbd_export_new reduces
>> the size to a multiple of BDRV_SECTOR_SIZE (can only happen
>> when offset is not sector-aligned, since bdrv_getlength()
>> currently rounds up), which can result in offset being greater
>> than the enforced length, but that's not fatal (the server
>> rejects client requests that exceed the advertised length).
>>
>> However, I'm finding it easier to work with the code if we are
>> consistent on having both callers pass in a valid length, and
>> just assert that things are sane in nbd_export_new.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>

>> +++ b/nbd/server.c
>> @@ -1499,13 +1499,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
>> dev_offset, off_t size,
>>       exp->name = g_strdup(name);
>>       exp->description = g_strdup(description);
>>       exp->nbdflags = nbdflags;
>> -    exp->size = size < 0 ? blk_getlength(blk) : size;
>> -    if (exp->size < 0) {
>> -        error_setg_errno(errp, -exp->size,
>> -                         "Failed to determine the NBD export's length");
>> -        goto fail;
>> -    }
>> -    exp->size -= exp->size % BDRV_SECTOR_SIZE;
>> +    assert(dev_offset <= size);
> 
> @size is not size of the image, but size of the export, so it may be less 
> than dev_offset
> (qemu-nbd.c do "fd_size -= dev_offset" before "nbd_export_new(bs, dev_offset, 
> fd_size, "

But the assert is fine because patch 3/19 fixed qemu-nbd.c to never pass
in dev_offset larger than size (it fails up front if dev_offset is out
of bounds, whether from the -o command line option or from what it read
from the partition header with the -P command line option).

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

signature.asc
Description: OpenPGP digital signature