Patchwork [1/7] migrate: add migration blockers

login
register
mail settings
Submitter Anthony Liguori
Date Nov. 12, 2011, 3:56 p.m.
Message ID <1321113420-3252-1-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/125347/
State New
Headers show

Comments

Anthony Liguori - Nov. 12, 2011, 3:56 p.m.
This lets different subsystems register an Error that is thrown whenever
migration is attempted.  This works nicely because it gracefully supports
things like hotplug.

Right now, if multiple errors are registered, only one of them is reported.
I expect that for 1.1, we'll extend query-migrate to return all of the reasons
why migration is disabled at any given point in time.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c |   18 ++++++++++++++++++
 migration.h |   15 +++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)
Kevin Wolf - Nov. 14, 2011, 9:08 a.m.
Am 12.11.2011 16:56, schrieb Anthony Liguori:
> This lets different subsystems register an Error that is thrown whenever
> migration is attempted.  This works nicely because it gracefully supports
> things like hotplug.
> 
> Right now, if multiple errors are registered, only one of them is reported.
> I expect that for 1.1, we'll extend query-migrate to return all of the reasons
> why migration is disabled at any given point in time.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  migration.c |   18 ++++++++++++++++++
>  migration.h |   15 +++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 41c3c24..6764d3a 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -398,6 +398,18 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
>      return s;
>  }
>  
> +static GSList *migration_blockers;

Any reason not to use qemu-queue.h? It's used everywhere else and being
consistent is usually a good thing. If you want to switch to the glib
functions as the standard for whatever reason, shouldn't we convert
everything at once?

Kevin
Kevin Wolf - Nov. 14, 2011, 9:12 a.m.
Am 12.11.2011 16:56, schrieb Anthony Liguori:
> This lets different subsystems register an Error that is thrown whenever
> migration is attempted.  This works nicely because it gracefully supports
> things like hotplug.
> 
> Right now, if multiple errors are registered, only one of them is reported.
> I expect that for 1.1, we'll extend query-migrate to return all of the reasons
> why migration is disabled at any given point in time.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  migration.c |   18 ++++++++++++++++++
>  migration.h |   15 +++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)

There doesn't seem to be a patch 0/n, so it goes here...

The series looks good in general. I commented on two patches, and there
are a few more image formats that must block migration (basically
everything that is writable, I would be surprised if there was one that
didn't cache anything).

Kevin
Juan Quintela - Nov. 14, 2011, 1 p.m.
Anthony Liguori <aliguori@us.ibm.com> wrote:
> This lets different subsystems register an Error that is thrown whenever
> migration is attempted.  This works nicely because it gracefully supports
> things like hotplug.
>
> Right now, if multiple errors are registered, only one of them is reported.
> I expect that for 1.1, we'll extend query-migrate to return all of the reasons
> why migration is disabled at any given point in time.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  migration.c |   18 ++++++++++++++++++
>  migration.h |   15 +++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 41c3c24..6764d3a 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -398,6 +398,18 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
>      return s;
>  }
>  
> +static GSList *migration_blockers;
> +

As said by Kevin, using another list implementation makes no sense on my
book.

Rest of idea ok.
Anthony Liguori - Nov. 14, 2011, 6:25 p.m.
On 11/14/2011 03:08 AM, Kevin Wolf wrote:
> Am 12.11.2011 16:56, schrieb Anthony Liguori:
>> This lets different subsystems register an Error that is thrown whenever
>> migration is attempted.  This works nicely because it gracefully supports
>> things like hotplug.
>>
>> Right now, if multiple errors are registered, only one of them is reported.
>> I expect that for 1.1, we'll extend query-migrate to return all of the reasons
>> why migration is disabled at any given point in time.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>   migration.c |   18 ++++++++++++++++++
>>   migration.h |   15 +++++++++++++++
>>   2 files changed, 33 insertions(+), 0 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 41c3c24..6764d3a 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -398,6 +398,18 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
>>       return s;
>>   }
>>
>> +static GSList *migration_blockers;
>
> Any reason not to use qemu-queue.h? It's used everywhere else and being
> consistent is usually a good thing. If you want to switch to the glib
> functions as the standard for whatever reason, shouldn't we convert
> everything at once?

They're fundamentally different list implementations.  GSList is a container 
while qemu-queues are embedded lists.  In this case, it's impossible to use 
qemu-queue without creating another data structure which would make things more 
complex than they need to be.

They aren't exact replacements for each other so I think they can co-exist and 
be used where they make sense respectively.

Regards,

Anthony Liguori

>
> Kevin
>

Patch

diff --git a/migration.c b/migration.c
index 41c3c24..6764d3a 100644
--- a/migration.c
+++ b/migration.c
@@ -398,6 +398,18 @@  static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
     return s;
 }
 
+static GSList *migration_blockers;
+
+void migrate_add_blocker(Error *reason)
+{
+    migration_blockers = g_slist_prepend(migration_blockers, reason);
+}
+
+void migrate_del_blocker(Error *reason)
+{
+    migration_blockers = g_slist_remove(migration_blockers, reason);
+}
+
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = migrate_get_current();
@@ -417,6 +429,12 @@  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }
 
+    if (migration_blockers) {
+        Error *err = migration_blockers->data;
+        qerror_report_err(err);
+        return -1;
+    }
+
     s = migrate_init(mon, detach, blk, inc);
 
     if (strstart(uri, "tcp:", &p)) {
diff --git a/migration.h b/migration.h
index 1b8ee58..0682179 100644
--- a/migration.h
+++ b/migration.h
@@ -17,6 +17,7 @@ 
 #include "qdict.h"
 #include "qemu-common.h"
 #include "notify.h"
+#include "error.h"
 
 typedef struct MigrationState MigrationState;
 
@@ -89,4 +90,18 @@  int ram_load(QEMUFile *f, void *opaque, int version_id);
 
 extern int incoming_expected;
 
+/**
+ * @migrate_add_blocker - prevent migration from proceeding
+ *
+ * @reason - an error to be returned whenever migration is attempted
+ */
+void migrate_add_blocker(Error *reason);
+
+/**
+ * @migrate_del_blocker - remove a blocking error from migration
+ *
+ * @reason - the error blocking migration
+ */
+void migrate_del_blocker(Error *reason);
+
 #endif