* [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
@ 2014-05-06 13:30 Kevin Wolf
2014-05-06 15:23 ` Paolo Bonzini
2014-05-06 19:53 ` Max Reitz
0 siblings, 2 replies; 4+ messages in thread
From: Kevin Wolf @ 2014-05-06 13:30 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha
bdrv_is_allocated() shouldn't return true for sectors that are
unallocated, but after the end of a short backing file, even though
such sectors are (correctly) marked as containing zeros.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 8 +++++---
include/block/block.h | 11 +++++++----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index c90c71a..d3a9906 100644
--- a/block.c
+++ b/block.c
@@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
*pnum, pnum);
}
+ if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
+ ret |= BDRV_BLOCK_ALLOCATED;
+ }
+
if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) {
if (bdrv_unallocated_blocks_are_zero(bs)) {
ret |= BDRV_BLOCK_ZERO;
@@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
if (ret < 0) {
return ret;
}
- return
- (ret & BDRV_BLOCK_DATA) ||
- ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
+ return (ret & BDRV_BLOCK_ALLOCATED);
}
/*
diff --git a/include/block/block.h b/include/block/block.h
index 2fda81c..ad4c7e8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -116,6 +116,8 @@ typedef enum {
/* BDRV_BLOCK_DATA: data is read from bs->file or another file
* BDRV_BLOCK_ZERO: sectors read as zero
* BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
+ * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
+ * layer (as opposed to the backing file)
* BDRV_BLOCK_RAW: used internally to indicate that the request
* was answered by the raw driver and that one
* should look in bs->file directly.
@@ -137,10 +139,11 @@ typedef enum {
* f t f not allocated or unknown offset, read as zero
* f f f not allocated or unknown offset, read from backing_hd
*/
-#define BDRV_BLOCK_DATA 1
-#define BDRV_BLOCK_ZERO 2
-#define BDRV_BLOCK_OFFSET_VALID 4
-#define BDRV_BLOCK_RAW 8
+#define BDRV_BLOCK_DATA 0x01
+#define BDRV_BLOCK_ZERO 0x02
+#define BDRV_BLOCK_OFFSET_VALID 0x04
+#define BDRV_BLOCK_RAW 0x08
+#define BDRV_BLOCK_ALLOCATED 0x10
#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
typedef enum {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
2014-05-06 13:30 [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files Kevin Wolf
@ 2014-05-06 15:23 ` Paolo Bonzini
2014-05-06 19:53 ` Max Reitz
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-05-06 15:23 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: stefanha
Il 06/05/2014 15:30, Kevin Wolf ha scritto:
> bdrv_is_allocated() shouldn't return true for sectors that are
> unallocated, but after the end of a short backing file, even though
> such sectors are (correctly) marked as containing zeros.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Nice. :)
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
2014-05-06 13:30 [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files Kevin Wolf
2014-05-06 15:23 ` Paolo Bonzini
@ 2014-05-06 19:53 ` Max Reitz
2014-05-07 8:31 ` Kevin Wolf
1 sibling, 1 reply; 4+ messages in thread
From: Max Reitz @ 2014-05-06 19:53 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: pbonzini, stefanha
On 06.05.2014 15:30, Kevin Wolf wrote:
> bdrv_is_allocated() shouldn't return true for sectors that are
> unallocated, but after the end of a short backing file, even though
> such sectors are (correctly) marked as containing zeros.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 8 +++++---
> include/block/block.h | 11 +++++++----
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/block.c b/block.c
> index c90c71a..d3a9906 100644
> --- a/block.c
> +++ b/block.c
> @@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
> *pnum, pnum);
> }
>
> + if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
> + ret |= BDRV_BLOCK_ALLOCATED;
> + }
> +
Shouldn't BDRV_BLOCK_ALLOCATED be set in the
!bs->drv->bdrv_co_get_block_status case as well?
> if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) {
> if (bdrv_unallocated_blocks_are_zero(bs)) {
> ret |= BDRV_BLOCK_ZERO;
> @@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
> if (ret < 0) {
> return ret;
> }
> - return
> - (ret & BDRV_BLOCK_DATA) ||
> - ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
> + return (ret & BDRV_BLOCK_ALLOCATED);
> }
>
> /*
> diff --git a/include/block/block.h b/include/block/block.h
> index 2fda81c..ad4c7e8 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -116,6 +116,8 @@ typedef enum {
> /* BDRV_BLOCK_DATA: data is read from bs->file or another file
> * BDRV_BLOCK_ZERO: sectors read as zero
> * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> + * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
> + * layer (as opposed to the backing file)
I guess this is above BDRV_BLOCK_RAW (albeit having a greater value)
because it is not only used internally? (to pick up on the topic of OCD :-P)
Max
> * BDRV_BLOCK_RAW: used internally to indicate that the request
> * was answered by the raw driver and that one
> * should look in bs->file directly.
> @@ -137,10 +139,11 @@ typedef enum {
> * f t f not allocated or unknown offset, read as zero
> * f f f not allocated or unknown offset, read from backing_hd
> */
> -#define BDRV_BLOCK_DATA 1
> -#define BDRV_BLOCK_ZERO 2
> -#define BDRV_BLOCK_OFFSET_VALID 4
> -#define BDRV_BLOCK_RAW 8
> +#define BDRV_BLOCK_DATA 0x01
> +#define BDRV_BLOCK_ZERO 0x02
> +#define BDRV_BLOCK_OFFSET_VALID 0x04
> +#define BDRV_BLOCK_RAW 0x08
> +#define BDRV_BLOCK_ALLOCATED 0x10
> #define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
>
> typedef enum {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
2014-05-06 19:53 ` Max Reitz
@ 2014-05-07 8:31 ` Kevin Wolf
0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2014-05-07 8:31 UTC (permalink / raw)
To: Max Reitz; +Cc: pbonzini, qemu-devel, stefanha
Am 06.05.2014 um 21:53 hat Max Reitz geschrieben:
> On 06.05.2014 15:30, Kevin Wolf wrote:
> >bdrv_is_allocated() shouldn't return true for sectors that are
> >unallocated, but after the end of a short backing file, even though
> >such sectors are (correctly) marked as containing zeros.
> >
> >Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >---
> > block.c | 8 +++++---
> > include/block/block.h | 11 +++++++----
> > 2 files changed, 12 insertions(+), 7 deletions(-)
> >
> >diff --git a/block.c b/block.c
> >index c90c71a..d3a9906 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
> > *pnum, pnum);
> > }
> >+ if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
> >+ ret |= BDRV_BLOCK_ALLOCATED;
> >+ }
> >+
>
> Shouldn't BDRV_BLOCK_ALLOCATED be set in the
> !bs->drv->bdrv_co_get_block_status case as well?
It should. Thanks, I'll send a v2.
> > if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) {
> > if (bdrv_unallocated_blocks_are_zero(bs)) {
> > ret |= BDRV_BLOCK_ZERO;
> >@@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
> > if (ret < 0) {
> > return ret;
> > }
> >- return
> >- (ret & BDRV_BLOCK_DATA) ||
> >- ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
> >+ return (ret & BDRV_BLOCK_ALLOCATED);
> > }
> > /*
> >diff --git a/include/block/block.h b/include/block/block.h
> >index 2fda81c..ad4c7e8 100644
> >--- a/include/block/block.h
> >+++ b/include/block/block.h
> >@@ -116,6 +116,8 @@ typedef enum {
> > /* BDRV_BLOCK_DATA: data is read from bs->file or another file
> > * BDRV_BLOCK_ZERO: sectors read as zero
> > * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> >+ * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
> >+ * layer (as opposed to the backing file)
>
> I guess this is above BDRV_BLOCK_RAW (albeit having a greater value)
> because it is not only used internally? (to pick up on the topic of
> OCD :-P)
Because it felt right. This may or may not be equivalent.
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-07 8:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 13:30 [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files Kevin Wolf
2014-05-06 15:23 ` Paolo Bonzini
2014-05-06 19:53 ` Max Reitz
2014-05-07 8:31 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).