[Qemu-devel] [PATCH v2 1/1] migrate-start-postcopy: Improve text

[Qemu-devel] [PATCH v2 1/1] migrate-start-postcopy: Improve text

[Dr. David Alan Gilbert (git)]

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

Improve the text in both the qapi-schema and hmp help to point out
you need to set the postcopy-ram capability prior to issuing
migrate-start-postcopy.

Also fix the text of the migrate_start_postcopy error that
deals with capabilities.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
v2
  Updated text to Erik's suggestion
  Fixed error text error spotted by Christian

 hmp-commands.hx       | 4 +++-
 migration/migration.c | 2 +-
 qapi-schema.json      | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8939b98..a3e5cb7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1011,7 +1011,9 @@ ETEXI
         .name       = "migrate_start_postcopy",
         .args_type  = "",
         .params     = "",
-        .help       = "Switch migration to postcopy mode",
+        .help       = "Followup to a migration command to switch the migration"
+                      " to postcopy mode. The x-postcopy-ram capability must "
+                      "be set before the original migration command.",
         .mhandler.cmd = hmp_migrate_start_postcopy,
     },
 
diff --git a/migration/migration.c b/migration/migration.c
index 5df490a..9bd2ce7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -768,7 +768,7 @@ void qmp_migrate_start_postcopy(Error **errp)
     MigrationState *s = migrate_get_current();
 
     if (!migrate_postcopy_ram()) {
-        error_setg(errp, "Enable postcopy with migration_set_capability before"
+        error_setg(errp, "Enable postcopy with migrate_set_capability before"
                          " the start of migration");
         return;
     }
diff --git a/qapi-schema.json b/qapi-schema.json
index 8c3a42a..3973fa6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -706,7 +706,9 @@
 ##
 # @migrate-start-postcopy
 #
-# Switch migration to postcopy mode
+# Followup to a migration command to switch the migration to postcopy mode.
+# The x-postcopy-ram capability must be set before the original migration
+# command.
 #
 # Since: 2.5
 { 'command': 'migrate-start-postcopy' }
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2 1/1] migrate-start-postcopy: Improve text

[Jason J. Herne]

On 11/12/2015 06:34 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Improve the text in both the qapi-schema and hmp help to point out
> you need to set the postcopy-ram capability prior to issuing
> migrate-start-postcopy.
>
> Also fix the text of the migrate_start_postcopy error that
> deals with capabilities.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> v2
>    Updated text to Erik's suggestion
>    Fixed error text error spotted by Christian
>
>   hmp-commands.hx       | 4 +++-
>   migration/migration.c | 2 +-
>   qapi-schema.json      | 4 +++-
>   3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8939b98..a3e5cb7 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1011,7 +1011,9 @@ ETEXI
>           .name       = "migrate_start_postcopy",
>           .args_type  = "",
>           .params     = "",
> -        .help       = "Switch migration to postcopy mode",
> +        .help       = "Followup to a migration command to switch the migration"
> +                      " to postcopy mode. The x-postcopy-ram capability must "
> +                      "be set before the original migration command.",
>           .mhandler.cmd = hmp_migrate_start_postcopy,
>       },
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 5df490a..9bd2ce7 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -768,7 +768,7 @@ void qmp_migrate_start_postcopy(Error **errp)
>       MigrationState *s = migrate_get_current();
>
>       if (!migrate_postcopy_ram()) {
> -        error_setg(errp, "Enable postcopy with migration_set_capability before"
> +        error_setg(errp, "Enable postcopy with migrate_set_capability before"
>                            " the start of migration");
>           return;
>       }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8c3a42a..3973fa6 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -706,7 +706,9 @@
>   ##
>   # @migrate-start-postcopy
>   #
> -# Switch migration to postcopy mode
> +# Followup to a migration command to switch the migration to postcopy mode.
> +# The x-postcopy-ram capability must be set before the original migration
> +# command.
>   #
>   # Since: 2.5
>   { 'command': 'migrate-start-postcopy' }
>

I like the rewording especially in the help text. This will make it far 
clearer to the user.

fwiw, Acked-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)

Re: [Qemu-devel] [PATCH v2 1/1] migrate-start-postcopy: Improve text

[Eric Blake]

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

On 11/12/2015 04:34 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Improve the text in both the qapi-schema and hmp help to point out
> you need to set the postcopy-ram capability prior to issuing
> migrate-start-postcopy.
> 
> Also fix the text of the migrate_start_postcopy error that
> deals with capabilities.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> v2
>   Updated text to Erik's suggestion

It's Eric, but you're not the first (nor the last) to make that typo.

> +++ b/qapi-schema.json
> @@ -706,7 +706,9 @@
>  ##
>  # @migrate-start-postcopy
>  #
> -# Switch migration to postcopy mode
> +# Followup to a migration command to switch the migration to postcopy mode.

Maybe "Followup to the migrate command to switch the migration..."

But the wording you have also covers the case if we add something other
than just 'migrate' to start a migration, so I'm fine either way.

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

-- 
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 v2 1/1] migrate-start-postcopy: Improve text

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 11/12/2015 04:34 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Improve the text in both the qapi-schema and hmp help to point out
> > you need to set the postcopy-ram capability prior to issuing
> > migrate-start-postcopy.
> > 
> > Also fix the text of the migrate_start_postcopy error that
> > deals with capabilities.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> > v2
> >   Updated text to Erik's suggestion
> 
> It's Eric, but you're not the first (nor the last) to make that typo.

Oops sorry about that.


> > +++ b/qapi-schema.json
> > @@ -706,7 +706,9 @@
> >  ##
> >  # @migrate-start-postcopy
> >  #
> > -# Switch migration to postcopy mode
> > +# Followup to a migration command to switch the migration to postcopy mode.
> 
> Maybe "Followup to the migrate command to switch the migration..."
> 
> But the wording you have also covers the case if we add something other
> than just 'migrate' to start a migration, so I'm fine either way.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks,

Dave

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


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