Commit 7596bbb3 authored by Eric Blake's avatar Eric Blake
Browse files

nbd/server: Hoist length check to qmp_nbd_server_add 



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.c 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) (someday, it would be nice to have
byte-accurate lengths - but not today).

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, meaning
that no negative values were passed, and that offset+size does
not exceed 63 bits (as that really is a fundamental limit to
later operations, whether we use off_t or uint64_t).

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <20190117193658.16413-6-eblake@redhat.com>
Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
parent 4485936b

blockdev-nbd.c

+9 −1
Original line number Diff line number Diff line
@@ -146,6 +146,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
    BlockDriverState *bs = NULL;

    BlockBackend *on_eject_blk;

    NBDExport *exp;

    int64_t len;



    if (!nbd_server) {

        error_setg(errp, "NBD server not running");
@@ -168,6 +169,13 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
        return;

    }



    len = bdrv_getlength(bs);

    if (len < 0) {

        error_setg_errno(errp, -len,

                         "Failed to determine the NBD export's length");

        return;

    }



    if (!has_writable) {

        writable = false;

    }
@@ -175,7 +183,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
        writable = false;

    }



    exp = nbd_export_new(bs, 0, -1, name, NULL, bitmap,

    exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,

                         writable ? 0 : NBD_FLAG_READ_ONLY,

                         NULL, false, on_eject_blk, errp);

    if (!exp) {

nbd/server.c

+3 −7
Original line number Diff line number Diff line
@@ -1495,17 +1495,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size, 
    exp->refcount = 1;

    QTAILQ_INIT(&exp->clients);

    exp->blk = blk;

    assert(dev_offset >= 0 && dev_offset <= INT64_MAX);

    exp->dev_offset = dev_offset;

    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(size >= 0 && size <= INT64_MAX - dev_offset);

    exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);



    if (bitmap) {

        BdrvDirtyBitmap *bm = NULL;