| Message ID | 1385124001-3576-9-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 22.11.2013 13:39, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/qcow2.c | 2 ++ > block/qed.c | 2 ++ > block/vdi.c | 1 + > block/vhdx.c | 3 +++ > block/vpc.c | 1 + > 5 files changed, 9 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 2fe37ed..1542750 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1892,6 +1892,8 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs) > static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > { > BDRVQcowState *s = bs->opaque; > + bdi->unallocated_blocks_are_zero = true; > + bdi->can_write_zeroes_with_unmap = (s->qcow_version >= 3); > bdi->cluster_size = s->cluster_size; > bdi->vm_state_offset = qcow2_vm_state_offset(s); > return 0; > diff --git a/block/qed.c b/block/qed.c > index adc2736..59516a5 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -1475,6 +1475,8 @@ static int bdrv_qed_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > memset(bdi, 0, sizeof(*bdi)); > bdi->cluster_size = s->header.cluster_size; > bdi->is_dirty = s->header.features & QED_F_NEED_CHECK; > + bdi->unallocated_blocks_are_zero = true; > + bdi->can_write_zeroes_with_unmap = true; > return 0; > } > > diff --git a/block/vdi.c b/block/vdi.c > index b6ec002..2d7490f 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -331,6 +331,7 @@ static int vdi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > logout("\n"); > bdi->cluster_size = s->block_size; > bdi->vm_state_offset = 0; > + bdi->unallocated_blocks_are_zero = true; > return 0; > } > > diff --git a/block/vhdx.c b/block/vhdx.c > index ed6fa53..67bbe10 100644 > --- a/block/vhdx.c > +++ b/block/vhdx.c > @@ -1049,6 +1049,9 @@ static int vhdx_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > > bdi->cluster_size = s->block_size; > > + bdi->unallocated_blocks_are_zero = > + (s->params.data_bits & VHDX_PARAMS_HAS_PARENT) == 0; > + > return 0; > } > > diff --git a/block/vpc.c b/block/vpc.c > index 551876f..1d326cb 100644 > --- a/block/vpc.c > +++ b/block/vpc.c > @@ -464,6 +464,7 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > bdi->cluster_size = s->block_size; > } > > + bdi->unallocated_blocks_are_zero = true; > return 0; > } > Reviewed-by: Peter Lieven <pl@kamp.de>
Am 22.11.2013 um 13:39 hat Paolo Bonzini geschrieben: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/qcow2.c | 2 ++ > block/qed.c | 2 ++ > block/vdi.c | 1 + > block/vhdx.c | 3 +++ > block/vpc.c | 1 + > 5 files changed, 9 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 2fe37ed..1542750 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1892,6 +1892,8 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs) > static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > { > BDRVQcowState *s = bs->opaque; > + bdi->unallocated_blocks_are_zero = true; > + bdi->can_write_zeroes_with_unmap = (s->qcow_version >= 3); > bdi->cluster_size = s->cluster_size; > bdi->vm_state_offset = qcow2_vm_state_offset(s); > return 0; We must change qcow2_discard_clusters() to set the zero flag instead of just deallocating the cluster. (We should be doing that anyway, because the backing file reappearing isn't very nice.) Not quite sure what to do with version 2 images, though. For the other formats, I guess this is only correct because they don't implement discard anyway? Kevin
Il 03/12/2013 16:09, Kevin Wolf ha scritto: >> > BDRVQcowState *s = bs->opaque; >> > + bdi->unallocated_blocks_are_zero = true; >> > + bdi->can_write_zeroes_with_unmap = (s->qcow_version >= 3); >> > bdi->cluster_size = s->cluster_size; >> > bdi->vm_state_offset = qcow2_vm_state_offset(s); >> > return 0; > We must change qcow2_discard_clusters() to set the zero flag instead of > just deallocating the cluster. (We should be doing that anyway, because > the backing file reappearing isn't very nice.) No, that's not needed: * unallocated_blocks_are_zero is only meaningful for bs->backing_hd == NULL (not too intuitive, but I didn't introduce that interface :)). In fact, v2 was checking backing_hd == NULL but I removed it after Peter noticed I was being inconsistent. * can_write_zeroes_with_unmap correctly returns true only if zero clusters are enabled > For the other formats, I guess this is only correct because they don't > implement discard anyway? No, it is correct because that's what their bdrv_co_readv (or similar) will return when a block is not allocated and there is no backing file. Of course, for many formats there will never be a backing file. Paolo
> unallocated_blocks_are_zero is false if there is a backing hd. Same as has_zero_init. Peter > Am 03.12.2013 um 16:21 schrieb Paolo Bonzini <pbonzini@redhat.com>: > > Il 03/12/2013 16:09, Kevin Wolf ha scritto: >>>> BDRVQcowState *s = bs->opaque; >>>> + bdi->unallocated_blocks_are_zero = true; >>>> + bdi->can_write_zeroes_with_unmap = (s->qcow_version >= 3); >>>> bdi->cluster_size = s->cluster_size; >>>> bdi->vm_state_offset = qcow2_vm_state_offset(s); >>>> return 0; >> We must change qcow2_discard_clusters() to set the zero flag instead of >> just deallocating the cluster. (We should be doing that anyway, because >> the backing file reappearing isn't very nice.) > > No, that's not needed: > > * unallocated_blocks_are_zero is only meaningful for bs->backing_hd == > NULL (not too intuitive, but I didn't introduce that interface :)). In > fact, v2 was checking backing_hd == NULL but I removed it after Peter > noticed I was being inconsistent. > > * can_write_zeroes_with_unmap correctly returns true only if zero > clusters are enabled > >> For the other formats, I guess this is only correct because they don't >> implement discard anyway? > > No, it is correct because that's what their bdrv_co_readv (or similar) > will return when a block is not allocated and there is no backing file. > Of course, for many formats there will never be a backing file. > > Paolo
diff --git a/block/qcow2.c b/block/qcow2.c index 2fe37ed..1542750 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1892,6 +1892,8 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs) static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) { BDRVQcowState *s = bs->opaque; + bdi->unallocated_blocks_are_zero = true; + bdi->can_write_zeroes_with_unmap = (s->qcow_version >= 3); bdi->cluster_size = s->cluster_size; bdi->vm_state_offset = qcow2_vm_state_offset(s); return 0; diff --git a/block/qed.c b/block/qed.c index adc2736..59516a5 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1475,6 +1475,8 @@ static int bdrv_qed_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) memset(bdi, 0, sizeof(*bdi)); bdi->cluster_size = s->header.cluster_size; bdi->is_dirty = s->header.features & QED_F_NEED_CHECK; + bdi->unallocated_blocks_are_zero = true; + bdi->can_write_zeroes_with_unmap = true; return 0; } diff --git a/block/vdi.c b/block/vdi.c index b6ec002..2d7490f 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -331,6 +331,7 @@ static int vdi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) logout("\n"); bdi->cluster_size = s->block_size; bdi->vm_state_offset = 0; + bdi->unallocated_blocks_are_zero = true; return 0; } diff --git a/block/vhdx.c b/block/vhdx.c index ed6fa53..67bbe10 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1049,6 +1049,9 @@ static int vhdx_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) bdi->cluster_size = s->block_size; + bdi->unallocated_blocks_are_zero = + (s->params.data_bits & VHDX_PARAMS_HAS_PARENT) == 0; + return 0; } diff --git a/block/vpc.c b/block/vpc.c index 551876f..1d326cb 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -464,6 +464,7 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) bdi->cluster_size = s->block_size; } + bdi->unallocated_blocks_are_zero = true; return 0; }
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block/qcow2.c | 2 ++ block/qed.c | 2 ++ block/vdi.c | 1 + block/vhdx.c | 3 +++ block/vpc.c | 1 + 5 files changed, 9 insertions(+)