[Qemu-devel] [PULL for-2.4 0/5] Block layer patches for 2.4.0-rc1

[Qemu-devel] [PULL for-2.4 0/5] Block layer patches for 2.4.0-rc1

[Jeff Cody]

The following changes since commit 661725da09f47eb92d356fac10a4cf3b7ad1f61d:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150714' into staging (2015-07-14 18:50:17 +0100)

are available in the git repository at:


  git@github.com:codyprime/qemu-kvm-jtc.git tags/jtc-for-upstream-pull-request

for you to fetch changes up to 796a060bc0fab40953997976a2e30d9d6235bc7b:

  block/curl: Don't lose original error when a connection fails. (2015-07-14 21:50:13 -0400)

----------------------------------------------------------------
Block layer patches for 2.4.0-rc1
----------------------------------------------------------------

Fam Zheng (1):
  block/mirror: Sleep periodically during bitmap scanning

Paolo Bonzini (1):
  blockdev: no need to drain in qmp_block_commit

Richard W.M. Jones (1):
  block/curl: Don't lose original error when a connection fails.

Stefan Hajnoczi (1):
  block: keep bitmap if incremental backup job is cancelled

Wen Congyang (1):
  mirror: correct buf_size

 block/backup.c |  2 +-
 block/curl.c   | 15 ++++++++++++++-
 block/mirror.c | 24 ++++++++++++++++++++++--
 blockdev.c     |  7 +------
 4 files changed, 38 insertions(+), 10 deletions(-)

-- 
1.9.3

[Qemu-devel] [PULL for-2.4 1/5] block/mirror: Sleep periodically during bitmap scanning

[Jeff Cody]

From: Fam Zheng <famz@redhat.com>

Before, we only yield after initializing dirty bitmap, where the QMP
command would return. That may take very long, and guest IO will be
blocked.

Add sleep points like the later mirror iterations.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1431486673-19280-1-git-send-email-famz@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/mirror.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index d409337..a2700ca 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -444,11 +444,23 @@ static void coroutine_fn mirror_run(void *opaque)
     sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
     mirror_free_init(s);
 
+    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     if (!s->is_none_mode) {
         /* First part, loop on the sectors and initialize the dirty bitmap.  */
         BlockDriverState *base = s->base;
         for (sector_num = 0; sector_num < end; ) {
             int64_t next = (sector_num | (sectors_per_chunk - 1)) + 1;
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
+            if (now - last_pause_ns > SLICE_TIME) {
+                last_pause_ns = now;
+                block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
+            }
+
+            if (block_job_is_cancelled(&s->common)) {
+                goto immediate_exit;
+            }
+
             ret = bdrv_is_allocated_above(bs, base,
                                           sector_num, next - sector_num, &n);
 
@@ -467,7 +479,6 @@ static void coroutine_fn mirror_run(void *opaque)
     }
 
     bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
-    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     for (;;) {
         uint64_t delay_ns = 0;
         int64_t cnt;
-- 
1.9.3

[Qemu-devel] [PULL for-2.4 2/5] blockdev: no need to drain in qmp_block_commit

[Jeff Cody]

From: Paolo Bonzini <pbonzini@redhat.com>

Draining is not necessary, I/O can happen as soon as the
commit coroutine yields.  Draining can be necessary before
reopening the file for read/write, or while modifying the
backing file chain, but that is done separately in
bdrv_reopen_multiple or bdrv_close; this particular
bdrv_drain_all does nothing for that.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1432822903-25821-1-git-send-email-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 7fee519..50421c8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2380,9 +2380,6 @@ void qmp_block_commit(const char *device,
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
-    /* drain all i/o before commits */
-    bdrv_drain_all();
-
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
         goto out;
     }
-- 
1.9.3

[Qemu-devel] [PULL for-2.4 3/5] block: keep bitmap if incremental backup job is cancelled

[Jeff Cody]

From: Stefan Hajnoczi <stefanha@redhat.com>

Reclaim the dirty bitmap if an incremental backup block job is
cancelled.  The ret variable may be 0 when the job is cancelled so it's
not enough to check ret < 0.

Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1434380534-7680-1-git-send-email-stefanha@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index d3c7d9f..965654d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -431,7 +431,7 @@ static void coroutine_fn backup_run(void *opaque)
 
     if (job->sync_bitmap) {
         BdrvDirtyBitmap *bm;
-        if (ret < 0) {
+        if (ret < 0 || block_job_is_cancelled(&job->common)) {
             /* Merge the successor back into the parent, delete nothing. */
             bm = bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap, NULL);
             assert(bm);
-- 
1.9.3

[Qemu-devel] [PULL for-2.4 4/5] mirror: correct buf_size

[Jeff Cody]

From: Wen Congyang <wency@cn.fujitsu.com>

If bus_size is less than 0, the command fails.
If buf_size is 0, use DEFAULT_MIRROR_BUF_SIZE.
If buf_size % granularity is not 0, mirror_free_init() will
do dangerous things.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 5555A588.3080907@cn.fujitsu.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/mirror.c | 11 ++++++++++-
 blockdev.c     |  4 +---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index a2700ca..323f747 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -20,6 +20,7 @@
 
 #define SLICE_TIME    100000000ULL /* ns */
 #define MAX_IN_FLIGHT 16
+#define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
 
 /* The mirroring buffer is a list of granularity-sized chunks.
  * Free chunks are organized in a list.
@@ -701,6 +702,14 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    if (buf_size < 0) {
+        error_setg(errp, "Invalid parameter 'buf-size'");
+        return;
+    }
+
+    if (buf_size == 0) {
+        buf_size = DEFAULT_MIRROR_BUF_SIZE;
+    }
 
     s = block_job_create(driver, bs, speed, cb, opaque, errp);
     if (!s) {
@@ -714,7 +723,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
-    s->buf_size = MAX(buf_size, granularity);
+    s->buf_size = ROUND_UP(buf_size, granularity);
     s->unmap = unmap;
 
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
diff --git a/blockdev.c b/blockdev.c
index 50421c8..62a4586 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2639,8 +2639,6 @@ out:
     aio_context_release(aio_context);
 }
 
-#define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
-
 void qmp_drive_mirror(const char *device, const char *target,
                       bool has_format, const char *format,
                       bool has_node_name, const char *node_name,
@@ -2682,7 +2680,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         granularity = 0;
     }
     if (!has_buf_size) {
-        buf_size = DEFAULT_MIRROR_BUF_SIZE;
+        buf_size = 0;
     }
     if (!has_unmap) {
         unmap = true;
-- 
1.9.3

[Qemu-devel] [PULL for-2.4 5/5] block/curl: Don't lose original error when a connection fails.

[Jeff Cody]

From: "Richard W.M. Jones" <rjones@redhat.com>

Currently if qemu is connected to a curl source (eg. web server), and
the web server fails / times out / dies, you always see a bogus EIO
"Input/output error".

For example, choose a large file located on any local webserver which
you control:

  $ qemu-img convert -p http://example.com/large.iso /tmp/test

Once it starts copying the file, stop the webserver and you will see
qemu-img fail with:

  qemu-img: error while reading sector 61440: Input/output error

This patch does two things: Firstly print the actual error from curl
so it doesn't get lost.  Secondly, change EIO to EPROTO.  EPROTO is a
POSIX.1 compatible errno which more accurately reflects that there was
a protocol error, rather than some kind of hardware failure.

After this patch is applied, the error changes to:

  $ qemu-img convert -p http://example.com/large.iso /tmp/test
  qemu-img: curl: transfer closed with 469989 bytes remaining to read
  qemu-img: error while reading sector 16384: Protocol error

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/curl.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 3a2b63e..032cc8a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "block/block_int.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
@@ -298,6 +299,18 @@ static void curl_multi_check_completion(BDRVCURLState *s)
             /* ACBs for successful messages get completed in curl_read_cb */
             if (msg->data.result != CURLE_OK) {
                 int i;
+                static int errcount = 100;
+
+                /* Don't lose the original error message from curl, since
+                 * it contains extra data.
+                 */
+                if (errcount > 0) {
+                    error_report("curl: %s", state->errmsg);
+                    if (--errcount == 0) {
+                        error_report("curl: further errors suppressed");
+                    }
+                }
+
                 for (i = 0; i < CURL_NUM_ACB; i++) {
                     CURLAIOCB *acb = state->acb[i];
 
@@ -305,7 +318,7 @@ static void curl_multi_check_completion(BDRVCURLState *s)
                         continue;
                     }
 
-                    acb->common.cb(acb->common.opaque, -EIO);
+                    acb->common.cb(acb->common.opaque, -EPROTO);
                     qemu_aio_unref(acb);
                     state->acb[i] = NULL;
                 }
-- 
1.9.3

Re: [Qemu-devel] [PULL for-2.4 0/5] Block layer patches for 2.4.0-rc1

[Peter Maydell]

On 15 July 2015 at 03:26, Jeff Cody <jcody@redhat.com> wrote:
> The following changes since commit 661725da09f47eb92d356fac10a4cf3b7ad1f61d:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150714' into staging (2015-07-14 18:50:17 +0100)
>
> are available in the git repository at:
>
>
>   git@github.com:codyprime/qemu-kvm-jtc.git tags/jtc-for-upstream-pull-request
>
> for you to fetch changes up to 796a060bc0fab40953997976a2e30d9d6235bc7b:
>
>   block/curl: Don't lose original error when a connection fails. (2015-07-14 21:50:13 -0400)
>
> ----------------------------------------------------------------
> Block layer patches for 2.4.0-rc1
> ----------------------------------------------------------------

Applied, thanks.

-- PMM