[PATCH 1/2] block: propagate discard alignment from format drivers to the guest

From: "Denis V. Lunev" <den@openvz.org>
To: qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>, John Snow <jsnow@redhat.com>
Subject: [PATCH 1/2] block: propagate discard alignment from format drivers to the guest
Date: Thu, 11 Jun 2020 20:16:07 +0300	[thread overview]
Message-ID: <20200611171608.22052-2-den@openvz.org> (raw)
In-Reply-To: <20200611171608.22052-1-den@openvz.org>

Nowaday SCSI drivers in guests are able to align UNMAP requests before
sending to the device. Right now QEMU provides an ability to set
this via "discard_granularity" property of the block device which could
be used by management layer.

Though, in particular, from the point of QEMU, there is
pdiscard_granularity on the format driver level, f.e. on QCOW2 or iSCSI.
It would be beneficial to pass this value as a default for this
property.

Technically this should reduce the amount of use less UNMAP requests
from the guest to the host. Basic test confirms this.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
CC: John Snow <jsnow@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Fam Zheng <fam@euphon.net>
---
 block/block-backend.c          | 11 +++++++++++
 hw/core/machine.c              | 15 ++++++++++++++-
 hw/ide/qdev.c                  |  3 ++-
 hw/scsi/scsi-disk.c            |  5 ++++-
 include/hw/block/block.h       |  2 +-
 include/sysemu/block-backend.h |  6 ++++++
 6 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6936b25c83..9342a475cb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2222,6 +2222,17 @@ int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo)
     return bdrv_probe_geometry(blk_bs(blk), geo);
 }
 
+int blk_discard_granularity(BlockBackend *blk)
+{
+    BlockDriverState *bs = blk_bs(blk);
+
+    if (bs == NULL) {
+        return DEFAULT_DISCARD_GRANULARITY;
+    }
+
+    return bs->bl.pdiscard_alignment;
+}
+
 /*
  * Updates the BlockBackendRootState object with data from the currently
  * attached BlockDriverState.
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bb3a7b18b1..08a242d606 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,7 +28,20 @@
 #include "hw/mem/nvdimm.h"
 #include "migration/vmstate.h"
 
-GlobalProperty hw_compat_5_0[] = {};
+GlobalProperty hw_compat_5_0[] = {
+    { "ide-cd", "discard_granularity", "0xffffffff" },
+    { "ide-hd", "discard_granularity", "0xffffffff" },
+    { "ide-drive", "discard_granularity", "0xffffffff" },
+    { "scsi-hd", "discard_granularity", "0xffffffff" },
+    { "scsi-cd", "discard_granularity", "0xffffffff" },
+    { "scsi-disk", "discard_granularity", "0xffffffff" },
+    { "virtio-blk-pci", "discard_granularity", "0xffffffff" },
+    { "xen-block", "discard_granularity", "0xffffffff" },
+    { "usb-storage", "discard_granularity", "0xffffffff" },
+    { "swim-drive", "discard_granularity", "0xffffffff" },
+    { "floppy", "discard_granularity", "0xffffffff" },
+    { "nvme", "discard_granularity", "0xffffffff" },
+};
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
 
 GlobalProperty hw_compat_4_2[] = {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 06b11583f5..e515dbeb0e 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -179,7 +179,8 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         }
     }
 
-    if (dev->conf.discard_granularity == -1) {
+    if (dev->conf.discard_granularity == -1 ||
+        dev->conf.discard_granularity == -2) {
         dev->conf.discard_granularity = 512;
     } else if (dev->conf.discard_granularity &&
                dev->conf.discard_granularity != 512) {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 387503e11b..6b809608e4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -48,7 +48,6 @@
 #define SCSI_MAX_INQUIRY_LEN        256
 #define SCSI_MAX_MODE_LEN           256
 
-#define DEFAULT_DISCARD_GRANULARITY (4 * KiB)
 #define DEFAULT_MAX_UNMAP_SIZE      (1 * GiB)
 #define DEFAULT_MAX_IO_SIZE         INT_MAX     /* 2 GB - 1 block */
 
@@ -2381,6 +2380,10 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     if (s->qdev.conf.discard_granularity == -1) {
         s->qdev.conf.discard_granularity =
             MAX(s->qdev.conf.logical_block_size, DEFAULT_DISCARD_GRANULARITY);
+    } else if (s->qdev.conf.discard_granularity == -2) {
+        s->qdev.conf.discard_granularity =
+            MAX(s->qdev.conf.logical_block_size,
+                blk_discard_granularity(s->qdev.conf.blk));
     }
 
     if (!s->version) {
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index d7246f3862..53d4a38044 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -54,7 +54,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
     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("discard_granularity", _state,                   \
-                       _conf.discard_granularity, -1),                  \
+                       _conf.discard_granularity, -2),                  \
     DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce,           \
                             ON_OFF_AUTO_AUTO),                          \
     DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8203d7f6f9..241a759432 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -13,6 +13,7 @@
 #ifndef BLOCK_BACKEND_H
 #define BLOCK_BACKEND_H
 
+#include "qemu/units.h"
 #include "qemu/iov.h"
 #include "block/throttle-groups.h"
 
@@ -25,6 +26,10 @@
  */
 #include "block/block.h"
 
+
+#define DEFAULT_DISCARD_GRANULARITY (4 * KiB)
+
+
 /* Callbacks for block device models */
 typedef struct BlockDevOps {
     /*
@@ -246,6 +251,7 @@ int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
 int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
 int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
 int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
+int blk_discard_granularity(BlockBackend *blk);
 BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
                                   BlockCompletionFunc *cb,
                                   void *opaque, int ret);
-- 
2.17.1


