Series comparison

-[Qemu-devel] [PULL 0/4] Block patches
+[Qemu-devel] [PULL for-4.0 0/1] Block patches
-The following changes since commit 20d6c7312f1b812bb9c750f4087f69ac8485cc90:
+The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
-  Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-3.2-part1' into staging (2019-01-03 13:26:30 +0000)
+  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 21:06:26 +0000)
 are available in the Git repository at:
   git://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 39a0408e768cd00142f5b57d27ab234282bf4df5:
+for you to fetch changes up to f357fcd890a8d6ced6d261338b859a41414561e9:
-  dmg: don't skip zero chunk (2019-01-04 11:15:09 +0000)
+  file-posix: add drop-cache=on|off option (2019-03-13 10:54:55 +0000)
 ----------------------------------------------------------------
 Pull request
-Bug fixes for the .dmg image file format.
+ * Add 'drop-cache=on|off' option to file-posix.c.  The default is on.
    Disabling the option fixes a QEMU 3.0.0 performance regression when live
    migrating on the same host with cache.direct=off.
 ----------------------------------------------------------------
-Julio Faracco (1):
+Stefan Hajnoczi (1):
-  dmg: Fixing wrong dmg block type value for block terminator.
+  file-posix: add drop-cache=on|off option
-yuchenlin (3):
+ qapi/block-core.json |  6 ++++++
-  dmg: fix binary search
+ block/file-posix.c   | 16 ++++++++++++++++
-  dmg: use enumeration type instead of hard coding number
+files changed, 22 insertions(+)
   dmg: don't skip zero chunk
  block/dmg.c | 31 ++++++++++++++++++++-----------
 file changed, 20 insertions(+), 11 deletions(-)
 --
 .20.1

-[Qemu-devel] [PULL 1/4] dmg: Fixing wrong dmg block type value for block terminator.
+Deleted patch
-From: Julio Faracco <jcfaracco@gmail.com>
-This is a trivial patch to fix a wrong value for block terminator.
-The old value was 0x7fffffff which is wrong. It was not affecting the
-code because QEMU dmg block is not handling block terminator right now.
-Neverthless, it should be fixed.
-Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
-Reviewed-by: yuchenlin <yuchenlin@synology.com>
-Message-id: 20181228145055.18039-1-jcfaracco@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/dmg.c | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/dmg.c b/block/dmg.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
-+++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ enum {
-     UDBZ,
-     ULFO,
-     UDCM = 0x7ffffffe, /* Comments */
--    UDLE               /* Last Entry */
-+    UDLE = 0xffffffff  /* Last Entry */
- };
- static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
---
-.20.1

-[Qemu-devel] [PULL 2/4] dmg: fix binary search
+[Qemu-devel] [PULL for-4.0 1/1] file-posix: add drop-cache=on|off option
-From: yuchenlin <npes87184@gmail.com>
+Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix:
 implement bdrv_co_invalidate_cache() on Linux") introduced page cache
 invalidation so that cache.direct=off live migration is safe on Linux.
-There is a possible hang in original binary search implementation. That is
+The invalidation takes a significant amount of time when the file is
-if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
+large and present in the page cache.  Normally this is not the case for
 cross-host live migration but it can happen when migrating between QEMU
 processes on the same host.
-The chunk1 will be still 4, and so on.
+On same-host migration we don't need to invalidate pages for correctness
 anyway, so an option to skip page cache invalidation is useful.  I
 investigated optimizing invalidation and detecting same-host migration,
 but both are hard to achieve so a user-visible option will suffice.
-Signed-off-by: yuchenlin <npes87184@gmail.com>
+As a bonus this option means that the cache invalidation feature will
-Message-id: 20190103114700.9686-2-npes87184@gmail.com
+now be detectable by libvirt via QMP schema introspection.
 Suggested-by: Neil Skrypuch <neil@tembosocial.com>
 Tested-by: Neil Skrypuch <neil@tembosocial.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20190307164941.3322-1-stefanha@redhat.com
 Message-Id: <20190307164941.3322-1-stefanha@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/dmg.c | 10 +++++++---
+ qapi/block-core.json |  6 ++++++
-file changed, 7 insertions(+), 3 deletions(-)
+ block/file-posix.c   | 16 ++++++++++++++++
 files changed, 22 insertions(+)
-diff --git a/block/dmg.c b/block/dmg.c
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
+--- a/qapi/block-core.json
-+++ b/block/dmg.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
+@@ -XXX,XX +XXX,XX @@
- {
+ # @locking:     whether to enable file locking. If set to 'auto', only enable
-     /* binary search */
+ #               when Open File Descriptor (OFD) locking API is available
-     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
+ #               (default: auto, since 2.10)
--    while (chunk1 != chunk2) {
++# @drop-cache:  invalidate page cache during live migration.  This prevents
-+    while (chunk1 <= chunk2) {
++#               stale data on the migration destination with cache.direct=off.
-         chunk3 = (chunk1 + chunk2) / 2;
++#               Currently only supported on Linux hosts.
-         if (s->sectors[chunk3] > sector_num) {
++#               (default: on, since: 4.0)
--            chunk2 = chunk3;
+ # @x-check-cache-dropped: whether to check that page cache was dropped on live
-+            if (chunk3 == 0) {
+ #                         migration.  May cause noticeable delays if the image
-+                goto err;
+ #                         file is large, do not use in production.
-+            }
+@@ -XXX,XX +XXX,XX @@
-+            chunk2 = chunk3 - 1;
+             '*pr-manager': 'str',
-         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
+             '*locking': 'OnOffAuto',
-             return chunk3;
+             '*aio': 'BlockdevAioOptions',
-         } else {
++        '*drop-cache': {'type': 'bool',
--            chunk1 = chunk3;
++                        'if': 'defined(CONFIG_LINUX)'},
-+            chunk1 = chunk3 + 1;
+             '*x-check-cache-dropped': 'bool' } }
  ##
 diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
      bool page_cache_inconsistent:1;
      bool has_fallocate;
      bool needs_alignment;
 +    bool drop_cache;
      bool check_cache_dropped;
      PRManager *pr_mgr;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
  typedef struct BDRVRawReopenState {
      int fd;
      int open_flags;
 +    bool drop_cache;
      bool check_cache_dropped;
  } BDRVRawReopenState;
@@ -XXX,XX +XXX,XX @@ static QemuOptsList raw_runtime_opts = {
              .type = QEMU_OPT_STRING,
              .help = "id of persistent reservation manager object (default: none)",
          },
 +#if defined(__linux__)
 +        {
 +            .name = "drop-cache",
 +            .type = QEMU_OPT_BOOL,
 +            .help = "invalidate page cache during live migration (default: on)",
 +        },
 +#endif
          {
              .name = "x-check-cache-dropped",
              .type = QEMU_OPT_BOOL,
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
          }
      }
-+err:
-     return s->n_chunks; /* error */
++    s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true);
- }
+     s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped",
+                                                false);
@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
          goto out;
      }
 +    rs->drop_cache = qemu_opt_get_bool_del(opts, "drop-cache", true);
      rs->check_cache_dropped =
          qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false);
@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
      BDRVRawState *s = state->bs->opaque;
      Error *local_err = NULL;
 +    s->drop_cache = rs->drop_cache;
      s->check_cache_dropped = rs->check_cache_dropped;
      s->open_flags = rs->open_flags;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs,
          return;
      }
 +    if (!s->drop_cache) {
 +        return;
 +    }
 +
      if (s->open_flags & O_DIRECT) {
          return; /* No host kernel page cache */
      }
 --
 .20.1

-[Qemu-devel] [PULL 3/4] dmg: use enumeration type instead of hard coding number
+Deleted patch
-From: yuchenlin <npes87184@gmail.com>
-Signed-off-by: yuchenlin <npes87184@gmail.com>
-Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20190103114700.9686-3-npes87184@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/dmg.c | 4 ++--
-file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/block/dmg.c b/block/dmg.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
-+++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
-         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-          * therefore be unbounded. */
--        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-+        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-             error_report("sector count %" PRIu64 " for chunk %" PRIu32
-                          " is larger than max (%u)",
-                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
-@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
-          * s->uncompressed_chunk may be too small to cover the large all-zeroes
-          * section. dmg_read_chunk is called to find s->current_chunk */
--        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
-+        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-             qemu_iovec_memset(qiov, i * 512, 0, 512);
-             continue;
-         }
---
-.20.1

-[Qemu-devel] [PULL 4/4] dmg: don't skip zero chunk
+Deleted patch
-From: yuchenlin <npes87184@gmail.com>
-The dmg file has many tables which describe: "start from sector XXX to
-sector XXX, the compression method is XXX and where the compressed data
-resides on".
-Each sector in the expanded file should be covered by a table. The table
-will describe the offset of compressed data (or raw depends on the type)
-in the dmg.
-For example:
-[-----------The expanded file------------]
-[---bzip table ---]/* zeros */[---zlib---]
-    ^
-    | if we want to read this sector.
-we will find bzip table which contains this sector, and get the
-compressed data offset, read it from dmg, uncompress it, finally write to
-expanded file.
-If we skip zero chunk (table), some sector cannot find the table which
-will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
-and finally causing dmg_co_preadv() return EIO.
-See:
-[-----------The expanded file------------]
-[---bzip table ---]/* zeros */[---zlib---]
-                    ^
-                    | if we want to read this sector.
-Oops, we cannot find the table contains it...
-In the original implementation, we don't have zero table. When we try to
-read sector inside the zero chunk. We will get EIO, and skip reading.
-After this patch, we treat zero chunk the same as ignore chunk, it will
-directly write zero and avoid some sector may not find the table.
-After this patch:
-[-----------The expanded file------------]
-[---bzip table ---][--zeros--][---zlib---]
-Signed-off-by: yuchenlin <npes87184@gmail.com>
-Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20190103114700.9686-4-npes87184@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/dmg.c | 19 ++++++++++++-------
-file changed, 12 insertions(+), 7 deletions(-)
-diff --git a/block/dmg.c b/block/dmg.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
-+++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
-     case UDRW: /* copy */
-         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
-         break;
--    case UDIG: /* zero */
-+    case UDZE: /* zero */
-+    case UDIG: /* ignore */
-         /* as the all-zeroes block may be large, it is treated specially: the
-          * sector is not copied from a large buffer, a simple memset is used
-          * instead. Therefore uncompressed_sectors does not need to be set. */
-@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
- static bool dmg_is_known_block_type(uint32_t entry_type)
- {
-     switch (entry_type) {
-+    case UDZE:    /* zeros */
-     case UDRW:    /* uncompressed */
--    case UDIG:    /* zeroes */
-+    case UDIG:    /* ignore */
-     case UDZO:    /* zlib */
-         return true;
-     case UDBZ:    /* bzip2 */
-@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
-         /* sector count */
-         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
--        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
--         * therefore be unbounded. */
--        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-+        /* all-zeroes sector (type UDZE and UDIG) does not need to be
-+         * "uncompressed" and can therefore be unbounded. */
-+        if (s->types[i] != UDZE && s->types[i] != UDIG
-+            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-             error_report("sector count %" PRIu64 " for chunk %" PRIu32
-                          " is larger than max (%u)",
-                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
-@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
-                 return -1;
-             }
-             break;
--        case UDIG: /* zero */
-+        case UDZE: /* zeros */
-+        case UDIG: /* ignore */
-             /* see dmg_read, it is treated specially. No buffer needs to be
-              * pre-filled, the zeroes can be set directly. */
-             break;
-@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
-          * s->uncompressed_chunk may be too small to cover the large all-zeroes
-          * section. dmg_read_chunk is called to find s->current_chunk */
--        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-+        if (s->types[s->current_chunk] == UDZE
-+            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-             qemu_iovec_memset(qiov, i * 512, 0, 512);
-             continue;
-         }
---
-.20.1

From: yuchenlin <npes87184@gmail.com>

There is a possible hang in original binary search implementation. That is
if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.

The chunk1 will be still 4, and so on.

Signed-off-by: yuchenlin <npes87184@gmail.com>
Message-id: 20190103114700.9686-2-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
 {
     /* binary search */
     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
-    while (chunk1 != chunk2) {
+    while (chunk1 <= chunk2) {
         chunk3 = (chunk1 + chunk2) / 2;
         if (s->sectors[chunk3] > sector_num) {
-            chunk2 = chunk3;
+            if (chunk3 == 0) {
+                goto err;
+            }
+            chunk2 = chunk3 - 1;
         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
             return chunk3;
         } else {
-            chunk1 = chunk3;
+            chunk1 = chunk3 + 1;
         }
     }
+err:
     return s->n_chunks; /* error */
 }
 
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-3-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
 
         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
          * therefore be unbounded. */
-        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

The dmg file has many tables which describe: "start from sector XXX to
sector XXX, the compression method is XXX and where the compressed data
resides on".

Each sector in the expanded file should be covered by a table. The table
will describe the offset of compressed data (or raw depends on the type)
in the dmg.

For example:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
    ^
    | if we want to read this sector.

we will find bzip table which contains this sector, and get the
compressed data offset, read it from dmg, uncompress it, finally write to
expanded file.

If we skip zero chunk (table), some sector cannot find the table which
will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
and finally causing dmg_co_preadv() return EIO.

See:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
                    ^
                    | if we want to read this sector.

Oops, we cannot find the table contains it...

In the original implementation, we don't have zero table. When we try to
read sector inside the zero chunk. We will get EIO, and skip reading.

After this patch, we treat zero chunk the same as ignore chunk, it will
directly write zero and avoid some sector may not find the table.

After this patch:

[-----------The expanded file------------]
[---bzip table ---][--zeros--][---zlib---]

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-4-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
     case UDRW: /* copy */
         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
         break;
-    case UDIG: /* zero */
+    case UDZE: /* zero */
+    case UDIG: /* ignore */
         /* as the all-zeroes block may be large, it is treated specially: the
          * sector is not copied from a large buffer, a simple memset is used
          * instead. Therefore uncompressed_sectors does not need to be set. */
@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
 static bool dmg_is_known_block_type(uint32_t entry_type)
 {
     switch (entry_type) {
+    case UDZE:    /* zeros */
     case UDRW:    /* uncompressed */
-    case UDIG:    /* zeroes */
+    case UDIG:    /* ignore */
     case UDZO:    /* zlib */
         return true;
     case UDBZ:    /* bzip2 */
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
         /* sector count */
         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
 
-        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-         * therefore be unbounded. */
-        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        /* all-zeroes sector (type UDZE and UDIG) does not need to be
+         * "uncompressed" and can therefore be unbounded. */
+        if (s->types[i] != UDZE && s->types[i] != UDIG
+            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return -1;
             }
             break;
-        case UDIG: /* zero */
+        case UDZE: /* zeros */
+        case UDIG: /* ignore */
             /* see dmg_read, it is treated specially. No buffer needs to be
              * pre-filled, the zeroes can be set directly. */
             break;
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDZE
+            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1

Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix:
implement bdrv_co_invalidate_cache() on Linux") introduced page cache
invalidation so that cache.direct=off live migration is safe on Linux.

The invalidation takes a significant amount of time when the file is
large and present in the page cache.  Normally this is not the case for
cross-host live migration but it can happen when migrating between QEMU
processes on the same host.

On same-host migration we don't need to invalidate pages for correctness
anyway, so an option to skip page cache invalidation is useful.  I
investigated optimizing invalidation and detecting same-host migration,
but both are hard to achieve so a user-visible option will suffice.

As a bonus this option means that the cache invalidation feature will
now be detectable by libvirt via QMP schema introspection.

Suggested-by: Neil Skrypuch <neil@tembosocial.com>
Tested-by: Neil Skrypuch <neil@tembosocial.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190307164941.3322-1-stefanha@redhat.com
Message-Id: <20190307164941.3322-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json |  6 ++++++
 block/file-posix.c   | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 # @locking:     whether to enable file locking. If set to 'auto', only enable
 #               when Open File Descriptor (OFD) locking API is available
 #               (default: auto, since 2.10)
+# @drop-cache:  invalidate page cache during live migration.  This prevents
+#               stale data on the migration destination with cache.direct=off.
+#               Currently only supported on Linux hosts.
+#               (default: on, since: 4.0)
 # @x-check-cache-dropped: whether to check that page cache was dropped on live
 #                         migration.  May cause noticeable delays if the image
 #                         file is large, do not use in production.
@@ -XXX,XX +XXX,XX @@
             '*pr-manager': 'str',
             '*locking': 'OnOffAuto',
             '*aio': 'BlockdevAioOptions',
+	    '*drop-cache': {'type': 'bool',
+	                    'if': 'defined(CONFIG_LINUX)'},
             '*x-check-cache-dropped': 'bool' } }
 
 ##
diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
     bool page_cache_inconsistent:1;
     bool has_fallocate;
     bool needs_alignment;
+    bool drop_cache;
     bool check_cache_dropped;
 
     PRManager *pr_mgr;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
 typedef struct BDRVRawReopenState {
     int fd;
     int open_flags;
+    bool drop_cache;
     bool check_cache_dropped;
 } BDRVRawReopenState;
 
@@ -XXX,XX +XXX,XX @@ static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "id of persistent reservation manager object (default: none)",
         },
+#if defined(__linux__)
+        {
+            .name = "drop-cache",
+            .type = QEMU_OPT_BOOL,
+            .help = "invalidate page cache during live migration (default: on)",
+        },
+#endif
         {
             .name = "x-check-cache-dropped",
             .type = QEMU_OPT_BOOL,
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         }
     }
 
+    s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true);
     s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped",
                                                false);
 
@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
         goto out;
     }
 
+    rs->drop_cache = qemu_opt_get_bool_del(opts, "drop-cache", true);
     rs->check_cache_dropped =
         qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false);
 
@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
     BDRVRawState *s = state->bs->opaque;
     Error *local_err = NULL;
 
+    s->drop_cache = rs->drop_cache;
     s->check_cache_dropped = rs->check_cache_dropped;
     s->open_flags = rs->open_flags;
 
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs,
         return;
     }
 
+    if (!s->drop_cache) {
+        return;
+    }
+
     if (s->open_flags & O_DIRECT) {
         return; /* No host kernel page cache */
     }
-- 
2.20.1