block: Get rid of bdrv_swap()
cover block: Get rid of bdrv_swap()
bdrv_swap() has always been an ugly hack that we would rather have avoided. When it was introduced, we simply didn't have the infrastructure to update pointers instead of transplanting the contents of BDS object, so we grudgingly added bdrv_swap() as a quick solution. Meanwhile, most of the infrastructure exists and this series implements the final step necessary to implement the required functionality in a less adventurous way.
Kevin Wolf (16):
block: Introduce BDS.file_child
vmdk: Use BdrvChild instead of BDS for references to extents
blkverify: Convert s->test_file to BdrvChild
quorum: Convert to BdrvChild
block: Convert bs->file to BdrvChild
block: Remove bdrv_open_image()
block: Convert bs->backing_hd to BdrvChild
block: Manage backing file references in bdrv_set_backing_hd()
block: Split bdrv_move_feature_fields()
block/io: Make bdrv_requests_pending() public
block-backend: Add blk_set_bs()
block: Introduce parents list
block: Implement bdrv_append() without bdrv_swap()
blockjob: Store device name at job creation
block: Add and use bdrv_replace_in_backing_chain()
block: Remove bdrv_swap()
block.c | 465 +++++++++++++++++++--------------------------- block/blkdebug.c | 32 ++-- block/blkverify.c | 68 +++---- block/block-backend.c | 16 ++ block/bochs.c | 8 +- block/cloop.c | 10 +- block/dmg.c | 20 +- block/io.c | 76 ++++---- block/mirror.c | 19 +- block/parallels.c | 38 ++-- block/qapi.c | 10 +- block/qcow.c | 46 ++--- block/qcow2-cache.c | 11 +- block/qcow2-cluster.c | 42 +++-- block/qcow2-refcount.c | 45 ++--- block/qcow2-snapshot.c | 30 +-- block/qcow2.c | 68 +++---- block/qed-table.c | 4 +- block/qed.c | 51 +++-- block/quorum.c | 63 ++++--- block/raw_bsd.c | 40 ++-- block/snapshot.c | 12 +- block/stream.c | 27 +-- block/vdi.c | 17 +- block/vhdx-log.c | 25 +-- block/vhdx.c | 36 ++-- block/vmdk.c | 133 ++++++------- block/vpc.c | 34 ++-- block/vvfat.c | 19 +- blockdev.c | 6 +- blockjob.c | 8 +- include/block/block.h | 15 +- include/block/block_int.h | 20 +- include/block/blockjob.h | 8 + include/qemu/queue.h | 6 - qemu-img.c | 24 +-- 36 files changed, 755 insertions(+), 797 deletions(-)
Signature · 1 line
1.8.3.1
PATCH 1/16 block: Introduce BDS.file_child
Store the BdrvChild for bs->file. At this point, bs->file_child->bs just duplicates the bs->file pointer. Later, it will completely replace it.
--- block.c | 12 +++++++++--- include/block/block_int.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-)
block.c +9 -3
@@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,assert (file ==NULL ); bs->open_flags = flags; - ret =bdrv_open_image (&file, filename, options,"file" , - bs, &child_file, true, &local_err); -if (ret <0 ) { + + bs->file_child =bdrv_open_child (filename, options,"file" , bs, + &child_file, true, &local_err); +if (local_err) { + ret = -EINVAL ; goto fail; } + +if (bs->file_child) { + file = bs->file_child->bs; + } }/* Image format probing */
include/block/block_int.h +1 -0
@@ -381,6 +381,7 @@ struct BlockDriverState {BlockDriverState *backing_hd;BdrvChild *backing_child;BlockDriverState *file; +BdrvChild *file_child;NotifierList close_notifiers;
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 01/16] block: Introduce BDS.file_child
Quoted 8 lines · Kevin Wolf
Store the BdrvChild for
bs->file. At this point,bs->file_child->bsjust duplicates thebs->filepointer. Later, it will completely replace it.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 12 +++++++++--- include/block/block_int.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-)
Although I have a small comment below:
block.c static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, · line 1488
Quoted 8 lines
block.c +0
-0@@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,assert (file ==NULL );
This looks strange now, without the direct connection to bdrv_open_image(). I'd drop this...
Quoted 5 lines
block.c +1
-3bs->open_flags = flags; - ret =bdrv_open_image (&file, filename, options,"file" , - bs, &child_file, true, &local_err); -if (ret <0 ) { +
block.c static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, · line 1500
Quoted 10 lines
block.c +8
-0+ bs->file_child =bdrv_open_child (filename, options,"file" , bs, + &child_file, true, &local_err); +if (local_err) { + ret = -EINVAL ; goto fail; } + +if (bs->file_child) { + file = bs->file_child->bs; + }
And make this file = bs->file_child ? bs->file_child->bs : NULL (or the respective long form, i.e.
if (bs->file_child) { ... } else { file = NULL; }).
We could even put this after this if block and drop the NULL initialization of file, but that might go overboard.
Max
Quoted 16 lines
block.c +0
-0}/* Image format probing */ include/block/block_int.h +1
-0@@ -381,6 +381,7 @@ struct BlockDriverState {BlockDriverState *backing_hd;BdrvChild *backing_child;BlockDriverState *file; +BdrvChild *file_child;NotifierList close_notifiers;
Re: [Qemu-devel] [PATCH 01/16] block: Introduce BDS.file_child
Quoted 46 lines · Max Reitz
Quoted 8 lines · Kevin Wolf
Store the BdrvChild for
bs->file. At this point,bs->file_child->bsjust duplicates thebs->filepointer. Later, it will completely replace it.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 12 +++++++++--- include/block/block_int.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-)Reviewed-by Max Reitz <[email protected]>Although I have a small comment below:
block.c static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, · line 1488
Quoted 8 lines
block.c +0
-0@@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,assert (file ==NULL );This looks strange now, without the direct connection to
bdrv_open_image(). I'd drop this...Quoted 5 lines
block.c +1
-3bs->open_flags = flags; - ret =bdrv_open_image (&file, filename, options,"file" , - bs, &child_file, true, &local_err); -if (ret <0 ) { +block.c static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, · line 1500
Quoted 10 lines
block.c +8
-0+ bs->file_child =bdrv_open_child (filename, options,"file" , bs, + &child_file, true, &local_err); +if (local_err) { + ret = -EINVAL ; goto fail; } + +if (bs->file_child) { + file = bs->file_child->bs; + }And make this file =
bs->file_child?bs->file_child->bs: NULL (or the respective long form, i.e.
if (bs->file_child) { ... } else { file = NULL; }).
Quoted 2 lines
We could even put this after this if block and drop the NULL initialization of file, but that might go overboard.
And expecting something like this, I skipped ahead to patch 5, which drops this altogether. An R-b without any comments it is, then. :-)
Max
Quoted 18 lines
Quoted 16 lines
block.c +0
-0}/* Image format probing */ include/block/block_int.h +1
-0@@ -381,6 +381,7 @@ struct BlockDriverState {BlockDriverState *backing_hd;BdrvChild *backing_child;BlockDriverState *file; +BdrvChild *file_child;NotifierList close_notifiers;
Re: [Qemu-devel] [PATCH 01/16] block: Introduce BDS.file_child
In reply to Kevin WolfQuoted 5 lines · Kevin Wolf
Store the BdrvChild for
bs->file. At this point,bs->file_child->bsjust duplicates thebs->filepointer. Later, it will completely replace it.Signed-off-by Kevin Wolf <[email protected]>---
Berto
PATCH 2/16 vmdk: Use BdrvChild instead of BDS for references to extents
In reply to Kevin Wolf--- block/vmdk.c | 99 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
block/vmdk.c +51 -48
@@ -87,7 +87,7 @@ typedef struct {#define L2_CACHE_SIZE 16 typedef struct VmdkExtent { - BlockDriverState *file; + BdrvChild *file; bool flat; bool compressed; bool has_marker;
@@ -221,8 +221,8 @@ static void vmdk_free_extents(BlockDriverState *bs)g_free (e->l2_cache);g_free (e->l1_backup_table);g_free (e->type); -if (e->file != bs->file) { -bdrv_unref (e->file); +if (e->file != bs->file_child) { +bdrv_unref_child (bs, e->file); } }g_free (s->extents);
@@ -367,7 +367,7 @@ static int vmdk_parent_open(BlockDriverState *bs)/* Create and append extent to the extent array. Return the added VmdkExtent * address. return NULL if allocation failed. */ static int vmdk_add_extent(BlockDriverState *bs, - BlockDriverState *file, bool flat, int64_t sectors, + BdrvChild *file, bool flat, int64_t sectors, int64_t l1_offset, int64_t l1_backup_offset, uint32_t l1_size, int l2_size, uint64_t cluster_sectors,
@@ -392,7 +392,7 @@ static int vmdk_add_extent(BlockDriverState *bs,return -EFBIG ; } - nb_sectors =bdrv_nb_sectors (file); + nb_sectors =bdrv_nb_sectors (file->bs);if (nb_sectors <0 ) {return nb_sectors; }
@@ -439,14 +439,14 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,return -ENOMEM ; } - ret =bdrv_pread (extent->file, + ret =bdrv_pread (extent->file->bs, extent->l1_table_offset, extent->l1_table, l1_size);if (ret <0 ) {error_setg_errno (errp, -ret,"Could not read l1 table from extent '%s'" , - extent->file->filename); + extent->file->bs->filename); goto fail_l1; }for (i =0 ; i < extent->l1_size; i++) {
@@ -459,14 +459,14 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, ret = -ENOMEM ; goto fail_l1; } - ret =bdrv_pread (extent->file, + ret =bdrv_pread (extent->file->bs, extent->l1_backup_table_offset, extent->l1_backup_table, l1_size);if (ret <0 ) {error_setg_errno (errp, -ret,"Could not read l1 backup table from extent '%s'" , - extent->file->filename); + extent->file->bs->filename); goto fail_l1b; }for (i =0 ; i < extent->l1_size; i++) {
@@ -485,7 +485,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, }static int vmdk_open_vmfs_sparse (BlockDriverState *bs, -BlockDriverState *file, +BdrvChild *file,int flags,Error **errp) {int ret;
@@ -493,11 +493,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,VMDK3Header header;VmdkExtent *extent; - ret =bdrv_pread (file,sizeof (magic), &header,sizeof (header)); + ret =bdrv_pread (file->bs,sizeof (magic), &header,sizeof (header));if (ret <0 ) {error_setg_errno (errp, -ret,"Could not read header from file '%s'" , - file->filename); + file->bs->filename);return ret; } ret = vmdk_add_extent(bs, file, false,
@@ -559,7 +559,7 @@ static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset, }static int vmdk_open_vmdk4 (BlockDriverState *bs, -BlockDriverState *file, +BdrvChild *file,int flags,QDict *options,Error **errp) {int ret;
@@ -570,17 +570,17 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,BDRVVmdkState *s = bs->opaque;int64_t l1_backup_offset =0 ; - ret =bdrv_pread (file,sizeof (magic), &header,sizeof (header)); + ret =bdrv_pread (file->bs,sizeof (magic), &header,sizeof (header));if (ret <0 ) {error_setg_errno (errp, -ret,"Could not read header from file '%s'" , - file->filename); + file->bs->filename);return -EINVAL ; }if (header.capacity ==0 ) {uint64_t desc_offset =le64_to_cpu (header.desc_offset);if (desc_offset) { -char *buf =vmdk_read_desc (file, desc_offset <<9 , errp); +char *buf =vmdk_read_desc (file->bs, desc_offset <<9 , errp);if (!buf) {return -EINVAL ; }
@@ -620,7 +620,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, }QEMU_PACKED eos_marker; }QEMU_PACKED footer; - ret =bdrv_pread (file, + ret =bdrv_pread (file->bs, bs->file->total_sectors *512 -1536 , &footer,sizeof (footer));if (ret <0 ) {
@@ -675,7 +675,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,if (le32_to_cpu (header.flags) &VMDK4_FLAG_RGD ) { l1_backup_offset =le64_to_cpu (header.rgd_offset) <<9 ; } -if (bdrv_nb_sectors (file) <le64_to_cpu (header.grain_offset)) { +if (bdrv_nb_sectors (file->bs) <le64_to_cpu (header.grain_offset)) {error_setg (errp,"File truncated, expecting at least %" PRId64" bytes" , (int64_t )(le64_to_cpu (header.grain_offset) *BDRV_SECTOR_SIZE ));
@@ -739,8 +739,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, }/* Open an extent file and append to bs array */ -static int vmdk_open_sparse (BlockDriverState *bs, -BlockDriverState *file,int flags, +static int vmdk_open_sparse (BlockDriverState *bs,BdrvChild *file,int flags,char *buf,QDict *options,Error **errp) {uint32_t magic;
@@ -773,10 +772,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,int64_t sectors =0 ;int64_t flat_offset;char *extent_path; -BlockDriverState *extent_file; +BdrvChild *extent_file;BDRVVmdkState *s = bs->opaque;VmdkExtent *extent;char extent_opt_prefix[32 ]; +Error *local_err =NULL ;while (*p) {/* parse extent line in one of below formats:
@@ -825,16 +825,16 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, extent_path =g_malloc0 (PATH_MAX );path_combine (extent_path,PATH_MAX , desc_file_path, fname); - extent_file =NULL ; ret =snprintf (extent_opt_prefix,32 ,"extents.%d" , s->num_extents);assert (ret <32 ); - ret =bdrv_open_image (&extent_file, extent_path, options, - extent_opt_prefix, bs, &child_file, false, errp); + extent_file =bdrv_open_child (extent_path, options, extent_opt_prefix, + bs, &child_file, false, &local_err);g_free (extent_path); -if (ret) { -return ret; +if (local_err) { +error_propagate (errp, local_err); +return -EINVAL ; }/* save to extents array */
@@ -844,13 +844,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, ret =vmdk_add_extent (bs, extent_file, true, sectors,0 ,0 ,0 ,0 ,0 , &extent, errp);if (ret <0 ) { -bdrv_unref (extent_file); +bdrv_unref_child (bs, extent_file);return ret; } extent->flat_start_offset = flat_offset <<9 ; }else if (!strcmp (type,"SPARSE" ) || !strcmp (type,"VMFSSPARSE" )) {/* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/ -char *buf =vmdk_read_desc (extent_file,0 , errp); +char *buf =vmdk_read_desc (extent_file->bs,0 , errp);if (!buf) { ret = -EINVAL ; }else {
@@ -859,13 +859,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, }g_free (buf);if (ret) { -bdrv_unref (extent_file); +bdrv_unref_child (bs, extent_file);return ret; } extent = &s->extents[s->num_extents -1 ]; }else {error_setg (errp,"Unsupported extent type '%s'" , type); -bdrv_unref (extent_file); +bdrv_unref_child (bs, extent_file);return -ENOTSUP ; } extent->type =g_strdup (type);
@@ -927,7 +927,8 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,switch (magic) {case VMDK3_MAGIC :case VMDK4_MAGIC : - ret =vmdk_open_sparse (bs, bs->file, flags, buf, options, errp); + ret =vmdk_open_sparse (bs, bs->file_child, flags, buf, options, + errp); s->desc_offset =0x200 ;break ;default :
@@ -1028,7 +1029,7 @@ static int get_whole_cluster(BlockDriverState *bs, goto exit; } } - ret =bdrv_write (extent->file, cluster_sector_num, whole_grain, + ret =bdrv_write (extent->file->bs, cluster_sector_num, whole_grain, skip_start_sector);if (ret <0 ) { ret =VMDK_ERROR ;
@@ -1046,7 +1047,7 @@ static int get_whole_cluster(BlockDriverState *bs, goto exit; } } - ret =bdrv_write (extent->file, cluster_sector_num + skip_end_sector, + ret =bdrv_write (extent->file->bs, cluster_sector_num + skip_end_sector, whole_grain + (skip_end_sector <<BDRV_SECTOR_BITS ), extent->cluster_sectors - skip_end_sector);if (ret <0 ) {
@@ -1066,7 +1067,7 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data, offset =cpu_to_le32 (offset);/* update L2 table */ if (bdrv_pwrite_sync ( - extent->file, + extent->file->bs, ((int64_t )m_data->l2_offset *512 ) + (m_data->l2_index *sizeof (offset)), &offset,sizeof (offset)) <0 ) {
@@ -1076,7 +1077,7 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data,if (extent->l1_backup_table_offset !=0 ) { m_data->l2_offset = extent->l1_backup_table[m_data->l1_index];if (bdrv_pwrite_sync ( - extent->file, + extent->file->bs, ((int64_t )m_data->l2_offset *512 ) + (m_data->l2_index *sizeof (offset)), &offset,sizeof (offset)) <0 ) {
@@ -1166,7 +1167,7 @@ static int get_cluster_offset(BlockDriverState *bs, } l2_table = extent->l2_cache + (min_index * extent->l2_size); if (bdrv_pread( - extent->file, + extent->file->bs, (int64_t)l2_offset * 512, l2_table, extent->l2_size * sizeof(uint32_t)
@@ -1274,7 +1275,7 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,break ;case VMDK_OK : ret =BDRV_BLOCK_DATA ; -if (extent->file == bs->file && !extent->compressed) { +if (extent->file == bs->file_child && !extent->compressed) { ret |=BDRV_BLOCK_OFFSET_VALID | offset; }
@@ -1320,7 +1321,7 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, write_len = buf_len +sizeof (VmdkGrainMarker); } write_offset = cluster_offset + offset_in_cluster, - ret =bdrv_pwrite (extent->file, write_offset, write_buf, write_len); + ret =bdrv_pwrite (extent->file->bs, write_offset, write_buf, write_len); write_end_sector =DIV_ROUND_UP (write_offset + write_len,BDRV_SECTOR_SIZE );
@@ -1355,7 +1356,7 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,if (!extent->compressed) { - ret =bdrv_pread (extent->file, + ret =bdrv_pread (extent->file->bs, cluster_offset + offset_in_cluster, buf, nb_sectors *512 );if (ret == nb_sectors *512 ) {
@@ -1369,7 +1370,7 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset, buf_bytes = cluster_bytes *2 ; cluster_buf =g_malloc (buf_bytes); uncomp_buf =g_malloc (cluster_bytes); - ret =bdrv_pread (extent->file, + ret =bdrv_pread (extent->file->bs, cluster_offset, cluster_buf, buf_bytes);if (ret <0 ) {
@@ -2035,7 +2036,7 @@ static coroutine_fn int vmdk_co_flush(BlockDriverState *bs)int ret =0 ;for (i =0 ; i < s->num_extents; i++) { - err =bdrv_co_flush (s->extents[i].file); + err =bdrv_co_flush (s->extents[i].file->bs);if (err <0 ) { ret = err; }
@@ -2055,10 +2056,10 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs)return ret; }for (i =0 ; i < s->num_extents; i++) { -if (s->extents[i].file == bs->file) { +if (s->extents[i].file == bs->file_child) {continue ; } - r =bdrv_get_allocated_file_size (s->extents[i].file); + r =bdrv_get_allocated_file_size (s->extents[i].file->bs);if (r <0 ) {return r; }
@@ -2076,7 +2077,7 @@ static int vmdk_has_zero_init(BlockDriverState *bs)* return 0. */ for (i =0 ; i < s->num_extents; i++) {if (s->extents[i].flat) { -if (!bdrv_has_zero_init (s->extents[i].file)) { +if (!bdrv_has_zero_init (s->extents[i].file->bs)) {return 0 ; } }
@@ -2089,7 +2090,7 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)ImageInfo *info =g_new0 (ImageInfo,1 ); *info = (ImageInfo ){ - .filename =g_strdup (extent->file->filename), + .filename =g_strdup (extent->file->bs->filename), .format =g_strdup (extent->type), .virtual_size = extent->sectors *BDRV_SECTOR_SIZE , .compressed = extent->compressed,
@@ -2135,7 +2136,9 @@ static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result, PRId64 "\n", sector_num);break ; } -if (ret ==VMDK_OK && cluster_offset >=bdrv_getlength (extent->file)) { +if (ret ==VMDK_OK && + cluster_offset >=bdrv_getlength (extent->file->bs)) + {fprintf (stderr,"ERROR: cluster offset for sector %" PRId64" points after EOF\n" , sector_num);
@@ -2211,7 +2214,7 @@ static void vmdk_detach_aio_context(BlockDriverState *bs)int i;for (i =0 ; i < s->num_extents; i++) { -bdrv_detach_aio_context (s->extents[i].file); +bdrv_detach_aio_context (s->extents[i].file->bs); } }
@@ -2222,7 +2225,7 @@ static void vmdk_attach_aio_context(BlockDriverState *bs,int i;for (i =0 ; i < s->num_extents; i++) { -bdrv_attach_aio_context (s->extents[i].file, new_context); +bdrv_attach_aio_context (s->extents[i].file->bs, new_context); } }
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 02/16] vmdk: Use BdrvChild instead of BDS for references to extents
Quoted 4 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/vmdk.c | 99 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
Re: [Qemu-devel] [PATCH 02/16] vmdk: Use BdrvChild instead of BDS for references to extents
In reply to Kevin WolfQuoted 4 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/vmdk.c | 99 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
Berto
PATCH 3/16 blkverify: Convert s->test_file to BdrvChild
In reply to Kevin Wolf--- block/blkverify.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
block/blkverify.c +21 -20
@@ -14,7 +14,7 @@#include "qapi/qmp/qstring.h" typedef struct { -BlockDriverState *test_file; +BdrvChild *test_file; }BDRVBlkverifyState ;typedef struct BlkverifyAIOCB BlkverifyAIOCB ;
@@ -132,12 +132,12 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, }/* Open the test file */ -assert (s->test_file ==NULL ); - ret =bdrv_open_image (&s->test_file,qemu_opt_get (opts,"x-image" ), options, -"test" , bs, &child_format, false, &local_err); -if (ret <0 ) { + s->test_file =bdrv_open_child (qemu_opt_get (opts,"x-image" ), options, +"test" , bs, &child_format, false, + &local_err); +if (local_err) { + ret = -EINVAL ;error_propagate (errp, local_err); - s->test_file =NULL ; goto fail; }
@@ -151,7 +151,7 @@ static void blkverify_close(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -bdrv_unref (s->test_file); +bdrv_unref_child (bs, s->test_file); s->test_file =NULL ; }
@@ -159,7 +159,7 @@ static int64_t blkverify_getlength(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -return bdrv_getlength (s->test_file); +return bdrv_getlength (s->test_file->bs); } static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool is_write,
@@ -242,7 +242,7 @@ static BlockAIOCB *blkverify_aio_readv(BlockDriverState *bs,qemu_iovec_init (&acb->raw_qiov, acb->qiov->niov);qemu_iovec_clone (&acb->raw_qiov, qiov, acb->buf); -bdrv_aio_readv (s->test_file, sector_num, qiov, nb_sectors, +bdrv_aio_readv (s->test_file->bs, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb);bdrv_aio_readv (bs->file, sector_num, &acb->raw_qiov, nb_sectors, blkverify_aio_cb, acb);
@@ -257,7 +257,7 @@ static BlockAIOCB *blkverify_aio_writev(BlockDriverState *bs,BlkverifyAIOCB *acb =blkverify_aio_get (bs, true, sector_num, qiov, nb_sectors, cb, opaque); -bdrv_aio_writev (s->test_file, sector_num, qiov, nb_sectors, +bdrv_aio_writev (s->test_file->bs, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb);bdrv_aio_writev (bs->file, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb);
@@ -271,7 +271,7 @@ static BlockAIOCB *blkverify_aio_flush(BlockDriverState *bs,BDRVBlkverifyState *s = bs->opaque;/* Only flush test file, the raw file is not important */ -return bdrv_aio_flush (s->test_file, cb, opaque); +return bdrv_aio_flush (s->test_file->bs, cb, opaque); } static bool blkverify_recurse_is_first_non_filter(BlockDriverState *bs,
@@ -285,7 +285,7 @@ static bool blkverify_recurse_is_first_non_filter(BlockDriverState *bs,return true; } -return bdrv_recurse_is_first_non_filter (s->test_file, candidate); +return bdrv_recurse_is_first_non_filter (s->test_file->bs, candidate); }/* Propagate AioContext changes to ->test_file */
@@ -293,7 +293,7 @@ static void blkverify_detach_aio_context(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -bdrv_detach_aio_context (s->test_file); +bdrv_detach_aio_context (s->test_file->bs); } static void blkverify_attach_aio_context(BlockDriverState *bs,
@@ -301,7 +301,7 @@ static void blkverify_attach_aio_context(BlockDriverState *bs, {BDRVBlkverifyState *s = bs->opaque; -bdrv_attach_aio_context (s->test_file, new_context); +bdrv_attach_aio_context (s->test_file->bs, new_context); }static void blkverify_refresh_filename (BlockDriverState *bs)
@@ -309,24 +309,25 @@ static void blkverify_refresh_filename(BlockDriverState *bs)BDRVBlkverifyState *s = bs->opaque;/* bs->file has already been refreshed */ -bdrv_refresh_filename (s->test_file); +bdrv_refresh_filename (s->test_file->bs); -if (bs->file->full_open_options && s->test_file->full_open_options) { +if (bs->file->full_open_options && s->test_file->bs->full_open_options) {QDict *opts =qdict_new ();qdict_put_obj (opts,"driver" ,QOBJECT (qstring_from_str ("blkverify" )));QINCREF (bs->file->full_open_options);qdict_put_obj (opts,"raw" ,QOBJECT (bs->file->full_open_options)); -QINCREF (s->test_file->full_open_options); -qdict_put_obj (opts,"test" ,QOBJECT (s->test_file->full_open_options)); +QINCREF (s->test_file->bs->full_open_options); +qdict_put_obj (opts,"test" , +QOBJECT (s->test_file->bs->full_open_options)); bs->full_open_options = opts; } -if (bs->file->exact_filename[0 ] && s->test_file->exact_filename[0 ]) { +if (bs->file->exact_filename[0 ] && s->test_file->bs->exact_filename[0 ]) {snprintf (bs->exact_filename,sizeof (bs->exact_filename),"blkverify:%s:%s" , - bs->file->exact_filename, s->test_file->exact_filename); + bs->file->exact_filename, s->test_file->bs->exact_filename); } }
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild
Quoted 4 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/blkverify.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild
In reply to Kevin Wolfblock/blkverify.c static void blkverify_close(BlockDriverState *bs) · line 156
Quoted 8 lines · Kevin Wolf
block/blkverify.c +1
-1@@ -151,7 +151,7 @@ static void blkverify_close(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -bdrv_unref (s->test_file); +bdrv_unref_child (bs, s->test_file); s->test_file =NULL ; }
You are using bdrv_unref_child() here whereas in quorum you kept bdrv_unref().
In principle both seem correct because if you don't detach the children in the driver's close function then bdrv_close() will take care of doing it, but is there a reason why you are using different methods?
Berto
Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild
Am 23.09.2015 um 15:01 hat Alberto Garcia geschrieben:
Quoted 14 lines · Kevin Wolf
block/blkverify.c static void blkverify_close(BlockDriverState *bs) · line 156
Quoted 9 lines
block/blkverify.c +1
-1@@ -151,7 +151,7 @@ static void blkverify_close(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -bdrv_unref (s->test_file); +bdrv_unref_child (bs, s->test_file); s->test_file =NULL ; }You are using
bdrv_unref_child()here whereas in quorum you keptbdrv_unref().
Quoted 3 lines
In principle both seem correct because if you don't detach the children in the driver's close function then
bdrv_close()will take care of doing it, but is there a reason why you are using different methods?
Because consistency is overrated? Or simply carelessness?
In the end (not in this series), I'd like to remove all of this from the close functions (as the comment in block.c says) and let it all be handled in bdrv_close(). But until then, we should stay reasonably consistent indeed.
VMDK uses bdrv_unref_child() as well, so I guess quorum is the one that should be changed?
Kevin
Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild
Quoted 12 lines · Kevin Wolf
Quoted 12 lines
block/blkverify.c static void blkverify_close(BlockDriverState *bs) · line 156
Quoted 8 lines
block/blkverify.c +1
-1@@ -151,7 +151,7 @@ static void blkverify_close(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -bdrv_unref (s->test_file); +bdrv_unref_child (bs, s->test_file); s->test_file =NULL ; }You are using
bdrv_unref_child()here whereas in quorum you keptbdrv_unref().
Quoted 6 lines
Quoted 4 lines
In principle both seem correct because if you don't detach the children in the driver's close function then
bdrv_close()will take care of doing it, but is there a reason why you are using different methods?Because consistency is overrated? Or simply carelessness?
Ah ok :-)
Quoted 2 lines
VMDK uses
bdrv_unref_child()as well, so I guess quorum is the one that should be changed?
You can keep my R-b if you do so.
Berto
Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild
In reply to Kevin WolfQuoted 5 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/blkverify.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
Berto
PATCH 4/16 quorum: Convert to BdrvChild
In reply to Kevin Wolf--- block/quorum.c | 63 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 30 deletions(-)
block/quorum.c +33 -30
@@ -64,7 +64,7 @@ typedef struct QuorumVotes {/* the following structure holds the state of one quorum instance */ typedef struct BDRVQuorumState { - BlockDriverState **bs; /* children BlockDriverStates */ + BdrvChild **children; /* children BlockDriverStates */ int num_children; /* children count */ int threshold; /* if less than threshold children reads gave the * same result a quorum error occurs.
@@ -336,7 +336,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,continue ; } QLIST_FOREACH(item, &version->items, next) { -quorum_report_bad (acb, s->bs[item->index]->node_name,0 ); +quorum_report_bad (acb, s->children[item->index]->bs->node_name,0 ); } } }
@@ -369,8 +369,9 @@ static bool quorum_rewrite_bad_versions(BDRVQuorumState *s, QuorumAIOCB *acb,continue ; } QLIST_FOREACH(item, &version->items, next) { -bdrv_aio_writev (s->bs[item->index], acb->sector_num, acb->qiov, - acb->nb_sectors, quorum_rewrite_aio_cb, acb); +bdrv_aio_writev (s->children[item->index]->bs, acb->sector_num, + acb->qiov, acb->nb_sectors, quorum_rewrite_aio_cb, + acb); } }
@@ -639,13 +640,13 @@ static BlockAIOCB *read_quorum_children(QuorumAIOCB *acb)int i;for (i =0 ; i < s->num_children; i++) { - acb->qcrs[i].buf =qemu_blockalign (s->bs[i], acb->qiov->size); + acb->qcrs[i].buf =qemu_blockalign (s->children[i]->bs, acb->qiov->size);qemu_iovec_init (&acb->qcrs[i].qiov, acb->qiov->niov);qemu_iovec_clone (&acb->qcrs[i].qiov, acb->qiov, acb->qcrs[i].buf); }for (i =0 ; i < s->num_children; i++) { -bdrv_aio_readv (s->bs[i], acb->sector_num, &acb->qcrs[i].qiov, +bdrv_aio_readv (s->children[i]->bs, acb->sector_num, &acb->qcrs[i].qiov, acb->nb_sectors, quorum_aio_cb, &acb->qcrs[i]); }
@@ -656,12 +657,12 @@ static BlockAIOCB *read_fifo_child(QuorumAIOCB *acb) {BDRVQuorumState *s = acb->common.bs->opaque; - acb->qcrs[acb->child_iter].buf =qemu_blockalign (s->bs[acb->child_iter], - acb->qiov->size); + acb->qcrs[acb->child_iter].buf = +qemu_blockalign (s->children[acb->child_iter]->bs, acb->qiov->size);qemu_iovec_init (&acb->qcrs[acb->child_iter].qiov, acb->qiov->niov);qemu_iovec_clone (&acb->qcrs[acb->child_iter].qiov, acb->qiov, acb->qcrs[acb->child_iter].buf); -bdrv_aio_readv (s->bs[acb->child_iter], acb->sector_num, +bdrv_aio_readv (s->children[acb->child_iter]->bs, acb->sector_num, &acb->qcrs[acb->child_iter].qiov, acb->nb_sectors, quorum_aio_cb, &acb->qcrs[acb->child_iter]);
@@ -702,8 +703,8 @@ static BlockAIOCB *quorum_aio_writev(BlockDriverState *bs,int i;for (i =0 ; i < s->num_children; i++) { - acb->qcrs[i].aiocb =bdrv_aio_writev (s->bs[i], sector_num, qiov, - nb_sectors, &quorum_aio_cb, + acb->qcrs[i].aiocb =bdrv_aio_writev (s->children[i]->bs, sector_num, + qiov, nb_sectors, &quorum_aio_cb, &acb->qcrs[i]); }
@@ -717,12 +718,12 @@ static int64_t quorum_getlength(BlockDriverState *bs)int i;/* check that all file have the same length */ - result =bdrv_getlength (s->bs[0 ]); + result =bdrv_getlength (s->children[0 ]->bs);if (result <0 ) {return result; }for (i =1 ; i < s->num_children; i++) { -int64_t value =bdrv_getlength (s->bs[i]); +int64_t value =bdrv_getlength (s->children[i]->bs);if (value <0 ) {return value; }
@@ -741,7 +742,7 @@ static void quorum_invalidate_cache(BlockDriverState *bs, Error **errp)int i;for (i =0 ; i < s->num_children; i++) { -bdrv_invalidate_cache (s->bs[i], &local_err); +bdrv_invalidate_cache (s->children[i]->bs, &local_err);if (local_err) {error_propagate (errp, local_err);return ;
@@ -762,7 +763,7 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs) error_votes.compare = quorum_64bits_compare;for (i =0 ; i < s->num_children; i++) { - result =bdrv_co_flush (s->bs[i]); + result =bdrv_co_flush (s->children[i]->bs); result_value.l = result;quorum_count_vote (&error_votes, &result_value, i); }
@@ -782,7 +783,7 @@ static bool quorum_recurse_is_first_non_filter(BlockDriverState *bs,int i;for (i =0 ; i < s->num_children; i++) { -bool perm =bdrv_recurse_is_first_non_filter (s->bs[i], +bool perm =bdrv_recurse_is_first_non_filter (s->children[i]->bs, candidate);if (perm) {return true;
@@ -923,7 +924,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, }/* allocate the children BlockDriverState array */ - s->bs = g_new0(BlockDriverState *, s->num_children); + s->children = g_new0(BdrvChild *, s->num_children); opened =g_new0 (bool, s->num_children);for (i =0 ; i < s->num_children; i++) {
@@ -931,9 +932,10 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, ret =snprintf (indexstr,32 ,"children.%d" , i);assert (ret <32 ); - ret =bdrv_open_image (&s->bs[i],NULL , options, indexstr, bs, - &child_format, false, &local_err); -if (ret <0 ) { + s->children[i] =bdrv_open_child (NULL , options, indexstr, bs, + &child_format, false, &local_err); +if (local_err) { + ret = -EINVAL ; goto close_exit; }
@@ -949,9 +951,9 @@ close_exit:if (!opened[i]) {continue ; } -bdrv_unref (s->bs[i]); +bdrv_unref_child (bs, s->children[i]); } -g_free (s->bs); +g_free (s->children);g_free (opened); exit:qemu_opts_del (opts);
@@ -968,10 +970,10 @@ static void quorum_close(BlockDriverState *bs)int i;for (i =0 ; i < s->num_children; i++) { -bdrv_unref (s->bs[i]); +bdrv_unref (s->children[i]->bs); } -g_free (s->bs); +g_free (s->children); }static void quorum_detach_aio_context (BlockDriverState *bs)
@@ -980,7 +982,7 @@ static void quorum_detach_aio_context(BlockDriverState *bs)int i;for (i =0 ; i < s->num_children; i++) { -bdrv_detach_aio_context (s->bs[i]); +bdrv_detach_aio_context (s->children[i]->bs); } }
@@ -991,7 +993,7 @@ static void quorum_attach_aio_context(BlockDriverState *bs,int i;for (i =0 ; i < s->num_children; i++) { -bdrv_attach_aio_context (s->bs[i], new_context); +bdrv_attach_aio_context (s->children[i]->bs, new_context); } }
@@ -1003,16 +1005,17 @@ static void quorum_refresh_filename(BlockDriverState *bs)int i;for (i =0 ; i < s->num_children; i++) { -bdrv_refresh_filename (s->bs[i]); -if (!s->bs[i]->full_open_options) { +bdrv_refresh_filename (s->children[i]->bs); +if (!s->children[i]->bs->full_open_options) {return ; } } children =qlist_new ();for (i =0 ; i < s->num_children; i++) { -QINCREF (s->bs[i]->full_open_options); -qlist_append_obj (children,QOBJECT (s->bs[i]->full_open_options)); +QINCREF (s->children[i]->bs->full_open_options); +qlist_append_obj (children, +QOBJECT (s->children[i]->bs->full_open_options)); } opts =qdict_new ();
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 04/16] quorum: Convert to BdrvChild
Quoted 4 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/quorum.c | 63 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 30 deletions(-)
Re: [Qemu-devel] [PATCH 04/16] quorum: Convert to BdrvChild
In reply to Kevin WolfQuoted 3 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/quorum.c | 63 ++++++++++++++++++++++++++++++----------------------------
Berto
PATCH 5/16 block: Convert bs->file to BdrvChild
In reply to Kevin WolfThis patch removes the temporary duplication between bs->file and bs->file_child by converting everything to BdrvChild.
--- block.c | 61 ++++++++++++++++++++++------------------------ block/blkdebug.c | 32 ++++++++++++------------ block/blkverify.c | 33 ++++++++++++++----------- block/bochs.c | 8 +++--- block/cloop.c | 10 ++++---- block/dmg.c | 20 +++++++-------- block/io.c | 50 +++++++++++++++++++------------------- block/parallels.c | 38 +++++++++++++++-------------- block/qapi.c | 2 +- block/qcow.c | 42 +++++++++++++++++--------------- block/qcow2-cache.c | 11 +++++---- block/qcow2-cluster.c | 38 +++++++++++++++++------------ block/qcow2-refcount.c | 45 ++++++++++++++++++---------------- block/qcow2-snapshot.c | 30 ++++++++++++----------- block/qcow2.c | 62 ++++++++++++++++++++++++----------------------- block/qed-table.c | 4 +-- block/qed.c | 32 ++++++++++++------------ block/raw_bsd.c | 40 +++++++++++++++--------------- block/snapshot.c | 12 ++++----- block/vdi.c | 17 +++++++------ block/vhdx-log.c | 25 ++++++++++--------- block/vhdx.c | 36 ++++++++++++++------------- block/vmdk.c | 27 +++++++++++---------- block/vpc.c | 34 ++++++++++++++------------ include/block/block.h | 8 +++++- include/block/block_int.h | 3 +-- 26 files changed, 377 insertions(+), 343 deletions(-)
block.c +29 -32
@@ -809,7 +809,7 @@ static QemuOptsList bdrv_runtime_opts = {* * Removes all processed options from *options. */ -static int bdrv_open_common (BlockDriverState *bs,BlockDriverState *file, +static int bdrv_open_common (BlockDriverState *bs,BdrvChild *file,QDict *options,int flags,BlockDriver *drv,Error **errp) {int ret, open_flags;
@@ -823,7 +823,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,assert (options !=NULL && bs->options != options);if (file !=NULL ) { - filename = file->filename; + filename = file->bs->filename; }else { filename =qdict_get_try_str (options,"filename" ); }
@@ -1401,7 +1401,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const BdrvChildRole *child_role, Error **errp) { int ret; -BlockDriverState *file =NULL , *bs; +BdrvChild *file =NULL ; +BlockDriverState *bs;BlockDriver *drv =NULL ;const char *drvname;Error *local_err =NULL ;
@@ -1485,25 +1486,20 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, flags =bdrv_backing_flags (flags); } -assert (file ==NULL ); bs->open_flags = flags; - bs->file_child =bdrv_open_child (filename, options,"file" , bs, - &child_file, true, &local_err); + file =bdrv_open_child (filename, options,"file" , bs, + &child_file, true, &local_err);if (local_err) { ret = -EINVAL ; goto fail; } - -if (bs->file_child) { - file = bs->file_child->bs; - } }/* Image format probing */ bs->probed = !drv;if (!drv && file) { - ret =find_image_format (file, filename, &drv, &local_err); + ret =find_image_format (file->bs, filename, &drv, &local_err);if (ret <0 ) { goto fail; }
@@ -1526,7 +1522,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, }if (file && (bs->file != file)) { -bdrv_unref (file); +bdrv_unref_child (bs, file); file =NULL ; }
@@ -1587,7 +1583,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, fail:if (file !=NULL ) { -bdrv_unref (file); +bdrv_unref_child (bs, file); }QDECREF (bs->options);QDECREF (options);
@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)bdrv_unref (backing_hd); } +if (bs->file !=NULL ) { +bdrv_unref (bs->file->bs); + bs->file =NULL ; + } + QLIST_FOREACH_SAFE(child, &bs->children, next, next) {/* TODO Remove bdrv_unref() from drivers' close function and use * bdrv_unref_child() here */
@@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs) bs->options =NULL ;QDECREF (bs->full_open_options); bs->full_open_options =NULL ; - -if (bs->file !=NULL ) { -bdrv_unref (bs->file); - bs->file =NULL ; - } }if (bs->blk) {
@@ -2565,7 +2561,7 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)return drv->bdrv_get_allocated_file_size (bs); }if (bs->file) { -return bdrv_get_allocated_file_size (bs->file); +return bdrv_get_allocated_file_size (bs->file->bs); }return -ENOTSUP ; }
@@ -3047,7 +3043,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event, const char *tag) {while (bs && bs->drv && !bs->drv->bdrv_debug_breakpoint) { - bs = bs->file; + bs = bs->file ? bs->file->bs :NULL ; }if (bs && bs->drv && bs->drv->bdrv_debug_breakpoint) {
@@ -3060,7 +3056,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,int bdrv_debug_remove_breakpoint (BlockDriverState *bs,const char *tag) {while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) { - bs = bs->file; + bs = bs->file ? bs->file->bs :NULL ; }if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
@@ -3073,7 +3069,7 @@ int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)int bdrv_debug_resume (BlockDriverState *bs,const char *tag) {while (bs && (!bs->drv || !bs->drv->bdrv_debug_resume)) { - bs = bs->file; + bs = bs->file ? bs->file->bs :NULL ; }if (bs && bs->drv && bs->drv->bdrv_debug_resume) {
@@ -3086,7 +3082,7 @@ int bdrv_debug_resume(BlockDriverState *bs, const char *tag)bool bdrv_debug_is_suspended (BlockDriverState *bs,const char *tag) {while (bs && bs->drv && !bs->drv->bdrv_debug_is_suspended) { - bs = bs->file; + bs = bs->file ? bs->file->bs :NULL ; }if (bs && bs->drv && bs->drv->bdrv_debug_is_suspended) {
@@ -3208,7 +3204,7 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)if (bs->drv->bdrv_invalidate_cache) { bs->drv->bdrv_invalidate_cache (bs, &local_err); }else if (bs->file) { -bdrv_invalidate_cache (bs->file, &local_err); +bdrv_invalidate_cache (bs->file->bs, &local_err); }if (local_err) {error_propagate (errp, local_err);
@@ -3938,7 +3934,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs) bs->drv->bdrv_detach_aio_context (bs); }if (bs->file) { -bdrv_detach_aio_context (bs->file); +bdrv_detach_aio_context (bs->file->bs); }if (bs->backing_hd) {bdrv_detach_aio_context (bs->backing_hd);
@@ -3962,7 +3958,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,bdrv_attach_aio_context (bs->backing_hd, new_context); }if (bs->file) { -bdrv_attach_aio_context (bs->file, new_context); +bdrv_attach_aio_context (bs->file->bs, new_context); }if (bs->drv->bdrv_attach_aio_context) { bs->drv->bdrv_attach_aio_context (bs, new_context);
@@ -4174,7 +4170,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)/* This BDS's file name will most probably depend on its file's name, so * refresh that first */ if (bs->file) { -bdrv_refresh_filename (bs->file); +bdrv_refresh_filename (bs->file->bs); }if (drv->bdrv_refresh_filename) {
@@ -4202,19 +4198,20 @@ void bdrv_refresh_filename(BlockDriverState *bs)/* If no specific options have been given for this BDS, the filename of * the underlying file should suffice for this one as well */ -if (bs->file->exact_filename[0 ] && !has_open_options) { -strcpy (bs->exact_filename, bs->file->exact_filename); +if (bs->file->bs->exact_filename[0 ] && !has_open_options) { +strcpy (bs->exact_filename, bs->file->bs->exact_filename); }/* Reconstructing the full options QDict is simple for most format block * drivers, as long as the full options are known for the underlying * file BDS. The full options QDict of that file BDS should somehow * contain a representation of the filename, therefore the following * suffices without querying the (exact_)filename of this BDS. */ -if (bs->file->full_open_options) { +if (bs->file->bs->full_open_options) {qdict_put_obj (opts,"driver" ,QOBJECT (qstring_from_str (drv->format_name))); -QINCREF (bs->file->full_open_options); -qdict_put_obj (opts,"file" ,QOBJECT (bs->file->full_open_options)); +QINCREF (bs->file->bs->full_open_options); +qdict_put_obj (opts,"file" , +QOBJECT (bs->file->bs->full_open_options)); bs->full_open_options = opts; }else {
block/blkdebug.c +17 -15
@@ -427,10 +427,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, s->state =1 ;/* Open the backing file */ -assert (bs->file ==NULL ); - ret =bdrv_open_image (&bs->file,qemu_opt_get (opts,"x-image" ), options,"image" , - bs, &child_file, false, &local_err); -if (ret <0 ) { + bs->file =bdrv_open_child (qemu_opt_get (opts,"x-image" ), options,"image" , + bs, &child_file, false, &local_err); +if (local_err) { + ret = -EINVAL ;error_propagate (errp, local_err); goto out; }
@@ -449,7 +449,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, goto out; fail_unref: -bdrv_unref (bs->file); +bdrv_unref (bs->file->bs); out:qemu_opts_del (opts);return ret;
@@ -510,7 +510,8 @@ static BlockAIOCB *blkdebug_aio_readv(BlockDriverState *bs,return inject_error (bs, cb, opaque, rule); } -return bdrv_aio_readv (bs->file, sector_num, qiov, nb_sectors, cb, opaque); +return bdrv_aio_readv (bs->file->bs, sector_num, qiov, nb_sectors, + cb, opaque); } static BlockAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
@@ -532,7 +533,8 @@ static BlockAIOCB *blkdebug_aio_writev(BlockDriverState *bs,return inject_error (bs, cb, opaque, rule); } -return bdrv_aio_writev (bs->file, sector_num, qiov, nb_sectors, cb, opaque); +return bdrv_aio_writev (bs->file->bs, sector_num, qiov, nb_sectors, + cb, opaque); } static BlockAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
@@ -551,7 +553,7 @@ static BlockAIOCB *blkdebug_aio_flush(BlockDriverState *bs,return inject_error (bs, cb, opaque, rule); } -return bdrv_aio_flush (bs->file, cb, opaque); +return bdrv_aio_flush (bs->file->bs, cb, opaque); }
@@ -716,12 +718,12 @@ static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)static int64_t blkdebug_getlength (BlockDriverState *bs) { -return bdrv_getlength (bs->file); +return bdrv_getlength (bs->file->bs); }static int blkdebug_truncate (BlockDriverState *bs,int64_t offset) { -return bdrv_truncate (bs->file, offset); +return bdrv_truncate (bs->file->bs, offset); }static void blkdebug_refresh_filename (BlockDriverState *bs)
@@ -741,24 +743,24 @@ static void blkdebug_refresh_filename(BlockDriverState *bs) } } -if (force_json && !bs->file->full_open_options) { +if (force_json && !bs->file->bs->full_open_options) {/* The config file cannot be recreated, so creating a plain filename * is impossible */ return ; } -if (!force_json && bs->file->exact_filename[0 ]) { +if (!force_json && bs->file->bs->exact_filename[0 ]) {snprintf (bs->exact_filename,sizeof (bs->exact_filename),"blkdebug:%s:%s" ,qdict_get_try_str (bs->options,"config" ) ?:"" , - bs->file->exact_filename); + bs->file->bs->exact_filename); } opts =qdict_new ();qdict_put_obj (opts,"driver" ,QOBJECT (qstring_from_str ("blkdebug" ))); -QINCREF (bs->file->full_open_options); -qdict_put_obj (opts,"image" ,QOBJECT (bs->file->full_open_options)); +QINCREF (bs->file->bs->full_open_options); +qdict_put_obj (opts,"image" ,QOBJECT (bs->file->bs->full_open_options));for (e =qdict_first (bs->options); e; e =qdict_next (bs->options, e)) { if (strcmp(qdict_entry_key(e), "x-image") &&
block/blkverify.c +19 -14
@@ -123,10 +123,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, }/* Open the raw file */ -assert (bs->file ==NULL ); - ret =bdrv_open_image (&bs->file,qemu_opt_get (opts,"x-raw" ), options, -"raw" , bs, &child_file, false, &local_err); -if (ret <0 ) { + bs->file =bdrv_open_child (qemu_opt_get (opts,"x-raw" ), options,"raw" , + bs, &child_file, false, &local_err); +if (local_err) { + ret = -EINVAL ;error_propagate (errp, local_err); goto fail; }
@@ -238,13 +238,13 @@ static BlockAIOCB *blkverify_aio_readv(BlockDriverState *bs, nb_sectors, cb, opaque); acb->verify = blkverify_verify_readv; - acb->buf =qemu_blockalign (bs->file, qiov->size); + acb->buf =qemu_blockalign (bs->file->bs, qiov->size);qemu_iovec_init (&acb->raw_qiov, acb->qiov->niov);qemu_iovec_clone (&acb->raw_qiov, qiov, acb->buf);bdrv_aio_readv (s->test_file->bs, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb); -bdrv_aio_readv (bs->file, sector_num, &acb->raw_qiov, nb_sectors, +bdrv_aio_readv (bs->file->bs, sector_num, &acb->raw_qiov, nb_sectors, blkverify_aio_cb, acb);return &acb->common; }
@@ -259,7 +259,7 @@ static BlockAIOCB *blkverify_aio_writev(BlockDriverState *bs,bdrv_aio_writev (s->test_file->bs, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb); -bdrv_aio_writev (bs->file, sector_num, qiov, nb_sectors, +bdrv_aio_writev (bs->file->bs, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb);return &acb->common; }
@@ -279,7 +279,7 @@ static bool blkverify_recurse_is_first_non_filter(BlockDriverState *bs, {BDRVBlkverifyState *s = bs->opaque; -bool perm =bdrv_recurse_is_first_non_filter (bs->file, candidate); +bool perm =bdrv_recurse_is_first_non_filter (bs->file->bs, candidate);if (perm) {return true;
@@ -308,15 +308,17 @@ static void blkverify_refresh_filename(BlockDriverState *bs) {BDRVBlkverifyState *s = bs->opaque; -/* bs->file has already been refreshed */ +/* bs->file->bs has already been refreshed */ bdrv_refresh_filename (s->test_file->bs); -if (bs->file->full_open_options && s->test_file->bs->full_open_options) { +if (bs->file->bs->full_open_options + && s->test_file->bs->full_open_options) + {QDict *opts =qdict_new ();qdict_put_obj (opts,"driver" ,QOBJECT (qstring_from_str ("blkverify" ))); -QINCREF (bs->file->full_open_options); -qdict_put_obj (opts,"raw" ,QOBJECT (bs->file->full_open_options)); +QINCREF (bs->file->bs->full_open_options); +qdict_put_obj (opts,"raw" ,QOBJECT (bs->file->bs->full_open_options));QINCREF (s->test_file->bs->full_open_options);qdict_put_obj (opts,"test" ,QOBJECT (s->test_file->bs->full_open_options));
@@ -324,10 +326,13 @@ static void blkverify_refresh_filename(BlockDriverState *bs) bs->full_open_options = opts; } -if (bs->file->exact_filename[0 ] && s->test_file->bs->exact_filename[0 ]) { +if (bs->file->bs->exact_filename[0 ] + && s->test_file->bs->exact_filename[0 ]) + {snprintf (bs->exact_filename,sizeof (bs->exact_filename),"blkverify:%s:%s" , - bs->file->exact_filename, s->test_file->bs->exact_filename); + bs->file->bs->exact_filename, + s->test_file->bs->exact_filename); } }
block/bochs.c +4 -4
@@ -103,7 +103,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, bs->read_only =1 ;// no write support yet - ret =bdrv_pread (bs->file,0 , &bochs,sizeof (bochs)); + ret =bdrv_pread (bs->file->bs,0 , &bochs,sizeof (bochs));if (ret <0 ) {return ret; }
@@ -137,7 +137,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,return -ENOMEM ; } - ret =bdrv_pread (bs->file,le32_to_cpu (bochs.header), s->catalog_bitmap, + ret =bdrv_pread (bs->file->bs,le32_to_cpu (bochs.header), s->catalog_bitmap, s->catalog_size *4 );if (ret <0 ) { goto fail;
@@ -206,7 +206,7 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) (s->extent_blocks + s->bitmap_blocks));/* read in bitmap for current extent */ - ret =bdrv_pread (bs->file, bitmap_offset + (extent_offset /8 ), + ret =bdrv_pread (bs->file->bs, bitmap_offset + (extent_offset /8 ), &bitmap_entry,1 );if (ret <0 ) {return ret;
@@ -229,7 +229,7 @@ static int bochs_read(BlockDriverState *bs, int64_t sector_num,if (block_offset <0 ) {return block_offset; }else if (block_offset >0 ) { - ret =bdrv_pread (bs->file, block_offset, buf,512 ); + ret =bdrv_pread (bs->file->bs, block_offset, buf,512 );if (ret <0 ) {return ret; }
block/cloop.c +5 -5
@@ -66,7 +66,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags, bs->read_only =1 ;/* read header */ - ret =bdrv_pread (bs->file,128 , &s->block_size,4 ); + ret =bdrv_pread (bs->file->bs,128 , &s->block_size,4 );if (ret <0 ) {return ret; }
@@ -92,7 +92,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,return -EINVAL ; } - ret =bdrv_pread (bs->file,128 +4 , &s->n_blocks,4 ); + ret =bdrv_pread (bs->file->bs,128 +4 , &s->n_blocks,4 );if (ret <0 ) {return ret; }
@@ -123,7 +123,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,return -ENOMEM ; } - ret =bdrv_pread (bs->file,128 +4 +4 , s->offsets, offsets_size); + ret =bdrv_pread (bs->file->bs,128 +4 +4 , s->offsets, offsets_size);if (ret <0 ) { goto fail; }
@@ -203,8 +203,8 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)int ret;uint32_t bytes = s->offsets[block_num +1 ] - s->offsets[block_num]; - ret =bdrv_pread (bs->file, s->offsets[block_num], s->compressed_block, - bytes); + ret =bdrv_pread (bs->file->bs, s->offsets[block_num], + s->compressed_block, bytes);if (ret != bytes) {return -1 ; }
block/dmg.c +10 -10
@@ -85,7 +85,7 @@ static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)uint64_t buffer;int ret; - ret =bdrv_pread (bs->file, offset, &buffer,8 ); + ret =bdrv_pread (bs->file->bs, offset, &buffer,8 );if (ret <0 ) {return ret; }
@@ -99,7 +99,7 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)uint32_t buffer;int ret; - ret =bdrv_pread (bs->file, offset, &buffer,4 ); + ret =bdrv_pread (bs->file->bs, offset, &buffer,4 );if (ret <0 ) {return ret; }
@@ -354,7 +354,7 @@ static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds, offset +=4 ; buffer =g_realloc (buffer, count); - ret =bdrv_pread (bs->file, offset, buffer, count); + ret =bdrv_pread (bs->file->bs, offset, buffer, count);if (ret <0 ) { goto fail; }
@@ -391,7 +391,7 @@ static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds, buffer =g_malloc (info_length +1 ); buffer[info_length] ='\0' ; - ret =bdrv_pread (bs->file, info_begin, buffer, info_length); + ret =bdrv_pread (bs->file->bs, info_begin, buffer, info_length);if (ret != info_length) { ret = -EINVAL ; goto fail;
@@ -446,7 +446,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, ds.max_sectors_per_chunk =1 ;/* locate the UDIF trailer */ - offset =dmg_find_koly_offset (bs->file, errp); + offset =dmg_find_koly_offset (bs->file->bs, errp);if (offset <0 ) { ret = offset; goto fail;
@@ -514,9 +514,9 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, }/* initialize zlib engine */ - s->compressed_chunk =qemu_try_blockalign (bs->file, + s->compressed_chunk =qemu_try_blockalign (bs->file->bs, ds.max_compressed_size +1 ); - s->uncompressed_chunk =qemu_try_blockalign (bs->file, + s->uncompressed_chunk =qemu_try_blockalign (bs->file->bs,512 * ds.max_sectors_per_chunk);if (s->compressed_chunk ==NULL || s->uncompressed_chunk ==NULL ) { ret = -ENOMEM ;
@@ -592,7 +592,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)case 0x80000005 : {/* zlib compressed */ /* we need to buffer, because only the chunk as whole can be * inflated. */ - ret =bdrv_pread (bs->file, s->offsets[chunk], + ret =bdrv_pread (bs->file->bs, s->offsets[chunk], s->compressed_chunk, s->lengths[chunk]);if (ret != s->lengths[chunk]) {return -1 ;
@@ -616,7 +616,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)case 0x80000006 :/* bzip2 compressed */ /* we need to buffer, because only the chunk as whole can be * inflated. */ - ret =bdrv_pread (bs->file, s->offsets[chunk], + ret =bdrv_pread (bs->file->bs, s->offsets[chunk], s->compressed_chunk, s->lengths[chunk]);if (ret != s->lengths[chunk]) {return -1 ;
@@ -641,7 +641,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)break ;#endif /* CONFIG_BZIP2 */ case 1 :/* copy */ - ret =bdrv_pread (bs->file, s->offsets[chunk], + ret =bdrv_pread (bs->file->bs, s->offsets[chunk], s->uncompressed_chunk, s->lengths[chunk]);if (ret != s->lengths[chunk]) {return -1 ;
block/io.c +25 -25
168 lines not on this page · raw message
block/parallels.c +20 -18
@@ -202,13 +202,13 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, to_allocate = (sector_num + *pnum + s->tracks -1 ) / s->tracks - idx; space = to_allocate * s->tracks; -if (s->data_end + space >bdrv_getlength (bs->file) >>BDRV_SECTOR_BITS ) { +if (s->data_end + space >bdrv_getlength (bs->file->bs) >>BDRV_SECTOR_BITS ) {int ret; space += s->prealloc_size;if (s->prealloc_mode ==PRL_PREALLOC_MODE_FALLOCATE ) { - ret =bdrv_write_zeroes (bs->file, s->data_end, space,0 ); + ret =bdrv_write_zeroes (bs->file->bs, s->data_end, space,0 ); }else { - ret =bdrv_truncate (bs->file, + ret =bdrv_truncate (bs->file->bs, (s->data_end + space) <<BDRV_SECTOR_BITS ); }if (ret <0 ) {
@@ -244,7 +244,8 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs)if (off + to_write > s->header_size) { to_write = s->header_size - off; } - ret =bdrv_pwrite (bs->file, off, (uint8_t *)s->header + off, to_write); + ret =bdrv_pwrite (bs->file->bs, off, (uint8_t *)s->header + off, + to_write);if (ret <0 ) {qemu_co_mutex_unlock (&s->lock);return ret;
@@ -303,7 +304,7 @@ static coroutine_fn int parallels_co_writev(BlockDriverState *bs,qemu_iovec_reset (&hd_qiov);qemu_iovec_concat (&hd_qiov, qiov, bytes_done, nbytes); - ret =bdrv_co_writev (bs->file, position, n, &hd_qiov); + ret =bdrv_co_writev (bs->file->bs, position, n, &hd_qiov);if (ret <0 ) {break ; }
@@ -343,7 +344,7 @@ static coroutine_fn int parallels_co_readv(BlockDriverState *bs,qemu_iovec_reset (&hd_qiov);qemu_iovec_concat (&hd_qiov, qiov, bytes_done, nbytes); - ret =bdrv_co_readv (bs->file, position, n, &hd_qiov); + ret =bdrv_co_readv (bs->file->bs, position, n, &hd_qiov);if (ret <0 ) {break ; }
@@ -369,7 +370,7 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,bool flush_bat = false;int cluster_size = s->tracks <<BDRV_SECTOR_BITS ; - size =bdrv_getlength (bs->file); + size =bdrv_getlength (bs->file->bs);if (size <0 ) { res->check_errors++;return size;
@@ -424,7 +425,7 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res, }if (flush_bat) { - ret =bdrv_pwrite_sync (bs->file,0 , s->header, s->header_size); + ret =bdrv_pwrite_sync (bs->file->bs,0 , s->header, s->header_size);if (ret <0 ) { res->check_errors++;return ret;
@@ -440,7 +441,7 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res, size - res->image_end_offset); res->leaks += count;if (fix &BDRV_FIX_LEAKS ) { - ret =bdrv_truncate (bs->file, res->image_end_offset); + ret =bdrv_truncate (bs->file->bs, res->image_end_offset);if (ret <0 ) { res->check_errors++;return ret;
@@ -546,12 +547,13 @@ static int parallels_probe(const uint8_t *buf, int buf_size,static int parallels_update_header (BlockDriverState *bs) {BDRVParallelsState *s = bs->opaque; -unsigned size =MAX (bdrv_opt_mem_align (bs->file),sizeof (ParallelsHeader)); +unsigned size =MAX (bdrv_opt_mem_align (bs->file->bs), +sizeof (ParallelsHeader));if (size > s->header_size) { size = s->header_size; } -return bdrv_pwrite_sync (bs->file,0 , s->header, size); +return bdrv_pwrite_sync (bs->file->bs,0 , s->header, size); } static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
@@ -564,7 +566,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,Error *local_err =NULL ;char *buf; - ret =bdrv_pread (bs->file,0 , &ph,sizeof (ph)); + ret =bdrv_pread (bs->file->bs,0 , &ph,sizeof (ph));if (ret <0 ) { goto fail; }
@@ -603,8 +605,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, } size =bat_entry_off (s->bat_size); - s->header_size =ROUND_UP (size,bdrv_opt_mem_align (bs->file)); - s->header =qemu_try_blockalign (bs->file, s->header_size); + s->header_size =ROUND_UP (size,bdrv_opt_mem_align (bs->file->bs)); + s->header =qemu_try_blockalign (bs->file->bs, s->header_size);if (s->header ==NULL ) { ret = -ENOMEM ; goto fail;
@@ -619,7 +621,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, s->header_size = size; } - ret =bdrv_pread (bs->file,0 , s->header, s->header_size); + ret =bdrv_pread (bs->file->bs,0 , s->header, s->header_size);if (ret <0 ) { goto fail; }
@@ -663,8 +665,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,if (local_err !=NULL ) { goto fail_options; } -if (!bdrv_has_zero_init (bs->file) || -bdrv_truncate (bs->file,bdrv_getlength (bs->file)) !=0 ) { +if (!bdrv_has_zero_init (bs->file->bs) || +bdrv_truncate (bs->file->bs,bdrv_getlength (bs->file->bs)) !=0 ) { s->prealloc_mode =PRL_PREALLOC_MODE_FALLOCATE ; }
@@ -707,7 +709,7 @@ static void parallels_close(BlockDriverState *bs) }if (bs->open_flags &BDRV_O_RDWR ) { -bdrv_truncate (bs->file, s->data_end <<BDRV_SECTOR_BITS ); +bdrv_truncate (bs->file->bs, s->data_end <<BDRV_SECTOR_BITS ); }g_free (s->bat_dirty_bmap);
block/qapi.c +1 -1
@@ -359,7 +359,7 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs,if (bs->file) { s->has_parent = true; - s->parent =bdrv_query_stats (bs->file, query_backing); + s->parent =bdrv_query_stats (bs->file->bs, query_backing); }if (query_backing && bs->backing_hd) {
block/qcow.c +22 -20
146 lines not on this page · raw message
block/qcow2-cache.c +6 -5
@@ -127,7 +127,7 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) c =g_new0 (Qcow2Cache,1 ); c->size = num_tables; c->entries =g_try_new0 (Qcow2CachedTable, num_tables); - c->table_array =qemu_try_blockalign (bs->file, + c->table_array =qemu_try_blockalign (bs->file->bs, (size_t ) num_tables * s->cluster_size);if (!c->entries || !c->table_array) {
@@ -185,7 +185,7 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i)if (c->depends) { ret =qcow2_cache_flush_dependency (bs, c); }else if (c->depends_on_flush) { - ret =bdrv_flush (bs->file); + ret =bdrv_flush (bs->file->bs);if (ret >=0 ) { c->depends_on_flush = false; }
@@ -216,7 +216,7 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i)BLKDBG_EVENT (bs->file,BLKDBG_L2_UPDATE ); } - ret =bdrv_pwrite (bs->file, c->entries[i].offset, + ret =bdrv_pwrite (bs->file->bs, c->entries[i].offset,qcow2_cache_get_table_addr (bs, c, i), s->cluster_size);if (ret <0 ) {return ret;
@@ -244,7 +244,7 @@ int qcow2_cache_flush(BlockDriverState *bs, Qcow2Cache *c) }if (result ==0 ) { - ret =bdrv_flush (bs->file); + ret =bdrv_flush (bs->file->bs);if (ret <0 ) { result = ret; }
@@ -356,7 +356,8 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,BLKDBG_EVENT (bs->file,BLKDBG_L2_LOAD ); } - ret =bdrv_pread (bs->file, offset,qcow2_cache_get_table_addr (bs, c, i), + ret =bdrv_pread (bs->file->bs, offset, +qcow2_cache_get_table_addr (bs, c, i), s->cluster_size);if (ret <0 ) {return ret;
block/qcow2-cluster.c +22 -16
@@ -72,7 +72,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,#endif new_l1_size2 =sizeof (uint64_t ) * new_l1_size; - new_l1_table =qemu_try_blockalign (bs->file, + new_l1_table =qemu_try_blockalign (bs->file->bs,align_offset (new_l1_size2,512 ));if (new_l1_table ==NULL ) {return -ENOMEM ;
@@ -105,7 +105,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,BLKDBG_EVENT (bs->file,BLKDBG_L1_GROW_WRITE_TABLE );for (i =0 ; i < s->l1_size; i++) new_l1_table[i] =cpu_to_be64 (new_l1_table[i]); - ret =bdrv_pwrite_sync (bs->file, new_l1_table_offset, new_l1_table, new_l1_size2); + ret =bdrv_pwrite_sync (bs->file->bs, new_l1_table_offset, + new_l1_table, new_l1_size2);if (ret <0 ) goto fail;for (i =0 ; i < s->l1_size; i++)
@@ -115,7 +116,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,BLKDBG_EVENT (bs->file,BLKDBG_L1_GROW_ACTIVATE_TABLE );cpu_to_be32w ((uint32_t *)data, new_l1_size);stq_be_p (data +4 , new_l1_table_offset); - ret =bdrv_pwrite_sync (bs->file, offsetof(QCowHeader , l1_size), data,sizeof (data)); + ret =bdrv_pwrite_sync (bs->file->bs, offsetof(QCowHeader , l1_size), + data,sizeof (data));if (ret <0 ) { goto fail; }
@@ -182,8 +184,9 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index) }BLKDBG_EVENT (bs->file,BLKDBG_L1_UPDATE ); - ret =bdrv_pwrite_sync (bs->file, s->l1_table_offset +8 * l1_start_index, - buf,sizeof (buf)); + ret =bdrv_pwrite_sync (bs->file->bs, + s->l1_table_offset +8 * l1_start_index, + buf,sizeof (buf));if (ret <0 ) {return ret; }
@@ -440,7 +443,8 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, }BLKDBG_EVENT (bs->file,BLKDBG_COW_WRITE ); - ret =bdrv_co_writev (bs->file, (cluster_offset >>9 ) + n_start, n, &qiov); + ret =bdrv_co_writev (bs->file->bs, (cluster_offset >>9 ) + n_start, n, + &qiov);if (ret <0 ) { goto out; }
@@ -817,7 +821,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)/* * If this was a COW, we need to decrease the refcount of the old cluster. -* Also flush bs->file to get the right order for L2 and refcount update. +* Also flush bs->file->bs to get the right order for L2 and refcount update. * * Don't discard clusters that reach a refcount of 0 (e.g. compressed * clusters), the next write will reuse them anyway.
@@ -1412,7 +1416,8 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) sector_offset = coffset &511 ; csize = nb_csectors *512 - sector_offset;BLKDBG_EVENT (bs->file,BLKDBG_READ_COMPRESSED ); - ret =bdrv_read (bs->file, coffset >>9 , s->cluster_data, nb_csectors); + ret =bdrv_read (bs->file->bs, coffset >>9 , s->cluster_data, + nb_csectors);if (ret <0 ) {return ret; }
@@ -1645,7 +1650,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,if (!is_active_l1) {/* inactive L2 tables require a buffer to be stored in when loading * them from disk */ - l2_table =qemu_try_blockalign (bs->file, s->cluster_size); + l2_table =qemu_try_blockalign (bs->file->bs, s->cluster_size);if (l2_table ==NULL ) {return -ENOMEM ; }
@@ -1679,8 +1684,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, (void **)&l2_table); }else {/* load inactive L2 tables from disk */ - ret =bdrv_read (bs->file, l2_offset /BDRV_SECTOR_SIZE , - (void *)l2_table, s->cluster_sectors); + ret =bdrv_read (bs->file->bs, l2_offset /BDRV_SECTOR_SIZE , + (void *)l2_table, s->cluster_sectors); }if (ret <0 ) { goto fail;
@@ -1754,7 +1759,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, goto fail; } - ret =bdrv_write_zeroes (bs->file, offset /BDRV_SECTOR_SIZE , + ret =bdrv_write_zeroes (bs->file->bs, offset /BDRV_SECTOR_SIZE , s->cluster_sectors,0 );if (ret <0 ) {if (!preallocated) {
@@ -1787,8 +1792,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, goto fail; } - ret =bdrv_write (bs->file, l2_offset /BDRV_SECTOR_SIZE , - (void *)l2_table, s->cluster_sectors); + ret =bdrv_write (bs->file->bs, l2_offset /BDRV_SECTOR_SIZE , + (void *)l2_table, s->cluster_sectors);if (ret <0 ) { goto fail; }
@@ -1861,8 +1866,9 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs, l1_table =g_realloc (l1_table, l1_sectors *BDRV_SECTOR_SIZE ); - ret =bdrv_read (bs->file, s->snapshots[i].l1_table_offset / -BDRV_SECTOR_SIZE , (void *)l1_table, l1_sectors); + ret =bdrv_read (bs->file->bs, + s->snapshots[i].l1_table_offset /BDRV_SECTOR_SIZE , + (void *)l1_table, l1_sectors);if (ret <0 ) { goto fail; }
block/qcow2-refcount.c +24 -21
156 lines not on this page · raw message
block/qcow2-snapshot.c +16 -14
@@ -64,7 +64,7 @@ int qcow2_read_snapshots(BlockDriverState *bs)for (i =0 ; i < s->nb_snapshots; i++) {/* Read statically sized part of the snapshot header */ offset =align_offset (offset,8 ); - ret =bdrv_pread (bs->file, offset, &h,sizeof (h)); + ret =bdrv_pread (bs->file->bs, offset, &h,sizeof (h));if (ret <0 ) { goto fail; }
@@ -83,7 +83,7 @@ int qcow2_read_snapshots(BlockDriverState *bs) name_size =be16_to_cpu (h.name_size);/* Read extra data */ - ret =bdrv_pread (bs->file, offset, &extra, + ret =bdrv_pread (bs->file->bs, offset, &extra,MIN (sizeof (extra), extra_data_size));if (ret <0 ) { goto fail;
@@ -102,7 +102,7 @@ int qcow2_read_snapshots(BlockDriverState *bs)/* Read snapshot ID */ sn->id_str =g_malloc (id_str_size +1 ); - ret =bdrv_pread (bs->file, offset, sn->id_str, id_str_size); + ret =bdrv_pread (bs->file->bs, offset, sn->id_str, id_str_size);if (ret <0 ) { goto fail; }
@@ -111,7 +111,7 @@ int qcow2_read_snapshots(BlockDriverState *bs)/* Read snapshot name */ sn->name =g_malloc (name_size +1 ); - ret =bdrv_pread (bs->file, offset, sn->name, name_size); + ret =bdrv_pread (bs->file->bs, offset, sn->name, name_size);if (ret <0 ) { goto fail; }
@@ -214,25 +214,25 @@ static int qcow2_write_snapshots(BlockDriverState *bs) h.name_size =cpu_to_be16 (name_size); offset =align_offset (offset,8 ); - ret =bdrv_pwrite (bs->file, offset, &h,sizeof (h)); + ret =bdrv_pwrite (bs->file->bs, offset, &h,sizeof (h));if (ret <0 ) { goto fail; } offset +=sizeof (h); - ret =bdrv_pwrite (bs->file, offset, &extra,sizeof (extra)); + ret =bdrv_pwrite (bs->file->bs, offset, &extra,sizeof (extra));if (ret <0 ) { goto fail; } offset +=sizeof (extra); - ret =bdrv_pwrite (bs->file, offset, sn->id_str, id_str_size); + ret =bdrv_pwrite (bs->file->bs, offset, sn->id_str, id_str_size);if (ret <0 ) { goto fail; } offset += id_str_size; - ret =bdrv_pwrite (bs->file, offset, sn->name, name_size); + ret =bdrv_pwrite (bs->file->bs, offset, sn->name, name_size);if (ret <0 ) { goto fail; }
@@ -254,7 +254,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs) header_data.nb_snapshots =cpu_to_be32 (s->nb_snapshots); header_data.snapshots_offset =cpu_to_be64 (snapshots_offset); - ret =bdrv_pwrite_sync (bs->file, offsetof(QCowHeader , nb_snapshots), + ret =bdrv_pwrite_sync (bs->file->bs, offsetof(QCowHeader , nb_snapshots), &header_data,sizeof (header_data));if (ret <0 ) { goto fail;
@@ -396,7 +396,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) goto fail; } - ret =bdrv_pwrite (bs->file, sn->l1_table_offset, l1_table, + ret =bdrv_pwrite (bs->file->bs, sn->l1_table_offset, l1_table, s->l1_size *sizeof (uint64_t ));if (ret <0 ) { goto fail;
@@ -509,7 +509,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) goto fail; } - ret =bdrv_pread (bs->file, sn->l1_table_offset, sn_l1_table, sn_l1_bytes); + ret =bdrv_pread (bs->file->bs, sn->l1_table_offset, + sn_l1_table, sn_l1_bytes);if (ret <0 ) { goto fail; }
@@ -526,7 +527,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) goto fail; } - ret =bdrv_pwrite_sync (bs->file, s->l1_table_offset, sn_l1_table, + ret =bdrv_pwrite_sync (bs->file->bs, s->l1_table_offset, sn_l1_table, cur_l1_bytes);if (ret <0 ) { goto fail;
@@ -706,13 +707,14 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,return -EFBIG ; } new_l1_bytes = sn->l1_size *sizeof (uint64_t ); - new_l1_table =qemu_try_blockalign (bs->file, + new_l1_table =qemu_try_blockalign (bs->file->bs,align_offset (new_l1_bytes,512 ));if (new_l1_table ==NULL ) {return -ENOMEM ; } - ret =bdrv_pread (bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes); + ret =bdrv_pread (bs->file->bs, sn->l1_table_offset, + new_l1_table, new_l1_bytes);if (ret <0 ) {error_setg (errp,"Failed to read l1 table for snapshot" );qemu_vfree (new_l1_table);
block/qcow2.c +32 -30
228 lines not on this page · raw message
block/qed-table.c +2 -2
@@ -63,7 +63,7 @@ static void qed_read_table(BDRVQEDState *s, uint64_t offset, QEDTable *table, read_table_cb->iov.iov_len = s->header.cluster_size * s->header.table_size,qemu_iovec_init_external (qiov, &read_table_cb->iov,1 ); -bdrv_aio_readv (s->bs->file, offset /BDRV_SECTOR_SIZE , qiov, +bdrv_aio_readv (s->bs->file->bs, offset /BDRV_SECTOR_SIZE , qiov, qiov->size /BDRV_SECTOR_SIZE , qed_read_table_cb, read_table_cb); }
@@ -152,7 +152,7 @@ static void qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table,/* Adjust for offset into table */ offset += start *sizeof (uint64_t ); -bdrv_aio_writev (s->bs->file, offset /BDRV_SECTOR_SIZE , +bdrv_aio_writev (s->bs->file->bs, offset /BDRV_SECTOR_SIZE , &write_table_cb->qiov, write_table_cb->qiov.size /BDRV_SECTOR_SIZE , qed_write_table_cb, write_table_cb);
block/qed.c +16 -16
@@ -82,7 +82,7 @@ int qed_write_header_sync(BDRVQEDState *s)int ret;qed_header_cpu_to_le (&s->header, &le); - ret =bdrv_pwrite (s->bs->file,0 , &le,sizeof (le)); + ret =bdrv_pwrite (s->bs->file->bs,0 , &le,sizeof (le));if (ret !=sizeof (le)) {return ret; }
@@ -119,7 +119,7 @@ static void qed_write_header_read_cb(void *opaque, int ret)/* Update header */ qed_header_cpu_to_le (&s->header, (QEDHeader *)write_header_cb->buf); -bdrv_aio_writev (s->bs->file,0 , &write_header_cb->qiov, +bdrv_aio_writev (s->bs->file->bs,0 , &write_header_cb->qiov, write_header_cb->nsectors, qed_write_header_cb, write_header_cb); }
@@ -152,7 +152,7 @@ static void qed_write_header(BDRVQEDState *s, BlockCompletionFunc cb, write_header_cb->iov.iov_len = len;qemu_iovec_init_external (&write_header_cb->qiov, &write_header_cb->iov,1 ); -bdrv_aio_readv (s->bs->file,0 , &write_header_cb->qiov, nsectors, +bdrv_aio_readv (s->bs->file->bs,0 , &write_header_cb->qiov, nsectors, qed_write_header_read_cb, write_header_cb); }
@@ -392,7 +392,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, s->bs = bs;QSIMPLEQ_INIT (&s->allocating_write_reqs); - ret =bdrv_pread (bs->file,0 , &le_header,sizeof (le_header)); + ret =bdrv_pread (bs->file->bs,0 , &le_header,sizeof (le_header));if (ret <0 ) {return ret; }
@@ -416,7 +416,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, }/* Round down file size to the last cluster */ - file_size =bdrv_getlength (bs->file); + file_size =bdrv_getlength (bs->file->bs);if (file_size <0 ) {return file_size; }
@@ -452,7 +452,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,return -EINVAL ; } - ret =qed_read_string (bs->file, s->header.backing_filename_offset, + ret =qed_read_string (bs->file->bs, s->header.backing_filename_offset, s->header.backing_filename_size, bs->backing_file,sizeof (bs->backing_file));if (ret <0 ) {
@@ -471,7 +471,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,* feature is no longer valid. */ if ((s->header.autoclear_features & ~QED_AUTOCLEAR_FEATURE_MASK ) !=0 && - !bdrv_is_read_only (bs->file) && !(flags &BDRV_O_INCOMING )) { + !bdrv_is_read_only (bs->file->bs) && !(flags &BDRV_O_INCOMING )) { s->header.autoclear_features &=QED_AUTOCLEAR_FEATURE_MASK ; ret =qed_write_header_sync (s);
@@ -480,7 +480,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, }/* From here on only known autoclear feature bits are valid */ -bdrv_flush (bs->file); +bdrv_flush (bs->file->bs); } s->l1_table =qed_alloc_table (s);
@@ -498,7 +498,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,* potentially inconsistent images to be opened read-only. This can * aid data recovery from an otherwise inconsistent image. */ -if (!bdrv_is_read_only (bs->file) && +if (!bdrv_is_read_only (bs->file->bs) && !(flags &BDRV_O_INCOMING )) {BdrvCheckResult result = {0 };
@@ -541,7 +541,7 @@ static void bdrv_qed_close(BlockDriverState *bs)bdrv_qed_detach_aio_context (bs);/* Ensure writes reach stable storage */ -bdrv_flush (bs->file); +bdrv_flush (bs->file->bs);/* Clean shutdown, no check required on next open */ if (s->header.features &QED_F_NEED_CHECK ) {
@@ -839,7 +839,7 @@ static void qed_copy_from_backing_file_write(void *opaque, int ret) }BLKDBG_EVENT (s->bs->file,BLKDBG_COW_WRITE ); -bdrv_aio_writev (s->bs->file, copy_cb->offset /BDRV_SECTOR_SIZE , +bdrv_aio_writev (s->bs->file->bs, copy_cb->offset /BDRV_SECTOR_SIZE , ©_cb->qiov, copy_cb->qiov.size /BDRV_SECTOR_SIZE , qed_copy_from_backing_file_cb, copy_cb); }
@@ -1055,7 +1055,7 @@ static void qed_aio_write_flush_before_l2_update(void *opaque, int ret)QEDAIOCB *acb = opaque;BDRVQEDState *s =acb_to_s (acb); -if (!bdrv_aio_flush (s->bs->file, qed_aio_write_l2_update_cb, opaque)) { +if (!bdrv_aio_flush (s->bs->file->bs, qed_aio_write_l2_update_cb, opaque)) {qed_aio_complete (acb, -EIO ); } }
@@ -1089,7 +1089,7 @@ static void qed_aio_write_main(void *opaque, int ret) }BLKDBG_EVENT (s->bs->file,BLKDBG_WRITE_AIO ); -bdrv_aio_writev (s->bs->file, offset /BDRV_SECTOR_SIZE , +bdrv_aio_writev (s->bs->file->bs, offset /BDRV_SECTOR_SIZE , &acb->cur_qiov, acb->cur_qiov.size /BDRV_SECTOR_SIZE , next_fn, acb); }
@@ -1321,7 +1321,7 @@ static void qed_aio_read_data(void *opaque, int ret, }BLKDBG_EVENT (bs->file,BLKDBG_READ_AIO ); -bdrv_aio_readv (bs->file, offset /BDRV_SECTOR_SIZE , +bdrv_aio_readv (bs->file->bs, offset /BDRV_SECTOR_SIZE , &acb->cur_qiov, acb->cur_qiov.size /BDRV_SECTOR_SIZE , qed_aio_next_io, acb);return ;
@@ -1580,7 +1580,7 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs, }/* Write new header */ - ret =bdrv_pwrite_sync (bs->file,0 , buffer, buffer_len); + ret =bdrv_pwrite_sync (bs->file->bs,0 , buffer, buffer_len);g_free (buffer);if (ret ==0 ) {memcpy (&s->header, &new_header,sizeof (new_header));
@@ -1596,7 +1596,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)bdrv_qed_close (bs); -bdrv_invalidate_cache (bs->file, &local_err); +bdrv_invalidate_cache (bs->file->bs, &local_err);if (local_err) {error_propagate (errp, local_err);return ;
block/raw_bsd.c +20 -20
@@ -52,7 +52,7 @@ static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO); -return bdrv_co_readv (bs->file, sector_num, nb_sectors, qiov); +return bdrv_co_readv (bs->file->bs, sector_num, nb_sectors, qiov); } static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -75,7 +75,7 @@ static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,return 0 ; } - buf =qemu_try_blockalign (bs->file,512 ); + buf =qemu_try_blockalign (bs->file->bs,512 );if (!buf) { ret = -ENOMEM ; goto fail;
@@ -102,7 +102,7 @@ static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num, }BLKDBG_EVENT (bs->file,BLKDBG_WRITE_AIO ); - ret =bdrv_co_writev (bs->file, sector_num, nb_sectors, qiov); + ret =bdrv_co_writev (bs->file->bs, sector_num, nb_sectors, qiov); fail:if (qiov == &local_qiov) {
@@ -125,58 +125,58 @@ static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) { - return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags); + return bdrv_co_write_zeroes(bs->file->bs, sector_num, nb_sectors, flags); } static int coroutine_fn raw_co_discard(BlockDriverState *bs,int64_t sector_num,int nb_sectors) { -return bdrv_co_discard (bs->file, sector_num, nb_sectors); +return bdrv_co_discard (bs->file->bs, sector_num, nb_sectors); }static int64_t raw_getlength (BlockDriverState *bs) { -return bdrv_getlength (bs->file); +return bdrv_getlength (bs->file->bs); }static int raw_get_info (BlockDriverState *bs,BlockDriverInfo *bdi) { -return bdrv_get_info (bs->file, bdi); +return bdrv_get_info (bs->file->bs, bdi); }static void raw_refresh_limits (BlockDriverState *bs,Error **errp) { - bs->bl = bs->file->bl; + bs->bl = bs->file->bs->bl; }static int raw_truncate (BlockDriverState *bs,int64_t offset) { -return bdrv_truncate (bs->file, offset); +return bdrv_truncate (bs->file->bs, offset); }static int raw_is_inserted (BlockDriverState *bs) { -return bdrv_is_inserted (bs->file); +return bdrv_is_inserted (bs->file->bs); }static int raw_media_changed (BlockDriverState *bs) { -return bdrv_media_changed (bs->file); +return bdrv_media_changed (bs->file->bs); }static void raw_eject (BlockDriverState *bs,bool eject_flag) { -bdrv_eject (bs->file, eject_flag); +bdrv_eject (bs->file->bs, eject_flag); }static void raw_lock_medium (BlockDriverState *bs,bool locked) { -bdrv_lock_medium (bs->file, locked); +bdrv_lock_medium (bs->file->bs, locked); }static int raw_ioctl (BlockDriverState *bs,unsigned long int req,void *buf) { -return bdrv_ioctl (bs->file, req, buf); +return bdrv_ioctl (bs->file->bs, req, buf); } static BlockAIOCB *raw_aio_ioctl(BlockDriverState *bs,
@@ -184,12 +184,12 @@ static BlockAIOCB *raw_aio_ioctl(BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque) { -return bdrv_aio_ioctl (bs->file, req, buf, cb, opaque); +return bdrv_aio_ioctl (bs->file->bs, req, buf, cb, opaque); }static int raw_has_zero_init (BlockDriverState *bs) { -return bdrv_has_zero_init (bs->file); +return bdrv_has_zero_init (bs->file->bs); }static int raw_create (const char *filename,QemuOpts *opts,Error **errp)
@@ -207,7 +207,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)static int raw_open (BlockDriverState *bs,QDict *options,int flags,Error **errp) { - bs->sg = bs->file->sg; + bs->sg = bs->file->bs->sg;if (bs->probed && !bdrv_is_read_only (bs)) { fprintf(stderr,
@@ -217,7 +217,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,"raw images, write operations on block 0 will be restricted.\n" " Specify the 'raw' format explicitly to remove the " "restrictions.\n" , - bs->file->filename); + bs->file->bs->filename); }return 0 ;
@@ -237,12 +237,12 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)static int raw_probe_blocksizes (BlockDriverState *bs,BlockSizes *bsz) { -return bdrv_probe_blocksizes (bs->file, bsz); +return bdrv_probe_blocksizes (bs->file->bs, bsz); }static int raw_probe_geometry (BlockDriverState *bs,HDGeometry *geo) { -return bdrv_probe_geometry (bs->file, geo); +return bdrv_probe_geometry (bs->file->bs, geo); }BlockDriver bdrv_raw = {
block/snapshot.c +6 -6
@@ -149,7 +149,7 @@ int bdrv_can_snapshot(BlockDriverState *bs)if (!drv->bdrv_snapshot_create) {if (bs->file !=NULL ) { -return bdrv_can_snapshot (bs->file); +return bdrv_can_snapshot (bs->file->bs); }return 0 ; }
@@ -168,7 +168,7 @@ int bdrv_snapshot_create(BlockDriverState *bs,return drv->bdrv_snapshot_create (bs, sn_info); }if (bs->file) { -return bdrv_snapshot_create (bs->file, sn_info); +return bdrv_snapshot_create (bs->file->bs, sn_info); }return -ENOTSUP ; }
@@ -188,10 +188,10 @@ int bdrv_snapshot_goto(BlockDriverState *bs,if (bs->file) { drv->bdrv_close (bs); - ret =bdrv_snapshot_goto (bs->file, snapshot_id); + ret =bdrv_snapshot_goto (bs->file->bs, snapshot_id); open_ret = drv->bdrv_open (bs,NULL , bs->open_flags,NULL );if (open_ret <0 ) { -bdrv_unref (bs->file); +bdrv_unref (bs->file->bs); bs->drv =NULL ;return open_ret; }
@@ -245,7 +245,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs,return drv->bdrv_snapshot_delete (bs, snapshot_id, name, errp); }if (bs->file) { -return bdrv_snapshot_delete (bs->file, snapshot_id, name, errp); +return bdrv_snapshot_delete (bs->file->bs, snapshot_id, name, errp); } error_setg(errp, "Block format '%s' used by device '%s' " "does not support internal snapshot deletion",
@@ -283,7 +283,7 @@ int bdrv_snapshot_list(BlockDriverState *bs,return drv->bdrv_snapshot_list (bs, psn_info); }if (bs->file) { -return bdrv_snapshot_list (bs->file, psn_info); +return bdrv_snapshot_list (bs->file->bs, psn_info); }return -ENOTSUP ; }
block/vdi.c +9 -8
@@ -399,7 +399,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,logout ("\n" ); - ret =bdrv_read (bs->file,0 , (uint8_t *)&header,1 ); + ret =bdrv_read (bs->file->bs,0 , (uint8_t *)&header,1 );if (ret <0 ) { goto fail; }
@@ -490,13 +490,14 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, bmap_size = header.blocks_in_image *sizeof (uint32_t ); bmap_size =DIV_ROUND_UP (bmap_size,SECTOR_SIZE ); - s->bmap =qemu_try_blockalign (bs->file, bmap_size *SECTOR_SIZE ); + s->bmap =qemu_try_blockalign (bs->file->bs, bmap_size *SECTOR_SIZE );if (s->bmap ==NULL ) { ret = -ENOMEM ; goto fail; } - ret =bdrv_read (bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size); + ret =bdrv_read (bs->file->bs, s->bmap_sector, (uint8_t *)s->bmap, + bmap_size);if (ret <0 ) { goto fail_free_bmap; }
@@ -585,7 +586,7 @@ static int vdi_co_read(BlockDriverState *bs,uint64_t offset = s->header.offset_data /SECTOR_SIZE + (uint64_t )bmap_entry * s->block_sectors + sector_in_block; - ret =bdrv_read (bs->file, offset, buf, n_sectors); + ret =bdrv_read (bs->file->bs, offset, buf, n_sectors); }logout ("%u sectors read\n" , n_sectors);
@@ -653,7 +654,7 @@ static int vdi_co_write(BlockDriverState *bs,* acquire the lock and thus the padded cluster is written before * the other coroutines can write to the affected area. */ qemu_co_mutex_lock (&s->write_lock); - ret =bdrv_write (bs->file, offset, block, s->block_sectors); + ret =bdrv_write (bs->file->bs, offset, block, s->block_sectors);qemu_co_mutex_unlock (&s->write_lock); }else { uint64_t offset = s->header.offset_data / SECTOR_SIZE +
@@ -669,7 +670,7 @@ static int vdi_co_write(BlockDriverState *bs,* that that write operation has returned (there may be other writes * in flight, but they do not concern this very operation). */ qemu_co_mutex_unlock (&s->write_lock); - ret =bdrv_write (bs->file, offset, buf, n_sectors); + ret =bdrv_write (bs->file->bs, offset, buf, n_sectors); } nb_sectors -= n_sectors;
@@ -694,7 +695,7 @@ static int vdi_co_write(BlockDriverState *bs,assert (VDI_IS_ALLOCATED (bmap_first)); *header = s->header;vdi_header_to_le (header); - ret =bdrv_write (bs->file,0 , block,1 ); + ret =bdrv_write (bs->file->bs,0 , block,1 );g_free (block); block =NULL ;
@@ -712,7 +713,7 @@ static int vdi_co_write(BlockDriverState *bs, base = ((uint8_t *)&s->bmap[0 ]) + bmap_first *SECTOR_SIZE ;logout ("will write %u block map sectors starting from entry %u\n" , n_sectors, bmap_first); - ret =bdrv_write (bs->file, offset, base, n_sectors); + ret =bdrv_write (bs->file->bs, offset, base, n_sectors); }return ret;
block/vhdx-log.c +13 -12
@@ -81,7 +81,7 @@ static int vhdx_log_peek_hdr(BlockDriverState *bs, VHDXLogEntries *log, offset = log->offset + read; - ret =bdrv_pread (bs->file, offset, hdr,sizeof (VHDXLogEntryHeader)); + ret =bdrv_pread (bs->file->bs, offset, hdr,sizeof (VHDXLogEntryHeader));if (ret <0 ) { goto exit; }
@@ -141,7 +141,7 @@ static int vhdx_log_read_sectors(BlockDriverState *bs, VHDXLogEntries *log, } offset = log->offset + read; - ret =bdrv_pread (bs->file, offset, buffer,VHDX_LOG_SECTOR_SIZE ); + ret =bdrv_pread (bs->file->bs, offset, buffer,VHDX_LOG_SECTOR_SIZE );if (ret <0 ) { goto exit; }
@@ -191,7 +191,8 @@ static int vhdx_log_write_sectors(BlockDriverState *bs, VHDXLogEntries *log,/* full */ break ; } - ret =bdrv_pwrite (bs->file, offset, buffer_tmp,VHDX_LOG_SECTOR_SIZE ); + ret =bdrv_pwrite (bs->file->bs, offset, buffer_tmp, +VHDX_LOG_SECTOR_SIZE );if (ret <0 ) { goto exit; }
@@ -353,7 +354,7 @@ static int vhdx_log_read_desc(BlockDriverState *bs, BDRVVHDXState *s, } desc_sectors =vhdx_compute_desc_sectors (hdr.descriptor_count); - desc_entries =qemu_try_blockalign (bs->file, + desc_entries =qemu_try_blockalign (bs->file->bs, desc_sectors *VHDX_LOG_SECTOR_SIZE );if (desc_entries ==NULL ) { ret = -ENOMEM ;
@@ -462,7 +463,7 @@ static int vhdx_log_flush_desc(BlockDriverState *bs, VHDXLogDescriptor *desc,/* count is only > 1 if we are writing zeroes */ for (i =0 ; i < count; i++) { - ret =bdrv_pwrite_sync (bs->file, file_offset, buffer, + ret =bdrv_pwrite_sync (bs->file->bs, file_offset, buffer,VHDX_LOG_SECTOR_SIZE );if (ret <0 ) { goto exit;
@@ -509,7 +510,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s,/* if the log shows a FlushedFileOffset larger than our current file * size, then that means the file has been truncated / corrupted, and * we must refused to open it / use it */ -if (hdr_tmp.flushed_file_offset >bdrv_getlength (bs->file)) { +if (hdr_tmp.flushed_file_offset >bdrv_getlength (bs->file->bs)) { ret = -EINVAL ; goto exit; }
@@ -539,12 +540,12 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, goto exit; } } -if (bdrv_getlength (bs->file) < desc_entries->hdr.last_file_offset) { +if (bdrv_getlength (bs->file->bs) < desc_entries->hdr.last_file_offset) { new_file_size = desc_entries->hdr.last_file_offset;if (new_file_size % (1024 *1024 )) {/* round up to nearest 1MB boundary */ new_file_size = ((new_file_size >>20 ) +1 ) <<20 ; -bdrv_truncate (bs->file, new_file_size); +bdrv_truncate (bs->file->bs, new_file_size); } }qemu_vfree (desc_entries);
@@ -908,8 +909,8 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, .sequence_number = s->log.sequence, .descriptor_count = sectors, .reserved = 0, - .flushed_file_offset = bdrv_getlength(bs->file), - .last_file_offset = bdrv_getlength(bs->file), + .flushed_file_offset = bdrv_getlength(bs->file->bs), + .last_file_offset = bdrv_getlength(bs->file->bs), }; new_hdr.log_guid = header->log_guid;
@@ -940,7 +941,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s,if (i ==0 && leading_length) {/* partial sector at the front of the buffer */ - ret =bdrv_pread (bs->file, file_offset, merged_sector, + ret =bdrv_pread (bs->file->bs, file_offset, merged_sector,VHDX_LOG_SECTOR_SIZE );if (ret <0 ) { goto exit;
@@ -950,7 +951,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, sector_write = merged_sector; }else if (i == sectors -1 && trailing_length) {/* partial sector at the end of the buffer */ - ret =bdrv_pread (bs->file, + ret =bdrv_pread (bs->file->bs, file_offset, merged_sector + trailing_length,VHDX_LOG_SECTOR_SIZE - trailing_length);
block/vhdx.c +19 -17
@@ -375,7 +375,7 @@ static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s, inactive_header->log_guid = *log_guid; } - ret =vhdx_write_header (bs->file, inactive_header, header_offset, true); + ret =vhdx_write_header (bs->file->bs, inactive_header, header_offset, true);if (ret <0 ) { goto exit; }
@@ -427,7 +427,8 @@ static void vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s,/* We have to read the whole VHDX_HEADER_SIZE instead of * sizeof(VHDXHeader), because the checksum is over the whole * region */ - ret =bdrv_pread (bs->file,VHDX_HEADER1_OFFSET , buffer,VHDX_HEADER_SIZE ); + ret =bdrv_pread (bs->file->bs,VHDX_HEADER1_OFFSET , buffer, +VHDX_HEADER_SIZE );if (ret <0 ) { goto fail; }
@@ -443,7 +444,8 @@ static void vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s, } } - ret =bdrv_pread (bs->file,VHDX_HEADER2_OFFSET , buffer,VHDX_HEADER_SIZE ); + ret =bdrv_pread (bs->file->bs,VHDX_HEADER2_OFFSET , buffer, +VHDX_HEADER_SIZE );if (ret <0 ) { goto fail; }
@@ -516,7 +518,7 @@ static int vhdx_open_region_tables(BlockDriverState *bs, BDRVVHDXState *s)* whole block */ buffer =qemu_blockalign (bs,VHDX_HEADER_BLOCK_SIZE ); - ret =bdrv_pread (bs->file,VHDX_REGION_TABLE_OFFSET , buffer, + ret =bdrv_pread (bs->file->bs,VHDX_REGION_TABLE_OFFSET , buffer,VHDX_HEADER_BLOCK_SIZE );if (ret <0 ) { goto fail;
@@ -629,7 +631,7 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s) buffer =qemu_blockalign (bs,VHDX_METADATA_TABLE_MAX_SIZE ); - ret =bdrv_pread (bs->file, s->metadata_rt.file_offset, buffer, + ret =bdrv_pread (bs->file->bs, s->metadata_rt.file_offset, buffer,VHDX_METADATA_TABLE_MAX_SIZE );if (ret <0 ) { goto exit;
@@ -732,7 +734,7 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s) goto exit; } - ret = bdrv_pread(bs->file, + ret = bdrv_pread(bs->file->bs, s->metadata_entries.file_parameters_entry.offset + s->metadata_rt.file_offset, &s->params,
@@ -767,7 +769,7 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)/* determine virtual disk size, logical sector size, * and phys sector size */ - ret = bdrv_pread(bs->file, + ret = bdrv_pread(bs->file->bs, s->metadata_entries.virtual_disk_size_entry.offset + s->metadata_rt.file_offset, &s->virtual_disk_size,
@@ -775,7 +777,7 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)if (ret <0 ) { goto exit; } - ret = bdrv_pread(bs->file, + ret = bdrv_pread(bs->file->bs, s->metadata_entries.logical_sector_size_entry.offset + s->metadata_rt.file_offset, &s->logical_sector_size,
@@ -783,7 +785,7 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)if (ret <0 ) { goto exit; } - ret = bdrv_pread(bs->file, + ret = bdrv_pread(bs->file->bs, s->metadata_entries.phys_sector_size_entry.offset + s->metadata_rt.file_offset, &s->physical_sector_size,
@@ -906,7 +908,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,QLIST_INIT (&s->regions);/* validate the file signature */ - ret =bdrv_pread (bs->file,0 , &signature,sizeof (uint64_t )); + ret =bdrv_pread (bs->file->bs,0 , &signature,sizeof (uint64_t ));if (ret <0 ) { goto fail; }
@@ -959,13 +961,13 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, }/* s->bat is freed in vhdx_close() */ - s->bat =qemu_try_blockalign (bs->file, s->bat_rt.length); + s->bat =qemu_try_blockalign (bs->file->bs, s->bat_rt.length);if (s->bat ==NULL ) { ret = -ENOMEM ; goto fail; } - ret =bdrv_pread (bs->file, s->bat_offset, s->bat, s->bat_rt.length); + ret =bdrv_pread (bs->file->bs, s->bat_offset, s->bat, s->bat_rt.length);if (ret <0 ) { goto fail; }
@@ -1118,7 +1120,7 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,break ;case PAYLOAD_BLOCK_FULLY_PRESENT :qemu_co_mutex_unlock (&s->lock); - ret =bdrv_co_readv (bs->file, + ret =bdrv_co_readv (bs->file->bs, sinfo.file_offset >>BDRV_SECTOR_BITS , sinfo.sectors_avail, &hd_qiov);qemu_co_mutex_lock (&s->lock);
@@ -1156,12 +1158,12 @@ exit:static int vhdx_allocate_block (BlockDriverState *bs,BDRVVHDXState *s,uint64_t *new_offset) { - *new_offset =bdrv_getlength (bs->file); + *new_offset =bdrv_getlength (bs->file->bs);/* per the spec, the address for a block is in units of 1MB */ *new_offset =ROUND_UP (*new_offset,1024 *1024 ); -return bdrv_truncate (bs->file, *new_offset + s->block_size); +return bdrv_truncate (bs->file->bs, *new_offset + s->block_size); }/*
@@ -1260,7 +1262,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,/* Queue another write of zero buffers if the underlying file * does not zero-fill on file extension */ -if (bdrv_has_zero_init (bs->file) ==0 ) { +if (bdrv_has_zero_init (bs->file->bs) ==0 ) { use_zero_buffers = true;/* zero fill the front, if any */
@@ -1327,7 +1329,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num, }/* block exists, so we can just overwrite it */ qemu_co_mutex_unlock (&s->lock); - ret =bdrv_co_writev (bs->file, + ret =bdrv_co_writev (bs->file->bs, sinfo.file_offset >>BDRV_SECTOR_BITS , sectors_to_write, &hd_qiov);qemu_co_mutex_lock (&s->lock);
block/vmdk.c +14 -13
@@ -221,7 +221,7 @@ static void vmdk_free_extents(BlockDriverState *bs)g_free (e->l2_cache);g_free (e->l1_backup_table);g_free (e->type); -if (e->file != bs->file_child) { +if (e->file != bs->file) {bdrv_unref_child (bs, e->file); } }
@@ -248,7 +248,7 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)BDRVVmdkState *s = bs->opaque;int ret; - ret =bdrv_pread (bs->file, s->desc_offset, desc,DESC_SIZE ); + ret =bdrv_pread (bs->file->bs, s->desc_offset, desc,DESC_SIZE );if (ret <0 ) {return 0 ; }
@@ -278,7 +278,7 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)BDRVVmdkState *s = bs->opaque;int ret; - ret =bdrv_pread (bs->file, s->desc_offset, desc,DESC_SIZE ); + ret =bdrv_pread (bs->file->bs, s->desc_offset, desc,DESC_SIZE );if (ret <0 ) {return ret; }
@@ -297,7 +297,7 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)pstrcat (desc,sizeof (desc), tmp_desc); } - ret =bdrv_pwrite_sync (bs->file, s->desc_offset, desc,DESC_SIZE ); + ret =bdrv_pwrite_sync (bs->file->bs, s->desc_offset, desc,DESC_SIZE );if (ret <0 ) {return ret; }
@@ -340,7 +340,7 @@ static int vmdk_parent_open(BlockDriverState *bs)int ret; desc[DESC_SIZE ] ='\0' ; - ret =bdrv_pread (bs->file, s->desc_offset, desc,DESC_SIZE ); + ret =bdrv_pread (bs->file->bs, s->desc_offset, desc,DESC_SIZE );if (ret <0 ) {return ret; }
@@ -621,7 +621,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, }QEMU_PACKED footer; ret =bdrv_pread (file->bs, - bs->file->total_sectors *512 -1536 , + bs->file->bs->total_sectors *512 -1536 , &footer,sizeof (footer));if (ret <0 ) {error_setg_errno (errp, -ret,"Failed to read footer" );
@@ -819,7 +819,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, !desc_file_path[0]) {error_setg (errp,"Cannot use relative extent paths with VMDK " -"descriptor file '%s'" , bs->file->filename); +"descriptor file '%s'" , bs->file->bs->filename);return -EINVAL ; }
@@ -905,7 +905,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, } s->create_type =g_strdup (ct); s->desc_offset =0 ; - ret =vmdk_parse_extents (buf, bs, bs->file->exact_filename, options, errp); + ret =vmdk_parse_extents (buf, bs, bs->file->bs->exact_filename, options, + errp); exit:return ret; }
@@ -918,7 +919,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,BDRVVmdkState *s = bs->opaque;uint32_t magic; - buf =vmdk_read_desc (bs->file,0 , errp); + buf =vmdk_read_desc (bs->file->bs,0 , errp);if (!buf) {return -EINVAL ; }
@@ -927,7 +928,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,switch (magic) {case VMDK3_MAGIC :case VMDK4_MAGIC : - ret =vmdk_open_sparse (bs, bs->file_child, flags, buf, options, + ret =vmdk_open_sparse (bs, bs->file, flags, buf, options, errp); s->desc_offset =0x200 ;break ;
@@ -1275,7 +1276,7 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,break ;case VMDK_OK : ret =BDRV_BLOCK_DATA ; -if (extent->file == bs->file_child && !extent->compressed) { +if (extent->file == bs->file && !extent->compressed) { ret |=BDRV_BLOCK_OFFSET_VALID | offset; }
@@ -2051,12 +2052,12 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs)int64_t r;BDRVVmdkState *s = bs->opaque; - ret =bdrv_get_allocated_file_size (bs->file); + ret =bdrv_get_allocated_file_size (bs->file->bs);if (ret <0 ) {return ret; }for (i =0 ; i < s->num_extents; i++) { -if (s->extents[i].file == bs->file_child) { +if (s->extents[i].file == bs->file) {continue ; } r =bdrv_get_allocated_file_size (s->extents[i].file->bs);
block/vpc.c +18 -16
@@ -172,14 +172,14 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,int disk_type =VHD_DYNAMIC ;int ret; - ret =bdrv_pread (bs->file,0 , s->footer_buf,HEADER_SIZE ); + ret =bdrv_pread (bs->file->bs,0 , s->footer_buf,HEADER_SIZE );if (ret <0 ) { goto fail; } footer = (VHDFooter *) s->footer_buf;if (strncmp (footer->creator,"conectix" ,8 )) { -int64_t offset =bdrv_getlength (bs->file); +int64_t offset =bdrv_getlength (bs->file->bs);if (offset <0 ) { ret = offset; goto fail;
@@ -189,7 +189,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, }/* If a fixed disk, the footer is found only at the end of the file */ - ret =bdrv_pread (bs->file, offset-HEADER_SIZE , s->footer_buf, + ret =bdrv_pread (bs->file->bs, offset-HEADER_SIZE , s->footer_buf,HEADER_SIZE );if (ret <0 ) { goto fail;
@@ -232,7 +232,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, }if (disk_type ==VHD_DYNAMIC ) { - ret =bdrv_pread (bs->file,be64_to_cpu (footer->data_offset), buf, + ret =bdrv_pread (bs->file->bs,be64_to_cpu (footer->data_offset), buf,HEADER_SIZE );if (ret <0 ) { goto fail;
@@ -280,7 +280,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, pagetable_size = (uint64_t ) s->max_table_entries *4 ; - s->pagetable =qemu_try_blockalign (bs->file, pagetable_size); + s->pagetable =qemu_try_blockalign (bs->file->bs, pagetable_size);if (s->pagetable ==NULL ) { ret = -ENOMEM ; goto fail;
@@ -288,7 +288,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, s->bat_offset =be64_to_cpu (dyndisk_header->table_offset); - ret =bdrv_pread (bs->file, s->bat_offset, s->pagetable, pagetable_size); + ret =bdrv_pread (bs->file->bs, s->bat_offset, s->pagetable, + pagetable_size);if (ret <0 ) { goto fail; }
@@ -308,7 +309,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, } } -if (s->free_data_block_offset >bdrv_getlength (bs->file)) { +if (s->free_data_block_offset >bdrv_getlength (bs->file->bs)) {error_setg (errp,"block-vpc: free_data_block_offset points after " "the end of file. The image has been truncated." ); ret = -EINVAL ;
@@ -383,7 +384,7 @@ static inline int64_t get_sector_offset(BlockDriverState *bs, s->last_bitmap_offset = bitmap_offset;memset (bitmap,0xff , s->bitmap_size); -bdrv_pwrite_sync (bs->file, bitmap_offset, bitmap, s->bitmap_size); +bdrv_pwrite_sync (bs->file->bs, bitmap_offset, bitmap, s->bitmap_size); }return block_offset;
@@ -401,7 +402,7 @@ static int rewrite_footer(BlockDriverState* bs)BDRVVPCState *s = bs->opaque;int64_t offset = s->free_data_block_offset; - ret =bdrv_pwrite_sync (bs->file, offset, s->footer_buf,HEADER_SIZE ); + ret =bdrv_pwrite_sync (bs->file->bs, offset, s->footer_buf,HEADER_SIZE );if (ret <0 )return ret;
@@ -436,7 +437,7 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)// Initialize the block's bitmap memset (bitmap,0xff , s->bitmap_size); - ret =bdrv_pwrite_sync (bs->file, s->free_data_block_offset, bitmap, + ret =bdrv_pwrite_sync (bs->file->bs, s->free_data_block_offset, bitmap, s->bitmap_size);if (ret <0 ) {return ret;
@@ -451,7 +452,7 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)// Write BAT entry to disk bat_offset = s->bat_offset + (4 * index); bat_value =cpu_to_be32 (s->pagetable[index]); - ret =bdrv_pwrite_sync (bs->file, bat_offset, &bat_value,4 ); + ret =bdrv_pwrite_sync (bs->file->bs, bat_offset, &bat_value,4 );if (ret <0 ) goto fail;
@@ -485,7 +486,7 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,VHDFooter *footer = (VHDFooter *) s->footer_buf;if (be32_to_cpu (footer->type) ==VHD_FIXED ) { -return bdrv_read (bs->file, sector_num, buf, nb_sectors); +return bdrv_read (bs->file->bs, sector_num, buf, nb_sectors); }while (nb_sectors >0 ) { offset =get_sector_offset (bs, sector_num,0 );
@@ -499,7 +500,7 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,if (offset ==-1 ) {memset (buf,0 , sectors *BDRV_SECTOR_SIZE ); }else { - ret =bdrv_pread (bs->file, offset, buf, + ret =bdrv_pread (bs->file->bs, offset, buf, sectors *BDRV_SECTOR_SIZE );if (ret != sectors *BDRV_SECTOR_SIZE ) {return -1 ;
@@ -534,7 +535,7 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,VHDFooter *footer = (VHDFooter *) s->footer_buf;if (be32_to_cpu (footer->type) ==VHD_FIXED ) { -return bdrv_write (bs->file, sector_num, buf, nb_sectors); +return bdrv_write (bs->file->bs, sector_num, buf, nb_sectors); }while (nb_sectors >0 ) { offset =get_sector_offset (bs, sector_num,1 );
@@ -551,7 +552,8 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,return -1 ; } - ret =bdrv_pwrite (bs->file, offset, buf, sectors *BDRV_SECTOR_SIZE ); + ret =bdrv_pwrite (bs->file->bs, offset, buf, + sectors *BDRV_SECTOR_SIZE );if (ret != sectors *BDRV_SECTOR_SIZE ) {return -1 ; }
@@ -878,7 +880,7 @@ static int vpc_has_zero_init(BlockDriverState *bs)VHDFooter *footer = (VHDFooter *) s->footer_buf;if (be32_to_cpu (footer->type) ==VHD_FIXED ) { -return bdrv_has_zero_init (bs->file); +return bdrv_has_zero_init (bs->file->bs); }else {return 1 ; }
include/block/block.h +7 -1
@@ -582,7 +582,13 @@ typedef enum {BLKDBG_EVENT_MAX , } BlkDebugEvent; -#define BLKDBG_EVENT (bs, evt) bdrv_debug_event(bs, evt) +#define BLKDBG_EVENT (child, evt) \ + do { \ + if (child) { \ + bdrv_debug_event(child->bs, evt); \ + } \ + } while(0) +void bdrv_debug_event (BlockDriverState *bs,BlkDebugEvent event); int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
include/block/block_int.h +1 -2
@@ -380,8 +380,7 @@ struct BlockDriverState {BlockDriverState *backing_hd;BdrvChild *backing_child; -BlockDriverState *file; -BdrvChild *file_child; +BdrvChild *file;NotifierList close_notifiers;
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild
Quoted 24 lines · Kevin Wolf
This patch removes the temporary duplication between
bs->fileandbs->file_childby converting everything to BdrvChild.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 61 ++++++++++++++++++++++------------------------ block/blkdebug.c | 32 ++++++++++++------------ block/blkverify.c | 33 ++++++++++++++----------- block/bochs.c | 8 +++--- block/cloop.c | 10 ++++---- block/dmg.c | 20 +++++++-------- block/io.c | 50 +++++++++++++++++++------------------- block/parallels.c | 38 +++++++++++++++-------------- block/qapi.c | 2 +- block/qcow.c | 42 +++++++++++++++++--------------- block/qcow2-cache.c | 11 +++++---- block/qcow2-cluster.c | 38 +++++++++++++++++------------ block/qcow2-refcount.c | 45 ++++++++++++++++++---------------- block/qcow2-snapshot.c | 30 ++++++++++++----------- block/qcow2.c | 62 ++++++++++++++++++++++++----------------------- block/qed-table.c | 4 +-- block/qed.c | 32 ++++++++++++------------ block/raw_bsd.c | 40 +++++++++++++++--------------- block/snapshot.c | 12 ++++-----
Quoted 8 lines
block/vdi.c | 17 +++++++------ block/vhdx-log.c | 25 ++++++++++--------- block/vhdx.c | 36 ++++++++++++++------------- block/vmdk.c | 27 +++++++++++---------- block/vpc.c | 34 ++++++++++++++------------ include/block/block.h | 8 +++++- include/block/block_int.h | 3 +-- 26 files changed, 377 insertions(+), 343 deletions(-)
Quoted 4 lines
block.c +0
-0
[...]
block.c void bdrv_close(BlockDriverState *bs) · line 1929
Quoted 6 lines
block.c +2
-0@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)bdrv_unref (backing_hd); } +if (bs->file !=NULL ) { +bdrv_unref (bs->file->bs);
I think we can use bdrv_unref_child(bs->file) here. I'd personally like it more, at least (because using bdrv_unref() and relying on the loop below is basically what the comment inside of the loop advises against).
Yes, I know, eventually, we want to drop this block anyway and let the loop below handle everything using bdrv_unref_child(). But when we do that conversion, it'll be obvious to drop a bdrv_unref_child() call, whereas dropping bdrv_unref() alone isn't so obvious.
Max
Quoted 18 lines
block.c +3
-5+ bs->file =NULL ; + } + QLIST_FOREACH_SAFE(child, &bs->children, next, next) {/* TODO Remove bdrv_unref() from drivers' close function and use * bdrv_unref_child() here */ @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs) bs->options =NULL ;QDECREF (bs->full_open_options); bs->full_open_options =NULL ; - -if (bs->file !=NULL ) { -bdrv_unref (bs->file); - bs->file =NULL ; - } }if (bs->blk) {
Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild
Am 22.09.2015 um 20:36 hat Max Reitz geschrieben:
Quoted 54 lines · Kevin Wolf
Quoted 24 lines
This patch removes the temporary duplication between
bs->fileandbs->file_childby converting everything to BdrvChild.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 61 ++++++++++++++++++++++------------------------ block/blkdebug.c | 32 ++++++++++++------------ block/blkverify.c | 33 ++++++++++++++----------- block/bochs.c | 8 +++--- block/cloop.c | 10 ++++---- block/dmg.c | 20 +++++++-------- block/io.c | 50 +++++++++++++++++++------------------- block/parallels.c | 38 +++++++++++++++-------------- block/qapi.c | 2 +- block/qcow.c | 42 +++++++++++++++++--------------- block/qcow2-cache.c | 11 +++++---- block/qcow2-cluster.c | 38 +++++++++++++++++------------ block/qcow2-refcount.c | 45 ++++++++++++++++++---------------- block/qcow2-snapshot.c | 30 ++++++++++++----------- block/qcow2.c | 62 ++++++++++++++++++++++++----------------------- block/qed-table.c | 4 +-- block/qed.c | 32 ++++++++++++------------ block/raw_bsd.c | 40 +++++++++++++++--------------- block/snapshot.c | 12 ++++-----Quoted 8 lines
block/vdi.c | 17 +++++++------ block/vhdx-log.c | 25 ++++++++++--------- block/vhdx.c | 36 ++++++++++++++------------- block/vmdk.c | 27 +++++++++++---------- block/vpc.c | 34 ++++++++++++++------------ include/block/block.h | 8 +++++- include/block/block_int.h | 3 +-- 26 files changed, 377 insertions(+), 343 deletions(-)Reviewed-by Max Reitz <[email protected]>Quoted 4 lines
block.c +0
-0[...]
block.c void bdrv_close(BlockDriverState *bs) · line 1929
Quoted 6 lines
block.c +2
-0@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)bdrv_unref (backing_hd); } +if (bs->file !=NULL ) { +bdrv_unref (bs->file->bs);I think we can use
bdrv_unref_child(bs->file)here. I'd personally like it more, at least (because usingbdrv_unref()and relying on the loop below is basically what the comment inside of the loop advises against).
Quoted 4 lines
Yes, I know, eventually, we want to drop this block anyway and let the loop below handle everything using
bdrv_unref_child(). But when we do that conversion, it'll be obvious to drop abdrv_unref_child()call, whereas droppingbdrv_unref()alone isn't so obvious.
Seems safe to do, and reads a bit nicer, so I'll change the patch accordingly. Thanks.
Kevin
Quoted 19 lines
Quoted 18 lines
block.c +3
-5+ bs->file =NULL ; + } + QLIST_FOREACH_SAFE(child, &bs->children, next, next) {/* TODO Remove bdrv_unref() from drivers' close function and use * bdrv_unref_child() here */ @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs) bs->options =NULL ;QDECREF (bs->full_open_options); bs->full_open_options =NULL ; - -if (bs->file !=NULL ) { -bdrv_unref (bs->file); - bs->file =NULL ; - } }if (bs->blk) {
Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild
In reply to Kevin Wolfblock.c void bdrv_close(BlockDriverState *bs) · line 1932
Quoted 9 lines · Kevin Wolf
block.c +5
-0@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)bdrv_unref (backing_hd); } +if (bs->file !=NULL ) { +bdrv_unref (bs->file->bs); + bs->file =NULL ; + } +
With Max's suggestion of using bdrv_unref_child(bs->file) instead,
Berto
Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild
In reply to Kevin WolfQuoted 13 lines · Kevin Wolf
block.c +5
-0@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)bdrv_unref (backing_hd); } +if (bs->file !=NULL ) { +bdrv_unref (bs->file->bs); + bs->file =NULL ; + } + QLIST_FOREACH_SAFE(child, &bs->children, next, next) {/* TODO Remove bdrv_unref() from drivers' close function and use * bdrv_unref_child() here */
block.c void bdrv_close(BlockDriverState *bs) · line 1957
Quoted 9 lines
block.c +0
-5@@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs) bs->options =NULL ;QDECREF (bs->full_open_options); bs->full_open_options =NULL ; - -if (bs->file !=NULL ) { -bdrv_unref (bs->file); - bs->file =NULL ; - } }
You are moving bdrv_unref(bs->file) up in the function and this seems to be causing a problem, by turning this:
bs->drv->bdrv_close(bs);
bs->drv = NULL;
bdrv_unref(bs->file);into this:
bs->drv->bdrv_close(bs);
bdrv_unref(bs->file);
bs->drv = NULL;In the latter case, closing bs->file calls aio_poll(). This can trigger new requests on bs, which at that point has a valid pointer to a BlockDriver that has already been closed. If throttling is enabled on bs those requests might be queued for later. At the point in which those requests are scheduled bs->drv is already NULL, crashing QEMU. I can reproduce this easily with x-data-plane=on.
I sent a separate patch that moves the bdrv_io_limits_disable() call to the beginning of bdrv_close(). That solves the crash, but I guess that this patch should also be changed.
Berto
PATCH 6/16 block: Remove bdrv_open_image()
In reply to Kevin WolfIt is unused now.
--- block.c | 34 ---------------------------------- include/block/block.h | 4 ---- 2 files changed, 38 deletions(-)
block.c +0 -34
@@ -1279,40 +1279,6 @@ done:return c; } -/* -* This is a version of bdrv_open_child() that returns 0/-EINVAL instead of -* a BdrvChild object. -* -* If allow_none is true, no image will be opened if filename is false and no -* BlockdevRef is given. *pbs will remain unchanged and 0 will be returned. -* -* To conform with the behavior of bdrv_open(), *pbs has to be NULL. -*/ -int bdrv_open_image (BlockDriverState **pbs,const char *filename, -QDict *options,const char *bdref_key, -BlockDriverState * parent,const BdrvChildRole *child_role, -bool allow_none,Error **errp) -{ -Error *local_err =NULL ; -BdrvChild *c; - -assert (pbs); -assert (*pbs ==NULL ); - - c =bdrv_open_child (filename, options, bdref_key, parent, child_role, - allow_none, &local_err); -if (local_err) { -error_propagate (errp, local_err); -return -EINVAL ; - } - -if (c !=NULL ) { - *pbs = c->bs; - } - -return 0 ; -} -int bdrv_append_temp_snapshot (BlockDriverState *bs,int flags,Error **errp) {/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
include/block/block.h +0 -4
@@ -204,10 +204,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top);int bdrv_parse_cache_flags (const char *mode,int *flags);int bdrv_parse_discard_flags (const char *mode,int *flags); -int bdrv_open_image (BlockDriverState **pbs,const char *filename, -QDict *options,const char *bdref_key, -BlockDriverState * parent,const BdrvChildRole *child_role, -bool allow_none,Error **errp); BdrvChild *bdrv_open_child(const char *filename, QDict *options, const char *bdref_key, BlockDriverState* parent,
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 06/16] block: Remove bdrv_open_image()
Quoted 7 lines · Kevin Wolf
It is unused now.
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 34 ---------------------------------- include/block/block.h | 4 ---- 2 files changed, 38 deletions(-)
Re: [Qemu-devel] [PATCH 06/16] block: Remove bdrv_open_image()
In reply to Kevin WolfQuoted 3 lines · Kevin Wolf
It is unused now.
Signed-off-by Kevin Wolf <[email protected]>
Berto
PATCH 7/16 block: Convert bs->backing_hd to BdrvChild
In reply to Kevin WolfThis is the final step in converting all of the BlockDriverState pointers that block drivers use to BdrvChild.
After this patch, bs->children contains the full list of child nodes that are referenced by a given BDS, and these children are only referenced through BdrvChild, so that updating the pointer in there is enough for changing edges in the graph.
--- block.c | 116 +++++++++++++++++++++++++--------------------- block/io.c | 24 +++++----- block/mirror.c | 7 +-- block/qapi.c | 8 ++-- block/qcow.c | 4 +- block/qcow2-cluster.c | 4 +- block/qcow2.c | 6 +-- block/qed.c | 12 ++--- block/stream.c | 10 ++-- block/vmdk.c | 21 +++++---- block/vvfat.c | 6 +-- blockdev.c | 6 +-- include/block/block_int.h | 12 +++-- qemu-img.c | 8 ++-- 14 files changed, 130 insertions(+), 114 deletions(-)
block.c +63 -53
@@ -721,7 +721,7 @@ const BdrvChildRole child_format = { };/* -* Returns the flags that bs->backing_hd should get, based on the given flags +* Returns the flags that bs->backing should get, based on the given flags * for the parent BDS */ static int bdrv_backing_flags (int flags)
@@ -1115,32 +1115,31 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)void bdrv_set_backing_hd (BlockDriverState *bs,BlockDriverState *backing_hd) { -if (bs->backing_hd) { +if (bs->backing) {assert (bs->backing_blocker); -bdrv_op_unblock_all (bs->backing_hd, bs->backing_blocker); -bdrv_detach_child (bs->backing_child); +bdrv_op_unblock_all (bs->backing->bs, bs->backing_blocker); +bdrv_detach_child (bs->backing); }else if (backing_hd) {error_setg (&bs->backing_blocker,"node is used as backing hd of '%s'" ,bdrv_get_device_or_node_name (bs)); } - bs->backing_hd = backing_hd;if (!backing_hd) {error_free (bs->backing_blocker); bs->backing_blocker =NULL ; - bs->backing_child =NULL ; + bs->backing =NULL ; goto out; } - bs->backing_child =bdrv_attach_child (bs, backing_hd, &child_backing); + bs->backing =bdrv_attach_child (bs, backing_hd, &child_backing); bs->open_flags &= ~BDRV_O_NO_BACKING ;pstrcpy (bs->backing_file,sizeof (bs->backing_file), backing_hd->filename);pstrcpy (bs->backing_format,sizeof (bs->backing_format), backing_hd->drv ? backing_hd->drv->format_name :"" ); -bdrv_op_block_all (bs->backing_hd, bs->backing_blocker); +bdrv_op_block_all (backing_hd, bs->backing_blocker);/* Otherwise we won't be able to commit due to check in bdrv_commit */ -bdrv_op_unblock (bs->backing_hd,BLOCK_OP_TYPE_COMMIT_TARGET , +bdrv_op_unblock (backing_hd,BLOCK_OP_TYPE_COMMIT_TARGET , bs->backing_blocker); out:bdrv_refresh_limits (bs,NULL );
@@ -1161,7 +1160,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)BlockDriverState *backing_hd;Error *local_err =NULL ; -if (bs->backing_hd !=NULL ) { +if (bs->backing !=NULL ) {QDECREF (options); goto free_exit; }
@@ -1201,7 +1200,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)qdict_put (options,"driver" ,qstring_from_str (bs->backing_format)); } -assert (bs->backing_hd ==NULL ); +assert (bs->backing ==NULL ); ret =bdrv_open_inherit (&backing_hd, *backing_filename ? backing_filename :NULL ,NULL , options,0 , bs, &child_backing, &local_err);
@@ -1885,8 +1884,8 @@ void bdrv_close(BlockDriverState *bs) bs->drv->bdrv_close (bs); -if (bs->backing_hd) { -BlockDriverState *backing_hd = bs->backing_hd; +if (bs->backing) { +BlockDriverState *backing_hd = bs->backing->bs;bdrv_set_backing_hd (bs,NULL );bdrv_unref (backing_hd); }
@@ -2204,20 +2203,20 @@ int bdrv_commit(BlockDriverState *bs)if (!drv)return -ENOMEDIUM ; -if (!bs->backing_hd) { +if (!bs->backing) {return -ENOTSUP ; }if (bdrv_op_is_blocked (bs,BLOCK_OP_TYPE_COMMIT_SOURCE ,NULL ) || -bdrv_op_is_blocked (bs->backing_hd,BLOCK_OP_TYPE_COMMIT_TARGET ,NULL )) { +bdrv_op_is_blocked (bs->backing->bs,BLOCK_OP_TYPE_COMMIT_TARGET ,NULL )) {return -EBUSY ; } - ro = bs->backing_hd->read_only; - open_flags = bs->backing_hd->open_flags; + ro = bs->backing->bs->read_only; + open_flags = bs->backing->bs->open_flags;if (ro) { -if (bdrv_reopen (bs->backing_hd, open_flags |BDRV_O_RDWR ,NULL )) { +if (bdrv_reopen (bs->backing->bs, open_flags |BDRV_O_RDWR ,NULL )) {return -EACCES ; } }
@@ -2228,7 +2227,7 @@ int bdrv_commit(BlockDriverState *bs) goto ro_cleanup; } - backing_length =bdrv_getlength (bs->backing_hd); + backing_length =bdrv_getlength (bs->backing->bs);if (backing_length <0 ) { ret = backing_length; goto ro_cleanup;
@@ -2238,7 +2237,7 @@ int bdrv_commit(BlockDriverState *bs)* grow the backing file image if possible. If not possible, * we must return an error */ if (length > backing_length) { - ret =bdrv_truncate (bs->backing_hd, length); + ret =bdrv_truncate (bs->backing->bs, length);if (ret <0 ) { goto ro_cleanup; }
@@ -2247,7 +2246,7 @@ int bdrv_commit(BlockDriverState *bs) total_sectors = length >>BDRV_SECTOR_BITS ;/* qemu_try_blockalign() for bs will choose an alignment that works for -* bs->backing_hd as well, so no need to compare the alignment manually. */ +* bs->backing->bs as well, so no need to compare the alignment manually. */ buf =qemu_try_blockalign (bs,COMMIT_BUF_SECTORS *BDRV_SECTOR_SIZE );if (buf ==NULL ) { ret = -ENOMEM ;
@@ -2265,7 +2264,7 @@ int bdrv_commit(BlockDriverState *bs) goto ro_cleanup; } - ret =bdrv_write (bs->backing_hd, sector, buf, n); + ret =bdrv_write (bs->backing->bs, sector, buf, n);if (ret <0 ) { goto ro_cleanup; }
@@ -2284,8 +2283,8 @@ int bdrv_commit(BlockDriverState *bs)* Make sure all data we wrote to the backing device is actually * stable on disk. */ -if (bs->backing_hd) { -bdrv_flush (bs->backing_hd); +if (bs->backing) { +bdrv_flush (bs->backing->bs); } ret =0 ;
@@ -2294,7 +2293,7 @@ ro_cleanup:if (ro) {/* ignoring error return here */ -bdrv_reopen (bs->backing_hd, open_flags & ~BDRV_O_RDWR ,NULL ); +bdrv_reopen (bs->backing->bs, open_flags & ~BDRV_O_RDWR ,NULL ); }return ret;
@@ -2308,7 +2307,7 @@ int bdrv_commit_all(void)AioContext *aio_context =bdrv_get_aio_context (bs);aio_context_acquire (aio_context); -if (bs->drv && bs->backing_hd) { +if (bs->drv && bs->backing) {int ret =bdrv_commit (bs);if (ret <0 ) {aio_context_release (aio_context);
@@ -2365,11 +2364,20 @@ int bdrv_change_backing_file(BlockDriverState *bs,BlockDriverState *bdrv_find_overlay (BlockDriverState *active,BlockDriverState *bs) { -while (active && bs != active->backing_hd) { - active = active->backing_hd; +while (active) { +if (active->backing) { +if (bs == active->backing->bs) { +return active; + } + active = active->backing->bs; + }else if (bs ==NULL ) { +return active; + }else { +return NULL ; + } } -return active; +return NULL ; }/* Given a BDS, searches for the base layer. */
@@ -2436,9 +2444,9 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, goto exit; } -/* special case of new_top_bs->backing_hd already pointing to base - nothing +/* special case of new_top_bs->backing->bs already pointing to base - nothing * to do, no intermediate images */ -if (new_top_bs->backing_hd == base) { +if (backing_bs (new_top_bs) == base) { ret =0 ; goto exit; }
@@ -2453,11 +2461,11 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, intermediate_state->bs = intermediate;QSIMPLEQ_INSERT_TAIL (&states_to_delete, intermediate_state, entry); -if (intermediate->backing_hd == base) { - base_bs = intermediate->backing_hd; +if (backing_bs (intermediate) == base) { + base_bs =backing_bs (intermediate);break ; } - intermediate = intermediate->backing_hd; + intermediate =backing_bs (intermediate); }if (base_bs ==NULL ) {/* something went wrong, we did not end at the base. safely
@@ -2676,25 +2684,27 @@ void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)int bdrv_is_encrypted (BlockDriverState *bs) { -if (bs->backing_hd && bs->backing_hd->encrypted) +if (bs->backing && bs->backing->bs->encrypted) {return 1 ; + }return bs->encrypted; }int bdrv_key_required (BlockDriverState *bs) { -BlockDriverState *backing_hd = bs->backing_hd; +BdrvChild *backing = bs->backing; -if (backing_hd && backing_hd->encrypted && !backing_hd->valid_key) +if (backing && backing->bs->encrypted && !backing->bs->valid_key) {return 1 ; + }return (bs->encrypted && !bs->valid_key); }int bdrv_set_key (BlockDriverState *bs,const char *key) {int ret; -if (bs->backing_hd && bs->backing_hd->encrypted) { - ret =bdrv_set_key (bs->backing_hd, key); +if (bs->backing && bs->backing->bs->encrypted) { + ret =bdrv_set_key (bs->backing->bs, key);if (ret <0 )return ret;if (!bs->encrypted)
@@ -2861,7 +2871,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device,bool bdrv_chain_contains (BlockDriverState *top,BlockDriverState *base) {while (top && top != base) { - top = top->backing_hd; + top =backing_bs (top); }return top !=NULL ;
@@ -2919,7 +2929,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)/* If BS is a copy on write image, it is initialized to the contents of the base image, which may not be zeroes. */ -if (bs->backing_hd) { +if (bs->backing) {return 0 ; }if (bs->drv->bdrv_has_zero_init) {
@@ -2934,7 +2944,7 @@ bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs) {BlockDriverInfo bdi; -if (bs->backing_hd) { +if (bs->backing) {return false; }
@@ -2949,7 +2959,7 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs) {BlockDriverInfo bdi; -if (bs->backing_hd || !(bs->open_flags &BDRV_O_UNMAP )) { +if (bs->backing || !(bs->open_flags &BDRV_O_UNMAP )) {return false; }
@@ -2962,7 +2972,7 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs)const char *bdrv_get_encrypted_filename (BlockDriverState *bs) { -if (bs->backing_hd && bs->backing_hd->encrypted) +if (bs->backing && bs->backing->bs->encrypted)return bs->backing_file;else if (bs->encrypted)return bs->filename;
@@ -3087,13 +3097,13 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, is_protocol =path_has_protocol (backing_file); -for (curr_bs = bs; curr_bs->backing_hd; curr_bs = curr_bs->backing_hd) { +for (curr_bs = bs; curr_bs->backing; curr_bs = curr_bs->backing->bs) {/* If either of the filename paths is actually a protocol, then * compare unmodified paths; otherwise make paths relative */ if (is_protocol ||path_has_protocol (curr_bs->backing_file)) {if (strcmp (backing_file, curr_bs->backing_file) ==0 ) { - retval = curr_bs->backing_hd; + retval = curr_bs->backing->bs;break ; } }else {
@@ -3117,7 +3127,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, }if (strcmp (backing_file_full, filename_full) ==0 ) { - retval = curr_bs->backing_hd; + retval = curr_bs->backing->bs;break ; } }
@@ -3135,11 +3145,11 @@ int bdrv_get_backing_file_depth(BlockDriverState *bs)return 0 ; } -if (!bs->backing_hd) { +if (!bs->backing) {return 0 ; } -return 1 +bdrv_get_backing_file_depth (bs->backing_hd); +return 1 +bdrv_get_backing_file_depth (bs->backing->bs); }void bdrv_init (void )
@@ -3902,8 +3912,8 @@ void bdrv_detach_aio_context(BlockDriverState *bs)if (bs->file) {bdrv_detach_aio_context (bs->file->bs); } -if (bs->backing_hd) { -bdrv_detach_aio_context (bs->backing_hd); +if (bs->backing) { +bdrv_detach_aio_context (bs->backing->bs); } bs->aio_context =NULL ;
@@ -3920,8 +3930,8 @@ void bdrv_attach_aio_context(BlockDriverState *bs, bs->aio_context = new_context; -if (bs->backing_hd) { -bdrv_attach_aio_context (bs->backing_hd, new_context); +if (bs->backing) { +bdrv_attach_aio_context (bs->backing->bs, new_context); }if (bs->file) {bdrv_attach_aio_context (bs->file->bs, new_context);
block/io.c +12 -12
@@ -170,24 +170,24 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) bs->bl.opt_mem_alignment =getpagesize (); } -if (bs->backing_hd) { -bdrv_refresh_limits (bs->backing_hd, &local_err); +if (bs->backing) { +bdrv_refresh_limits (bs->backing->bs, &local_err);if (local_err) {error_propagate (errp, local_err);return ; } bs->bl.opt_transfer_length =MAX (bs->bl.opt_transfer_length, - bs->backing_hd->bl.opt_transfer_length); + bs->backing->bs->bl.opt_transfer_length); bs->bl.max_transfer_length =MIN_NON_ZERO (bs->bl.max_transfer_length, - bs->backing_hd->bl.max_transfer_length); + bs->backing->bs->bl.max_transfer_length); bs->bl.opt_mem_alignment =MAX (bs->bl.opt_mem_alignment, - bs->backing_hd->bl.opt_mem_alignment); + bs->backing->bs->bl.opt_mem_alignment); bs->bl.min_mem_alignment =MAX (bs->bl.min_mem_alignment, - bs->backing_hd->bl.min_mem_alignment); + bs->backing->bs->bl.min_mem_alignment); }/* Then let the driver override it */
@@ -227,7 +227,7 @@ static bool bdrv_requests_pending(BlockDriverState *bs)if (bs->file &&bdrv_requests_pending (bs->file->bs)) {return true; } -if (bs->backing_hd &&bdrv_requests_pending (bs->backing_hd)) { +if (bs->backing &&bdrv_requests_pending (bs->backing->bs)) {return true; }return false;
@@ -1495,8 +1495,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, }else {if (bdrv_unallocated_blocks_are_zero (bs)) { ret |=BDRV_BLOCK_ZERO ; - }else if (bs->backing_hd) { -BlockDriverState *bs2 = bs->backing_hd; + }else if (bs->backing) { +BlockDriverState *bs2 = bs->backing->bs;int64_t nb_sectors2 =bdrv_nb_sectors (bs2);if (nb_sectors2 >=0 && sector_num >= nb_sectors2) { ret |=BDRV_BLOCK_ZERO ;
@@ -1541,7 +1541,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,int64_t ret =0 ;assert (bs != base); -for (p = bs; p != base; p = p->backing_hd) { +for (p = bs; p != base; p =backing_bs (p)) { ret =bdrv_co_get_block_status (p, sector_num, nb_sectors, pnum);if (ret <0 || ret &BDRV_BLOCK_ALLOCATED ) {break ;
@@ -1604,7 +1604,7 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,int64_t sector_num, int nb_sectors, int *pnum) { - return bdrv_get_block_status_above(bs, bs->backing_hd, + return bdrv_get_block_status_above(bs, backing_bs(bs),sector_num ,nb_sectors ,pnum ); }
@@ -1662,7 +1662,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, n = pnum_inter; } - intermediate = intermediate->backing_hd; + intermediate = intermediate->backing ? intermediate->backing->bs :NULL ; } *pnum = n;
block/mirror.c +4 -3
@@ -371,7 +371,8 @@ static void mirror_exit(BlockJob *job, void *opaque)if (s->common.driver->job_type ==BLOCK_JOB_TYPE_COMMIT ) {/* drop the bs loop chain formed by the swap: break the loop then * trigger the unref from the top one */ -BlockDriverState *p = s->base->backing_hd; +BlockDriverState *p = s->base->backing + ? s->base->backing->bs :NULL ;bdrv_set_backing_hd (s->base,NULL );bdrv_unref (p); }
@@ -431,7 +432,7 @@ static void coroutine_fn mirror_run(void *opaque)*/ bdrv_get_backing_filename (s->target, backing_filename,sizeof (backing_filename)); -if (backing_filename[0 ] && !s->target->backing_hd) { +if (backing_filename[0 ] && !s->target->backing) { ret =bdrv_get_info (s->target, &bdi);if (ret <0 ) { goto immediate_exit;
@@ -764,7 +765,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,return ; } is_none_mode = mode ==MIRROR_SYNC_MODE_NONE ; - base = mode ==MIRROR_SYNC_MODE_TOP ? bs->backing_hd :NULL ; + base = mode ==MIRROR_SYNC_MODE_TOP && bs->backing ? bs->backing->bs :NULL ; mirror_start_job(bs, target, replaces, speed, granularity, buf_size, on_source_error, on_target_error, unmap, cb, opaque, errp,
block/qapi.c +4 -4
@@ -110,8 +110,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)qapi_free_BlockDeviceInfo (info);return NULL ; } -if (bs0->drv && bs0->backing_hd) { - bs0 = bs0->backing_hd; +if (bs0->drv && bs0->backing) { + bs0 = bs0->backing->bs; (*p_image_info)->has_backing_image = true; p_image_info = &((*p_image_info)->backing_image); }else {
@@ -362,9 +362,9 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs, s->parent =bdrv_query_stats (bs->file->bs, query_backing); } -if (query_backing && bs->backing_hd) { +if (query_backing && bs->backing) { s->has_backing = true; - s->backing =bdrv_query_stats (bs->backing_hd, query_backing); + s->backing =bdrv_query_stats (bs->backing->bs, query_backing); }return s;
block/qcow.c +2 -2
@@ -596,13 +596,13 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, }if (!cluster_offset) { -if (bs->backing_hd) { +if (bs->backing) {/* read from the base image */ hd_iov.iov_base = (void *)buf; hd_iov.iov_len = n *512 ;qemu_iovec_init_external (&hd_qiov, &hd_iov,1 );qemu_co_mutex_unlock (&s->lock); - ret =bdrv_co_readv (bs->backing_hd, sector_num, + ret =bdrv_co_readv (bs->backing->bs, sector_num, n, &hd_qiov);qemu_co_mutex_lock (&s->lock);if (ret <0 ) {
block/qcow2-cluster.c +2 -2
@@ -1474,7 +1474,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,*/ switch (qcow2_get_cluster_type (old_l2_entry)) {case QCOW2_CLUSTER_UNALLOCATED : -if (full_discard || !bs->backing_hd) { +if (full_discard || !bs->backing) {continue ; }break ;
@@ -1708,7 +1708,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, }if (!preallocated) { -if (!bs->backing_hd) { +if (!bs->backing) {/* not backed; therefore we can simply deallocate the * cluster */ l2_table[j] =0 ;
block/qcow2.c +3 -3
@@ -1369,9 +1369,9 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,switch (ret) {case QCOW2_CLUSTER_UNALLOCATED : -if (bs->backing_hd) { +if (bs->backing) {/* read from the base image */ - n1 =qcow2_backing_read1 (bs->backing_hd, &hd_qiov, + n1 =qcow2_backing_read1 (bs->backing->bs, &hd_qiov, sector_num, cur_nr_sectors);if (n1 >0 ) {QEMUIOVector local_qiov;
@@ -1382,7 +1382,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,BLKDBG_EVENT (bs->file,BLKDBG_READ_BACKING_AIO );qemu_co_mutex_unlock (&s->lock); - ret =bdrv_co_readv (bs->backing_hd, sector_num, + ret =bdrv_co_readv (bs->backing->bs, sector_num, n1, &local_qiov);qemu_co_mutex_lock (&s->lock);
block/qed.c +6 -6
@@ -772,8 +772,8 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,/* If there is a backing file, get its length. Treat the absence of a * backing file like a zero length backing file. */ -if (s->bs->backing_hd) { -int64_t l =bdrv_getlength (s->bs->backing_hd); +if (s->bs->backing) { +int64_t l =bdrv_getlength (s->bs->backing->bs);if (l <0 ) {cb (opaque, l);return ;
@@ -802,7 +802,7 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,qemu_iovec_concat (*backing_qiov, qiov,0 , size);BLKDBG_EVENT (s->bs->file,BLKDBG_READ_BACKING_AIO ); -bdrv_aio_readv (s->bs->backing_hd, pos /BDRV_SECTOR_SIZE , +bdrv_aio_readv (s->bs->backing->bs, pos /BDRV_SECTOR_SIZE , *backing_qiov, size /BDRV_SECTOR_SIZE , cb, opaque); }
@@ -1081,7 +1081,7 @@ static void qed_aio_write_main(void *opaque, int ret)if (acb->find_cluster_ret ==QED_CLUSTER_FOUND ) { next_fn = qed_aio_next_io; }else { -if (s->bs->backing_hd) { +if (s->bs->backing) { next_fn = qed_aio_write_flush_before_l2_update; }else { next_fn = qed_aio_write_l2_update_cb;
@@ -1139,7 +1139,7 @@ static void qed_aio_write_prefill(void *opaque, int ret)static bool qed_should_set_need_check (BDRVQEDState *s) {/* The flush before L2 update path ensures consistency */ -if (s->bs->backing_hd) { +if (s->bs->backing) {return false; }
@@ -1443,7 +1443,7 @@ static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs,struct iovec iov;/* Refuse if there are untouched backing file sectors */ -if (bs->backing_hd) { +if (bs->backing) {if (qed_offset_into_cluster (s, sector_num *BDRV_SECTOR_SIZE ) !=0 ) {return -ENOTSUP ; }
block/stream.c +5 -5
@@ -56,7 +56,7 @@ static void close_unused_images(BlockDriverState *top, BlockDriverState *base, const char *base_id) {BlockDriverState *intermediate; - intermediate = top->backing_hd; + intermediate = top->backing ? top->backing->bs :NULL ;/* Must assign before bdrv_delete() to prevent traversing dangling pointer * while we delete backing image instances.
@@ -72,7 +72,7 @@ static void close_unused_images(BlockDriverState *top, BlockDriverState *base, } unused = intermediate; - intermediate = intermediate->backing_hd; + intermediate = intermediate->backing ? intermediate->backing->bs :NULL ;bdrv_set_backing_hd (unused,NULL );bdrv_unref (unused); }
@@ -121,7 +121,7 @@ static void coroutine_fn stream_run(void *opaque)int n =0 ;void *buf; -if (!bs->backing_hd) { +if (!bs->backing) {block_job_completed (&s->common,0 );return ; }
@@ -166,8 +166,8 @@ wait: }else if (ret >=0 ) {/* Copy if allocated in the intermediate images. Limit to the * known-unallocated area [sector_num, sector_num+n). */ - ret =bdrv_is_allocated_above (bs->backing_hd, base, - sector_num, n, &n); + ret =bdrv_is_allocated_above (bs->backing ? bs->backing->bs :NULL , + base, sector_num, n, &n);/* Finish early if end of backing file has been reached */ if (ret ==0 && n ==0 ) {
block/vmdk.c +11 -10
@@ -308,10 +308,11 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)static int vmdk_is_cid_valid (BlockDriverState *bs) {BDRVVmdkState *s = bs->opaque; -BlockDriverState *p_bs = bs->backing_hd;uint32_t cur_pcid; -if (!s->cid_checked && p_bs) { +if (!s->cid_checked && bs->backing) { +BlockDriverState *p_bs = bs->backing->bs; + cur_pcid =vmdk_read_cid (p_bs,0 );if (s->parent_cid != cur_pcid) {/* CID not valid */
@@ -1006,7 +1007,7 @@ static int get_whole_cluster(BlockDriverState *bs, cluster_bytes = extent->cluster_sectors <<BDRV_SECTOR_BITS ; whole_grain =qemu_blockalign (bs, cluster_bytes); -if (!bs->backing_hd) { +if (!bs->backing) {memset (whole_grain,0 , skip_start_sector <<BDRV_SECTOR_BITS );memset (whole_grain + (skip_end_sector <<BDRV_SECTOR_BITS ),0 , cluster_bytes - (skip_end_sector <<BDRV_SECTOR_BITS ));
@@ -1015,15 +1016,15 @@ static int get_whole_cluster(BlockDriverState *bs,assert (skip_end_sector <= extent->cluster_sectors);/* we will be here if it's first write on non-exist grain(cluster). * try to read from parent image, if exist */ -if (bs->backing_hd && !vmdk_is_cid_valid (bs)) { +if (bs->backing && !vmdk_is_cid_valid (bs)) { ret =VMDK_ERROR ; goto exit; }/* Read backing data before skip range */ if (skip_start_sector >0 ) { -if (bs->backing_hd) { - ret =bdrv_read (bs->backing_hd, sector_num, +if (bs->backing) { + ret =bdrv_read (bs->backing->bs, sector_num, whole_grain, skip_start_sector);if (ret <0 ) { ret =VMDK_ERROR ;
@@ -1039,8 +1040,8 @@ static int get_whole_cluster(BlockDriverState *bs, }/* Read backing data after skip range */ if (skip_end_sector < extent->cluster_sectors) { -if (bs->backing_hd) { - ret =bdrv_read (bs->backing_hd, sector_num + skip_end_sector, +if (bs->backing) { + ret =bdrv_read (bs->backing->bs, sector_num + skip_end_sector, whole_grain + (skip_end_sector <<BDRV_SECTOR_BITS ), extent->cluster_sectors - skip_end_sector);if (ret <0 ) {
@@ -1433,11 +1434,11 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num, }if (ret !=VMDK_OK ) {/* if not allocated, try to read from parent image, if exist */ -if (bs->backing_hd && ret !=VMDK_ZEROED ) { +if (bs->backing && ret !=VMDK_ZEROED ) {if (!vmdk_is_cid_valid (bs)) {return -EINVAL ; } - ret =bdrv_read (bs->backing_hd, sector_num, buf, n); + ret =bdrv_read (bs->backing->bs, sector_num, buf, n);if (ret <0 ) {return ret; }
block/vvfat.c +3 -3
@@ -2972,9 +2972,9 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)#endif bdrv_set_backing_hd (s->bs,bdrv_new ()); - s->bs->backing_hd->drv = &vvfat_write_target; - s->bs->backing_hd->opaque = g_new(void *, 1); - *(void **)s->bs->backing_hd->opaque = s; + s->bs->backing->bs->drv = &vvfat_write_target; + s->bs->backing->bs->opaque = g_new(void *, 1); + *(void **)s->bs->backing->bs->opaque = s;return 0 ;
blockdev.c +3 -3
@@ -2508,10 +2508,10 @@ void qmp_drive_backup(const char *device, const char *target,/* See if we have a backing HD we can use to create our new image * on top of. */ if (sync ==MIRROR_SYNC_MODE_TOP ) { - source = bs->backing_hd; -if (!source) { +if (!bs->backing) { sync =MIRROR_SYNC_MODE_FULL ; } + source = bs->backing->bs; }if (sync ==MIRROR_SYNC_MODE_NONE ) { source = bs;
@@ -2716,7 +2716,7 @@ void qmp_drive_mirror(const char *device, const char *target, } flags = bs->open_flags |BDRV_O_RDWR ; - source = bs->backing_hd; + source = bs->backing ? bs->backing->bs :NULL ;if (!source && sync ==MIRROR_SYNC_MODE_TOP ) { sync =MIRROR_SYNC_MODE_FULL ; }
include/block/block_int.h +8 -4
@@ -378,8 +378,7 @@ struct BlockDriverState {QDict *full_open_options;char exact_filename[PATH_MAX ]; -BlockDriverState *backing_hd; -BdrvChild *backing_child; +BdrvChild *backing;BdrvChild *file;NotifierList close_notifiers;
@@ -458,6 +457,11 @@ struct BlockDriverState {NotifierWithReturn write_threshold_notifier; }; +static inline BlockDriverState *backing_bs (BlockDriverState *bs) +{ +return bs->backing ? bs->backing->bs :NULL ; +} +/* Essential block drivers which must always be statically linked into qemu, and * which therefore can be accessed without using bdrv_find_format() */
@@ -496,7 +500,7 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,* * May be called from .bdrv_detach_aio_context() to detach children from the * current #AioContext. This is only needed by block drivers that manage their -* own children. Both ->file and ->backing_hd are automatically handled and +* own children. Both ->file and ->backing are automatically handled and * block drivers should not call this function on them explicitly. */ void bdrv_detach_aio_context (BlockDriverState *bs);
@@ -506,7 +510,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs);* * May be called from .bdrv_attach_aio_context() to attach children to the new * #AioContext. This is only needed by block drivers that manage their own -* children. Both ->file and ->backing_hd are automatically handled and block +* children. Both ->file and ->backing are automatically handled and block * drivers should not call this function on them explicitly. */ void bdrv_attach_aio_context(BlockDriverState *bs,
qemu-img.c +4 -4
@@ -747,11 +747,11 @@ static int img_commit(int argc, char **argv)/* This is different from QMP, which by default uses the deepest file in * the backing chain (i.e., the very base); however, the traditional * behavior of qemu-img commit is using the immediate backing file. */ - base_bs = bs->backing_hd; -if (!base_bs) { +if (!bs->backing) {error_setg (&local_err,"Image does not have a backing file" ); goto done; } + base_bs = bs->backing->bs; } cbi = (CommonBlockJobCBInfo ){
@@ -2206,11 +2206,11 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,if (ret & (BDRV_BLOCK_ZERO |BDRV_BLOCK_DATA )) {break ; } - bs = bs->backing_hd; -if (bs ==NULL ) { +if (bs->backing ==NULL ) { ret =0 ;break ; } + bs = bs->backing->bs; depth++; }
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 07/16] block: Convert bs->backing_hd to BdrvChild
Quoted 26 lines · Kevin Wolf
This is the final step in converting all of the BlockDriverState pointers that block drivers use to BdrvChild.
After this patch,
bs->childrencontains the full list of child nodes that are referenced by a given BDS, and these children are only referenced through BdrvChild, so that updating the pointer in there is enough for changing edges in the graph.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 116 +++++++++++++++++++++++++--------------------- block/io.c | 24 +++++----- block/mirror.c | 7 +-- block/qapi.c | 8 ++-- block/qcow.c | 4 +- block/qcow2-cluster.c | 4 +- block/qcow2.c | 6 +-- block/qed.c | 12 ++--- block/stream.c | 10 ++-- block/vmdk.c | 21 +++++---- block/vvfat.c | 6 +-- blockdev.c | 6 +-- include/block/block_int.h | 12 +++-- qemu-img.c | 8 ++-- 14 files changed, 130 insertions(+), 114 deletions(-)
[...]
Quoted 4 lines
block/io.c +0
-0
[...]
Quoted 10 lines
block/io.c +1
-1@@ -1604,7 +1604,7 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,int64_t sector_num, int nb_sectors, int *pnum) { - return bdrv_get_block_status_above(bs, bs->backing_hd, + return bdrv_get_block_status_above(bs, backing_bs(bs),sector_num ,nb_sectors ,pnum ); }
block/io.c int bdrv_is_allocated_above(BlockDriverState *top, · line 1665
Quoted 5 lines
block/io.c +1
-1@@ -1662,7 +1662,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, n = pnum_inter; } - intermediate = intermediate->backing_hd; + intermediate = intermediate->backing ? intermediate->backing->bs :NULL ;
backing_bs(intermediate)?
Quoted 8 lines
block/io.c +0
-0} *pnum = n;block/mirror.c +0
-0
block/mirror.c static void mirror_exit(BlockJob *job, void *opaque) · line 375
Quoted 6 lines
block/mirror.c +2
-1@@ -371,7 +371,8 @@ static void mirror_exit(BlockJob *job, void *opaque)if (s->common.driver->job_type ==BLOCK_JOB_TYPE_COMMIT ) {/* drop the bs loop chain formed by the swap: break the loop then * trigger the unref from the top one */ -BlockDriverState *p = s->base->backing_hd; +BlockDriverState *p = s->base->backing + ? s->base->backing->bs :NULL ;
Maybe you don't want to use backing_bs() outside of the core block layer, but it could be used here, too.
(There are two similar expressions in block/stream.c, and maybe elsewhere, too)
Quoted 3 lines
block/mirror.c +0
-0bdrv_set_backing_hd (s->base,NULL );bdrv_unref (p); }
[...]
Quoted 5 lines
blockdev.c +0
-0
blockdev.c void qmp_drive_backup(const char *device, const char *target, · line 2514
Quoted 9 lines
blockdev.c +2
-2@@ -2508,10 +2508,10 @@ void qmp_drive_backup(const char *device, const char *target,/* See if we have a backing HD we can use to create our new image * on top of. */ if (sync ==MIRROR_SYNC_MODE_TOP ) { - source = bs->backing_hd; -if (!source) { +if (!bs->backing) { sync =MIRROR_SYNC_MODE_FULL ; } + source = bs->backing->bs;
That doesn't seem right. In case of !bs->backing, this won't abort but just continue and run into bs->backing->bs, which should therefore probably be backing_bs(bs) instead.
blockdev.c void qmp_drive_mirror(const char *device, const char *target, · line 2719
Quoted 9 lines
blockdev.c +1
-1}if (sync ==MIRROR_SYNC_MODE_NONE ) { source = bs;@@ -2716,7 +2716,7 @@ void qmp_drive_mirror(const char *device, const char *target, } flags = bs->open_flags |BDRV_O_RDWR ; - source = bs->backing_hd; + source = bs->backing ? bs->backing->bs :NULL ;
Why not source = backing_bs(bs)?
Quoted 3 lines
blockdev.c +0
-0if (!source && sync ==MIRROR_SYNC_MODE_TOP ) { sync =MIRROR_SYNC_MODE_FULL ; }
[...]
Quoted 4 lines
qemu-img.c +0
-0
[...]
qemu-img.c static int get_block_status(BlockDriverState *bs, int64_t sector_num, · line 2213
Quoted 11 lines
qemu-img.c +2
-2@@ -2206,11 +2206,11 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,if (ret & (BDRV_BLOCK_ZERO |BDRV_BLOCK_DATA )) {break ; } - bs = bs->backing_hd; -if (bs ==NULL ) { +if (bs->backing ==NULL ) { ret =0 ;break ; } + bs = bs->backing->bs;
This changes behavior. bs needs to be set to NULL in the
if (bs->backing == NULL) block, or the break will break: Before, if bs->backing_hd == NULL, the loop was left with bs == NULL. Now, bs won't be NULL anymore (but its value is used after the loop and stored in e->bs).
Max
Quoted 4 lines
qemu-img.c +0
-0depth++; }
PATCH 8/16 block: Manage backing file references in bdrv_set_backing_hd()
In reply to Kevin WolfThis simplifies the code somewhat, especially when dropping whole backing file subchains.
The exception is the mirroring code that does adventurous things with bdrv_swap() and in order to keep it working, I had to duplicate most of bdrv_set_backing_hd() locally. We'll get rid again of this ugliness shortly.
--- block.c | 35 ++++++++++++++--------------------- block/mirror.c | 17 ++++++++++++----- block/stream.c | 21 --------------------- block/vvfat.c | 6 +++++- include/block/block.h | 1 + 5 files changed, 32 insertions(+), 48 deletions(-)
block.c +14 -21
@@ -1094,7 +1094,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -static void bdrv_detach_child (BdrvChild *child) +void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);g_free (child);
@@ -1112,13 +1112,20 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)bdrv_unref (child_bs); } +/* +* Sets the backing file link of a BDS. A new reference is created; callers +* which don't need their own reference any more must call bdrv_unref(). +*/ void bdrv_set_backing_hd (BlockDriverState *bs,BlockDriverState *backing_hd) { +if (backing_hd) { +bdrv_ref (backing_hd); + }if (bs->backing) {assert (bs->backing_blocker);bdrv_op_unblock_all (bs->backing->bs, bs->backing_blocker); -bdrv_detach_child (bs->backing); +bdrv_unref_child (bs, bs->backing); }else if (backing_hd) { error_setg(&bs->backing_blocker, "node is used as backing hd of '%s'",
@@ -1214,7 +1221,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) goto free_exit; } +/* Hook up the backing file link; drop our reference, bs owns the +* backing_hd reference now */ bdrv_set_backing_hd (bs, backing_hd); +bdrv_unref (backing_hd); free_exit:g_free (backing_filename);
@@ -1884,11 +1894,7 @@ void bdrv_close(BlockDriverState *bs) bs->drv->bdrv_close (bs); -if (bs->backing) { -BlockDriverState *backing_hd = bs->backing->bs; -bdrv_set_backing_hd (bs,NULL ); -bdrv_unref (backing_hd); - } +bdrv_set_backing_hd (bs,NULL );if (bs->file !=NULL ) {bdrv_unref (bs->file->bs);
@@ -2427,12 +2433,9 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,BlockDriverState *intermediate;BlockDriverState *base_bs =NULL ;BlockDriverState *new_top_bs =NULL ; -BlkIntermediateStates *intermediate_state, *next; +BlkIntermediateStates *intermediate_state;int ret = -EIO ; - QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete; -QSIMPLEQ_INIT (&states_to_delete); -if (!top->drv || !base->drv) { goto exit; }
@@ -2459,7 +2462,6 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,while (intermediate) { intermediate_state =g_new0 (BlkIntermediateStates,1 ); intermediate_state->bs = intermediate; -QSIMPLEQ_INSERT_TAIL (&states_to_delete, intermediate_state, entry);if (backing_bs (intermediate) == base) { base_bs =backing_bs (intermediate);
@@ -2482,17 +2484,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, }bdrv_set_backing_hd (new_top_bs, base_bs); - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { -/* so that bdrv_close() does not recursively close the chain */ -bdrv_set_backing_hd (intermediate_state->bs,NULL ); -bdrv_unref (intermediate_state->bs); - } ret =0 ; - exit: - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { -g_free (intermediate_state); - }return ret; }
block/mirror.c +12 -5
@@ -370,11 +370,18 @@ static void mirror_exit(BlockJob *job, void *opaque)bdrv_swap (s->target, to_replace);if (s->common.driver->job_type ==BLOCK_JOB_TYPE_COMMIT ) {/* drop the bs loop chain formed by the swap: break the loop then -* trigger the unref from the top one */ -BlockDriverState *p = s->base->backing - ? s->base->backing->bs :NULL ; -bdrv_set_backing_hd (s->base,NULL ); -bdrv_unref (p); +* trigger the unref */ +/* FIXME This duplicates bdrv_set_backing_hd(), except for the +* actual detach/unref so that the loop can be broken. When +* bdrv_swap() gets replaced, this will become sane again. */ +BlockDriverState *backing = s->base->backing->bs; +assert (s->base->backing_blocker); +bdrv_op_unblock_all (s->base->backing->bs, s->base->backing_blocker); +error_free (s->base->backing_blocker); + s->base->backing_blocker =NULL ; +bdrv_detach_child (s->base->backing); + s->base->backing =NULL ; +bdrv_unref (backing); } }if (s->to_replace) {
block/stream.c +0 -21
@@ -55,28 +55,7 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,static void close_unused_images (BlockDriverState *top,BlockDriverState *base,const char *base_id) { -BlockDriverState *intermediate; - intermediate = top->backing ? top->backing->bs :NULL ; - -/* Must assign before bdrv_delete() to prevent traversing dangling pointer -* while we delete backing image instances. -*/ bdrv_set_backing_hd (top, base); - -while (intermediate) { -BlockDriverState *unused; - -/* reached base */ -if (intermediate == base) { -break ; - } - - unused = intermediate; - intermediate = intermediate->backing ? intermediate->backing->bs :NULL ; -bdrv_set_backing_hd (unused,NULL ); -bdrv_unref (unused); - } -bdrv_refresh_limits (top,NULL ); }
block/vvfat.c +5 -1
@@ -2923,6 +2923,7 @@ static BlockDriver vvfat_write_target = {static int enable_write_target (BDRVVVFATState *s,Error **errp) {BlockDriver *bdrv_qcow =NULL ; +BlockDriverState *backing;QemuOpts *opts =NULL ;int ret;int size =sector2cluster (s, s->sector_count);
@@ -2971,7 +2972,10 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)unlink (s->qcow_filename);#endif -bdrv_set_backing_hd (s->bs,bdrv_new ()); + backing =bdrv_new (); +bdrv_set_backing_hd (s->bs, backing); +bdrv_unref (backing); + s->bs->backing->bs->drv = &vvfat_write_target; s->bs->backing->bs->opaque = g_new(void *, 1); *(void **)s->bs->backing->bs->opaque = s;
include/block/block.h +1 -0
@@ -510,6 +510,7 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);void bdrv_ref (BlockDriverState *bs);void bdrv_unref (BlockDriverState *bs);void bdrv_unref_child (BlockDriverState *parent,BdrvChild *child); +void bdrv_detach_child (BdrvChild *child);bool bdrv_op_is_blocked (BlockDriverState *bs,BlockOpType op,Error **errp);void bdrv_op_block (BlockDriverState *bs,BlockOpType op,Error *reason);
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 08/16] block: Manage backing file references in bdrv_set_backing_hd()
Quoted 106 lines · Kevin Wolf
This simplifies the code somewhat, especially when dropping whole backing file subchains.
The exception is the mirroring code that does adventurous things with
bdrv_swap()and in order to keep it working, I had to duplicate most ofbdrv_set_backing_hd()locally. We'll get rid again of this ugliness shortly.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 35 ++++++++++++++--------------------- block/mirror.c | 17 ++++++++++++----- block/stream.c | 21 --------------------- block/vvfat.c | 6 +++++- include/block/block.h | 1 + 5 files changed, 32 insertions(+), 48 deletions(-)block.c +14
-16@@ -1094,7 +1094,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -static void bdrv_detach_child (BdrvChild *child) +void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);g_free (child);@@ -1112,13 +1112,20 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)bdrv_unref (child_bs); } +/* +* Sets the backing file link of a BDS. A new reference is created; callers +* which don't need their own reference any more must call bdrv_unref(). +*/ void bdrv_set_backing_hd (BlockDriverState *bs,BlockDriverState *backing_hd) { +if (backing_hd) { +bdrv_ref (backing_hd); + }if (bs->backing) {assert (bs->backing_blocker);bdrv_op_unblock_all (bs->backing->bs, bs->backing_blocker); -bdrv_detach_child (bs->backing); +bdrv_unref_child (bs, bs->backing); }else if (backing_hd) { error_setg(&bs->backing_blocker, "node is used as backing hd of '%s'",@@ -1214,7 +1221,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) goto free_exit; } +/* Hook up the backing file link; drop our reference, bs owns the +* backing_hd reference now */ bdrv_set_backing_hd (bs, backing_hd); +bdrv_unref (backing_hd); free_exit:g_free (backing_filename);@@ -1884,11 +1894,7 @@ void bdrv_close(BlockDriverState *bs) bs->drv->bdrv_close (bs); -if (bs->backing) { -BlockDriverState *backing_hd = bs->backing->bs; -bdrv_set_backing_hd (bs,NULL ); -bdrv_unref (backing_hd); - } +bdrv_set_backing_hd (bs,NULL );if (bs->file !=NULL ) {bdrv_unref (bs->file->bs);@@ -2427,12 +2433,9 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,BlockDriverState *intermediate;BlockDriverState *base_bs =NULL ;BlockDriverState *new_top_bs =NULL ; -BlkIntermediateStates *intermediate_state, *next; +BlkIntermediateStates *intermediate_state;int ret = -EIO ; - QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete; -QSIMPLEQ_INIT (&states_to_delete); -if (!top->drv || !base->drv) { goto exit; }@@ -2459,7 +2462,6 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,while (intermediate) { intermediate_state =g_new0 (BlkIntermediateStates,1 ); intermediate_state->bs = intermediate; -QSIMPLEQ_INSERT_TAIL (&states_to_delete, intermediate_state, entry);if (backing_bs (intermediate) == base) { base_bs =backing_bs (intermediate);@@ -2482,17 +2484,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, }bdrv_set_backing_hd (new_top_bs, base_bs); - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { -/* so that bdrv_close() does not recursively close the chain */ -bdrv_set_backing_hd (intermediate_state->bs,NULL ); -bdrv_unref (intermediate_state->bs);
block.c int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, · line 2491
Quoted 10 lines
block.c +0
-5- } ret =0 ; - exit: - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { -g_free (intermediate_state); - }return ret; }
To me that begs the question why before we didn't simply bdrv_ref(base_bs), and then bdrv_unref(top). Well, now it's even simpler, so that's good.
Quoted 10 lines
block/mirror.c +0
-2@@ -370,11 +370,18 @@ static void mirror_exit(BlockJob *job, void *opaque)bdrv_swap (s->target, to_replace);if (s->common.driver->job_type ==BLOCK_JOB_TYPE_COMMIT ) {/* drop the bs loop chain formed by the swap: break the loop then -* trigger the unref from the top one */ -BlockDriverState *p = s->base->backing
block/mirror.c static void mirror_exit(BlockJob *job, void *opaque) · line 379
Quoted 10 lines
block/mirror.c +7
-3- ? s->base->backing->bs :NULL ; -bdrv_set_backing_hd (s->base,NULL ); -bdrv_unref (p); +* trigger the unref */ +/* FIXME This duplicates bdrv_set_backing_hd(), except for the +* actual detach/unref so that the loop can be broken. When +* bdrv_swap() gets replaced, this will become sane again. */ +BlockDriverState *backing = s->base->backing->bs; +assert (s->base->backing_blocker); +bdrv_op_unblock_all (s->base->backing->bs, s->base->backing_blocker);
I know it's dropped later on anwyay, but I can't stop my fingers from suggesting s/s->base->backing->bs/backing/.
Anyway, with or without that:
Re: [Qemu-devel] [PATCH 08/16] block: Manage backing file references in bdrv_set_backing_hd()
In reply to Kevin WolfQuoted 20 lines · Kevin Wolf
block/stream.c +0
-13@@ -55,28 +55,7 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,static void close_unused_images (BlockDriverState *top,BlockDriverState *base,const char *base_id) { -BlockDriverState *intermediate; - intermediate = top->backing ? top->backing->bs :NULL ; - -/* Must assign before bdrv_delete() to prevent traversing dangling pointer -* while we delete backing image instances. -*/ bdrv_set_backing_hd (top, base); - -while (intermediate) { -BlockDriverState *unused; - -/* reached base */ -if (intermediate == base) { -break ;
block/stream.c static int coroutine_fn stream_populate(BlockDriverState *bs, · line 60
Quoted 10 lines
block/stream.c +0
-8- } - - unused = intermediate; - intermediate = intermediate->backing ? intermediate->backing->bs :NULL ; -bdrv_set_backing_hd (unused,NULL ); -bdrv_unref (unused); - } -bdrv_refresh_limits (top,NULL ); }
bdrv_refresh_limits(bs, NULL) is already called at the end of bdrv_set_backing_hd(), so there's no need to call it again here.
And then close_unused_images() is used only once in this file and it would only contain the bdrv_set_backing_hd() call, so you don't need a separate function anymore.
Berto
PATCH 9/16 block: Split bdrv_move_feature_fields()
In reply to Kevin WolfAfter bdrv_swap(), some fields must be moved back to their original BDS to compensate for the effects that a swap of the contents of the objects has while keeping the old addresses. Other fields must be moved back because they should logically be moved and must stay on top
When replacing bdrv_swap() with operations changing the pointers in the parents, we only need the latter and must avoid swapping the former. Split the function accordingly.
--- block.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
block.c +12 -0
@@ -1984,6 +1984,8 @@ static void bdrv_rebind(BlockDriverState *bs) } } +/* Fields that need to stay with the top-level BDS, no matter whether the +* address of the top-level BDS stays the same or not. */ static void bdrv_move_feature_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) {
@@ -2019,7 +2021,13 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,/* dirty bitmap */ bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps; +} +/* Fields that only need to be swapped if the contents of BDSes is swapped +* rather than pointers being changed in the parents. */ +static void bdrv_move_reference_fields (BlockDriverState *bs_dest, +BlockDriverState *bs_src) +{/* reference count */ bs_dest->refcnt = bs_src->refcnt;
@@ -2090,6 +2098,10 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_move_feature_fields (bs_old, bs_new);bdrv_move_feature_fields (bs_new, &tmp); +bdrv_move_reference_fields (&tmp, bs_old); +bdrv_move_reference_fields (bs_old, bs_new); +bdrv_move_reference_fields (bs_new, &tmp); +/* bs_new must remain unattached */ assert (!bs_new->blk);
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 09/16] block: Split bdrv_move_feature_fields()
Quoted 31 lines · Kevin Wolf
After
bdrv_swap(), some fields must be moved back to their original BDS to compensate for the effects that a swap of the contents of the objects has while keeping the old addresses. Other fields must be moved back because they should logically be moved and must stay on topWhen replacing
bdrv_swap()with operations changing the pointers in the parents, we only need the latter and must avoid swapping the former. Split the function accordingly.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)block.c +2
-0@@ -1984,6 +1984,8 @@ static void bdrv_rebind(BlockDriverState *bs) } } +/* Fields that need to stay with the top-level BDS, no matter whether the +* address of the top-level BDS stays the same or not. */ static void bdrv_move_feature_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) {@@ -2019,7 +2021,13 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,/* dirty bitmap */ bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps;
block.c static void bdrv_move_feature_fields(BlockDriverState *bs_dest, · line 2033
Quoted 10 lines
block.c +6
-0+} +/* Fields that only need to be swapped if the contents of BDSes is swapped +* rather than pointers being changed in the parents. */ +static void bdrv_move_reference_fields (BlockDriverState *bs_dest, +BlockDriverState *bs_src) +{/* reference count */ bs_dest->refcnt = bs_src->refcnt;
I'm not sure whether the op blockers should be moved in this function... I think they should be moved in bdrv_move_feasture_fields(), because they generally depend on the position within the node graph and not on the BDS itself, don't they?
Max
Quoted 12 lines
block.c +4
-0@@ -2090,6 +2098,10 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_move_feature_fields (bs_old, bs_new);bdrv_move_feature_fields (bs_new, &tmp); +bdrv_move_reference_fields (&tmp, bs_old); +bdrv_move_reference_fields (bs_old, bs_new); +bdrv_move_reference_fields (bs_new, &tmp); +/* bs_new must remain unattached */ assert (!bs_new->blk);
Re: [Qemu-devel] [PATCH 09/16] block: Split bdrv_move_feature_fields()
Am 23.09.2015 um 18:36 hat Max Reitz geschrieben:
Quoted 34 lines · Kevin Wolf
Quoted 33 lines
After
bdrv_swap(), some fields must be moved back to their original BDS to compensate for the effects that a swap of the contents of the objects has while keeping the old addresses. Other fields must be moved back because they should logically be moved and must stay on topWhen replacing
bdrv_swap()with operations changing the pointers in the parents, we only need the latter and must avoid swapping the former. Split the function accordingly.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)block.c +3
-0@@ -1984,6 +1984,8 @@ static void bdrv_rebind(BlockDriverState *bs) } } +/* Fields that need to stay with the top-level BDS, no matter whether the +* address of the top-level BDS stays the same or not. */ static void bdrv_move_feature_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) {@@ -2019,7 +2021,13 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,/* dirty bitmap */ bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps; +}
Quoted 13 lines
block.c static void bdrv_move_feature_fields(BlockDriverState *bs_dest, · line 2033
Quoted 8 lines
block.c +5
-0+/* Fields that only need to be swapped if the contents of BDSes is swapped +* rather than pointers being changed in the parents. */ +static void bdrv_move_reference_fields (BlockDriverState *bs_dest, +BlockDriverState *bs_src) +{/* reference count */ bs_dest->refcnt = bs_src->refcnt;I'm not sure whether the op blockers should be moved in this function... I think they should be moved in
bdrv_move_feasture_fields(), because they generally depend on the position within the node graph and not on the BDS itself, don't they?
Op blockers are a mess. I believe we have both kinds. In any case, we should be relatively safe here because the operation involving something like bdrv_append() or bdrv_replace_in_backing_chain() would have been blocked in the first place if there were an op blocker that needs to be moved.
I seem to remember that there were two reasons for not moving op
blockers: The first one is that the next action the block jobs want to do is to unblock (because the job has completed) and it's nicer to do this on the same BDS as they blocked initially. The other one is that bs->backing_blocker is already managed by bdrv_set_backing_hd() and swapping in addition broke things.
Oh, and the third one is that swapping is plain wrong. We're not really performing a symmetrical swap any more. If BDS A is replaced by BDS B (or B is appended to a chain that had A as it's root before), all parents of _both_ A and B point to B afterwards. So swapping would move some op blockers to now completely unrelated BDSes.
Kevin
Re: [Qemu-devel] [PATCH 09/16] block: Split bdrv_move_feature_fields()
In reply to Kevin WolfQuoted 5 lines · Kevin Wolf
After
bdrv_swap(), some fields must be moved back to their original BDS to compensate for the effects that a swap of the contents of the objects has while keeping the old addresses. Other fields must be moved back because they should logically be moved and must stay on top
Quoted 8 lines
When replacing
bdrv_swap()with operations changing the pointers in the parents, we only need the latter and must avoid swapping the former. Split the function accordingly.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
(and an ACK to "Op blockers are a mess.")
PATCH 10/16 block/io: Make bdrv_requests_pending() public
In reply to Kevin Wolf--- block/io.c | 2 +- include/block/block_int.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
block/io.c +1 -1
@@ -213,7 +213,7 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs) }/* Check if any requests are in-flight (including throttled requests) */ -static bool bdrv_requests_pending (BlockDriverState *bs) +bool bdrv_requests_pending (BlockDriverState *bs) {if (!QLIST_EMPTY (&bs->tracked_requests)) {return true;
include/block/block_int.h +1 -0
@@ -667,5 +667,6 @@ bool blk_dev_is_medium_locked(BlockBackend *blk);void blk_dev_resize_cb (BlockBackend *blk);void bdrv_set_dirty (BlockDriverState *bs,int64_t cur_sector,int nr_sectors); +bool bdrv_requests_pending (BlockDriverState *bs);#endif /* BLOCK_INT_H */
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 10/16] block/io: Make bdrv_requests_pending() public
Quoted 5 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/io.c | 2 +- include/block/block_int.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
In addition: Shouldn't we iterate over all children in that function instead of just file and backing?
Max
Re: [Qemu-devel] [PATCH 10/16] block/io: Make bdrv_requests_pending() public
Am 23.09.2015 um 17:40 hat Max Reitz geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 5 lines
Signed-off-by Kevin Wolf <[email protected]>--- block/io.c | 2 +- include/block/block_int.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)Reviewed-by Max Reitz <[email protected]>In addition: Shouldn't we iterate over all children in that function instead of just file and backing?
I guess we should. If noone beats me to it, I'll send it as a patch separate from this series.
Kevin
Re: [Qemu-devel] [PATCH 10/16] block/io: Make bdrv_requests_pending() public
In reply to Kevin WolfQuoted 5 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block/io.c | 2 +- include/block/block_int.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
Berto
PATCH 11/16 block-backend: Add blk_set_bs()
In reply to Kevin WolfIt allows changing the BlockDriverState that a BlockBackend points to.
--- block/block-backend.c | 16 ++++++++++++++++ include/block/block_int.h | 2 ++ 2 files changed, 18 insertions(+)
block/block-backend.c +16 -0
@@ -239,6 +239,22 @@ BlockDriverState *blk_bs(BlockBackend *blk) }/* +* Changes the BlockDriverState attached to @blk +*/ +void blk_set_bs (BlockBackend *blk,BlockDriverState *bs) +{ +bdrv_ref (bs); + +if (blk->bs) { + blk->bs->blk =NULL ; +bdrv_unref (blk->bs); + } + + blk->bs = bs; + bs->blk = blk; +} + +/* * Return @blk's DriveInfo if any, else null. */ DriveInfo *blk_legacy_dinfo (BlockBackend *blk)
include/block/block_int.h +2 -0
@@ -659,6 +659,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, BlockCompletionFunc *cb, void *opaque, Error **errp); +void blk_set_bs (BlockBackend *blk,BlockDriverState *bs); +void blk_dev_change_media_cb (BlockBackend *blk,bool load);bool blk_dev_has_removable_media (BlockBackend *blk);void blk_dev_eject_request (BlockBackend *blk,bool force);
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 11/16] block-backend: Add blk_set_bs()
Quoted 22 lines · Kevin Wolf
It allows changing the BlockDriverState that a BlockBackend points to.
Signed-off-by Kevin Wolf <[email protected]>--- block/block-backend.c | 16 ++++++++++++++++ include/block/block_int.h | 2 ++ 2 files changed, 18 insertions(+)block/block-backend.c +6
-0@@ -239,6 +239,22 @@ BlockDriverState *blk_bs(BlockBackend *blk) }/* +* Changes the BlockDriverState attached to @blk +*/ +void blk_set_bs (BlockBackend *blk,BlockDriverState *bs) +{ +bdrv_ref (bs); +
block/block-backend.c BlockDriverState *blk_bs(BlockBackend *blk) · line 257
Quoted 10 lines
block/block-backend.c +10
-0+if (blk->bs) { + blk->bs->blk =NULL ; +bdrv_unref (blk->bs); + } + + blk->bs = bs; + bs->blk = blk; +} + +/*
As discussed in the BlockBackend and media thread, you should probably assert(bs->blk == NULL), and I'd rather have this implemented as blk_remove_bs(blk); blk_insert_bs(blk, bs);, but that's probably something that won't happen until I do so myself.
Max
Quoted 17 lines
block/block-backend.c +0
-0* Return @blk's DriveInfo if any, else null. */ DriveInfo *blk_legacy_dinfo (BlockBackend *blk)include/block/block_int.h +2
-0@@ -659,6 +659,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, BlockCompletionFunc *cb, void *opaque, Error **errp); +void blk_set_bs (BlockBackend *blk,BlockDriverState *bs); +void blk_dev_change_media_cb (BlockBackend *blk,bool load);bool blk_dev_has_removable_media (BlockBackend *blk);void blk_dev_eject_request (BlockBackend *blk,bool force);
Re: [Qemu-devel] [PATCH 11/16] block-backend: Add blk_set_bs()
Am 23.09.2015 um 17:46 hat Max Reitz geschrieben:
Quoted 28 lines · Kevin Wolf
Quoted 27 lines
It allows changing the BlockDriverState that a BlockBackend points to.
Signed-off-by Kevin Wolf <[email protected]>--- block/block-backend.c | 16 ++++++++++++++++ include/block/block_int.h | 2 ++ 2 files changed, 18 insertions(+)block/block-backend.c +11
-0@@ -239,6 +239,22 @@ BlockDriverState *blk_bs(BlockBackend *blk) }/* +* Changes the BlockDriverState attached to @blk +*/ +void blk_set_bs (BlockBackend *blk,BlockDriverState *bs) +{ +bdrv_ref (bs); + +if (blk->bs) { + blk->bs->blk =NULL ; +bdrv_unref (blk->bs); + } +
Quoted 8 lines
block/block-backend.c BlockDriverState *blk_bs(BlockBackend *blk) · line 257
Quoted 5 lines
block/block-backend.c +5
-0+ blk->bs = bs; + bs->blk = blk; +} + +/* As discussed in the BlockBackend and media thread, you should probably
assert(bs->blk == NULL),
Yes, my current branch has this already.
Quoted 3 lines
and I'd rather have this implemented as
blk_remove_bs(blk);blk_insert_bs(blk, bs);, but that's probably something that won't happen until I do so myself.
Let's see which series hits master first. If yours is first, I'll happily change this patch to make use of the new functions; otherwise, you get to rebase.
Kevin
PATCH 12/16 block: Introduce parents list
In reply to Kevin Wolf--- block.c | 3 +++ include/block/block_int.h | 2 ++ 2 files changed, 5 insertions(+)
block.c +3 -0
@@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, };QLIST_INSERT_HEAD (&parent_bs->children, child, next); +QLIST_INSERT_HEAD (&child_bs->parents, child, next_parent);return child; }
@@ -1097,6 +1098,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next); +QLIST_REMOVE (child, next_parent);g_free (child); }
@@ -2037,6 +2039,7 @@ static void bdrv_move_reference_fields(BlockDriverState *bs_dest,/* keep the same entry in bdrv_states */ bs_dest->device_list = bs_src->device_list; bs_dest->blk = bs_src->blk; + bs_dest->parents = bs_src->parents;memcpy (bs_dest->op_blockers, bs_src->op_blockers,sizeof (bs_dest->op_blockers));
include/block/block_int.h +2 -0
@@ -339,6 +339,7 @@ struct BdrvChild {BlockDriverState *bs;const BdrvChildRole *role;QLIST_ENTRY (BdrvChild) next; +QLIST_ENTRY (BdrvChild) next_parent; };/*
@@ -445,6 +446,7 @@ struct BlockDriverState {* parent node of this node. */ BlockDriverState *inherits_from; QLIST_HEAD(, BdrvChild) children; + QLIST_HEAD(, BdrvChild) parents;QDict *options;BlockdevDetectZeroesOptions detect_zeroes;
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 12/16] block: Introduce parents list
Quoted 5 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 3 +++ include/block/block_int.h | 2 ++ 2 files changed, 5 insertions(+)
Re: [Qemu-devel] [PATCH 12/16] block: Introduce parents list
In reply to Kevin Wolfblock.c static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, · line 1096
Quoted 8 lines · Kevin Wolf
block.c +1
-0@@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, };QLIST_INSERT_HEAD (&parent_bs->children, child, next); +QLIST_INSERT_HEAD (&child_bs->parents, child, next_parent);return child; }
Ok, I'm probably slow today, but what is this used for? :-? And why is it called 'parents'? The list doesn't contain pointers to the parents of child_bs, but to child_bs itself...
I would expect a BdrvChild *parent, with parent->bs = parent_bs.
Berto
Re: [Qemu-devel] [PATCH 12/16] block: Introduce parents list
Am 28.09.2015 um 15:09 hat Alberto Garcia geschrieben:
Quoted 8 lines · Kevin Wolf
Quoted 7 lines
block.c +1
-0@@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, };QLIST_INSERT_HEAD (&parent_bs->children, child, next); +QLIST_INSERT_HEAD (&child_bs->parents, child, next_parent);
Quoted 8 lines
block.c static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, · line 1096
Quoted 2 lines
block.c +0
-0return child; }Ok, I'm probably slow today, but what is this used for? :-? And why is it called 'parents'? The list doesn't contain pointers to the parents of
child_bs, but tochild_bsitself...I would expect a BdrvChild *parent, with
parent->bs=parent_bs.
It's the list of BdrvChild objects that point to the given BlockDriverState. This is good enough for updating the pointers. If we ever need the actual parent BDS, we can add a pointer to BdrvChild.
Kevin
Re: [Qemu-devel] [PATCH 12/16] block: Introduce parents list
Quoted 13 lines · Kevin Wolf
Quoted 6 lines
Quoted 6 lines
block.c +1
-0@@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, };QLIST_INSERT_HEAD (&parent_bs->children, child, next); +QLIST_INSERT_HEAD (&child_bs->parents, child, next_parent);Quoted 6 lines
block.c static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, · line 1096
Quoted 2 lines
block.c +0
-0return child; }Ok, I'm probably slow today, but what is this used for? :-? And why is it called 'parents'? The list doesn't contain pointers to the parents of
child_bs, but tochild_bsitself...
Quoted 2 lines
It's the list of BdrvChild objects that point to the given BlockDriverState.
Ok I see now. I think the names here are misleading, 'parent' means the BdrvChild object, not parent_bs.
The name child_bs->parents suggests that you can actually iterate over the BDSs that have child_bs as a child, but that's not the case, it's a mechanism that allows children to change the pointers that are pointing at them.
I'm not sure what a better name would be ("reverse refs"?), but at least I think this deserves a comment.
Berto
PATCH 13/16 block: Implement bdrv_append() without bdrv_swap()
In reply to Kevin WolfRemember all parent nodes and just change the pointers there instead of swapping the contents of the BlockDriverState.
Handling of snapshot=on must be moved further down in bdrv_open() because *pbs (which is the bs pointer in the BlockBackend) must already be set before bdrv_append() is called. Otherwise bdrv_append() changes the BB's pointer to the temporary snapshot, but bdrv_open() overwrites it with the read-only original image.
--- block.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 81 insertions(+), 28 deletions(-)
block.c +81 -28
@@ -1516,15 +1516,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,bdrv_refresh_filename (bs); -/* For snapshot=on, create a temporary qcow2 overlay. bs points to the -* temporary snapshot afterwards. */ -if (snapshot_flags) { - ret =bdrv_append_temp_snapshot (bs, snapshot_flags, &local_err); -if (local_err) { - goto close_and_fail; - } - } -/* Check if any unknown options were used */ if (options && (qdict_size (options) !=0 )) {const QDictEntry *entry =qdict_first (options);
@@ -1556,6 +1547,16 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,QDECREF (options); *pbs = bs; + +/* For snapshot=on, create a temporary qcow2 overlay. bs points to the +* temporary snapshot afterwards. */ +if (snapshot_flags) { + ret =bdrv_append_temp_snapshot (bs, snapshot_flags, &local_err); +if (local_err) { + goto close_and_fail; + } + } +return 0 ; fail:
@@ -1999,20 +2000,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, bs_dest->enable_write_cache = bs_src->enable_write_cache; -/* i/o throttled req */ - bs_dest->throttle_state = bs_src->throttle_state, - bs_dest->io_limits_enabled = bs_src->io_limits_enabled; - bs_dest->pending_reqs[0 ] = bs_src->pending_reqs[0 ]; - bs_dest->pending_reqs[1 ] = bs_src->pending_reqs[1 ]; - bs_dest->throttled_reqs[0 ] = bs_src->throttled_reqs[0 ]; - bs_dest->throttled_reqs[1 ] = bs_src->throttled_reqs[1 ]; -memcpy (&bs_dest->round_robin, - &bs_src->round_robin, -sizeof (bs_dest->round_robin)); -memcpy (&bs_dest->throttle_timers, - &bs_src->throttle_timers, -sizeof (ThrottleTimers)); -/* r/w error */ bs_dest->on_read_error = bs_src->on_read_error; bs_dest->on_write_error = bs_src->on_write_error;
@@ -2026,10 +2013,25 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, }/* Fields that only need to be swapped if the contents of BDSes is swapped -* rather than pointers being changed in the parents. */ +* rather than pointers being changed in the parents, and throttling fields +* because only bdrv_swap() messes with internals of throttling. */ static void bdrv_move_reference_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) { +/* i/o throttled req */ + bs_dest->throttle_state = bs_src->throttle_state, + bs_dest->io_limits_enabled = bs_src->io_limits_enabled; + bs_dest->pending_reqs[0 ] = bs_src->pending_reqs[0 ]; + bs_dest->pending_reqs[1 ] = bs_src->pending_reqs[1 ]; + bs_dest->throttled_reqs[0 ] = bs_src->throttled_reqs[0 ]; + bs_dest->throttled_reqs[1 ] = bs_src->throttled_reqs[1 ]; +memcpy (&bs_dest->round_robin, + &bs_src->round_robin, +sizeof (bs_dest->round_robin)); +memcpy (&bs_dest->throttle_timers, + &bs_src->throttle_timers, +sizeof (ThrottleTimers)); +/* reference count */ bs_dest->refcnt = bs_src->refcnt;
@@ -2155,6 +2157,42 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_rebind (bs_old); } +static void change_parent_backing_link (BlockDriverState *from, +BlockDriverState *to) +{ +BdrvChild *c, *next; + + QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { +assert (c->role != &child_backing); + c->bs = to; +QLIST_REMOVE (c, next_parent); +QLIST_INSERT_HEAD (&to->parents, c, next_parent); + } +if (from->blk) { +blk_set_bs (from->blk, to); +if (!to->device_list.tqe_prev) { +QTAILQ_INSERT_BEFORE (from, to, device_list); + } +QTAILQ_REMOVE (&bdrv_states, from, device_list); + } +} + +static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; + +bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +} +/* * Add new bs contents at the top of an image chain while the chain is * live, while keeping required fields on the top layer.
@@ -2165,14 +2203,29 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)* bs_new must not be attached to a BlockBackend. * * This function does not create any image files. +* +* bdrv_append() takes ownership of a bs_new reference and unrefs it because +* that's what the callers commonly need. bs_new will be referenced by the old +* parents of bs_top after bdrv_append() returns. If the caller needs to keep a +* reference of its own, it must call bdrv_ref(). */ void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top) { -bdrv_swap (bs_new, bs_top); +assert (!bdrv_requests_pending (bs_top)); +assert (!bdrv_requests_pending (bs_new)); + +bdrv_ref (bs_top); +change_parent_backing_link (bs_top, bs_new); + +/* Some fields always stay on top of the backing file chain */ +swap_feature_fields (bs_top, bs_new); + +bdrv_set_backing_hd (bs_new, bs_top); +bdrv_unref (bs_top); -/* The contents of 'tmp' will become bs_top, as we are -* swapping bs_new and bs_top contents. */ -bdrv_set_backing_hd (bs_top, bs_new); +/* bs_new is now referenced by its new parents, we don't need the +* additional reference any more. */ +bdrv_unref (bs_new); }static void bdrv_delete (BlockDriverState *bs)
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Quoted 5 lines · Kevin Wolf
block.c +5
-0+static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; +
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2194
Quoted 10 lines
block.c +10
-0+bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +}
I would use 'if (bs_top->throttle_state != NULL)' instead. That pointer is set if and only if the BDS has I/O limits set.
bs->io_limits_enabled can be set to false in order to bypass the limits temporarily without removing the BDS's throttling settings (e.g in bdrv_read_unthrottled()).
I actually think that we can get rid of io_limits_enabled in several places (if not completely), but I will take care of that in a separate patch.
The only scenario that could cause problems is if bs->throttle_state is set but bs->io_limits_enabled is false when swap_feature_fields() is called, but I don't think that's possible in this case.
Berto
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Am 18.09.2015 um 13:45 hat Alberto Garcia geschrieben:
Quoted 25 lines · Kevin Wolf
Quoted 6 lines
block.c +5
-0+static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; +block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2194
Quoted 10 lines
block.c +10
-0+bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +}I would use 'if (
bs_top->throttle_state!= NULL)' instead. That pointer is set if and only if the BDS has I/O limits set.
bs->io_limits_enabledcan be set to false in order to bypass the limits temporarily without removing the BDS's throttling settings (e.g inbdrv_read_unthrottled()).
Quoted 7 lines
I actually think that we can get rid of
io_limits_enabledin several places (if not completely), but I will take care of that in a separate patch.The only scenario that could cause problems is if
bs->throttle_stateis set butbs->io_limits_enabledis false whenswap_feature_fields()is called, but I don't think that's possible in this case.
So something like this? (Specifically, is the assertion right?)
assert(!bs_new->throttle_state);
if (bs_top->throttle_state) {
assert(bs_top->io_limits_enabled);
bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
bdrv_io_limits_disable(bs_top);
}If the assertion doesn't hold true, I guess we would need to call throttle_group_register() directly for the cases where io_limits_enabled wasn't true.
Kevin
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Quoted 31 lines · Kevin Wolf
Quoted 23 lines
Quoted 5 lines
block.c +5
-0+static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; +block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2194
Quoted 10 lines
block.c +10
-0+bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +}I would use 'if (
bs_top->throttle_state!= NULL)' instead. That pointer is set if and only if the BDS has I/O limits set.
bs->io_limits_enabledcan be set to false in order to bypass the limits temporarily without removing the BDS's throttling settings (e.g inbdrv_read_unthrottled()).Quoted 7 lines
I actually think that we can get rid of
io_limits_enabledin several places (if not completely), but I will take care of that in a separate patch.The only scenario that could cause problems is if
bs->throttle_stateis set butbs->io_limits_enabledis false whenswap_feature_fields()is called, but I don't think that's possible in this case.
Quoted 8 lines
So something like this? (Specifically, is the assertion right?)
assert(!bs_new->throttle_state); if (bs_top->throttle_state) { assert(bs_top->io_limits_enabled); bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top)); bdrv_io_limits_disable(bs_top); }
Yes, I think that's fine.
Quoted 3 lines
If the assertion doesn't hold true, I guess we would need to call
throttle_group_register()directly for the cases whereio_limits_enabledwasn't true.
If io_limits_enabled is not true but throttle_state is set it means that there's an ongoing request that needs to bypass the I/O limits.
There's two places where that can happen:
1) bdrv_start_throttled_reqs(), that's either a result of bdrv_io_limits_disable() or bdrv_flush_io_queue() (i.e. bdrv_drain()).
2) blk_read_unthrottled() (that comes from hd_geometry_guess()).
Is there any scenario where the feature fields can be swapped in the middle of one of these two operations? I understand that the BDS must be drained first, that's why I don't think there's any risk.
Berto
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Am 18.09.2015 um 15:31 hat Alberto Garcia geschrieben:
Quoted 57 lines · Kevin Wolf
Quoted 32 lines
Quoted 24 lines
Quoted 6 lines
block.c +5
-0+static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; +block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2194
Quoted 10 lines
block.c +10
-0+bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +}I would use 'if (
bs_top->throttle_state!= NULL)' instead. That pointer is set if and only if the BDS has I/O limits set.
bs->io_limits_enabledcan be set to false in order to bypass the limits temporarily without removing the BDS's throttling settings (e.g inbdrv_read_unthrottled()).Quoted 7 lines
I actually think that we can get rid of
io_limits_enabledin several places (if not completely), but I will take care of that in a separate patch.The only scenario that could cause problems is if
bs->throttle_stateis set butbs->io_limits_enabledis false whenswap_feature_fields()is called, but I don't think that's possible in this case.Quoted 8 lines
So something like this? (Specifically, is the assertion right?)
assert(!bs_new->throttle_state); if (bs_top->throttle_state) { assert(bs_top->io_limits_enabled); bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top)); bdrv_io_limits_disable(bs_top); }Yes, I think that's fine.
Quoted 3 lines
If the assertion doesn't hold true, I guess we would need to call
throttle_group_register()directly for the cases whereio_limits_enabledwasn't true.If
io_limits_enabledis not true butthrottle_stateis set it means that there's an ongoing request that needs to bypass the I/O limits.There's two places where that can happen:
1)
bdrv_start_throttled_reqs(), that's either a result ofbdrv_io_limits_disable()orbdrv_flush_io_queue()(i.e.bdrv_drain()).
Quoted 5 lines
2)
blk_read_unthrottled()(that comes fromhd_geometry_guess()).Is there any scenario where the feature fields can be swapped in the middle of one of these two operations? I understand that the BDS must be drained first, that's why I don't think there's any risk.
Yes, I'm asserting !bdrv_requests_pending() for both BDSes, so that should be fine. (But I neglect to actually drain requests in mirror.c. That's probably a bug.)
Kevin
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
In reply to Kevin WolfQuoted 103 lines · Kevin Wolf
Remember all parent nodes and just change the pointers there instead of swapping the contents of the BlockDriverState.
Handling of snapshot=on must be moved further down in
bdrv_open()because *pbs (which is the bs pointer in the BlockBackend) must already be set beforebdrv_append()is called. Otherwisebdrv_append()changes the BB's pointer to the temporary snapshot, butbdrv_open()overwrites it with the read-only original image.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 81 insertions(+), 28 deletions(-)block.c +26
-24@@ -1516,15 +1516,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,bdrv_refresh_filename (bs); -/* For snapshot=on, create a temporary qcow2 overlay. bs points to the -* temporary snapshot afterwards. */ -if (snapshot_flags) { - ret =bdrv_append_temp_snapshot (bs, snapshot_flags, &local_err); -if (local_err) { - goto close_and_fail; - } - } -/* Check if any unknown options were used */ if (options && (qdict_size (options) !=0 )) {const QDictEntry *entry =qdict_first (options);@@ -1556,6 +1547,16 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,QDECREF (options); *pbs = bs; + +/* For snapshot=on, create a temporary qcow2 overlay. bs points to the +* temporary snapshot afterwards. */ +if (snapshot_flags) { + ret =bdrv_append_temp_snapshot (bs, snapshot_flags, &local_err); +if (local_err) { + goto close_and_fail; + } + } +return 0 ; fail:@@ -1999,20 +2000,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, bs_dest->enable_write_cache = bs_src->enable_write_cache; -/* i/o throttled req */ - bs_dest->throttle_state = bs_src->throttle_state, - bs_dest->io_limits_enabled = bs_src->io_limits_enabled; - bs_dest->pending_reqs[0 ] = bs_src->pending_reqs[0 ]; - bs_dest->pending_reqs[1 ] = bs_src->pending_reqs[1 ]; - bs_dest->throttled_reqs[0 ] = bs_src->throttled_reqs[0 ]; - bs_dest->throttled_reqs[1 ] = bs_src->throttled_reqs[1 ]; -memcpy (&bs_dest->round_robin, - &bs_src->round_robin, -sizeof (bs_dest->round_robin)); -memcpy (&bs_dest->throttle_timers, - &bs_src->throttle_timers, -sizeof (ThrottleTimers)); -/* r/w error */ bs_dest->on_read_error = bs_src->on_read_error; bs_dest->on_write_error = bs_src->on_write_error;@@ -2026,10 +2013,25 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, }/* Fields that only need to be swapped if the contents of BDSes is swapped -* rather than pointers being changed in the parents. */ +* rather than pointers being changed in the parents, and throttling fields +* because only bdrv_swap() messes with internals of throttling. */ static void bdrv_move_reference_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) { +/* i/o throttled req */ + bs_dest->throttle_state = bs_src->throttle_state, + bs_dest->io_limits_enabled = bs_src->io_limits_enabled; + bs_dest->pending_reqs[0 ] = bs_src->pending_reqs[0 ]; + bs_dest->pending_reqs[1 ] = bs_src->pending_reqs[1 ]; + bs_dest->throttled_reqs[0 ] = bs_src->throttled_reqs[0 ]; + bs_dest->throttled_reqs[1 ] = bs_src->throttled_reqs[1 ]; +memcpy (&bs_dest->round_robin, + &bs_src->round_robin, +sizeof (bs_dest->round_robin)); +memcpy (&bs_dest->throttle_timers, + &bs_src->throttle_timers, +sizeof (ThrottleTimers)); +/* reference count */ bs_dest->refcnt = bs_src->refcnt;@@ -2155,6 +2157,42 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_rebind (bs_old); }
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2169
Quoted 10 lines
block.c +10
-0+static void change_parent_backing_link (BlockDriverState *from, +BlockDriverState *to) +{ +BdrvChild *c, *next; + + QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { +assert (c->role != &child_backing); + c->bs = to; +QLIST_REMOVE (c, next_parent); +QLIST_INSERT_HEAD (&to->parents, c, next_parent);
This drops a reference from the parent BDS to @from, and adds a new one from the parent BDS to @to. However, this is not reflected here.
Quoted 51 lines
block.c +40
-1+ } +if (from->blk) { +blk_set_bs (from->blk, to); +if (!to->device_list.tqe_prev) { +QTAILQ_INSERT_BEFORE (from, to, device_list); + } +QTAILQ_REMOVE (&bdrv_states, from, device_list); + } +} + +static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; + +bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +} +/* * Add new bs contents at the top of an image chain while the chain is * live, while keeping required fields on the top layer. @@ -2165,14 +2203,29 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)* bs_new must not be attached to a BlockBackend. * * This function does not create any image files. +* +* bdrv_append() takes ownership of a bs_new reference and unrefs it because +* that's what the callers commonly need. bs_new will be referenced by the old +* parents of bs_top after bdrv_append() returns. If the caller needs to keep a +* reference of its own, it must call bdrv_ref(). */ void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top) { -bdrv_swap (bs_new, bs_top); +assert (!bdrv_requests_pending (bs_top)); +assert (!bdrv_requests_pending (bs_new)); + +bdrv_ref (bs_top); +change_parent_backing_link (bs_top, bs_new); + +/* Some fields always stay on top of the backing file chain */ +swap_feature_fields (bs_top, bs_new); +
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2229
Quoted 10 lines
block.c +5
-3+bdrv_set_backing_hd (bs_new, bs_top); +bdrv_unref (bs_top); -/* The contents of 'tmp' will become bs_top, as we are -* swapping bs_new and bs_top contents. */ -bdrv_set_backing_hd (bs_top, bs_new); +/* bs_new is now referenced by its new parents, we don't need the +* additional reference any more. */ +bdrv_unref (bs_new); }
Before, all pointers to @bs_new were moved to @bs_top. Now, they stay at @bs_new. I suppose we are assuming there are no pointers to @bs_new, should we assert that, and/or point it out in the documentation?
Max
Quoted 3 lines
block.c +0
-0static void bdrv_delete (BlockDriverState *bs)
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Am 23.09.2015 um 18:36 hat Max Reitz geschrieben:
Quoted 16 lines · Kevin Wolf
Quoted 15 lines
Remember all parent nodes and just change the pointers there instead of swapping the contents of the BlockDriverState.
Handling of snapshot=on must be moved further down in
bdrv_open()because *pbs (which is the bs pointer in the BlockBackend) must already be set beforebdrv_append()is called. Otherwisebdrv_append()changes the BB's pointer to the temporary snapshot, butbdrv_open()overwrites it with the read-only original image.Signed-off-by Kevin Wolf <[email protected]>block.c +0
-0@@ -2155,6 +2157,42 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_rebind (bs_old); }
Quoted 13 lines
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2169
Quoted 10 lines
block.c +10
-0+static void change_parent_backing_link (BlockDriverState *from, +BlockDriverState *to) +{ +BdrvChild *c, *next; + + QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { +assert (c->role != &child_backing); + c->bs = to; +QLIST_REMOVE (c, next_parent); +QLIST_INSERT_HEAD (&to->parents, c, next_parent);This drops a reference from the parent BDS to @from, and adds a new one from the parent BDS to @to. However, this is not reflected here.
You mean bdrv_ref(to); bdrv_unref(from); ?
Quoted 54 lines
Quoted 54 lines
block.c +42
-1+ } +if (from->blk) { +blk_set_bs (from->blk, to); +if (!to->device_list.tqe_prev) { +QTAILQ_INSERT_BEFORE (from, to, device_list); + } +QTAILQ_REMOVE (&bdrv_states, from, device_list); + } +} + +static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; + +bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +} +/* * Add new bs contents at the top of an image chain while the chain is * live, while keeping required fields on the top layer. @@ -2165,14 +2203,29 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)* bs_new must not be attached to a BlockBackend. * * This function does not create any image files. +* +* bdrv_append() takes ownership of a bs_new reference and unrefs it because +* that's what the callers commonly need. bs_new will be referenced by the old +* parents of bs_top after bdrv_append() returns. If the caller needs to keep a +* reference of its own, it must call bdrv_ref(). */ void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top) { -bdrv_swap (bs_new, bs_top); +assert (!bdrv_requests_pending (bs_top)); +assert (!bdrv_requests_pending (bs_new)); + +bdrv_ref (bs_top); +change_parent_backing_link (bs_top, bs_new); + +/* Some fields always stay on top of the backing file chain */ +swap_feature_fields (bs_top, bs_new); + +bdrv_set_backing_hd (bs_new, bs_top); +bdrv_unref (bs_top);
Quoted 11 lines
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2229
Quoted 7 lines
block.c +3
-3-/* The contents of 'tmp' will become bs_top, as we are -* swapping bs_new and bs_top contents. */ -bdrv_set_backing_hd (bs_top, bs_new); +/* bs_new is now referenced by its new parents, we don't need the +* additional reference any more. */ +bdrv_unref (bs_new); }Before, all pointers to @bs_new were moved to @bs_top. Now, they stay at @bs_new. I suppose we are assuming there are no pointers to @bs_new, should we assert that, and/or point it out in the documentation?
How would you assert something like this? Also, I think it's currently true, but there's no reason why it should stay so. The important part is just that it's true while applying the patch because the semantics changes. Once it's applied, we have sane behaviour and can make use of it.
Kevin
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Quoted 31 lines · Kevin Wolf
Am 23.09.2015 um 18:36 hat Max Reitz geschrieben:
Quoted 16 lines · Kevin Wolf
Quoted 15 lines
Remember all parent nodes and just change the pointers there instead of swapping the contents of the BlockDriverState.
Handling of snapshot=on must be moved further down in
bdrv_open()because *pbs (which is the bs pointer in the BlockBackend) must already be set beforebdrv_append()is called. Otherwisebdrv_append()changes the BB's pointer to the temporary snapshot, butbdrv_open()overwrites it with the read-only original image.Signed-off-by Kevin Wolf <[email protected]>block.c +0
-0@@ -2155,6 +2157,42 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)bdrv_rebind (bs_old); }Quoted 13 lines
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2169
Quoted 10 lines
block.c +10
-0+static void change_parent_backing_link (BlockDriverState *from, +BlockDriverState *to) +{ +BdrvChild *c, *next; + + QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { +assert (c->role != &child_backing); + c->bs = to; +QLIST_REMOVE (c, next_parent); +QLIST_INSERT_HEAD (&to->parents, c, next_parent);This drops a reference from the parent BDS to @from, and adds a new one from the parent BDS to @to. However, this is not reflected here.
Quoted 1 line
You mean
bdrv_ref(to);bdrv_unref(from); ?
Yes.
Quoted 66 lines
Quoted 54 lines
Quoted 54 lines
block.c +42
-1+ } +if (from->blk) { +blk_set_bs (from->blk, to); +if (!to->device_list.tqe_prev) { +QTAILQ_INSERT_BEFORE (from, to, device_list); + } +QTAILQ_REMOVE (&bdrv_states, from, device_list); + } +} + +static void swap_feature_fields (BlockDriverState *bs_top, +BlockDriverState *bs_new) +{ +BlockDriverState tmp; + +bdrv_move_feature_fields (&tmp, bs_top); +bdrv_move_feature_fields (bs_top, bs_new); +bdrv_move_feature_fields (bs_new, &tmp); + +assert (!bs_new->io_limits_enabled); +if (bs_top->io_limits_enabled) { +bdrv_io_limits_enable (bs_new,throttle_group_get_name (bs_top)); +bdrv_io_limits_disable (bs_top); + } +} +/* * Add new bs contents at the top of an image chain while the chain is * live, while keeping required fields on the top layer. @@ -2165,14 +2203,29 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)* bs_new must not be attached to a BlockBackend. * * This function does not create any image files. +* +* bdrv_append() takes ownership of a bs_new reference and unrefs it because +* that's what the callers commonly need. bs_new will be referenced by the old +* parents of bs_top after bdrv_append() returns. If the caller needs to keep a +* reference of its own, it must call bdrv_ref(). */ void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top) { -bdrv_swap (bs_new, bs_top); +assert (!bdrv_requests_pending (bs_top)); +assert (!bdrv_requests_pending (bs_new)); + +bdrv_ref (bs_top); +change_parent_backing_link (bs_top, bs_new); + +/* Some fields always stay on top of the backing file chain */ +swap_feature_fields (bs_top, bs_new); + +bdrv_set_backing_hd (bs_new, bs_top); +bdrv_unref (bs_top);Quoted 11 lines
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2229
Quoted 7 lines
block.c +3
-3-/* The contents of 'tmp' will become bs_top, as we are -* swapping bs_new and bs_top contents. */ -bdrv_set_backing_hd (bs_top, bs_new); +/* bs_new is now referenced by its new parents, we don't need the +* additional reference any more. */ +bdrv_unref (bs_new); }Before, all pointers to @bs_new were moved to @bs_top. Now, they stay at @bs_new. I suppose we are assuming there are no pointers to @bs_new, should we assert that, and/or point it out in the documentation?
Quoted 1 line
How would you assert something like this?
Of course, I have no idea.
Quoted 5 lines
Also, I think it's currently
true, but there's no reason why it should stay so. The important part is just that it's true while applying the patch because the semantics changes. Once it's applied, we have sane behaviour and can make use of it.
The thing is that this is exactly the reason for the bug Berto found. external_snapshot_commit() keeps a reference to state->new_bs (@bs_new) and uses it afterwards for bdrv_reopen(), whereas it should be using state->old_bs (@bs_top) after this series.
Max
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()
Am 30.09.2015 um 16:45 hat Max Reitz geschrieben:
Quoted 40 lines · Kevin Wolf
Quoted 29 lines
Am 23.09.2015 um 18:36 hat Max Reitz geschrieben:
Quoted 16 lines · Kevin Wolf
Quoted 15 lines
void
bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top){block.c +11
-1-bdrv_swap (bs_new, bs_top); +assert (!bdrv_requests_pending (bs_top)); +assert (!bdrv_requests_pending (bs_new)); + +bdrv_ref (bs_top); +change_parent_backing_link (bs_top, bs_new); + +/* Some fields always stay on top of the backing file chain */ +swap_feature_fields (bs_top, bs_new); + +bdrv_set_backing_hd (bs_new, bs_top); +bdrv_unref (bs_top);Quoted 11 lines
block.c void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) · line 2229
Quoted 7 lines
block.c +3
-3-/* The contents of 'tmp' will become bs_top, as we are -* swapping bs_new and bs_top contents. */ -bdrv_set_backing_hd (bs_top, bs_new); +/* bs_new is now referenced by its new parents, we don't need the +* additional reference any more. */ +bdrv_unref (bs_new); }Before, all pointers to @bs_new were moved to @bs_top. Now, they stay at @bs_new. I suppose we are assuming there are no pointers to @bs_new, should we assert that, and/or point it out in the documentation?
Quoted 1 line
How would you assert something like this?
Of course, I have no idea.
Quoted 5 lines
Also, I think it's currently
true, but there's no reason why it should stay so. The important part is just that it's true while applying the patch because the semantics changes. Once it's applied, we have sane behaviour and can make use of it.
Quoted 4 lines
The thing is that this is exactly the reason for the bug Berto found.
external_snapshot_commit()keeps a reference tostate->new_bs(@bs_new) and uses it afterwards forbdrv_reopen(), whereas it should be usingstate->old_bs(@bs_top) after this series.
Yes, the interface changes. That means that you need to review all callers. There aren't that many, so it's doable.
Kevin
PATCH 14/16 blockjob: Store device name at job creation
In reply to Kevin WolfSome block jobs change the block device graph on completion. This means that the device that owns the job and originally was addressed with its device name may no longer be what the corresponding BlockBackend points to.
Previously, the effects of bdrv_swap() ensured that the job was (at least partially) transferred to the target image. Events that contain the device name could still use bdrv_get_device_name(job->bs) and get the same result.
After removing bdrv_swap(), this won't work any more. Instead, save the device name at job creation and use that copy for the events.
--- blockjob.c | 8 +++++--- include/block/blockjob.h | 8 ++++++++ 2 files changed, 13 insertions(+), 3 deletions(-)
blockjob.c +5 -3
@@ -54,6 +54,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,bdrv_op_unblock (bs,BLOCK_OP_TYPE_DATAPLANE , job->blocker); job->driver = driver; + job->id =g_strdup (bdrv_get_device_name (bs)); job->bs = bs; job->cb = cb; job->opaque = opaque;
@@ -81,6 +82,7 @@ void block_job_release(BlockDriverState *bs) bs->job =NULL ;bdrv_op_unblock_all (bs, job->blocker);error_free (job->blocker); +g_free (job->id);g_free (job); }
@@ -291,7 +293,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error)void block_job_event_cancelled (BlockJob *job) { qapi_event_send_block_job_cancelled(job->driver->job_type, - bdrv_get_device_name(job->bs), + job->id, job->len, job->offset, job->speed,
@@ -301,7 +303,7 @@ void block_job_event_cancelled(BlockJob *job)void block_job_event_completed (BlockJob *job,const char *msg) { qapi_event_send_block_job_completed(job->driver->job_type, - bdrv_get_device_name(job->bs), + job->id, job->len, job->offset, job->speed,
@@ -315,7 +317,7 @@ void block_job_event_ready(BlockJob *job) job->ready = true;qapi_event_send_block_job_ready (job->driver->job_type, -bdrv_get_device_name (job->bs), + job->id, job->len, job->offset, job->speed, &error_abort);
include/block/blockjob.h +8 -0
@@ -65,6 +65,14 @@ struct BlockJob {BlockDriverState *bs;/** +* The ID of the block job. Currently the BlockBackend name of the BDS +* owning the job at the time when the job is started. +* +* TODO Decouple block job IDs from BlockBackend names +*/ +char *id; + +/** * The coroutine that executes the job. If not NULL, it is * reentered when busy is false and the job is cancelled. */
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 14/16] blockjob: Store device name at job creation
Quoted 10 lines · Kevin Wolf
Some block jobs change the block device graph on completion. This means that the device that owns the job and originally was addressed with its device name may no longer be what the corresponding BlockBackend points to.
Previously, the effects of
bdrv_swap()ensured that the job was (at least partially) transferred to the target image. Events that contain the device name could still usebdrv_get_device_name(job->bs)and get the same result.
Quoted 8 lines
After removing
bdrv_swap(), this won't work any more. Instead, save the device name at job creation and use that copy for the events.Signed-off-by Kevin Wolf <[email protected]>--- blockjob.c | 8 +++++--- include/block/blockjob.h | 8 ++++++++ 2 files changed, 13 insertions(+), 3 deletions(-)
Why not %s/bdrv_get_device_name(job->bs)/job->id/?
Max
PATCH 15/16 block: Add and use bdrv_replace_in_backing_chain()
In reply to Kevin WolfThis cleans up the mess we left behind in the mirror code after the previous patch. Instead of using bdrv_swap(), just change pointers.
The interface change of the mirror job that callers must consider is that after job completion, their local BDS pointers still point to the same node now. qemu-img must change its code accordingly (which makes it easier to understand); the other callers stays unchanged because after completion they don't do anything with the BDS, but just with the job, and the job is still owned by the source BDS.
--- block.c | 32 +++++++++++++++++++++++++++++++- block/mirror.c | 23 +++++++---------------- include/block/block.h | 4 +++- qemu-img.c | 16 ++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-)
block.c +31 -1
@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -void bdrv_detach_child (BdrvChild *child) +static void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);QLIST_REMOVE (child, next_parent);
@@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)bdrv_unref (bs_new); } +void bdrv_replace_in_backing_chain (BlockDriverState *old,BlockDriverState *new) +{ +assert (!bdrv_requests_pending (old)); +assert (!bdrv_requests_pending (new)); + +bdrv_ref (old); + +if (old->blk) { +/* As long as these fields aren't in BlockBackend, but in the top-level +* BlockDriverState, it's not possible for a BDS to have two BBs. +* +* We really want to copy the fields from old to new, but we go for a +* swap instead so that pointers aren't duplicated and cause trouble. +* (Also, bdrv_swap() used to do the same.) */ +assert (!new->blk); +swap_feature_fields (old, new); + } +change_parent_backing_link (old, new); + +/* Change backing files if a previously independent node is added to the +* chain. For active commit, we replace top by its own (indirect) backing +* file and don't do anything here so we don't build a loop. */ +if (new->backing ==NULL && !bdrv_chain_contains (backing_bs (old), new)) { +bdrv_set_backing_hd (new,backing_bs (old)); +bdrv_set_backing_hd (old,NULL ); + } + +bdrv_unref (old); +} +static void bdrv_delete (BlockDriverState *bs) {assert (!bs->job);
block/mirror.c +7 -16
@@ -353,6 +353,11 @@ static void mirror_exit(BlockJob *job, void *opaque)MirrorBlockJob *s =container_of (job, MirrorBlockJob, common);MirrorExitData *data = opaque;AioContext *replace_aio_context =NULL ; +BlockDriverState *src = s->common.bs; + +/* Make sure that the source BDS doesn't go away before we called +* block_job_completed(). */ +bdrv_ref (src);if (s->to_replace) { replace_aio_context =bdrv_get_aio_context (s->to_replace);
@@ -367,22 +372,7 @@ static void mirror_exit(BlockJob *job, void *opaque)if (bdrv_get_flags (s->target) !=bdrv_get_flags (to_replace)) {bdrv_reopen (s->target,bdrv_get_flags (to_replace),NULL ); } -bdrv_swap (s->target, to_replace); -if (s->common.driver->job_type ==BLOCK_JOB_TYPE_COMMIT ) { -/* drop the bs loop chain formed by the swap: break the loop then -* trigger the unref */ -/* FIXME This duplicates bdrv_set_backing_hd(), except for the -* actual detach/unref so that the loop can be broken. When -* bdrv_swap() gets replaced, this will become sane again. */ -BlockDriverState *backing = s->base->backing->bs; -assert (s->base->backing_blocker); -bdrv_op_unblock_all (s->base->backing->bs, s->base->backing_blocker); -error_free (s->base->backing_blocker); - s->base->backing_blocker =NULL ; -bdrv_detach_child (s->base->backing); - s->base->backing =NULL ; -bdrv_unref (backing); - } +bdrv_replace_in_backing_chain (to_replace, s->target); }if (s->to_replace) {bdrv_op_unblock_all (s->to_replace, s->replace_blocker);
@@ -396,6 +386,7 @@ static void mirror_exit(BlockJob *job, void *opaque)bdrv_unref (s->target);block_job_completed (&s->common, data->ret);g_free (data); +bdrv_unref (src); } static void coroutine_fn mirror_run(void *opaque)
include/block/block.h +3 -1
@@ -202,6 +202,9 @@ BlockDriverState *bdrv_new(void);void bdrv_make_anon (BlockDriverState *bs);void bdrv_swap (BlockDriverState *bs_new,BlockDriverState *bs_old);void bdrv_append (BlockDriverState *bs_new,BlockDriverState *bs_top); +void bdrv_replace_in_backing_chain (BlockDriverState *old, +BlockDriverState *new); +int bdrv_parse_cache_flags (const char *mode,int *flags);int bdrv_parse_discard_flags (const char *mode,int *flags); BdrvChild *bdrv_open_child(const char *filename,
@@ -510,7 +513,6 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);void bdrv_ref (BlockDriverState *bs);void bdrv_unref (BlockDriverState *bs);void bdrv_unref_child (BlockDriverState *parent,BdrvChild *child); -void bdrv_detach_child (BdrvChild *child);bool bdrv_op_is_blocked (BlockDriverState *bs,BlockOpType op,Error **errp);void bdrv_op_block (BlockDriverState *bs,BlockOpType op,Error *reason);
qemu-img.c +8 -8
@@ -765,12 +765,12 @@ static int img_commit(int argc, char **argv) goto done; } -/* The block job will swap base_bs and bs (which is not what we really want -* here, but okay) and unref base_bs (after the swap, i.e., the old top -* image). In order to still be able to empty that top image afterwards, -* increment the reference counter here preemptively. */ +/* When the block job completes, the BlockBackend reference will point to +* the old backing file. In order to avoid that the top image is already +* deleted, so we can still empty it afterwards, increment the reference +* counter here preemptively. */ if (!drop) { -bdrv_ref (base_bs); +bdrv_ref (bs); }run_block_job (bs->job, &local_err);
@@ -778,8 +778,8 @@ static int img_commit(int argc, char **argv) goto unref_backing; } -if (!drop && base_bs->drv->bdrv_make_empty) { - ret = base_bs->drv->bdrv_make_empty (base_bs); +if (!drop && bs->drv->bdrv_make_empty) { + ret = bs->drv->bdrv_make_empty (bs);if (ret) {error_setg_errno (&local_err, -ret,"Could not empty %s" , filename);
@@ -789,7 +789,7 @@ static int img_commit(int argc, char **argv) unref_backing:if (!drop) { -bdrv_unref (base_bs); +bdrv_unref (bs); } done:
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain()
Quoted 50 lines · Kevin Wolf
This cleans up the mess we left behind in the mirror code after the previous patch. Instead of using
bdrv_swap(), just change pointers.The interface change of the mirror job that callers must consider is that after job completion, their local BDS pointers still point to the same node now. qemu-img must change its code accordingly (which makes it easier to understand); the other callers stays unchanged because after completion they don't do anything with the BDS, but just with the job, and the job is still owned by the source BDS.
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 32 +++++++++++++++++++++++++++++++- block/mirror.c | 23 +++++++---------------- include/block/block.h | 4 +++- qemu-img.c | 16 ++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-)block.c +16
-1@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -void bdrv_detach_child (BdrvChild *child) +static void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);QLIST_REMOVE (child, next_parent);@@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)bdrv_unref (bs_new); } +void bdrv_replace_in_backing_chain (BlockDriverState *old,BlockDriverState *new) +{ +assert (!bdrv_requests_pending (old)); +assert (!bdrv_requests_pending (new)); + +bdrv_ref (old); + +if (old->blk) { +/* As long as these fields aren't in BlockBackend, but in the top-level +* BlockDriverState, it's not possible for a BDS to have two BBs. +* +* We really want to copy the fields from old to new, but we go for a +* swap instead so that pointers aren't duplicated and cause trouble. +* (Also, bdrv_swap() used to do the same.) */ +assert (!new->blk);
block.c void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) · line 2255
Quoted 10 lines
block.c +10
-0+swap_feature_fields (old, new); + } +change_parent_backing_link (old, new); + +/* Change backing files if a previously independent node is added to the +* chain. For active commit, we replace top by its own (indirect) backing +* file and don't do anything here so we don't build a loop. */ +if (new->backing ==NULL && !bdrv_chain_contains (backing_bs (old), new)) { +bdrv_set_backing_hd (new,backing_bs (old)); +bdrv_set_backing_hd (old,NULL );
Wouldn't we want @old to keep its backing file?
Then bdrv_append() would basically be a special case of this function, with it additionally decrementing the refcount of @bs_new.
Max
Quoted 8 lines
block.c +5
-0+ } + +bdrv_unref (old); +} +static void bdrv_delete (BlockDriverState *bs) {assert (!bs->job);
Re: [Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain()
Am 23.09.2015 um 19:08 hat Max Reitz geschrieben:
Quoted 55 lines · Kevin Wolf
Quoted 54 lines
This cleans up the mess we left behind in the mirror code after the previous patch. Instead of using
bdrv_swap(), just change pointers.The interface change of the mirror job that callers must consider is that after job completion, their local BDS pointers still point to the same node now. qemu-img must change its code accordingly (which makes it easier to understand); the other callers stays unchanged because after completion they don't do anything with the BDS, but just with the job, and the job is still owned by the source BDS.
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 32 +++++++++++++++++++++++++++++++- block/mirror.c | 23 +++++++---------------- include/block/block.h | 4 +++- qemu-img.c | 16 ++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-)block.c +20
-1@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -void bdrv_detach_child (BdrvChild *child) +static void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);QLIST_REMOVE (child, next_parent);@@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)bdrv_unref (bs_new); } +void bdrv_replace_in_backing_chain (BlockDriverState *old,BlockDriverState *new) +{ +assert (!bdrv_requests_pending (old)); +assert (!bdrv_requests_pending (new)); + +bdrv_ref (old); + +if (old->blk) { +/* As long as these fields aren't in BlockBackend, but in the top-level +* BlockDriverState, it's not possible for a BDS to have two BBs. +* +* We really want to copy the fields from old to new, but we go for a +* swap instead so that pointers aren't duplicated and cause trouble. +* (Also, bdrv_swap() used to do the same.) */ +assert (!new->blk); +swap_feature_fields (old, new); + } +change_parent_backing_link (old, new); +
Quoted 8 lines
block.c void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) · line 2255
Quoted 6 lines
block.c +6
-0+/* Change backing files if a previously independent node is added to the +* chain. For active commit, we replace top by its own (indirect) backing +* file and don't do anything here so we don't build a loop. */ +if (new->backing ==NULL && !bdrv_chain_contains (backing_bs (old), new)) { +bdrv_set_backing_hd (new,backing_bs (old)); +bdrv_set_backing_hd (old,NULL );Wouldn't we want @old to keep its backing file?
Would we? The operation I had in mind was: Given a backing file chain, one node in that chain and an independent node, replace the node in the chain. That is:
A <- B <- C <- D
Xbecomes
A <- X <- C <- D
BOf course, you could define a different operation, but this seemed to be the obvious one that the mirror completion needs.
Quoted 2 lines
Then
bdrv_append()would basically be a special case of this function, with it additionally decrementing the refcount of @bs_new.
Hm, less duplication sounds nice, but as long as the current way is technically correct, can we leave this for a cleanup on top?
Kevin
Quoted 11 lines
Max
Quoted 8 lines
block.c +5
-0+ } + +bdrv_unref (old); +} +static void bdrv_delete (BlockDriverState *bs) {assert (!bs->job);
Re: [Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain()
Quoted 73 lines · Kevin Wolf
Am 23.09.2015 um 19:08 hat Max Reitz geschrieben:
Quoted 55 lines · Kevin Wolf
Quoted 54 lines
This cleans up the mess we left behind in the mirror code after the previous patch. Instead of using
bdrv_swap(), just change pointers.The interface change of the mirror job that callers must consider is that after job completion, their local BDS pointers still point to the same node now. qemu-img must change its code accordingly (which makes it easier to understand); the other callers stays unchanged because after completion they don't do anything with the BDS, but just with the job, and the job is still owned by the source BDS.
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 32 +++++++++++++++++++++++++++++++- block/mirror.c | 23 +++++++---------------- include/block/block.h | 4 +++- qemu-img.c | 16 ++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-)block.c +20
-1@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,return child; } -void bdrv_detach_child (BdrvChild *child) +static void bdrv_detach_child (BdrvChild *child) {QLIST_REMOVE (child, next);QLIST_REMOVE (child, next_parent);@@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)bdrv_unref (bs_new); } +void bdrv_replace_in_backing_chain (BlockDriverState *old,BlockDriverState *new) +{ +assert (!bdrv_requests_pending (old)); +assert (!bdrv_requests_pending (new)); + +bdrv_ref (old); + +if (old->blk) { +/* As long as these fields aren't in BlockBackend, but in the top-level +* BlockDriverState, it's not possible for a BDS to have two BBs. +* +* We really want to copy the fields from old to new, but we go for a +* swap instead so that pointers aren't duplicated and cause trouble. +* (Also, bdrv_swap() used to do the same.) */ +assert (!new->blk); +swap_feature_fields (old, new); + } +change_parent_backing_link (old, new); +Quoted 8 lines
block.c void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) · line 2255
Quoted 6 lines
block.c +6
-0+/* Change backing files if a previously independent node is added to the +* chain. For active commit, we replace top by its own (indirect) backing +* file and don't do anything here so we don't build a loop. */ +if (new->backing ==NULL && !bdrv_chain_contains (backing_bs (old), new)) { +bdrv_set_backing_hd (new,backing_bs (old)); +bdrv_set_backing_hd (old,NULL );Wouldn't we want @old to keep its backing file?
Would we? The operation I had in mind was: Given a backing file chain, one node in that chain and an independent node, replace the node in the chain. That is:
A <- B <- C <- D X
Quoted 8 lines
becomes
A <- X <- C <- D BOf course, you could define a different operation, but this seemed to be the obvious one that the mirror completion needs.
Oops, apparently I missed the backing_bs(old) inbdrv_set_backing_hd(new, ...).
Quoted 5 lines
Quoted 2 lines
Then
bdrv_append()would basically be a special case of this function, with it additionally decrementing the refcount of @bs_new.Hm, less duplication sounds nice, but as long as the current way is technically correct, can we leave this for a cleanup on top?
And with the backing_bs() taken into account, it is not so duplicated after all.
Max
Quoted 15 lines
Kevin
Quoted 11 lines
Max
Quoted 8 lines
block.c +5
-0+ } + +bdrv_unref (old); +} +static void bdrv_delete (BlockDriverState *bs) {assert (!bs->job);
Re: [Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain()
In reply to Kevin WolfQuoted 10 lines · Kevin Wolf
This cleans up the mess we left behind in the mirror code after the previous patch. Instead of using
bdrv_swap(), just change pointers.The interface change of the mirror job that callers must consider is that after job completion, their local BDS pointers still point to the same node now. qemu-img must change its code accordingly (which makes it easier to understand); the other callers stays unchanged because after completion they don't do anything with the BDS, but just with the job, and the job is still owned by the source BDS.
Quoted 7 lines
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 32 +++++++++++++++++++++++++++++++- block/mirror.c | 23 +++++++---------------- include/block/block.h | 4 +++- qemu-img.c | 16 ++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-)
PATCH 16/16 block: Remove bdrv_swap()
In reply to Kevin Wolfbdrv_swap() is unused now. Remove it and all functions that have no other users than bdrv_swap(). In particular, this removes the .bdrv_rebind callbacks from block drivers.
--- block.c | 155 +--------------------------------------------- block/qed.c | 7 --- block/vvfat.c | 7 --- include/block/block_int.h | 1 - include/qemu/queue.h | 6 -- 5 files changed, 1 insertion(+), 175 deletions(-)
block.c +1 -154
@@ -1980,15 +1980,7 @@ void bdrv_make_anon(BlockDriverState *bs) bs->node_name[0 ] ='\0' ; } -static void bdrv_rebind (BlockDriverState *bs) -{ -if (bs->drv && bs->drv->bdrv_rebind) { - bs->drv->bdrv_rebind (bs); - } -} - -/* Fields that need to stay with the top-level BDS, no matter whether the -* address of the top-level BDS stays the same or not. */ +/* Fields that need to stay with the top-level BDS */ static void bdrv_move_feature_fields (BlockDriverState *bs_dest,BlockDriverState *bs_src) {
@@ -2012,151 +2004,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps; } -/* Fields that only need to be swapped if the contents of BDSes is swapped -* rather than pointers being changed in the parents, and throttling fields -* because only bdrv_swap() messes with internals of throttling. */ -static void bdrv_move_reference_fields (BlockDriverState *bs_dest, -BlockDriverState *bs_src) -{ -/* i/o throttled req */ - bs_dest->throttle_state = bs_src->throttle_state, - bs_dest->io_limits_enabled = bs_src->io_limits_enabled; - bs_dest->pending_reqs[0 ] = bs_src->pending_reqs[0 ]; - bs_dest->pending_reqs[1 ] = bs_src->pending_reqs[1 ]; - bs_dest->throttled_reqs[0 ] = bs_src->throttled_reqs[0 ]; - bs_dest->throttled_reqs[1 ] = bs_src->throttled_reqs[1 ]; -memcpy (&bs_dest->round_robin, - &bs_src->round_robin, -sizeof (bs_dest->round_robin)); -memcpy (&bs_dest->throttle_timers, - &bs_src->throttle_timers, -sizeof (ThrottleTimers)); - -/* reference count */ - bs_dest->refcnt = bs_src->refcnt; - -/* job */ - bs_dest->job = bs_src->job; - -/* keep the same entry in bdrv_states */ - bs_dest->device_list = bs_src->device_list; - bs_dest->blk = bs_src->blk; - bs_dest->parents = bs_src->parents; - -memcpy (bs_dest->op_blockers, bs_src->op_blockers, -sizeof (bs_dest->op_blockers)); -} - -/* -* Swap bs contents for two image chains while they are live, -* while keeping required fields on the BlockDriverState that is -* actually attached to a device. -* -* This will modify the BlockDriverState fields, and swap contents -* between bs_new and bs_old. Both bs_new and bs_old are modified. -* -* bs_new must not be attached to a BlockBackend. -* -* This function does not create any image files. -*/ -void bdrv_swap (BlockDriverState *bs_new,BlockDriverState *bs_old) -{ -BlockDriverState tmp; -BdrvChild *child; - -bdrv_drain (bs_new); -bdrv_drain (bs_old); - -/* The code needs to swap the node_name but simply swapping node_list won't -* work so first remove the nodes from the graph list, do the swap then -* insert them back if needed. -*/ -if (bs_new->node_name[0 ] !='\0' ) { -QTAILQ_REMOVE (&graph_bdrv_states, bs_new, node_list); - } -if (bs_old->node_name[0 ] !='\0' ) { -QTAILQ_REMOVE (&graph_bdrv_states, bs_old, node_list); - } - -/* If the BlockDriverState is part of a throttling group acquire -* its lock since we're going to mess with the protected fields. -* Otherwise there's no need to worry since no one else can touch -* them. */ -if (bs_old->throttle_state) { -throttle_group_lock (bs_old); - } - -/* bs_new must be unattached and shouldn't have anything fancy enabled */ -assert (!bs_new->blk); -assert (QLIST_EMPTY (&bs_new->dirty_bitmaps)); -assert (bs_new->job ==NULL ); -assert (bs_new->io_limits_enabled == false); -assert (bs_new->throttle_state ==NULL ); -assert (!throttle_timers_are_initialized (&bs_new->throttle_timers)); - - tmp = *bs_new; - *bs_new = *bs_old; - *bs_old = tmp; - -/* there are some fields that should not be swapped, move them back */ -bdrv_move_feature_fields (&tmp, bs_old); -bdrv_move_feature_fields (bs_old, bs_new); -bdrv_move_feature_fields (bs_new, &tmp); - -bdrv_move_reference_fields (&tmp, bs_old); -bdrv_move_reference_fields (bs_old, bs_new); -bdrv_move_reference_fields (bs_new, &tmp); - -/* bs_new must remain unattached */ -assert (!bs_new->blk); - -/* Check a few fields that should remain attached to the device */ -assert (bs_new->job ==NULL ); -assert (bs_new->io_limits_enabled == false); -assert (bs_new->throttle_state ==NULL ); -assert (!throttle_timers_are_initialized (&bs_new->throttle_timers)); - -/* Release the ThrottleGroup lock */ -if (bs_old->throttle_state) { -throttle_group_unlock (bs_old); - } - -/* insert the nodes back into the graph node list if needed */ -if (bs_new->node_name[0 ] !='\0' ) { -QTAILQ_INSERT_TAIL (&graph_bdrv_states, bs_new, node_list); - } -if (bs_old->node_name[0 ] !='\0' ) { -QTAILQ_INSERT_TAIL (&graph_bdrv_states, bs_old, node_list); - } - -/* -* Update lh_first.le_prev for non-empty lists. -* -* The head of the op blocker list doesn't change because it is moved back -* in bdrv_move_feature_fields(). -*/ -assert (QLIST_EMPTY (&bs_old->tracked_requests)); -assert (QLIST_EMPTY (&bs_new->tracked_requests)); - -QLIST_FIX_HEAD_PTR (&bs_new->children, next); -QLIST_FIX_HEAD_PTR (&bs_old->children, next); - -/* Update references in bs->opaque and children */ - QLIST_FOREACH(child, &bs_old->children, next) { -if (child->bs->inherits_from == bs_new) { - child->bs->inherits_from = bs_old; - } - } - QLIST_FOREACH(child, &bs_new->children, next) { -if (child->bs->inherits_from == bs_old) { - child->bs->inherits_from = bs_new; - } - } - -bdrv_rebind (bs_new); -bdrv_rebind (bs_old); -} -static void change_parent_backing_link (BlockDriverState *from,BlockDriverState *to) {
block/qed.c +0 -7
@@ -354,12 +354,6 @@ static void qed_cancel_need_check_timer(BDRVQEDState *s)timer_del (s->need_check_timer); } -static void bdrv_qed_rebind (BlockDriverState *bs) -{ -BDRVQEDState *s = bs->opaque; - s->bs = bs; -} -static void bdrv_qed_detach_aio_context (BlockDriverState *bs) {BDRVQEDState *s = bs->opaque;
@@ -1664,7 +1658,6 @@ static BlockDriver bdrv_qed = { .supports_backing = true, .bdrv_probe = bdrv_qed_probe, - .bdrv_rebind = bdrv_qed_rebind, .bdrv_open = bdrv_qed_open, .bdrv_close = bdrv_qed_close, .bdrv_reopen_prepare = bdrv_qed_reopen_prepare,
block/vvfat.c +0 -7
@@ -985,12 +985,6 @@ static BDRVVVFATState *vvv = NULL;static int enable_write_target (BDRVVVFATState *s,Error **errp);static int is_consistent (BDRVVVFATState *s); -static void vvfat_rebind (BlockDriverState *bs) -{ -BDRVVVFATState *s = bs->opaque; - s->bs = bs; -} -static QemuOptsList runtime_opts = { .name ="vvfat" , .head =QTAILQ_HEAD_INITIALIZER (runtime_opts.head),
@@ -3012,7 +3006,6 @@ static BlockDriver bdrv_vvfat = { .bdrv_parse_filename = vvfat_parse_filename, .bdrv_file_open = vvfat_open, .bdrv_close = vvfat_close, - .bdrv_rebind = vvfat_rebind, .bdrv_read = vvfat_co_read, .bdrv_write = vvfat_co_write,
include/block/block_int.h +0 -1
@@ -122,7 +122,6 @@ struct BlockDriver {int (*bdrv_write)(BlockDriverState *bs,int64_t sector_num,const uint8_t *buf,int nb_sectors);void (*bdrv_close)(BlockDriverState *bs); -void (*bdrv_rebind)(BlockDriverState *bs);int (*bdrv_create)(const char *filename,QemuOpts *opts,Error **errp);int (*bdrv_set_key)(BlockDriverState *bs,const char *key);int (*bdrv_make_empty)(BlockDriverState *bs);
include/qemu/queue.h +0 -6
@@ -117,12 +117,6 @@ struct { \ } \ } while (/*CONSTCOND*/0) -#define QLIST_FIX_HEAD_PTR(head, field) do { \ - if ((head)->lh_first != NULL) { \ - (head)->lh_first->field.le_prev = &(head)->lh_first; \ - } \ -} while (/*CONSTCOND*/0) -#define QLIST_INSERT_AFTER (listelm, elm, field) do { \ if (((elm)->field.le_next = (listelm)->field.le_next) != NULL) \ (listelm)->field.le_next->field.le_prev = \
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 16/16] block: Remove bdrv_swap()
Quoted 4 lines · Kevin Wolf
bdrv_swap()is unused now. Remove it and all functions that have no other users thanbdrv_swap(). In particular, this removes the .bdrv_rebind callbacks from block drivers.
Quoted 8 lines
Signed-off-by Kevin Wolf <[email protected]>--- block.c | 155 +--------------------------------------------- block/qed.c | 7 --- block/vvfat.c | 7 --- include/block/block_int.h | 1 - include/qemu/queue.h | 6 -- 5 files changed, 1 inse
Re: [Qemu-devel] [PATCH 16/16] block: Remove bdrv_swap()
In reply to Kevin WolfQuoted 5 lines · Kevin Wolf
bdrv_swap()is unused now. Remove it and all functions that have no other users thanbdrv_swap(). In particular, this removes the .bdrv_rebind callbacks from block drivers.Signed-off-by Kevin Wolf <[email protected]>
Berto
Quoted 7 lines · Kevin Wolf
bdrv_swap()has always been an ugly hack that we would rather have avoided. When it was introduced, we simply didn't have the infrastructure to update pointers instead of transplanting the contents of BDS object, so we grudgingly addedbdrv_swap()as a quick solution. Meanwhile, most of the infrastructure exists and this series implements the final step necessary to implement the required functionality in a less adventurous way.
I didn't have the chance to debug this further, but with this series if I make a snapshot of a device with ongoing I/O I get lots of errors in the guest:
Buffer I/O error on device vda1, logical block 3801549end_request: I/O error, dev vda, sector 7604516 [...]
The drive is attached with -drive if=virtio,file=foo.qcow2,node-name=bar
Berto
Am 18.09.2015 um 13:03 hat Alberto Garcia geschrieben:
Quoted 14 lines · Kevin Wolf
Quoted 8 lines
bdrv_swap()has always been an ugly hack that we would rather have avoided. When it was introduced, we simply didn't have the infrastructure to update pointers instead of transplanting the contents of BDS object, so we grudgingly addedbdrv_swap()as a quick solution. Meanwhile, most of the infrastructure exists and this series implements the final step necessary to implement the required functionality in a less adventurous way.I didn't have the chance to debug this further, but with this series if I make a snapshot of a device with ongoing I/O I get lots of errors in the guest:
Quoted 5 lines
Buffer I/O error on device vda1, logical block 3801549
end_request: I/O error, dev vda, sector 7604516 [...]The drive is attached with -drive if=virtio,file=foo.qcow2,node-name=bar
This is a stupid one. Now that bdrv_append() doesn't swap the meaning of its parameters any more, the snapshotting code makes the wrong node read-only.
The more important point, though, is probably that qemu-iotests never tries to write to an image after taking an internal snapshot. Perhaps we should port the monitor to qemu-io (Benoît had some patches to add it to qemu-nbd, which should be similar; except that a "qmp" command in qemu-io could even be simpler), so we can test monitor commands more thoroughly without launching a full qemu (which allows to use the monitor commands in isolation, but mostly means that we don't do a lot of actual I/O).
Anyway, patch below.
Kevin
blockdev.c +1 -1
@@ -1546,7 +1546,7 @@ static void external_snapshot_commit(BlkTransactionState *common)/* We don't need (or want) to use the transactional * bdrv_reopen_multiple() across all the entries at once, because we * don't want to abort all of them if one of them fails the reopen */ -bdrv_reopen (state->new_bs, state->new_bs->open_flags & ~BDRV_O_RDWR , +bdrv_reopen (state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR ,NULL );aio_context_release (state->aio_context);
Quoted 7 lines · Kevin Wolf
bdrv_swap()has always been an ugly hack that we would rather have avoided. When it was introduced, we simply didn't have the infrastructure to update pointers instead of transplanting the contents of BDS object, so we grudgingly addedbdrv_swap()as a quick solution. Meanwhile, most of the infrastructure exists and this series implements the final step necessary to implement the required functionality in a less adventurous way.
I managed to crash QEMU with this series. I don't think it's a problem it the series itself, but rather some older bug that is triggered by your patches.
I think I found the cause and I have a preliminary patch, I'll try to send it tomorrow.
Berto