Commit 061ca8a3 authored by Kevin Wolf's avatar Kevin Wolf
Browse files

block: Convert .bdrv_truncate callback to coroutine_fn 



bdrv_truncate() is an operation that can block (even for a quite long
time, depending on the PreallocMode) in I/O paths that shouldn't block.
Convert it to a coroutine_fn so that we have the infrastructure for
drivers to make their .bdrv_co_truncate implementation asynchronous.

This change could potentially introduce new race conditions because
bdrv_truncate() isn't necessarily executed atomically any more. Whether
this is a problem needs to be evaluated for each block driver that
supports truncate:

* file-posix/win32, gluster, iscsi, nfs, rbd, ssh, sheepdog: The
  protocol drivers are trivially safe because they don't actually yield
  yet, so there is no change in behaviour.

* copy-on-read, crypto, raw-format: Essentially just filter drivers that
  pass the request to a child node, no problem.

* qcow2: The implementation modifies metadata, so it needs to hold
  s->lock to be safe with concurrent I/O requests. In order to avoid
  double locking, this requires pulling the locking out into
  preallocate_co() and using qcow2_write_caches() instead of
  bdrv_flush().

* qed: Does a single header update, this is fine without locking.

Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent ae5475e8

block.c

+55 −8
Original line number Diff line number Diff line
@@ -3788,8 +3788,8 @@ exit: 
/**

 * Truncate file to 'offset' bytes (needed only for file protocols)

 */

int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,

                  Error **errp)

int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,

                                  PreallocMode prealloc, Error **errp)

{

    BlockDriverState *bs = child->bs;

    BlockDriver *drv = bs->drv;
@@ -3807,23 +3807,28 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, 
        return -EINVAL;

    }



    if (!drv->bdrv_truncate) {

    bdrv_inc_in_flight(bs);



    if (!drv->bdrv_co_truncate) {

        if (bs->file && drv->is_filter) {

            return bdrv_truncate(bs->file, offset, prealloc, errp);

            ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);

            goto out;

        }

        error_setg(errp, "Image format driver does not support resize");

        return -ENOTSUP;

        ret = -ENOTSUP;

        goto out;

    }

    if (bs->read_only) {

        error_setg(errp, "Image is read-only");

        return -EACCES;

        ret = -EACCES;

        goto out;

    }



    assert(!(bs->open_flags & BDRV_O_INACTIVE));



    ret = drv->bdrv_truncate(bs, offset, prealloc, errp);

    ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);

    if (ret < 0) {

        return ret;

        goto out;

    }

    ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);

    if (ret < 0) {
@@ -3834,9 +3839,51 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, 
    bdrv_dirty_bitmap_truncate(bs, offset);

    bdrv_parent_cb_resize(bs);

    atomic_inc(&bs->write_gen);



out:

    bdrv_dec_in_flight(bs);

    return ret;

}



typedef struct TruncateCo {

    BdrvChild *child;

    int64_t offset;

    PreallocMode prealloc;

    Error **errp;

    int ret;

} TruncateCo;



static void coroutine_fn bdrv_truncate_co_entry(void *opaque)

{

    TruncateCo *tco = opaque;

    tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc,

                                tco->errp);

}



int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,

                  Error **errp)

{

    Coroutine *co;

    TruncateCo tco = {

        .child      = child,

        .offset     = offset,

        .prealloc   = prealloc,

        .errp       = errp,

        .ret        = NOT_DONE,

    };



    if (qemu_in_coroutine()) {

        /* Fast-path if already in coroutine context */

        bdrv_truncate_co_entry(&tco);

    } else {

        co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);

        qemu_coroutine_enter(co);

        BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);

    }



    return tco.ret;

}



/**

 * Length of a allocated file in bytes. Sparse files are counted by actual

 * allocated space. Return < 0 if error or unknown.

block/copy-on-read.c

+4 −4
Original line number Diff line number Diff line
@@ -80,10 +80,10 @@ static int64_t cor_getlength(BlockDriverState *bs) 
}





static int cor_truncate(BlockDriverState *bs, int64_t offset,

static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset,

                                        PreallocMode prealloc, Error **errp)

{

    return bdrv_truncate(bs->file, offset, prealloc, errp);

    return bdrv_co_truncate(bs->file, offset, prealloc, errp);

}
@@ -147,7 +147,7 @@ BlockDriver bdrv_copy_on_read = { 
    .bdrv_child_perm                    = cor_child_perm,



    .bdrv_getlength                     = cor_getlength,

    .bdrv_truncate                      = cor_truncate,

    .bdrv_co_truncate                   = cor_co_truncate,



    .bdrv_co_preadv                     = cor_co_preadv,

    .bdrv_co_pwritev                    = cor_co_pwritev,

block/crypto.c

+5 −4
Original line number Diff line number Diff line
@@ -357,7 +357,8 @@ static int block_crypto_co_create_generic(BlockDriverState *bs, 
    return ret;

}



static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,

static int coroutine_fn

block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,

                         PreallocMode prealloc, Error **errp)

{

    BlockCrypto *crypto = bs->opaque;
@@ -371,7 +372,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset, 


    offset += payload_offset;



    return bdrv_truncate(bs->file, offset, prealloc, errp);

    return bdrv_co_truncate(bs->file, offset, prealloc, errp);

}



static void block_crypto_close(BlockDriverState *bs)
@@ -700,7 +701,7 @@ BlockDriver bdrv_crypto_luks = { 
    .bdrv_child_perm    = bdrv_format_default_perms,

    .bdrv_co_create     = block_crypto_co_create_luks,

    .bdrv_co_create_opts = block_crypto_co_create_opts_luks,

    .bdrv_truncate      = block_crypto_truncate,

    .bdrv_co_truncate   = block_crypto_co_truncate,

    .create_opts        = &block_crypto_create_opts_luks,



    .bdrv_reopen_prepare = block_crypto_reopen_prepare,

block/file-posix.c

+6 −6
Original line number Diff line number Diff line
@@ -1878,7 +1878,7 @@ out: 
    return result;

}



static int raw_truncate(BlockDriverState *bs, int64_t offset,

static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,

                                        PreallocMode prealloc, Error **errp)

{

    BDRVRawState *s = bs->opaque;
@@ -2625,7 +2625,7 @@ BlockDriver bdrv_file = { 
    .bdrv_io_unplug = raw_aio_unplug,

    .bdrv_attach_aio_context = raw_aio_attach_aio_context,



    .bdrv_truncate = raw_truncate,

    .bdrv_co_truncate = raw_co_truncate,

    .bdrv_getlength = raw_getlength,

    .bdrv_get_info = raw_get_info,

    .bdrv_get_allocated_file_size
@@ -3105,7 +3105,7 @@ static BlockDriver bdrv_host_device = { 
    .bdrv_io_plug = raw_aio_plug,

    .bdrv_io_unplug = raw_aio_unplug,



    .bdrv_truncate      = raw_truncate,

    .bdrv_co_truncate       = raw_co_truncate,

    .bdrv_getlength	= raw_getlength,

    .bdrv_get_info = raw_get_info,

    .bdrv_get_allocated_file_size
@@ -3227,7 +3227,7 @@ static BlockDriver bdrv_host_cdrom = { 
    .bdrv_io_plug = raw_aio_plug,

    .bdrv_io_unplug = raw_aio_unplug,



    .bdrv_truncate      = raw_truncate,

    .bdrv_co_truncate    = raw_co_truncate,

    .bdrv_getlength      = raw_getlength,

    .has_variable_length = true,

    .bdrv_get_allocated_file_size
@@ -3357,7 +3357,7 @@ static BlockDriver bdrv_host_cdrom = { 
    .bdrv_io_plug = raw_aio_plug,

    .bdrv_io_unplug = raw_aio_unplug,



    .bdrv_truncate      = raw_truncate,

    .bdrv_co_truncate    = raw_co_truncate,

    .bdrv_getlength      = raw_getlength,

    .has_variable_length = true,

    .bdrv_get_allocated_file_size

block/file-win32.c

+3 −3
Original line number Diff line number Diff line
@@ -467,7 +467,7 @@ static void raw_close(BlockDriverState *bs) 
    }

}



static int raw_truncate(BlockDriverState *bs, int64_t offset,

static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,

                                        PreallocMode prealloc, Error **errp)

{

    BDRVRawState *s = bs->opaque;
@@ -640,7 +640,7 @@ BlockDriver bdrv_file = { 
    .bdrv_aio_pwritev   = raw_aio_pwritev,

    .bdrv_aio_flush     = raw_aio_flush,



    .bdrv_truncate	= raw_truncate,

    .bdrv_co_truncate   = raw_co_truncate,

    .bdrv_getlength	= raw_getlength,

    .bdrv_get_allocated_file_size

                        = raw_get_allocated_file_size,