[Qemu-devel] [PATCH V2 0/3] Block migration when streaming block jobs are running

[Qemu-devel] [PATCH V2 0/3] Block migration when streaming block jobs are running

[benoit.canet]

From: Benoît Canet <benoit@irqsave.net>

This patchset is designed to avoid starting a live migration while one or more
streaming block jobs are running.

Tested with the following sequence:
QEMU 1.1.50 monitor - type 'help' for more information
(qemu) block_stream virtio0 1k
(qemu) migrate tcp:localhost:4444
migrate: Streaming blocks migration
(qemu) migrate tcp:localhost:4444
migrate: Streaming blocks migration
(qemu) block_job_cancel virtio0
(qemu) migrate tcp:localhost:4444
migrate: Connection can not be completed immediately
(qemu)
=> migration then succeed

in v2:
stefanha: Rename bdrv_have_block_jobs() to bdrv_are_busy() and make it return -EBUSY.
paolo: remove spurious bdrv_close()

Benoît Canet (3):
  block: Add bdrv_are_busy() so migration code abort if needed.
  qerror: Add error telling that streaming blocks migration
  migration: block migration when streaming block jobs are running.

 block.c     |   13 +++++++++++++
 block.h     |    2 ++
 migration.c |    5 +++++
 qerror.c    |    4 ++++
 qerror.h    |    3 +++
 5 files changed, 27 insertions(+)

-- 
1.7.9.5

[Qemu-devel] [PATCH V2 1/3] block: Add bdrv_are_busy()

[benoit.canet]

From: Benoît Canet <benoit@irqsave.net>

bdrv_are_busy will be used to check if any of the bs are in use
or if one of them have a running block job.

The first user will be qmp_migrate().

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c |   13 +++++++++++++
 block.h |    2 ++
 2 files changed, 15 insertions(+)

diff --git a/block.c b/block.c
index ce7eb8f..bc8f160 100644
--- a/block.c
+++ b/block.c
@@ -4027,6 +4027,19 @@ out:
     return ret;
 }
 
+int bdrv_are_busy(void)
+{
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        if (bs->job || bdrv_in_use(bs)) {
+            return -EBUSY;
+        }
+    }
+
+    return 0;
+}
+
 void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
                        int64_t speed, BlockDriverCompletionFunc *cb,
                        void *opaque, Error **errp)
diff --git a/block.h b/block.h
index c89590d..0a3de2f 100644
--- a/block.h
+++ b/block.h
@@ -337,6 +337,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
+int bdrv_are_busy(void);
+
 enum BlockAcctType {
     BDRV_ACCT_READ,
     BDRV_ACCT_WRITE,
-- 
1.7.9.5

[Qemu-devel] [PATCH V2 2/3] qerror: Add error telling that streaming blocks migration

[benoit.canet]

From: Benoît Canet <benoit@irqsave.net>

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+)

diff --git a/qerror.c b/qerror.c
index 92c4eff..bcd74b7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -283,6 +283,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Could not set password",
     },
     {
+        .error_fmt = QERR_STREAMING_BLOCKS_MIGRATION,
+        .desc      = "Streaming blocks migration",
+    },
+    {
         .error_fmt = QERR_TOO_MANY_FILES,
         .desc      = "Too many open files",
     },
diff --git a/qerror.h b/qerror.h
index b4c8758..95d9e8d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -233,6 +233,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_SET_PASSWD_FAILED \
     "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
+#define QERR_STREAMING_BLOCKS_MIGRATION \
+    "{ 'class': 'StreamingFeatureBlocksMigration', 'data': {} }"
+
 #define QERR_TOO_MANY_FILES \
     "{ 'class': 'TooManyFiles', 'data': {} }"
 
-- 
1.7.9.5

[Qemu-devel] [PATCH V2 3/3] migration: block migration when streaming block jobs are running.

[benoit.canet]

From: Benoît Canet <benoit@irqsave.net>

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 migration.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/migration.c b/migration.c
index 8db1b43..5196d7e 100644
--- a/migration.c
+++ b/migration.c
@@ -425,6 +425,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
+    if (bdrv_are_busy()) {
+        error_set(errp, QERR_STREAMING_BLOCKS_MIGRATION);
+        return;
+    }
+
     s = migrate_init(&params);
 
     if (strstart(uri, "tcp:", &p)) {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH V2 2/3] qerror: Add error telling that streaming blocks migration

[Peter Maydell]

On 23 July 2012 12:23,  <benoit.canet@gmail.com> wrote:
> --- a/qerror.c
> +++ b/qerror.c
> @@ -283,6 +283,10 @@ static const QErrorStringTable qerror_table[] = {
>          .desc      = "Could not set password",
>      },
>      {
> +        .error_fmt = QERR_STREAMING_BLOCKS_MIGRATION,
> +        .desc      = "Streaming blocks migration",
> +    },

An error should be a description of something that went wrong
("this isn't supported", "I couldn't do X", "couldn't find Y",
"that isn't valid on devices of type Z"). This text isn't
describing anything that's gone wrong in any comprehensible
way.

In particular I just parsed "Streaming blocks migration"
as "(streaming blocks) migration" ie "migration of
streaming blocks". If you meant "Migration was blocked
by streaming" or something similar you need to rephrase.

-- PMM