Re: [PATCH] virtio-blk: Respect discard granularity

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

On Fri, Feb 19, 2021 at 07:19:19PM +0900, Akihiko Odaki wrote:
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  hw/block/virtio-blk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index bac2d6fa2b2..692fd17b0e0 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -965,7 +965,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>          virtio_stl_p(vdev, &blkcfg.max_discard_sectors,
>                       s->conf.max_discard_sectors);
>          virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,
> -                     blk_size >> BDRV_SECTOR_BITS);
> +                     conf->discard_granularity >> BDRV_SECTOR_BITS);

Please handle the -1 default value like this:

  uint32_t discard_granularity = conf->discard_granularity;
  if (conf->discard_granularity == -1) {
      discard_granularity = BDRV_SECTOR_SIZE;
  }

I noticed this when comparing the blk_size and discard_granularity
values when I run QEMU:

  $ qemu-system-x86_64 -M accel=kvm -m 1G -cpu host -drive if=virtio,file=test.img,format=raw
  blk_size 512
  discard_granularity 4294967295

Also, please add a compat prop in hw/core/machine.c to ensure that
existing machine types are unaffected by this change. This can be done
by adding DEFINE_PROP_BOOL("report-discard-granularity", ...) in
hw/block/virtio-blk.c and then setting it to false for existing machine
types in hw/core/machine.c. Then new machine types benefit from the new
feature but existing machine types will be unchanged (eliminating the
risk of live migration/snapshot incompatibilities when the device
unexpectedly changes behavior while the guest is running).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]