qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [v5 1/3] block/file-posix: Optimize for macOS
@ 2021-04-02 16:02 Akihiko Odaki
  2021-04-02 16:02 ` [v5 2/3] block: Add backend_defaults property Akihiko Odaki
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Akihiko Odaki @ 2021-04-02 16:02 UTC (permalink / raw)
  Cc: Kevin Wolf, Fam Zheng, pkrempa, Akihiko Odaki, qemu-block,
	Markus Armbruster, qemu-devel, Max Reitz, Konstantin Nazarov,
	Stefan Hajnoczi, John Snow, dgilbert

This commit introduces "punch hole" operation and optimizes transfer
block size for macOS.

Thanks to Konstantin Nazarov for detailed analysis of a flaw in an
old version of this change:
https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3654667

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 block/file-posix.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 05079b40cae..89e8ed9f801 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -44,6 +44,7 @@
 #if defined(__APPLE__) && (__MACH__)
 #include <paths.h>
 #include <sys/param.h>
+#include <sys/mount.h>
 #include <IOKit/IOKitLib.h>
 #include <IOKit/IOBSD.h>
 #include <IOKit/storage/IOMediaBSDClient.h>
@@ -1248,6 +1249,15 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
 
+#if defined(__APPLE__) && (__MACH__)
+    struct statfs buf;
+
+    if (!fstatfs(s->fd, &buf)) {
+        bs->bl.opt_transfer = buf.f_iosize;
+        bs->bl.pdiscard_alignment = buf.f_bsize;
+    }
+#endif
+
     if (bs->sg) {
         int ret = sg_get_max_transfer_length(s->fd);
 
@@ -1586,6 +1596,7 @@ out:
     }
 }
 
+#if defined(CONFIG_FALLOCATE) || defined(BLKZEROOUT) || defined(BLKDISCARD)
 static int translate_err(int err)
 {
     if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP ||
@@ -1594,6 +1605,7 @@ static int translate_err(int err)
     }
     return err;
 }
+#endif
 
 #ifdef CONFIG_FALLOCATE
 static int do_fallocate(int fd, int mode, off_t offset, off_t len)
@@ -1795,16 +1807,27 @@ static int handle_aiocb_discard(void *opaque)
             }
         } while (errno == EINTR);
 
-        ret = -errno;
+        ret = translate_err(-errno);
 #endif
     } else {
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
         ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
                            aiocb->aio_offset, aiocb->aio_nbytes);
+        ret = translate_err(-errno);
+#elif defined(__APPLE__) && (__MACH__)
+        fpunchhole_t fpunchhole;
+        fpunchhole.fp_flags = 0;
+        fpunchhole.reserved = 0;
+        fpunchhole.fp_offset = aiocb->aio_offset;
+        fpunchhole.fp_length = aiocb->aio_nbytes;
+        if (fcntl(s->fd, F_PUNCHHOLE, &fpunchhole) == -1) {
+            ret = errno == ENODEV ? -ENOTSUP : -errno;
+        } else {
+            ret = 0;
+        }
 #endif
     }
 
-    ret = translate_err(ret);
     if (ret == -ENOTSUP) {
         s->has_discard = false;
     }
-- 
2.24.3 (Apple Git-128)



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [v5 2/3] block: Add backend_defaults property
  2021-04-02 16:02 [v5 1/3] block/file-posix: Optimize for macOS Akihiko Odaki
@ 2021-04-02 16:02 ` Akihiko Odaki
  2021-04-02 16:02 ` [v5 3/3] block/io: Merge discard request alignments Akihiko Odaki
  2021-07-05 10:39 ` [v5 1/3] block/file-posix: Optimize for macOS Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Akihiko Odaki @ 2021-04-02 16:02 UTC (permalink / raw)
  Cc: Kevin Wolf, Fam Zheng, pkrempa, Akihiko Odaki, qemu-block,
	Markus Armbruster, qemu-devel, Max Reitz, Konstantin Nazarov,
	Stefan Hajnoczi, John Snow, dgilbert

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 for block
sizes and ignores the others, which is consistent with the older
versions.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 hw/block/block.c         | 42 ++++++++++++++++++++++++++++++++++++----
 include/hw/block/block.h |  3 +++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index 1e34573da71..d47ebf005ad 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -65,24 +65,58 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
 {
     BlockBackend *blk = conf->blk;
     BlockSizes blocksizes;
-    int backend_ret;
+    BlockDriverState *bs;
+    bool use_blocksizes;
+    bool use_bs;
+
+    switch (conf->backend_defaults) {
+    case ON_OFF_AUTO_AUTO:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+        use_bs = false;
+        break;
+
+    case ON_OFF_AUTO_ON:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+        bs = blk_bs(blk);
+        use_bs = bs;
+        break;
+
+    case ON_OFF_AUTO_OFF:
+        use_blocksizes = false;
+        use_bs = 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) {
+        if (use_blocksizes) {
            conf->physical_block_size = blocksizes.phys;
         } else {
             conf->physical_block_size = BDRV_SECTOR_SIZE;
         }
     }
     if (!conf->logical_block_size) {
-        if (!backend_ret) {
+        if (use_blocksizes) {
             conf->logical_block_size = blocksizes.log;
         } else {
             conf->logical_block_size = BDRV_SECTOR_SIZE;
         }
     }
+    if (use_bs) {
+        if (!conf->opt_io_size) {
+            conf->opt_io_size = bs->bl.opt_transfer;
+        }
+        if (conf->discard_granularity == -1) {
+            if (bs->bl.pdiscard_alignment) {
+                conf->discard_granularity = bs->bl.pdiscard_alignment;
+            } else if (bs->bl.request_alignment != 1) {
+                conf->discard_granularity = bs->bl.request_alignment;
+            }
+        }
+    }
 
     if (conf->logical_block_size > conf->physical_block_size) {
         error_setg(errp,
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,                \
-- 
2.24.3 (Apple Git-128)



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [v5 3/3] block/io: Merge discard request alignments
  2021-04-02 16:02 [v5 1/3] block/file-posix: Optimize for macOS Akihiko Odaki
  2021-04-02 16:02 ` [v5 2/3] block: Add backend_defaults property Akihiko Odaki
@ 2021-04-02 16:02 ` Akihiko Odaki
  2021-07-05 10:39 ` [v5 1/3] block/file-posix: Optimize for macOS Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Akihiko Odaki @ 2021-04-02 16:02 UTC (permalink / raw)
  Cc: Kevin Wolf, Fam Zheng, pkrempa, Akihiko Odaki, qemu-block,
	Markus Armbruster, qemu-devel, Max Reitz, Konstantin Nazarov,
	Stefan Hajnoczi, John Snow, dgilbert

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 block/io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/io.c b/block/io.c
index ca2dca30070..aad74bd6714 100644
--- a/block/io.c
+++ b/block/io.c
@@ -124,6 +124,8 @@ void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll)
 
 static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
 {
+    dst->pdiscard_alignment = MAX(dst->pdiscard_alignment,
+                                  src->pdiscard_alignment);
     dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
     dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer);
     dst->opt_mem_alignment = MAX(dst->opt_mem_alignment,
-- 
2.24.3 (Apple Git-128)



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [v5 1/3] block/file-posix: Optimize for macOS
  2021-04-02 16:02 [v5 1/3] block/file-posix: Optimize for macOS Akihiko Odaki
  2021-04-02 16:02 ` [v5 2/3] block: Add backend_defaults property Akihiko Odaki
  2021-04-02 16:02 ` [v5 3/3] block/io: Merge discard request alignments Akihiko Odaki
@ 2021-07-05 10:39 ` Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2021-07-05 10:39 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: Kevin Wolf, Fam Zheng, pkrempa, qemu-block, Markus Armbruster,
	qemu-devel, Max Reitz, Konstantin Nazarov, shajnocz, John Snow,
	dgilbert

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

On Fri, Apr 02, 2021 at 09:02:54AM -0700, Akihiko Odaki wrote:
> This commit introduces "punch hole" operation and optimizes transfer
> block size for macOS.
> 
> Thanks to Konstantin Nazarov for detailed analysis of a flaw in an
> old version of this change:
> https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3654667
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  block/file-posix.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)

I encountered the following qemu-iotests failure after applying this
series:

172                             fail       [11:09:32] [11:09:36]   3.6s                 output mismatch (see 172.out.bad)
--- /home/stefanha/qemu/tests/qemu-iotests/172.out
+++ 172.out.bad
@@ -21,6 +21,7 @@
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
...

You can run the test case like this:

  $ (cd tests/qemu-iotests && ./check -qcow2 172)

It looks like the 172.out test output file just needs to be updated to
include the new backend_defaults property.

Stefan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-07-05 10:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02 16:02 [v5 1/3] block/file-posix: Optimize for macOS Akihiko Odaki
2021-04-02 16:02 ` [v5 2/3] block: Add backend_defaults property Akihiko Odaki
2021-04-02 16:02 ` [v5 3/3] block/io: Merge discard request alignments Akihiko Odaki
2021-07-05 10:39 ` [v5 1/3] block/file-posix: Optimize for macOS Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).