[Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This is a series of minor fixes from Markus's review:
http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04079.html

To be applied after the exisitng set of -incoming defer
patches.

Dr. David Alan Gilbert (3):
  migrate_incoming: use hmp_handle_error
  Warn against the use of the string as uri parameter to
    migrate-incoming
  migrate_incoming: Cleanup/clarify error messages

 hmp.c                 | 6 +-----
 migration/migration.c | 8 ++++++--
 qapi-schema.json      | 3 +++
 3 files changed, 10 insertions(+), 7 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/3] migrate_incoming: use hmp_handle_error

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hmp.c b/hmp.c
index 761e5ef..e713e06 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1091,11 +1091,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
 
     qmp_migrate_incoming(uri, &err);
 
-    if (err) {
-        monitor_printf(mon, "%s\n", error_get_pretty(err));
-        error_free(err);
-        return;
-    }
+    hmp_handle_error(mon, &err);
 }
 
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] Warn against the use of the string as uri parameter to migrate-incoming

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 qapi-schema.json | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7a80081..cfd59d0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1750,6 +1750,9 @@
 # Returns: nothing on success
 #
 # Since: 2.3
+# Note: It's a bad idea to use a string for the uri, but it needs to stay
+# compatible with -incoming and the format of the uri is already exposed
+# above libvirt
 ##
 { 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
 
-- 
2.1.0

[Qemu-devel] [PATCH 3/3] migrate_incoming: Cleanup/clarify error messages

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Create a separate error for the case where migrate_incoming is
used after a succesful migrate_incoming.

Reword the error in the case where '-incoming defer' is missing
to omit the command name so it's right for both hmp and qmp.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 2c805f1..e6ac3de 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -435,11 +435,15 @@ void migrate_del_blocker(Error *reason)
 void qmp_migrate_incoming(const char *uri, Error **errp)
 {
     Error *local_err = NULL;
+    static bool once = true;
 
     if (!deferred_incoming) {
-        error_setg(errp, "'-incoming defer' is required for migrate_incoming");
+        error_setg(errp, "For use with '-incoming defer'");
         return;
     }
+    if (!once) {
+        error_setg(errp, "The incoming migration has already been started");
+    }
 
     qemu_start_incoming_migration(uri, &local_err);
 
@@ -448,7 +452,7 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
         return;
     }
 
-    deferred_incoming = false;
+    once = false;
 }
 
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Markus Armbruster]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> This is a series of minor fixes from Markus's review:
> http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04079.html
>
> To be applied after the exisitng set of -incoming defer
> patches.

You haven't addressed my review of "[PATCH v3 3/3] Document -incoming
options", yet.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

On 02/26/2015 07:54 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This is a series of minor fixes from Markus's review:
> http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04079.html
> 
> To be applied after the exisitng set of -incoming defer
> patches.

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> Dr. David Alan Gilbert (3):
>   migrate_incoming: use hmp_handle_error
>   Warn against the use of the string as uri parameter to
>     migrate-incoming
>   migrate_incoming: Cleanup/clarify error messages
> 
>  hmp.c                 | 6 +-----
>  migration/migration.c | 8 ++++++--
>  qapi-schema.json      | 3 +++
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] migrate_incoming: Cleanup/clarify error messages

[Amit Shah]

On (Thu) 26 Feb 2015 [14:54:41], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Create a separate error for the case where migrate_incoming is
> used after a succesful migrate_incoming.
> 
> Reword the error in the case where '-incoming defer' is missing
> to omit the command name so it's right for both hmp and qmp.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/migration.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 2c805f1..e6ac3de 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -435,11 +435,15 @@ void migrate_del_blocker(Error *reason)
>  void qmp_migrate_incoming(const char *uri, Error **errp)
>  {
>      Error *local_err = NULL;
> +    static bool once = true;
>  
>      if (!deferred_incoming) {
> -        error_setg(errp, "'-incoming defer' is required for migrate_incoming");
> +        error_setg(errp, "For use with '-incoming defer'");
>          return;
>      }
> +    if (!once) {
> +        error_setg(errp, "The incoming migration has already been started");

Let me apologise in advance for picking on another error message...

This reads like we already have an incoming migration, when we mean we
just have a URI for one.

		Amit

Re: [Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Amit Shah]

On (Thu) 26 Feb 2015 [14:54:38], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This is a series of minor fixes from Markus's review:
> http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04079.html
> 
> To be applied after the exisitng set of -incoming defer
> patches.

ACK to series, just one comment on an error message.

		Amit

Re: [Qemu-devel] [PATCH 3/3] migrate_incoming: Cleanup/clarify error messages

[Dr. David Alan Gilbert]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Thu) 26 Feb 2015 [14:54:41], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Create a separate error for the case where migrate_incoming is
> > used after a succesful migrate_incoming.
> > 
> > Reword the error in the case where '-incoming defer' is missing
> > to omit the command name so it's right for both hmp and qmp.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  migration/migration.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 2c805f1..e6ac3de 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -435,11 +435,15 @@ void migrate_del_blocker(Error *reason)
> >  void qmp_migrate_incoming(const char *uri, Error **errp)
> >  {
> >      Error *local_err = NULL;
> > +    static bool once = true;
> >  
> >      if (!deferred_incoming) {
> > -        error_setg(errp, "'-incoming defer' is required for migrate_incoming");
> > +        error_setg(errp, "For use with '-incoming defer'");
> >          return;
> >      }
> > +    if (!once) {
> > +        error_setg(errp, "The incoming migration has already been started");
> 
> Let me apologise in advance for picking on another error message...
> 
> This reads like we already have an incoming migration, when we mean we
> just have a URI for one.

No, if you hit this error you have at least started listening on the connection.

Dave

> 
> 		Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/3] migrate_incoming cleanups

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> This is a series of minor fixes from Markus's review:
> http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04079.html
>
> To be applied after the exisitng set of -incoming defer
> patches.

Series applied, thanks.