15.01.2019 18:25, Eric Blake wrote:
> 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 <ebl...@redhat.com>
>>>
>
>>> +++ 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).
>
Don't follow =(
Assume, image size 3M, and we have offset 2M, i.e. -o 2M.
than in qemu-nbd.c, we have
fd_size = blk_getlength(blk); # 3M
...
fd_size -= dev_offset; # 1M
...
export = nbd_export_new(bs, dev_offset, fd_size # bs, 2M, 1M
in nbd_export_new:
assert(dev_offset <= size); # 2M <= 1M
fail.
--
Best regards,
Vladimir
