Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Sat, Dec 11, 2010 at 12:50:20PM +, Paul Brook wrote: It's guest visible state, so it must not change due to migrations. For the current implementation all values for it work anyway - if it's smaller than the block size we'll zero out the remainder of the block. That sounds wrong. Surely we should leave partial blocks untouched. While zeroing them is not required for qemu, the general semantics of the XFS ioctl require it. It punches a hole, which means it's makes the new area equivalent to a hole create by truncating a file to a larger size and then only writing at the larger offset. The semantics for a hole in all Unix filesystems is that we read back zeroes from them. If we write into a sparse file at a not block aligned offset the zeroing of the partial block also happens.
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Sat, Dec 11, 2010 at 12:50:20PM +, Paul Brook wrote: It's guest visible state, so it must not change due to migrations. For the current implementation all values for it work anyway - if it's smaller than the block size we'll zero out the remainder of the block. That sounds wrong. Surely we should leave partial blocks untouched. While zeroing them is not required for qemu, the general semantics of the XFS ioctl require it. It punches a hole, which means it's makes the new area equivalent to a hole create by truncating a file to a larger size and then only writing at the larger offset. The semantics for a hole in all Unix filesystems is that we read back zeroes from them. If we write into a sparse file at a not block aligned offset the zeroing of the partial block also happens. Ah, so it was just inconsistent use of the term block. When the erase region includes part of a block, we zero that part of the block and leave the rest of the block untouched. Paul
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Thu, Dec 02, 2010 at 01:12:13PM +0100, Kevin Wolf wrote: DEFINE_PROP_UINT16(physical_block_size, _state, \ _conf.physical_block_size, 512), \ DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \ -DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0) +DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \ +DEFINE_PROP_UINT32(discard_granularity, _state, \ + _conf.discard_granularity, 0) Is there no way to get this value automatically? At least for non-raw images (and it should be trivial to implement this in qcow2) I guess we'll want to set it to the cluster size of the image instead of requiring the user to set this value. It's guest visible state, so it must not change due to migrations. For the current implementation all values for it work anyway - if it's smaller than the block size we'll zero out the remainder of the block. That sounds wrong. Surely we should leave partial blocks untouched. Paul
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Thu, Dec 02, 2010 at 01:12:13PM +0100, Kevin Wolf wrote: DEFINE_PROP_UINT16(physical_block_size, _state, \ _conf.physical_block_size, 512), \ DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \ -DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0) +DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \ +DEFINE_PROP_UINT32(discard_granularity, _state, \ + _conf.discard_granularity, 0) Is there no way to get this value automatically? At least for non-raw images (and it should be trivial to implement this in qcow2) I guess we'll want to set it to the cluster size of the image instead of requiring the user to set this value. It's guest visible state, so it must not change due to migrations. For the current implementation all values for it work anyway - if it's smaller than the block size we'll zero out the remainder of the block.
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
Am 01.12.2010 16:35, schrieb Christoph Hellwig:
Add a new bdrv_discard method to free blocks in a mapping image, and a new
drive property to set the granularity for these discard. If no discard
granularity support is set discard support is disabled.
Signed-off-by: Christoph Hellwig h...@lst.de
Index: qemu/block.c
===
--- qemu.orig/block.c 2010-11-25 16:17:32.922003704 +0100
+++ qemu/block.c 2010-11-29 14:10:21.793255565 +0100
@@ -1499,6 +1499,15 @@ int bdrv_has_zero_init(BlockDriverState
return 1;
}
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+if (!bs-drv)
+return -ENOMEDIUM;
+if (!bs-drv-bdrv_discard)
+return 0;
Missing braces.
+return bs-drv-bdrv_discard(bs, sector_num, nb_sectors);
+}
+
/*
* Returns true iff the specified sector is present in the disk image.
Drivers
* not implementing the functionality are assumed to not support backing
files,
Index: qemu/block.h
===
--- qemu.orig/block.h 2010-11-25 16:17:32.929004193 +0100
+++ qemu/block.h 2010-11-29 14:07:00.267003145 +0100
@@ -146,6 +146,7 @@ int bdrv_flush(BlockDriverState *bs);
void bdrv_flush_all(void);
void bdrv_close_all(void);
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int
nb_sectors,
int *pnum);
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h 2010-11-25 16:17:32.935003774 +0100
+++ qemu/block_int.h 2010-11-29 14:09:31.926264704 +0100
@@ -73,6 +73,8 @@ struct BlockDriver {
BlockDriverCompletionFunc *cb, void *opaque);
BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors);
int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
int num_reqs);
@@ -227,6 +229,7 @@ typedef struct BlockConf {
uint16_t logical_block_size;
uint16_t min_io_size;
uint32_t opt_io_size;
+uint32_t discard_granularity;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -249,6 +252,8 @@ static inline unsigned int get_physical_
DEFINE_PROP_UINT16(physical_block_size, _state, \
_conf.physical_block_size, 512), \
DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
-DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0)
+DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \
+DEFINE_PROP_UINT32(discard_granularity, _state, \
+ _conf.discard_granularity, 0)
Is there no way to get this value automatically?
At least for non-raw images (and it should be trivial to implement this
in qcow2) I guess we'll want to set it to the cluster size of the image
instead of requiring the user to set this value.
Kevin
[Qemu-devel] [PATCH 1/5] block: add discard support
Add a new bdrv_discard method to free blocks in a mapping image, and a new
drive property to set the granularity for these discard. If no discard
granularity support is set discard support is disabled.
Signed-off-by: Christoph Hellwig h...@lst.de
Index: qemu/block.c
===
--- qemu.orig/block.c 2010-11-25 16:17:32.922003704 +0100
+++ qemu/block.c2010-11-29 14:10:21.793255565 +0100
@@ -1499,6 +1499,15 @@ int bdrv_has_zero_init(BlockDriverState
return 1;
}
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+if (!bs-drv)
+return -ENOMEDIUM;
+if (!bs-drv-bdrv_discard)
+return 0;
+return bs-drv-bdrv_discard(bs, sector_num, nb_sectors);
+}
+
/*
* Returns true iff the specified sector is present in the disk image. Drivers
* not implementing the functionality are assumed to not support backing files,
Index: qemu/block.h
===
--- qemu.orig/block.h 2010-11-25 16:17:32.929004193 +0100
+++ qemu/block.h2010-11-29 14:07:00.267003145 +0100
@@ -146,6 +146,7 @@ int bdrv_flush(BlockDriverState *bs);
void bdrv_flush_all(void);
void bdrv_close_all(void);
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h 2010-11-25 16:17:32.935003774 +0100
+++ qemu/block_int.h2010-11-29 14:09:31.926264704 +0100
@@ -73,6 +73,8 @@ struct BlockDriver {
BlockDriverCompletionFunc *cb, void *opaque);
BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors);
int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
int num_reqs);
@@ -227,6 +229,7 @@ typedef struct BlockConf {
uint16_t logical_block_size;
uint16_t min_io_size;
uint32_t opt_io_size;
+uint32_t discard_granularity;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -249,6 +252,8 @@ static inline unsigned int get_physical_
DEFINE_PROP_UINT16(physical_block_size, _state, \
_conf.physical_block_size, 512), \
DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
-DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0)
+DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \
+DEFINE_PROP_UINT32(discard_granularity, _state, \
+ _conf.discard_granularity, 0)
#endif /* BLOCK_INT_H */
Index: qemu/block/raw.c
===
--- qemu.orig/block/raw.c 2010-11-25 16:17:32.943003495 +0100
+++ qemu/block/raw.c2010-11-29 14:07:00.278003495 +0100
@@ -65,6 +65,11 @@ static int raw_probe(const uint8_t *buf,
return 1; /* everything can be opened as raw image */
}
+static int raw_discard(BlockDriverState *bs, int64_t sector_num, int
nb_sectors)
+{
+return bdrv_discard(bs-file, sector_num, nb_sectors);
+}
+
static int raw_is_inserted(BlockDriverState *bs)
{
return bdrv_is_inserted(bs-file);
@@ -130,6 +135,7 @@ static BlockDriver bdrv_raw = {
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev= raw_aio_writev,
.bdrv_aio_flush = raw_aio_flush,
+.bdrv_discard = raw_discard,
.bdrv_is_inserted = raw_is_inserted,
.bdrv_eject = raw_eject,
[Qemu-devel] [PATCH 1/5] block: add discard support
Add a new bdrv_discard method to free blocks in a mapping image, and a new
drive property to set the granularity for these discard. If no discard
granularity support is set discard support is disabled.
Signed-off-by: Christoph Hellwig h...@lst.de
Index: qemu/block.c
===
--- qemu.orig/block.c 2010-11-25 12:34:10.580256183 +0100
+++ qemu/block.c2010-11-25 12:34:41.761254654 +0100
@@ -1499,6 +1499,13 @@ int bdrv_has_zero_init(BlockDriverState
return 1;
}
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+if (!bs-drv || !bs-drv-bdrv_discard)
+return 0;
+return bs-drv-bdrv_discard(bs, sector_num, nb_sectors);
+}
+
/*
* Returns true iff the specified sector is present in the disk image. Drivers
* not implementing the functionality are assumed to not support backing files,
Index: qemu/block.h
===
--- qemu.orig/block.h 2010-11-25 12:34:10.589253738 +0100
+++ qemu/block.h2010-11-25 12:34:12.148012156 +0100
@@ -146,6 +146,7 @@ int bdrv_flush(BlockDriverState *bs);
void bdrv_flush_all(void);
void bdrv_close_all(void);
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h 2010-11-25 12:34:10.595254018 +0100
+++ qemu/block_int.h2010-11-25 12:34:12.150013832 +0100
@@ -73,6 +73,8 @@ struct BlockDriver {
BlockDriverCompletionFunc *cb, void *opaque);
BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors);
int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
int num_reqs);
@@ -227,6 +229,7 @@ typedef struct BlockConf {
uint16_t logical_block_size;
uint16_t min_io_size;
uint32_t opt_io_size;
+uint32_t discard_granularity;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -249,6 +252,8 @@ static inline unsigned int get_physical_
DEFINE_PROP_UINT16(physical_block_size, _state, \
_conf.physical_block_size, 512), \
DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
-DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0)
+DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \
+DEFINE_PROP_UINT32(discard_granularity, _state, \
+ _conf.discard_granularity, 0)
#endif /* BLOCK_INT_H */
Index: qemu/block/raw.c
===
--- qemu.orig/block/raw.c 2010-11-25 12:34:10.601259605 +0100
+++ qemu/block/raw.c2010-11-25 12:34:12.155004124 +0100
@@ -65,6 +65,11 @@ static int raw_probe(const uint8_t *buf,
return 1; /* everything can be opened as raw image */
}
+static int raw_discard(BlockDriverState *bs, int64_t sector_num, int
nb_sectors)
+{
+return bdrv_discard(bs-file, sector_num, nb_sectors);
+}
+
static int raw_is_inserted(BlockDriverState *bs)
{
return bdrv_is_inserted(bs-file);
@@ -130,6 +135,7 @@ static BlockDriver bdrv_raw = {
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev= raw_aio_writev,
.bdrv_aio_flush = raw_aio_flush,
+.bdrv_discard = raw_discard,
.bdrv_is_inserted = raw_is_inserted,
.bdrv_eject = raw_eject,
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Thu, 25 Nov 2010, Christoph Hellwig wrote:
Add a new bdrv_discard method to free blocks in a mapping image, and a new
drive property to set the granularity for these discard. If no discard
granularity support is set discard support is disabled.
Signed-off-by: Christoph Hellwig h...@lst.de
Index: qemu/block.c
===
--- qemu.orig/block.c 2010-11-25 12:34:10.580256183 +0100
+++ qemu/block.c 2010-11-25 12:34:41.761254654 +0100
@@ -1499,6 +1499,13 @@ int bdrv_has_zero_init(BlockDriverState
return 1;
}
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+if (!bs-drv || !bs-drv-bdrv_discard)
+return 0;
+return bs-drv-bdrv_discard(bs, sector_num, nb_sectors);
+}
+
/*
* Returns true iff the specified sector is present in the disk image.
Drivers
* not implementing the functionality are assumed to not support backing
files,
Index: qemu/block.h
===
--- qemu.orig/block.h 2010-11-25 12:34:10.589253738 +0100
+++ qemu/block.h 2010-11-25 12:34:12.148012156 +0100
@@ -146,6 +146,7 @@ int bdrv_flush(BlockDriverState *bs);
void bdrv_flush_all(void);
void bdrv_close_all(void);
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int
nb_sectors,
int *pnum);
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h 2010-11-25 12:34:10.595254018 +0100
+++ qemu/block_int.h 2010-11-25 12:34:12.150013832 +0100
@@ -73,6 +73,8 @@ struct BlockDriver {
BlockDriverCompletionFunc *cb, void *opaque);
BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors);
int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
int num_reqs);
@@ -227,6 +229,7 @@ typedef struct BlockConf {
uint16_t logical_block_size;
uint16_t min_io_size;
uint32_t opt_io_size;
+uint32_t discard_granularity;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -249,6 +252,8 @@ static inline unsigned int get_physical_
DEFINE_PROP_UINT16(physical_block_size, _state, \
_conf.physical_block_size, 512), \
DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
-DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0)
+DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0), \
+DEFINE_PROP_UINT32(discard_granularity, _state, \
+ _conf.discard_granularity, 0)
Tab here.
#endif /* BLOCK_INT_H */
Index: qemu/block/raw.c
===
--- qemu.orig/block/raw.c 2010-11-25 12:34:10.601259605 +0100
+++ qemu/block/raw.c 2010-11-25 12:34:12.155004124 +0100
@@ -65,6 +65,11 @@ static int raw_probe(const uint8_t *buf,
return 1; /* everything can be opened as raw image */
}
+static int raw_discard(BlockDriverState *bs, int64_t sector_num, int
nb_sectors)
+{
+return bdrv_discard(bs-file, sector_num, nb_sectors);
+}
+
static int raw_is_inserted(BlockDriverState *bs)
{
return bdrv_is_inserted(bs-file);
@@ -130,6 +135,7 @@ static BlockDriver bdrv_raw = {
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev= raw_aio_writev,
.bdrv_aio_flush = raw_aio_flush,
+.bdrv_discard = raw_discard,
.bdrv_is_inserted = raw_is_inserted,
.bdrv_eject = raw_eject,
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 1/5] block: add discard support
On Thu, Nov 25, 2010 at 1:57 PM, Christoph Hellwig h...@lst.de wrote:
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+ if (!bs-drv || !bs-drv-bdrv_discard)
+ return 0;
!bs-drv is normally -ENOMEDIUM. Perhaps we shouldn't lump it in with
!bs-drv-bdrv_discard.
Stefan
