Re: [Qemu-devel] [PATCH 1/5] block: add discard support

[Kevin Wolf <kwolf@redhat.com>]

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 <hch@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