qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Sam Eiderman <shmuel.eiderman@oracle.com>
To: kwolf@redhat.com, mreitz@redhat.com, qemu-block@nongnu.org,
	qemu-devel@nongnu.org
Cc: shmuel.eiderman@oracle.com, eyal.moscovici@oracle.com,
	sagi.amit@oracle.com, karl.heubaum@oracle.com,
	liran.alon@oracle.com, arbel.moshe@oracle.com
Subject: [Qemu-devel] [PATCH 2/3] qemu-img: rebase: Reduce reads on in-chain rebase
Date: Thu,  2 May 2019 11:50:28 +0300	[thread overview]
Message-ID: <20190502085029.30776-3-shmuel.eiderman@oracle.com> (raw)
In-Reply-To: <20190502085029.30776-1-shmuel.eiderman@oracle.com>

In the following case:

(base) A <- B <- C (tip)

when running:

    qemu-img rebase -b A C

QEMU would read all sectors not allocated in the file being rebased (C)
and compare them to the new base image (A), regardless of whether they
were changed or even allocated anywhere along the chain between the new
base and the top image (B). This causes many unneeded reads when
rebasing an image which represents a small diff of a large disk, as it
would read most of the disk's sectors.

Instead, use bdrv_is_allocated_above() to reduce the number of
unnecessary reads.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
Signed-off-by: Eyal Moscovici <eyal.moscovici@oracle.com>
---
 qemu-img.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index d9b609b3f0..7f20858cb9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3152,7 +3152,7 @@ static int img_rebase(int argc, char **argv)
     BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
     uint8_t *buf_old = NULL;
     uint8_t *buf_new = NULL;
-    BlockDriverState *bs = NULL;
+    BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
     char *filename;
     const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
     int c, flags, src_flags, ret;
@@ -3343,6 +3343,12 @@ static int img_rebase(int argc, char **argv)
                 goto out;
             }
 
+            /*
+             * Find out whether we rebase an image on top of a previous image
+             * in its chain.
+             */
+            prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
+
             blk_new_backing = blk_new_open(out_real_path, NULL,
                                            options, src_flags, &local_err);
             g_free(out_real_path);
@@ -3422,6 +3428,23 @@ static int img_rebase(int argc, char **argv)
                 continue;
             }
 
+            if (prefix_chain_bs) {
+                /*
+                 * If cluster wasn't changed since prefix_chain, we don't need
+                 * to take action
+                 */
+                ret = bdrv_is_allocated_above(bs, prefix_chain_bs,
+                                              offset, n, &n);
+                if (ret < 0) {
+                    error_report("error while reading image metadata: %s",
+                                 strerror(-ret));
+                    goto out;
+                }
+                if (!ret) {
+                    continue;
+                }
+            }
+
             /*
              * Read old and new backing file and take into consideration that
              * backing files may be smaller than the COW image.
-- 
2.13.3

WARNING: multiple messages have this Message-ID (diff)
From: Sam Eiderman <shmuel.eiderman@oracle.com>
To: kwolf@redhat.com, mreitz@redhat.com, qemu-block@nongnu.org,
	qemu-devel@nongnu.org
Cc: eyal.moscovici@oracle.com, arbel.moshe@oracle.com,
	liran.alon@oracle.com, shmuel.eiderman@oracle.com,
	sagi.amit@oracle.com, karl.heubaum@oracle.com
Subject: [Qemu-devel] [PATCH 2/3] qemu-img: rebase: Reduce reads on in-chain rebase
Date: Thu,  2 May 2019 11:50:28 +0300	[thread overview]
Message-ID: <20190502085029.30776-3-shmuel.eiderman@oracle.com> (raw)
Message-ID: <20190502085028.7aKo24JORC_h5ZyO7Ro6iP0JaIHYPE7mNWdB4r07UiQ@z> (raw)
In-Reply-To: <20190502085029.30776-1-shmuel.eiderman@oracle.com>

In the following case:

(base) A <- B <- C (tip)

when running:

    qemu-img rebase -b A C

QEMU would read all sectors not allocated in the file being rebased (C)
and compare them to the new base image (A), regardless of whether they
were changed or even allocated anywhere along the chain between the new
base and the top image (B). This causes many unneeded reads when
rebasing an image which represents a small diff of a large disk, as it
would read most of the disk's sectors.

Instead, use bdrv_is_allocated_above() to reduce the number of
unnecessary reads.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
Signed-off-by: Eyal Moscovici <eyal.moscovici@oracle.com>
---
 qemu-img.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index d9b609b3f0..7f20858cb9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3152,7 +3152,7 @@ static int img_rebase(int argc, char **argv)
     BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
     uint8_t *buf_old = NULL;
     uint8_t *buf_new = NULL;
-    BlockDriverState *bs = NULL;
+    BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
     char *filename;
     const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
     int c, flags, src_flags, ret;
@@ -3343,6 +3343,12 @@ static int img_rebase(int argc, char **argv)
                 goto out;
             }
 
+            /*
+             * Find out whether we rebase an image on top of a previous image
+             * in its chain.
+             */
+            prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
+
             blk_new_backing = blk_new_open(out_real_path, NULL,
                                            options, src_flags, &local_err);
             g_free(out_real_path);
@@ -3422,6 +3428,23 @@ static int img_rebase(int argc, char **argv)
                 continue;
             }
 
+            if (prefix_chain_bs) {
+                /*
+                 * If cluster wasn't changed since prefix_chain, we don't need
+                 * to take action
+                 */
+                ret = bdrv_is_allocated_above(bs, prefix_chain_bs,
+                                              offset, n, &n);
+                if (ret < 0) {
+                    error_report("error while reading image metadata: %s",
+                                 strerror(-ret));
+                    goto out;
+                }
+                if (!ret) {
+                    continue;
+                }
+            }
+
             /*
              * Read old and new backing file and take into consideration that
              * backing files may be smaller than the COW image.
-- 
2.13.3



  parent reply	other threads:[~2019-05-02  8:53 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-02  8:50 [Qemu-devel] [PATCH 0/3] qemu-img: rebase: Improve/optimize rebase operation Sam Eiderman
2019-05-02  8:50 ` Sam Eiderman
2019-05-02  8:50 ` [Qemu-devel] [PATCH 1/3] qemu-img: rebase: Reuse parent BlockDriverState Sam Eiderman
2019-05-02  8:50   ` Sam Eiderman
2019-05-02  8:50 ` Sam Eiderman [this message]
2019-05-02  8:50   ` [Qemu-devel] [PATCH 2/3] qemu-img: rebase: Reduce reads on in-chain rebase Sam Eiderman
2019-05-02  8:50 ` [Qemu-devel] [PATCH 3/3] qemu-img: rebase: Reuse in-chain BlockDriverState Sam Eiderman
2019-05-02  8:50   ` Sam Eiderman
2019-05-02 13:58 ` [Qemu-devel] [PATCH v2 0/3] qemu-img: rebase: Improve/optimize rebase operation Sam Eiderman
2019-05-02 13:58   ` Sam Eiderman
2019-05-02 13:58   ` [Qemu-devel] [PATCH v2 1/3] qemu-img: rebase: Reuse parent BlockDriverState Sam Eiderman
2019-05-02 13:58     ` Sam Eiderman
2019-05-23 13:07     ` Max Reitz
2019-05-02 13:58   ` [Qemu-devel] [PATCH v2 2/3] qemu-img: rebase: Reduce reads on in-chain rebase Sam Eiderman
2019-05-02 13:58     ` Sam Eiderman
2019-05-23 14:01     ` Max Reitz
2019-05-23 14:09       ` Sam Eiderman
2019-05-23 14:26         ` Max Reitz
2019-05-23 15:37           ` Sam Eiderman
2019-05-02 13:58   ` [Qemu-devel] [PATCH v2 3/3] qemu-img: rebase: Reuse in-chain BlockDriverState Sam Eiderman
2019-05-02 13:58     ` Sam Eiderman
2019-05-23 14:04     ` Max Reitz
2019-05-02 14:21   ` [Qemu-devel] [PATCH v2 0/3] qemu-img: rebase: Improve/optimize rebase operation Eric Blake
2019-05-02 14:21     ` Eric Blake
2019-05-02 14:38     ` Sam
2019-05-02 14:38       ` Sam
2019-05-19  8:34     ` Sam Eiderman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190502085029.30776-3-shmuel.eiderman@oracle.com \
    --to=shmuel.eiderman@oracle.com \
    --cc=arbel.moshe@oracle.com \
    --cc=eyal.moscovici@oracle.com \
    --cc=karl.heubaum@oracle.com \
    --cc=kwolf@redhat.com \
    --cc=liran.alon@oracle.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sagi.amit@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).