* [PATCH] virtio-blk: Respect discard granularity @ 2021-02-19 10:19 Akihiko Odaki 2021-02-22 16:49 ` Stefan Hajnoczi 0 siblings, 1 reply; 10+ messages in thread From: Akihiko Odaki @ 2021-02-19 10:19 UTC (permalink / raw) Cc: Kevin Wolf, Akihiko Odaki, qemu-block, Michael S. Tsirkin, qemu-devel, Max Reitz, Stefan Hajnoczi 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); /* * We support only one segment per request since multiple segments * are not widely used and there are no userspace APIs that allow -- 2.24.3 (Apple Git-128) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio-blk: Respect discard granularity 2021-02-19 10:19 [PATCH] virtio-blk: Respect discard granularity Akihiko Odaki @ 2021-02-22 16:49 ` Stefan Hajnoczi 2021-02-23 5:36 ` [PATCH v2] " Akihiko Odaki 0 siblings, 1 reply; 10+ messages in thread From: Stefan Hajnoczi @ 2021-02-22 16:49 UTC (permalink / raw) To: Akihiko Odaki Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Michael S. Tsirkin [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] virtio-blk: Respect discard granularity 2021-02-22 16:49 ` Stefan Hajnoczi @ 2021-02-23 5:36 ` Akihiko Odaki 2021-02-23 11:24 ` Stefano Garzarella 0 siblings, 1 reply; 10+ messages in thread From: Akihiko Odaki @ 2021-02-23 5:36 UTC (permalink / raw) Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, qemu-devel, Max Reitz, Akihiko Odaki Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- hw/block/virtio-blk.c | 8 +++++++- hw/core/machine.c | 9 ++++++++- include/hw/virtio/virtio-blk.h | 1 + 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index bac2d6fa2b2..f4378e61182 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blkcfg.wce = blk_enable_write_cache(s->blk); virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { + uint32_t discard_granularity = conf->discard_granularity; + if (discard_granularity == -1 || !s->conf.report_discard_granularity) { + discard_granularity = blk_size; + } 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); + discard_granularity >> BDRV_SECTOR_BITS); /* * We support only one segment per request since multiple segments * are not widely used and there are no userspace APIs that allow @@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { IOThread *), DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, VIRTIO_BLK_F_DISCARD, true), + DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, + conf.report_discard_granularity, true), DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, VIRTIO_BLK_F_WRITE_ZEROES, true), DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, diff --git a/hw/core/machine.c b/hw/core/machine.c index de3b8f1b318..3ba976e5bbc 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -33,7 +33,9 @@ #include "migration/global_state.h" #include "migration/vmstate.h" -GlobalProperty hw_compat_5_2[] = {}; +GlobalProperty hw_compat_5_2[] = { + { "virtio-blk-device", "report-discard-granularity", "off" }, +}; const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); GlobalProperty hw_compat_5_1[] = { @@ -41,6 +43,7 @@ GlobalProperty hw_compat_5_1[] = { { "vhost-user-blk", "num-queues", "1"}, { "vhost-user-scsi", "num_queues", "1"}, { "virtio-blk-device", "num-queues", "1"}, + { "virtio-blk-device", "report-discard-granularity", "off" }, { "virtio-scsi-device", "num_queues", "1"}, { "nvme", "use-intel-id", "on"}, { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */ @@ -50,6 +53,7 @@ const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1); GlobalProperty hw_compat_5_0[] = { { "pci-host-bridge", "x-config-reg-migration-enabled", "off" }, { "virtio-balloon-device", "page-poison", "false" }, + { "virtio-blk-device", "report-discard-granularity", "off" }, { "vmport", "x-read-set-eax", "off" }, { "vmport", "x-signal-unsupported-cmd", "off" }, { "vmport", "x-report-vmx-type", "off" }, @@ -59,6 +63,7 @@ GlobalProperty hw_compat_5_0[] = { const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0); GlobalProperty hw_compat_4_2[] = { + { "virtio-blk-device", "report-discard-granularity", "off" }, { "virtio-blk-device", "queue-size", "128"}, { "virtio-scsi-device", "virtqueue_size", "128"}, { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" }, @@ -74,6 +79,7 @@ GlobalProperty hw_compat_4_2[] = { const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2); GlobalProperty hw_compat_4_1[] = { + { "virtio-blk-device", "report-discard-granularity", "off" }, { "virtio-pci", "x-pcie-flr-init", "off" }, { "virtio-device", "use-disabled-flag", "false" }, }; @@ -83,6 +89,7 @@ GlobalProperty hw_compat_4_0[] = { { "VGA", "edid", "false" }, { "secondary-vga", "edid", "false" }, { "bochs-display", "edid", "false" }, + { "virtio-blk-device", "report-discard-granularity", "off" }, { "virtio-vga", "edid", "false" }, { "virtio-gpu-device", "edid", "false" }, { "virtio-device", "use-started", "false" }, diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 214ab748229..29655a406dd 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -41,6 +41,7 @@ struct VirtIOBlkConf uint16_t num_queues; uint16_t queue_size; bool seg_max_adjust; + bool report_discard_granularity; uint32_t max_discard_sectors; uint32_t max_write_zeroes_sectors; bool x_enable_wce_if_config_wce; -- 2.24.3 (Apple Git-128) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] virtio-blk: Respect discard granularity 2021-02-23 5:36 ` [PATCH v2] " Akihiko Odaki @ 2021-02-23 11:24 ` Stefano Garzarella 2021-02-23 12:09 ` [PATCH v3] " Akihiko Odaki 0 siblings, 1 reply; 10+ messages in thread From: Stefano Garzarella @ 2021-02-23 11:24 UTC (permalink / raw) To: Akihiko Odaki Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Michael S. Tsirkin On Tue, Feb 23, 2021 at 02:36:16PM +0900, Akihiko Odaki wrote: >Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> >--- > hw/block/virtio-blk.c | 8 +++++++- > hw/core/machine.c | 9 ++++++++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 16 insertions(+), 2 deletions(-) > >diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c >index bac2d6fa2b2..f4378e61182 100644 >--- a/hw/block/virtio-blk.c >+++ b/hw/block/virtio-blk.c >@@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) > blkcfg.wce = blk_enable_write_cache(s->blk); > virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); > if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { >+ uint32_t discard_granularity = conf->discard_granularity; >+ if (discard_granularity == -1 || !s->conf.report_discard_granularity) { >+ discard_granularity = blk_size; >+ } > 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); >+ discard_granularity >> BDRV_SECTOR_BITS); > /* > * We support only one segment per request since multiple segments > * are not widely used and there are no userspace APIs that allow >@@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { > IOThread *), > DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, > VIRTIO_BLK_F_DISCARD, true), >+ DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, >+ conf.report_discard_granularity, true), > DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, > VIRTIO_BLK_F_WRITE_ZEROES, true), > DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, >diff --git a/hw/core/machine.c b/hw/core/machine.c >index de3b8f1b318..3ba976e5bbc 100644 >--- a/hw/core/machine.c >+++ b/hw/core/machine.c >@@ -33,7 +33,9 @@ > #include "migration/global_state.h" > #include "migration/vmstate.h" > >-GlobalProperty hw_compat_5_2[] = {}; >+GlobalProperty hw_compat_5_2[] = { >+ { "virtio-blk-device", "report-discard-granularity", "off" }, IIUC older machines inherit the properties set for newer machines, so I think only this one is enough. Thanks, Stefano >+}; > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); > > GlobalProperty hw_compat_5_1[] = { >@@ -41,6 +43,7 @@ GlobalProperty hw_compat_5_1[] = { > { "vhost-user-blk", "num-queues", "1"}, > { "vhost-user-scsi", "num_queues", "1"}, > { "virtio-blk-device", "num-queues", "1"}, >+ { "virtio-blk-device", "report-discard-granularity", "off" }, > { "virtio-scsi-device", "num_queues", "1"}, > { "nvme", "use-intel-id", "on"}, > { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */ >@@ -50,6 +53,7 @@ const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1); > GlobalProperty hw_compat_5_0[] = { > { "pci-host-bridge", "x-config-reg-migration-enabled", "off" }, > { "virtio-balloon-device", "page-poison", "false" }, >+ { "virtio-blk-device", "report-discard-granularity", "off" }, > { "vmport", "x-read-set-eax", "off" }, > { "vmport", "x-signal-unsupported-cmd", "off" }, > { "vmport", "x-report-vmx-type", "off" }, >@@ -59,6 +63,7 @@ GlobalProperty hw_compat_5_0[] = { > const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0); > > GlobalProperty hw_compat_4_2[] = { >+ { "virtio-blk-device", "report-discard-granularity", "off" }, > { "virtio-blk-device", "queue-size", "128"}, > { "virtio-scsi-device", "virtqueue_size", "128"}, > { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" }, >@@ -74,6 +79,7 @@ GlobalProperty hw_compat_4_2[] = { > const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2); > > GlobalProperty hw_compat_4_1[] = { >+ { "virtio-blk-device", "report-discard-granularity", "off" }, > { "virtio-pci", "x-pcie-flr-init", "off" }, > { "virtio-device", "use-disabled-flag", "false" }, > }; >@@ -83,6 +89,7 @@ GlobalProperty hw_compat_4_0[] = { > { "VGA", "edid", "false" }, > { "secondary-vga", "edid", "false" }, > { "bochs-display", "edid", "false" }, >+ { "virtio-blk-device", "report-discard-granularity", "off" }, > { "virtio-vga", "edid", "false" }, > { "virtio-gpu-device", "edid", "false" }, > { "virtio-device", "use-started", "false" }, >diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h >index 214ab748229..29655a406dd 100644 >--- a/include/hw/virtio/virtio-blk.h >+++ b/include/hw/virtio/virtio-blk.h >@@ -41,6 +41,7 @@ struct VirtIOBlkConf > uint16_t num_queues; > uint16_t queue_size; > bool seg_max_adjust; >+ bool report_discard_granularity; > uint32_t max_discard_sectors; > uint32_t max_write_zeroes_sectors; > bool x_enable_wce_if_config_wce; >-- >2.24.3 (Apple Git-128) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] virtio-blk: Respect discard granularity 2021-02-23 11:24 ` Stefano Garzarella @ 2021-02-23 12:09 ` Akihiko Odaki 2021-02-23 14:26 ` Michael S. Tsirkin 2021-02-24 10:17 ` Stefano Garzarella 0 siblings, 2 replies; 10+ messages in thread From: Akihiko Odaki @ 2021-02-23 12:09 UTC (permalink / raw) Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, qemu-devel, Max Reitz, Akihiko Odaki Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- hw/block/virtio-blk.c | 8 +++++++- hw/core/machine.c | 4 +++- include/hw/virtio/virtio-blk.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index bac2d6fa2b2..f4378e61182 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blkcfg.wce = blk_enable_write_cache(s->blk); virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { + uint32_t discard_granularity = conf->discard_granularity; + if (discard_granularity == -1 || !s->conf.report_discard_granularity) { + discard_granularity = blk_size; + } 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); + discard_granularity >> BDRV_SECTOR_BITS); /* * We support only one segment per request since multiple segments * are not widely used and there are no userspace APIs that allow @@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { IOThread *), DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, VIRTIO_BLK_F_DISCARD, true), + DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, + conf.report_discard_granularity, true), DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, VIRTIO_BLK_F_WRITE_ZEROES, true), DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, diff --git a/hw/core/machine.c b/hw/core/machine.c index de3b8f1b318..e4df5797e72 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -33,7 +33,9 @@ #include "migration/global_state.h" #include "migration/vmstate.h" -GlobalProperty hw_compat_5_2[] = {}; +GlobalProperty hw_compat_5_2[] = { + { "virtio-blk-device", "report-discard-granularity", "off" }, +}; const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); GlobalProperty hw_compat_5_1[] = { diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 214ab748229..29655a406dd 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -41,6 +41,7 @@ struct VirtIOBlkConf uint16_t num_queues; uint16_t queue_size; bool seg_max_adjust; + bool report_discard_granularity; uint32_t max_discard_sectors; uint32_t max_write_zeroes_sectors; bool x_enable_wce_if_config_wce; -- 2.24.3 (Apple Git-128) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] virtio-blk: Respect discard granularity 2021-02-23 12:09 ` [PATCH v3] " Akihiko Odaki @ 2021-02-23 14:26 ` Michael S. Tsirkin 2021-02-24 10:17 ` Stefano Garzarella 1 sibling, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2021-02-23 14:26 UTC (permalink / raw) To: Akihiko Odaki; +Cc: Kevin Wolf, qemu-devel, qemu-block, Max Reitz On Tue, Feb 23, 2021 at 09:09:40PM +0900, Akihiko Odaki wrote: > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> whoever knows more about the detail here, feel free to merge. > --- > hw/block/virtio-blk.c | 8 +++++++- > hw/core/machine.c | 4 +++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index bac2d6fa2b2..f4378e61182 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) > blkcfg.wce = blk_enable_write_cache(s->blk); > virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); > if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { > + uint32_t discard_granularity = conf->discard_granularity; > + if (discard_granularity == -1 || !s->conf.report_discard_granularity) { > + discard_granularity = blk_size; > + } > 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); > + discard_granularity >> BDRV_SECTOR_BITS); > /* > * We support only one segment per request since multiple segments > * are not widely used and there are no userspace APIs that allow > @@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { > IOThread *), > DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, > VIRTIO_BLK_F_DISCARD, true), > + DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, > + conf.report_discard_granularity, true), > DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, > VIRTIO_BLK_F_WRITE_ZEROES, true), > DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, > diff --git a/hw/core/machine.c b/hw/core/machine.c > index de3b8f1b318..e4df5797e72 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -33,7 +33,9 @@ > #include "migration/global_state.h" > #include "migration/vmstate.h" > > -GlobalProperty hw_compat_5_2[] = {}; > +GlobalProperty hw_compat_5_2[] = { > + { "virtio-blk-device", "report-discard-granularity", "off" }, > +}; > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); > > GlobalProperty hw_compat_5_1[] = { > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index 214ab748229..29655a406dd 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -41,6 +41,7 @@ struct VirtIOBlkConf > uint16_t num_queues; > uint16_t queue_size; > bool seg_max_adjust; > + bool report_discard_granularity; > uint32_t max_discard_sectors; > uint32_t max_write_zeroes_sectors; > bool x_enable_wce_if_config_wce; > -- > 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] virtio-blk: Respect discard granularity 2021-02-23 12:09 ` [PATCH v3] " Akihiko Odaki 2021-02-23 14:26 ` Michael S. Tsirkin @ 2021-02-24 10:17 ` Stefano Garzarella 2021-02-25 0:12 ` [PATCH v4] " Akihiko Odaki 1 sibling, 1 reply; 10+ messages in thread From: Stefano Garzarella @ 2021-02-24 10:17 UTC (permalink / raw) To: Akihiko Odaki Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, qemu-devel, Max Reitz, Stefan Hajnoczi +Cc stefanha@redhat.com Please explain a bit the changes in the commit message, for example that you added 'report-discard-granularity', disabled it for older machines, that we use blk_size as default granularity, etc. Something like this: Report the configured granularity for discard operation to the guest. If this is not set use the block size. Since until now we have ignored the configured discard granularity and always reported the block size, let's add 'report-discard-granularity' property and disable it for older machine types to avoid migration issues. And use ./scripts/get_maintainer.pl to CC all the maintainers (e.g. Stefan Hajnoczi was missing) Other than that, the patch LGTM. Thanks, Stefano On Tue, Feb 23, 2021 at 09:09:40PM +0900, Akihiko Odaki wrote: >Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> >--- > hw/block/virtio-blk.c | 8 +++++++- > hw/core/machine.c | 4 +++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 11 insertions(+), 2 deletions(-) > >diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c >index bac2d6fa2b2..f4378e61182 100644 >--- a/hw/block/virtio-blk.c >+++ b/hw/block/virtio-blk.c >@@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) > blkcfg.wce = blk_enable_write_cache(s->blk); > virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); > if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { >+ uint32_t discard_granularity = conf->discard_granularity; >+ if (discard_granularity == -1 || !s->conf.report_discard_granularity) { >+ discard_granularity = blk_size; >+ } > 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); >+ discard_granularity >> BDRV_SECTOR_BITS); > /* > * We support only one segment per request since multiple segments > * are not widely used and there are no userspace APIs that allow >@@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { > IOThread *), > DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, > VIRTIO_BLK_F_DISCARD, true), >+ DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, >+ conf.report_discard_granularity, true), > DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, > VIRTIO_BLK_F_WRITE_ZEROES, true), > DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, >diff --git a/hw/core/machine.c b/hw/core/machine.c >index de3b8f1b318..e4df5797e72 100644 >--- a/hw/core/machine.c >+++ b/hw/core/machine.c >@@ -33,7 +33,9 @@ > #include "migration/global_state.h" > #include "migration/vmstate.h" > >-GlobalProperty hw_compat_5_2[] = {}; >+GlobalProperty hw_compat_5_2[] = { >+ { "virtio-blk-device", "report-discard-granularity", "off" }, >+}; > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); > > GlobalProperty hw_compat_5_1[] = { >diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h >index 214ab748229..29655a406dd 100644 >--- a/include/hw/virtio/virtio-blk.h >+++ b/include/hw/virtio/virtio-blk.h >@@ -41,6 +41,7 @@ struct VirtIOBlkConf > uint16_t num_queues; > uint16_t queue_size; > bool seg_max_adjust; >+ bool report_discard_granularity; > uint32_t max_discard_sectors; > uint32_t max_write_zeroes_sectors; > bool x_enable_wce_if_config_wce; >-- >2.24.3 (Apple Git-128) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4] virtio-blk: Respect discard granularity 2021-02-24 10:17 ` Stefano Garzarella @ 2021-02-25 0:12 ` Akihiko Odaki 2021-02-25 8:34 ` Stefano Garzarella 2021-03-08 16:52 ` Stefan Hajnoczi 0 siblings, 2 replies; 10+ messages in thread From: Akihiko Odaki @ 2021-02-25 0:12 UTC (permalink / raw) Cc: Kevin Wolf, Akihiko Odaki, Eduardo Habkost, qemu-block, Michael S . Tsirkin, Stefan Hajnoczi, qemu-devel, Max Reitz, Stefano Garzarella Report the configured granularity for discard operation to the guest. If this is not set use the block size. Since until now we have ignored the configured discard granularity and always reported the block size, let's add 'report-discard-granularity' property and disable it for older machine types to avoid migration issues. Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- hw/block/virtio-blk.c | 8 +++++++- hw/core/machine.c | 4 +++- include/hw/virtio/virtio-blk.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index bac2d6fa2b2..f4378e61182 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -962,10 +962,14 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blkcfg.wce = blk_enable_write_cache(s->blk); virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { + uint32_t discard_granularity = conf->discard_granularity; + if (discard_granularity == -1 || !s->conf.report_discard_granularity) { + discard_granularity = blk_size; + } 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); + discard_granularity >> BDRV_SECTOR_BITS); /* * We support only one segment per request since multiple segments * are not widely used and there are no userspace APIs that allow @@ -1299,6 +1303,8 @@ static Property virtio_blk_properties[] = { IOThread *), DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, VIRTIO_BLK_F_DISCARD, true), + DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock, + conf.report_discard_granularity, true), DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, VIRTIO_BLK_F_WRITE_ZEROES, true), DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, diff --git a/hw/core/machine.c b/hw/core/machine.c index de3b8f1b318..e4df5797e72 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -33,7 +33,9 @@ #include "migration/global_state.h" #include "migration/vmstate.h" -GlobalProperty hw_compat_5_2[] = {}; +GlobalProperty hw_compat_5_2[] = { + { "virtio-blk-device", "report-discard-granularity", "off" }, +}; const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); GlobalProperty hw_compat_5_1[] = { diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 214ab748229..29655a406dd 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -41,6 +41,7 @@ struct VirtIOBlkConf uint16_t num_queues; uint16_t queue_size; bool seg_max_adjust; + bool report_discard_granularity; uint32_t max_discard_sectors; uint32_t max_write_zeroes_sectors; bool x_enable_wce_if_config_wce; -- 2.24.3 (Apple Git-128) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] virtio-blk: Respect discard granularity 2021-02-25 0:12 ` [PATCH v4] " Akihiko Odaki @ 2021-02-25 8:34 ` Stefano Garzarella 2021-03-08 16:52 ` Stefan Hajnoczi 1 sibling, 0 replies; 10+ messages in thread From: Stefano Garzarella @ 2021-02-25 8:34 UTC (permalink / raw) To: Akihiko Odaki Cc: Kevin Wolf, Eduardo Habkost, qemu-block, Michael S . Tsirkin, Stefan Hajnoczi, qemu-devel, Max Reitz On Thu, Feb 25, 2021 at 09:12:39AM +0900, Akihiko Odaki wrote: >Report the configured granularity for discard operation to the >guest. If this is not set use the block size. > >Since until now we have ignored the configured discard granularity >and always reported the block size, let's add >'report-discard-granularity' property and disable it for older >machine types to avoid migration issues. > >Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> >--- > hw/block/virtio-blk.c | 8 +++++++- > hw/core/machine.c | 4 +++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 11 insertions(+), 2 deletions(-) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] virtio-blk: Respect discard granularity 2021-02-25 0:12 ` [PATCH v4] " Akihiko Odaki 2021-02-25 8:34 ` Stefano Garzarella @ 2021-03-08 16:52 ` Stefan Hajnoczi 1 sibling, 0 replies; 10+ messages in thread From: Stefan Hajnoczi @ 2021-03-08 16:52 UTC (permalink / raw) To: Akihiko Odaki Cc: Kevin Wolf, Eduardo Habkost, qemu-block, Michael S . Tsirkin, qemu-devel, Max Reitz, Stefano Garzarella [-- Attachment #1: Type: text/plain, Size: 763 bytes --] On Thu, Feb 25, 2021 at 09:12:39AM +0900, Akihiko Odaki wrote: > Report the configured granularity for discard operation to the > guest. If this is not set use the block size. > > Since until now we have ignored the configured discard granularity > and always reported the block size, let's add > 'report-discard-granularity' property and disable it for older > machine types to avoid migration issues. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> > --- > hw/block/virtio-blk.c | 8 +++++++- > hw/core/machine.c | 4 +++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 11 insertions(+), 2 deletions(-) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-08 16:57 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-19 10:19 [PATCH] virtio-blk: Respect discard granularity Akihiko Odaki 2021-02-22 16:49 ` Stefan Hajnoczi 2021-02-23 5:36 ` [PATCH v2] " Akihiko Odaki 2021-02-23 11:24 ` Stefano Garzarella 2021-02-23 12:09 ` [PATCH v3] " Akihiko Odaki 2021-02-23 14:26 ` Michael S. Tsirkin 2021-02-24 10:17 ` Stefano Garzarella 2021-02-25 0:12 ` [PATCH v4] " Akihiko Odaki 2021-02-25 8:34 ` Stefano Garzarella 2021-03-08 16:52 ` Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.