[v3,2/2] block: Add backend_defaults property

Commit Message

Akihiko Odaki March 10, 2021, 3:39 p.m. UTC

backend_defaults property allow users to control if default block
properties should be decided with backend information.

If it is off, any backend information will be discarded, which is
suitable if you plan to perform live migration to a different disk backend.

If it is on, a block device may utilize backend information more
aggressively.

By default, it is auto, which uses backend information from physical
devices and ignores one from file. It is consistent with the older
versions, and should be aligned with the user expectation that a file
backend is more independent of host than a physical device backend.

Signed-off-by: Akihiko Odaki <[e-mail]>
---
 block/file-posix.c       |  2 ++
 block/nvme.c             |  1 +
 hw/block/block.c         | 27 ++++++++++++++++++++++-----
 include/block/block.h    |  1 +
 include/hw/block/block.h |  3 +++
 5 files changed, 29 insertions(+), 5 deletions(-)

Comments

Stefan Hajnoczi April 1, 2021, 3:58 p.m. UTC | #1

On Thu, Mar 11, 2021 at 12:39:16AM +0900, Akihiko Odaki wrote:
> backend_defaults property allow users to control if default block
> properties should be decided with backend information.
> 
> If it is off, any backend information will be discarded, which is
> suitable if you plan to perform live migration to a different disk backend.
> 
> If it is on, a block device may utilize backend information more
> aggressively.
> 
> By default, it is auto, which uses backend information from physical
> devices and ignores one from file. It is consistent with the older
> versions, and should be aligned with the user expectation that a file
> backend is more independent of host than a physical device backend.

Can BlockLimits pdiscard_alignment and opt_transfer be used instead of
adding discard_granularity and opt_io fields to BlockSizes? They seem to
do the same thing except the QEMU block layer currently uses BlockLimits
values internally to ensure that requests are suitable for the host
device.

Patch

diff --git a/block/file-posix.c b/block/file-posix.c
index 21bdaf969c5..2631bd972a5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1293,6 +1293,7 @@  static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
     if (check_for_dasd(s->fd) < 0) {
         return -ENOTSUP;
     }
+    bsz->file = false;
     bsz->opt_io = 0;
     bsz->discard_granularity = -1;
     ret = probe_logical_blocksize(s->fd, &bsz->log);
@@ -2135,6 +2136,7 @@  static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
     struct statfs buf;
 
     if (!fstatfs(s->fd, &buf)) {
+        bsz->file = true;
         bsz->phys = 0;
         bsz->log = 0;
         bsz->opt_io = buf.f_iosize;
diff --git a/block/nvme.c b/block/nvme.c
index 1845d07577b..8438c8a2b90 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -987,6 +987,7 @@  static uint32_t nvme_get_blocksize(BlockDriverState *bs)
 static int nvme_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
 {
     uint32_t blocksize = nvme_get_blocksize(bs);
+    bsz->file = false;
     bsz->phys = blocksize;
     bsz->log = blocksize;
     bsz->opt_io = 0;
diff --git a/hw/block/block.c b/hw/block/block.c
index c907e5a7722..d42cc40a1c8 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -65,25 +65,42 @@  bool blkconf_blocksizes(BlockConf *conf, Error **errp)
 {
     BlockBackend *blk = conf->blk;
     BlockSizes blocksizes;
-    int backend_ret;
+    bool use_blocksizes;
+
+    switch (conf->backend_defaults) {
+    case ON_OFF_AUTO_AUTO:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes) &&
+                         !blocksizes.file;
+        break;
+
+    case ON_OFF_AUTO_ON:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+        break;
+
+    case ON_OFF_AUTO_OFF:
+        use_blocksizes = false;
+        break;
+
+    default:
+        abort();
+    }
 
-    backend_ret = blk_probe_blocksizes(blk, &blocksizes);
     /* fill in detected values if they are not defined via qemu command line */
     if (!conf->physical_block_size) {
-        if (!backend_ret && blocksizes.phys) {
+        if (use_blocksizes && blocksizes.phys) {
            conf->physical_block_size = blocksizes.phys;
         } else {
             conf->physical_block_size = BDRV_SECTOR_SIZE;
         }
     }
     if (!conf->logical_block_size) {
-        if (!backend_ret && blocksizes.log) {
+        if (use_blocksizes && blocksizes.log) {
             conf->logical_block_size = blocksizes.log;
         } else {
             conf->logical_block_size = BDRV_SECTOR_SIZE;
         }
     }
-    if (!backend_ret) {
+    if (use_blocksizes) {
         if (!conf->opt_io_size) {
             conf->opt_io_size = blocksizes.opt_io;
         }
diff --git a/include/block/block.h b/include/block/block.h
index d12471a6cc4..ccc3ec5c847 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -91,6 +91,7 @@  typedef enum {
 } BdrvRequestFlags;
 
 typedef struct BlockSizes {
+    bool file;
     uint32_t phys;
     uint32_t log;
     uint32_t discard_granularity;
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index c172cbe65f1..5902c0440a5 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -19,6 +19,7 @@ 
 
 typedef struct BlockConf {
     BlockBackend *blk;
+    OnOffAuto backend_defaults;
     uint32_t physical_block_size;
     uint32_t logical_block_size;
     uint32_t min_io_size;
@@ -48,6 +49,8 @@  static inline unsigned int get_physical_block_exp(BlockConf *conf)
 }
 
 #define DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)                     \
+    DEFINE_PROP_ON_OFF_AUTO("backend_defaults", _state,                 \
+                            _conf.backend_defaults, ON_OFF_AUTO_AUTO),  \
     DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
                           _conf.logical_block_size),                    \
     DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \