Skip to content

Commit 061ca8a

Browse files
kevmwkevmw
committed
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: Kevin Wolf <[email protected]> Reviewed-by: Stefan Hajnoczi <[email protected]>
1 parent ae5475e commit 061ca8a

16 files changed

+162
-89
lines changed

‎block.c

+55-8
Original file line numberDiff line numberDiff line change
@@ -3788,8 +3788,8 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
37883788
/**
37893789
* Truncate file to 'offset' bytes (needed only for file protocols)
37903790
*/
3791-
int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
3792-
Error **errp)
3791+
int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
3792+
PreallocMode prealloc, Error **errp)
37933793
{
37943794
BlockDriverState *bs = child->bs;
37953795
BlockDriver *drv = bs->drv;
@@ -3807,23 +3807,28 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
38073807
return -EINVAL;
38083808
}
38093809

3810-
if (!drv->bdrv_truncate) {
3810+
bdrv_inc_in_flight(bs);
3811+
3812+
if (!drv->bdrv_co_truncate) {
38113813
if (bs->file && drv->is_filter) {
3812-
return bdrv_truncate(bs->file, offset, prealloc, errp);
3814+
ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
3815+
goto out;
38133816
}
38143817
error_setg(errp, "Image format driver does not support resize");
3815-
return -ENOTSUP;
3818+
ret = -ENOTSUP;
3819+
goto out;
38163820
}
38173821
if (bs->read_only) {
38183822
error_setg(errp, "Image is read-only");
3819-
return -EACCES;
3823+
ret = -EACCES;
3824+
goto out;
38203825
}
38213826

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

3824-
ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
3829+
ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
38253830
if (ret < 0) {
3826-
return ret;
3831+
goto out;
38273832
}
38283833
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
38293834
if (ret < 0) {
@@ -3834,9 +3839,51 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
38343839
bdrv_dirty_bitmap_truncate(bs, offset);
38353840
bdrv_parent_cb_resize(bs);
38363841
atomic_inc(&bs->write_gen);
3842+
3843+
out:
3844+
bdrv_dec_in_flight(bs);
38373845
return ret;
38383846
}
38393847

3848+
typedef struct TruncateCo {
3849+
BdrvChild *child;
3850+
int64_t offset;
3851+
PreallocMode prealloc;
3852+
Error **errp;
3853+
int ret;
3854+
} TruncateCo;
3855+
3856+
static void coroutine_fn bdrv_truncate_co_entry(void *opaque)
3857+
{
3858+
TruncateCo *tco = opaque;
3859+
tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc,
3860+
tco->errp);
3861+
}
3862+
3863+
int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
3864+
Error **errp)
3865+
{
3866+
Coroutine *co;
3867+
TruncateCo tco = {
3868+
.child = child,
3869+
.offset = offset,
3870+
.prealloc = prealloc,
3871+
.errp = errp,
3872+
.ret = NOT_DONE,
3873+
};
3874+
3875+
if (qemu_in_coroutine()) {
3876+
/* Fast-path if already in coroutine context */
3877+
bdrv_truncate_co_entry(&tco);
3878+
} else {
3879+
co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);
3880+
qemu_coroutine_enter(co);
3881+
BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);
3882+
}
3883+
3884+
return tco.ret;
3885+
}
3886+
38403887
/**
38413888
* Length of a allocated file in bytes. Sparse files are counted by actual
38423889
* allocated space. Return < 0 if error or unknown.

‎block/copy-on-read.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ static int64_t cor_getlength(BlockDriverState *bs)
8080
}
8181

8282

83-
static int cor_truncate(BlockDriverState *bs, int64_t offset,
84-
PreallocMode prealloc, Error **errp)
83+
static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset,
84+
PreallocMode prealloc, Error **errp)
8585
{
86-
return bdrv_truncate(bs->file, offset, prealloc, errp);
86+
return bdrv_co_truncate(bs->file, offset, prealloc, errp);
8787
}
8888

8989

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

149149
.bdrv_getlength = cor_getlength,
150-
.bdrv_truncate = cor_truncate,
150+
.bdrv_co_truncate = cor_co_truncate,
151151

152152
.bdrv_co_preadv = cor_co_preadv,
153153
.bdrv_co_pwritev = cor_co_pwritev,

‎block/crypto.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,9 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
357357
return ret;
358358
}
359359

360-
static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
361-
PreallocMode prealloc, Error **errp)
360+
static int coroutine_fn
361+
block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,
362+
PreallocMode prealloc, Error **errp)
362363
{
363364
BlockCrypto *crypto = bs->opaque;
364365
uint64_t payload_offset =
@@ -371,7 +372,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
371372

372373
offset += payload_offset;
373374

374-
return bdrv_truncate(bs->file, offset, prealloc, errp);
375+
return bdrv_co_truncate(bs->file, offset, prealloc, errp);
375376
}
376377

377378
static void block_crypto_close(BlockDriverState *bs)
@@ -700,7 +701,7 @@ BlockDriver bdrv_crypto_luks = {
700701
.bdrv_child_perm = bdrv_format_default_perms,
701702
.bdrv_co_create = block_crypto_co_create_luks,
702703
.bdrv_co_create_opts = block_crypto_co_create_opts_luks,
703-
.bdrv_truncate = block_crypto_truncate,
704+
.bdrv_co_truncate = block_crypto_co_truncate,
704705
.create_opts = &block_crypto_create_opts_luks,
705706

706707
.bdrv_reopen_prepare = block_crypto_reopen_prepare,

‎block/file-posix.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -1878,8 +1878,8 @@ static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
18781878
return result;
18791879
}
18801880

1881-
static int raw_truncate(BlockDriverState *bs, int64_t offset,
1882-
PreallocMode prealloc, Error **errp)
1881+
static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
1882+
PreallocMode prealloc, Error **errp)
18831883
{
18841884
BDRVRawState *s = bs->opaque;
18851885
struct stat st;
@@ -2625,7 +2625,7 @@ BlockDriver bdrv_file = {
26252625
.bdrv_io_unplug = raw_aio_unplug,
26262626
.bdrv_attach_aio_context = raw_aio_attach_aio_context,
26272627

2628-
.bdrv_truncate = raw_truncate,
2628+
.bdrv_co_truncate = raw_co_truncate,
26292629
.bdrv_getlength = raw_getlength,
26302630
.bdrv_get_info = raw_get_info,
26312631
.bdrv_get_allocated_file_size
@@ -3105,7 +3105,7 @@ static BlockDriver bdrv_host_device = {
31053105
.bdrv_io_plug = raw_aio_plug,
31063106
.bdrv_io_unplug = raw_aio_unplug,
31073107

3108-
.bdrv_truncate = raw_truncate,
3108+
.bdrv_co_truncate = raw_co_truncate,
31093109
.bdrv_getlength = raw_getlength,
31103110
.bdrv_get_info = raw_get_info,
31113111
.bdrv_get_allocated_file_size
@@ -3227,7 +3227,7 @@ static BlockDriver bdrv_host_cdrom = {
32273227
.bdrv_io_plug = raw_aio_plug,
32283228
.bdrv_io_unplug = raw_aio_unplug,
32293229

3230-
.bdrv_truncate = raw_truncate,
3230+
.bdrv_co_truncate = raw_co_truncate,
32313231
.bdrv_getlength = raw_getlength,
32323232
.has_variable_length = true,
32333233
.bdrv_get_allocated_file_size
@@ -3357,7 +3357,7 @@ static BlockDriver bdrv_host_cdrom = {
33573357
.bdrv_io_plug = raw_aio_plug,
33583358
.bdrv_io_unplug = raw_aio_unplug,
33593359

3360-
.bdrv_truncate = raw_truncate,
3360+
.bdrv_co_truncate = raw_co_truncate,
33613361
.bdrv_getlength = raw_getlength,
33623362
.has_variable_length = true,
33633363
.bdrv_get_allocated_file_size

‎block/file-win32.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,8 @@ static void raw_close(BlockDriverState *bs)
467467
}
468468
}
469469

470-
static int raw_truncate(BlockDriverState *bs, int64_t offset,
471-
PreallocMode prealloc, Error **errp)
470+
static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
471+
PreallocMode prealloc, Error **errp)
472472
{
473473
BDRVRawState *s = bs->opaque;
474474
LONG low, high;
@@ -640,7 +640,7 @@ BlockDriver bdrv_file = {
640640
.bdrv_aio_pwritev = raw_aio_pwritev,
641641
.bdrv_aio_flush = raw_aio_flush,
642642

643-
.bdrv_truncate = raw_truncate,
643+
.bdrv_co_truncate = raw_co_truncate,
644644
.bdrv_getlength = raw_getlength,
645645
.bdrv_get_allocated_file_size
646646
= raw_get_allocated_file_size,

‎block/gluster.c

+8-6
Original file line numberDiff line numberDiff line change
@@ -1177,8 +1177,10 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
11771177
return acb.ret;
11781178
}
11791179

1180-
static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset,
1181-
PreallocMode prealloc, Error **errp)
1180+
static coroutine_fn int qemu_gluster_co_truncate(BlockDriverState *bs,
1181+
int64_t offset,
1182+
PreallocMode prealloc,
1183+
Error **errp)
11821184
{
11831185
BDRVGlusterState *s = bs->opaque;
11841186
return qemu_gluster_do_truncate(s->fd, offset, prealloc, errp);
@@ -1499,7 +1501,7 @@ static BlockDriver bdrv_gluster = {
14991501
.bdrv_co_create_opts = qemu_gluster_co_create_opts,
15001502
.bdrv_getlength = qemu_gluster_getlength,
15011503
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
1502-
.bdrv_truncate = qemu_gluster_truncate,
1504+
.bdrv_co_truncate = qemu_gluster_co_truncate,
15031505
.bdrv_co_readv = qemu_gluster_co_readv,
15041506
.bdrv_co_writev = qemu_gluster_co_writev,
15051507
.bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk,
@@ -1528,7 +1530,7 @@ static BlockDriver bdrv_gluster_tcp = {
15281530
.bdrv_co_create_opts = qemu_gluster_co_create_opts,
15291531
.bdrv_getlength = qemu_gluster_getlength,
15301532
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
1531-
.bdrv_truncate = qemu_gluster_truncate,
1533+
.bdrv_co_truncate = qemu_gluster_co_truncate,
15321534
.bdrv_co_readv = qemu_gluster_co_readv,
15331535
.bdrv_co_writev = qemu_gluster_co_writev,
15341536
.bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk,
@@ -1557,7 +1559,7 @@ static BlockDriver bdrv_gluster_unix = {
15571559
.bdrv_co_create_opts = qemu_gluster_co_create_opts,
15581560
.bdrv_getlength = qemu_gluster_getlength,
15591561
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
1560-
.bdrv_truncate = qemu_gluster_truncate,
1562+
.bdrv_co_truncate = qemu_gluster_co_truncate,
15611563
.bdrv_co_readv = qemu_gluster_co_readv,
15621564
.bdrv_co_writev = qemu_gluster_co_writev,
15631565
.bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk,
@@ -1592,7 +1594,7 @@ static BlockDriver bdrv_gluster_rdma = {
15921594
.bdrv_co_create_opts = qemu_gluster_co_create_opts,
15931595
.bdrv_getlength = qemu_gluster_getlength,
15941596
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
1595-
.bdrv_truncate = qemu_gluster_truncate,
1597+
.bdrv_co_truncate = qemu_gluster_co_truncate,
15961598
.bdrv_co_readv = qemu_gluster_co_readv,
15971599
.bdrv_co_writev = qemu_gluster_co_writev,
15981600
.bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk,

‎block/iscsi.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -2085,8 +2085,8 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
20852085
}
20862086
}
20872087

2088-
static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
2089-
PreallocMode prealloc, Error **errp)
2088+
static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
2089+
PreallocMode prealloc, Error **errp)
20902090
{
20912091
IscsiLun *iscsilun = bs->opaque;
20922092
Error *local_err = NULL;
@@ -2431,7 +2431,7 @@ static BlockDriver bdrv_iscsi = {
24312431

24322432
.bdrv_getlength = iscsi_getlength,
24332433
.bdrv_get_info = iscsi_get_info,
2434-
.bdrv_truncate = iscsi_truncate,
2434+
.bdrv_co_truncate = iscsi_co_truncate,
24352435
.bdrv_refresh_limits = iscsi_refresh_limits,
24362436

24372437
.bdrv_co_block_status = iscsi_co_block_status,
@@ -2468,7 +2468,7 @@ static BlockDriver bdrv_iser = {
24682468

24692469
.bdrv_getlength = iscsi_getlength,
24702470
.bdrv_get_info = iscsi_get_info,
2471-
.bdrv_truncate = iscsi_truncate,
2471+
.bdrv_co_truncate = iscsi_co_truncate,
24722472
.bdrv_refresh_limits = iscsi_refresh_limits,
24732473

24742474
.bdrv_co_block_status = iscsi_co_block_status,

‎block/nfs.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -743,8 +743,9 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
743743
return (task.ret < 0 ? task.ret : st.st_blocks * 512);
744744
}
745745

746-
static int nfs_file_truncate(BlockDriverState *bs, int64_t offset,
747-
PreallocMode prealloc, Error **errp)
746+
static int coroutine_fn
747+
nfs_file_co_truncate(BlockDriverState *bs, int64_t offset,
748+
PreallocMode prealloc, Error **errp)
748749
{
749750
NFSClient *client = bs->opaque;
750751
int ret;
@@ -873,7 +874,7 @@ static BlockDriver bdrv_nfs = {
873874

874875
.bdrv_has_zero_init = nfs_has_zero_init,
875876
.bdrv_get_allocated_file_size = nfs_get_allocated_file_size,
876-
.bdrv_truncate = nfs_file_truncate,
877+
.bdrv_co_truncate = nfs_file_co_truncate,
877878

878879
.bdrv_file_open = nfs_file_open,
879880
.bdrv_close = nfs_file_close,

0 commit comments

Comments
 (0)