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

[Qemu-devel] [PATCH 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

Benoît Canet (3):
  block: Add bdrv_have_block_jobs() 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     |   14 ++++++++++++++
 block.h     |    2 ++
 migration.c |    5 +++++
 qerror.c    |    4 ++++
 qerror.h    |    3 +++
 5 files changed, 28 insertions(+)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/3] block: Add bdrv_have_block_jobs() so migration code abort if needed.

[benoit.canet]

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

qmp_migrate() will be able to check if some block jobs are
running using bdrv_have_block_jobs() and abort safely if needed.

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

diff --git a/block.c b/block.c
index ce7eb8f..2b7b999 100644
--- a/block.c
+++ b/block.c
@@ -4027,6 +4027,20 @@ out:
     return ret;
 }
 
+bool bdrv_have_block_jobs(void)
+{
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        if (bs->job || bdrv_in_use(bs)) {
+            return true;
+        }
+        bdrv_close(bs);
+    }
+
+    return false;
+}
+
 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..1335a2b 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);
 
+bool bdrv_have_block_jobs(void);
+
 enum BlockAcctType {
     BDRV_ACCT_READ,
     BDRV_ACCT_WRITE,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 1/3] block: Add bdrv_have_block_jobs() so migration code abort if needed.

[Paolo Bonzini]

Il 20/07/2012 21:32, benoit.canet@gmail.com ha scritto:
> +bool bdrv_have_block_jobs(void)
> +{
> +    BlockDriverState *bs;
> +
> +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
> +        if (bs->job || bdrv_in_use(bs)) {
> +            return true;
> +        }
> +        bdrv_close(bs);

Why close the device here?

Paolo

> +    }
> +
> +    return false;
> +}
> +

Re: [Qemu-devel] [PATCH 1/3] block: Add bdrv_have_block_jobs() so migration code abort if needed.

[Benoît Canet]

> Why close the device here?

Thanks

Benoît

[Qemu-devel] [PATCH 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 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..dfce680 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_have_block_jobs()) {
+        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 3/3] migration: block migration when streaming block jobs are running.

[Stefan Hajnoczi]

On Fri, Jul 20, 2012 at 8:32 PM,  <benoit.canet@gmail.com> wrote:
> 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..dfce680 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_have_block_jobs()) {
> +        error_set(errp, QERR_STREAMING_BLOCKS_MIGRATION);
> +        return;
> +    }

I think bdrv_have_block_jobs() is too specific and would use
bdrv_in_use(bs) here to give basically an EBUSY-type error.

Stefan

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

[Benoît Canet]

Would

int bdrv_are_busy(void)
{
    BlockDriverState *bs;

    QTAILQ_FOREACH(bs, &bdrv_states, list) {
        if (bs->job || bdrv_in_use(bs)) {
            return -EBUSY;
        }
    }

    return 0;
}

be more acceptable ?

Benoît

Le Monday 23 Jul 2012 à 10:55:41 (+0100), Stefan Hajnoczi a écrit :
> On Fri, Jul 20, 2012 at 8:32 PM,  <benoit.canet@gmail.com> wrote:
> > 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..dfce680 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_have_block_jobs()) {
> > +        error_set(errp, QERR_STREAMING_BLOCKS_MIGRATION);
> > +        return;
> > +    }
> 
> I think bdrv_have_block_jobs() is too specific and would use
> bdrv_in_use(bs) here to give basically an EBUSY-type error.
> 
> Stefan

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

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 11:17 AM, Benoît Canet <benoit.canet@irqsave.net> wrote:
> Would
>
> int bdrv_are_busy(void)
> {
>     BlockDriverState *bs;
>
>     QTAILQ_FOREACH(bs, &bdrv_states, list) {
>         if (bs->job || bdrv_in_use(bs)) {
>             return -EBUSY;
>         }
>     }
>
>     return 0;
> }
>
> be more acceptable ?

Looks fine to me.

Stefan