[Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Anthony Liguori]

To be replaced with live block copy.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 653a3c1..babccf4 100644
--- a/migration.c
+++ b/migration.c
@@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     MigrationParams params;
     const char *p;
     int ret;
+    static bool suppress_deprecation_message;
 
     params.blk = blk;
     params.shared = inc;
 
+    if (blk && !suppress_deprecation_message) {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                      "Block migration is deprecated.  "
+                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
+                      "for an alternative syntax.");
+        suppress_deprecation_message = true;
+    }
+
     if (s->state == MIG_STATE_ACTIVE) {
         error_set(errp, QERR_MIGRATION_ACTIVE);
         return;
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Kevin Wolf]

Am 14.08.2012 15:32, schrieb Anthony Liguori:
> To be replaced with live block copy.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Might be worth adding a deprecation note in qapi-schema.json.

> ---
>  migration.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 653a3c1..babccf4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      MigrationParams params;
>      const char *p;
>      int ret;
> +    static bool suppress_deprecation_message;
>  
>      params.blk = blk;
>      params.shared = inc;
>  
> +    if (blk && !suppress_deprecation_message) {

Hm, it's consistent with when we start block migration, but has_blk is
completely ignored for that and blk seems to be uninitialised if
!has_blk. I think this needs to be fixed. (Why does qmp-marshal.c even
compile when it can use blk uninitialised...?)

Kevin

> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> +                      "Block migration is deprecated.  "
> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> +                      "for an alternative syntax.");
> +        suppress_deprecation_message = true;
> +    }
> +
>      if (s->state == MIG_STATE_ACTIVE) {
>          error_set(errp, QERR_MIGRATION_ACTIVE);
>          return;
> 

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Luiz Capitulino]

On Tue, 14 Aug 2012 15:48:37 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 14.08.2012 15:32, schrieb Anthony Liguori:
> > To be replaced with live block copy.
> > 
> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> Might be worth adding a deprecation note in qapi-schema.json.
> 
> > ---
> >  migration.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/migration.c b/migration.c
> > index 653a3c1..babccf4 100644
> > --- a/migration.c
> > +++ b/migration.c
> > @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >      MigrationParams params;
> >      const char *p;
> >      int ret;
> > +    static bool suppress_deprecation_message;
> >  
> >      params.blk = blk;
> >      params.shared = inc;
> >  
> > +    if (blk && !suppress_deprecation_message) {
> 
> Hm, it's consistent with when we start block migration, but has_blk is
> completely ignored for that and blk seems to be uninitialised if
> !has_blk. I think this needs to be fixed. (Why does qmp-marshal.c even
> compile when it can use blk uninitialised...?)

Are you referring to qmp_marshal_input_migrate()? Where does it use
blk uninitialized?

> 
> Kevin
> 
> > +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> > +                      "Block migration is deprecated.  "
> > +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> > +                      "for an alternative syntax.");
> > +        suppress_deprecation_message = true;
> > +    }
> > +
> >      if (s->state == MIG_STATE_ACTIVE) {
> >          error_set(errp, QERR_MIGRATION_ACTIVE);
> >          return;
> > 
> 

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Kevin Wolf]

Am 14.08.2012 16:52, schrieb Luiz Capitulino:
> On Tue, 14 Aug 2012 15:48:37 +0200
> Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> Am 14.08.2012 15:32, schrieb Anthony Liguori:
>>> To be replaced with live block copy.
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Might be worth adding a deprecation note in qapi-schema.json.
>>
>>> ---
>>>  migration.c |    9 +++++++++
>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/migration.c b/migration.c
>>> index 653a3c1..babccf4 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>>      MigrationParams params;
>>>      const char *p;
>>>      int ret;
>>> +    static bool suppress_deprecation_message;
>>>  
>>>      params.blk = blk;
>>>      params.shared = inc;
>>>  
>>> +    if (blk && !suppress_deprecation_message) {
>>
>> Hm, it's consistent with when we start block migration, but has_blk is
>> completely ignored for that and blk seems to be uninitialised if
>> !has_blk. I think this needs to be fixed. (Why does qmp-marshal.c even
>> compile when it can use blk uninitialised...?)
> 
> Are you referring to qmp_marshal_input_migrate()? Where does it use
> blk uninitialized?

Yes, I am. Maybe I'm missing the obvious thing, but:

bool blk;
...
if (has_blk) {
    visit_type_bool(v, &blk, "blk", errp);
}
...
qmp_migrate(uri, has_blk, blk, has_inc, inc, has_detach, detach, errp);

If has_blk is false, blk is never assigned a value before the
qmp_migrate() call, which uses it.

Kevin

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Luiz Capitulino]

On Tue, 14 Aug 2012 17:02:26 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 14.08.2012 16:52, schrieb Luiz Capitulino:
> > On Tue, 14 Aug 2012 15:48:37 +0200
> > Kevin Wolf <kwolf@redhat.com> wrote:
> > 
> >> Am 14.08.2012 15:32, schrieb Anthony Liguori:
> >>> To be replaced with live block copy.
> >>>
> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >>
> >> Might be worth adding a deprecation note in qapi-schema.json.
> >>
> >>> ---
> >>>  migration.c |    9 +++++++++
> >>>  1 files changed, 9 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/migration.c b/migration.c
> >>> index 653a3c1..babccf4 100644
> >>> --- a/migration.c
> >>> +++ b/migration.c
> >>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >>>      MigrationParams params;
> >>>      const char *p;
> >>>      int ret;
> >>> +    static bool suppress_deprecation_message;
> >>>  
> >>>      params.blk = blk;
> >>>      params.shared = inc;
> >>>  
> >>> +    if (blk && !suppress_deprecation_message) {
> >>
> >> Hm, it's consistent with when we start block migration, but has_blk is
> >> completely ignored for that and blk seems to be uninitialised if
> >> !has_blk. I think this needs to be fixed. (Why does qmp-marshal.c even
> >> compile when it can use blk uninitialised...?)
> > 
> > Are you referring to qmp_marshal_input_migrate()? Where does it use
> > blk uninitialized?
> 
> Yes, I am. Maybe I'm missing the obvious thing, but:
> 
> bool blk;
> ...
> if (has_blk) {
>     visit_type_bool(v, &blk, "blk", errp);
> }
> ...
> qmp_migrate(uri, has_blk, blk, has_inc, inc, has_detach, detach, errp);
> 
> If has_blk is false, blk is never assigned a value before the
> qmp_migrate() call, which uses it.

Yes, qmp_migrate() should only use blk if has_blk is true. The point you
made above about this patch is correct. I thought you were talking about
other usages.

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Kevin Wolf]

Am 14.08.2012 17:48, schrieb Luiz Capitulino:
> On Tue, 14 Aug 2012 17:02:26 +0200
> Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> Am 14.08.2012 16:52, schrieb Luiz Capitulino:
>>> On Tue, 14 Aug 2012 15:48:37 +0200
>>> Kevin Wolf <kwolf@redhat.com> wrote:
>>>
>>>> Am 14.08.2012 15:32, schrieb Anthony Liguori:
>>>>> To be replaced with live block copy.
>>>>>
>>>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>>>
>>>> Might be worth adding a deprecation note in qapi-schema.json.
>>>>
>>>>> ---
>>>>>  migration.c |    9 +++++++++
>>>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/migration.c b/migration.c
>>>>> index 653a3c1..babccf4 100644
>>>>> --- a/migration.c
>>>>> +++ b/migration.c
>>>>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>>>>      MigrationParams params;
>>>>>      const char *p;
>>>>>      int ret;
>>>>> +    static bool suppress_deprecation_message;
>>>>>  
>>>>>      params.blk = blk;
>>>>>      params.shared = inc;
>>>>>  
>>>>> +    if (blk && !suppress_deprecation_message) {
>>>>
>>>> Hm, it's consistent with when we start block migration, but has_blk is
>>>> completely ignored for that and blk seems to be uninitialised if
>>>> !has_blk. I think this needs to be fixed. (Why does qmp-marshal.c even
>>>> compile when it can use blk uninitialised...?)
>>>
>>> Are you referring to qmp_marshal_input_migrate()? Where does it use
>>> blk uninitialized?
>>
>> Yes, I am. Maybe I'm missing the obvious thing, but:
>>
>> bool blk;
>> ...
>> if (has_blk) {
>>     visit_type_bool(v, &blk, "blk", errp);
>> }
>> ...
>> qmp_migrate(uri, has_blk, blk, has_inc, inc, has_detach, detach, errp);
>>
>> If has_blk is false, blk is never assigned a value before the
>> qmp_migrate() call, which uses it.
> 
> Yes, qmp_migrate() should only use blk if has_blk is true. The point you
> made above about this patch is correct. I thought you were talking about
> other usages.

I thought the compiler should warn about it independent of the
implementation of qmp_migrate(), because strictly speaking you're
already using blk in qmp_marshal_input_migrate(), even though
qmp_migrate() doesn't look at the parameter.

Kevin

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Stefan Hajnoczi]

On Tue, Aug 14, 2012 at 08:32:31AM -0500, Anthony Liguori wrote:
> To be replaced with live block copy.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  migration.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 653a3c1..babccf4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      MigrationParams params;
>      const char *p;
>      int ret;
> +    static bool suppress_deprecation_message;
> 
>      params.blk = blk;
>      params.shared = inc;
> 
> +    if (blk && !suppress_deprecation_message) {
> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,

qerror_report_once() would be nice :).

> +                      "Block migration is deprecated.  "
> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "

The page doesn't exist, I think it should be:
http://wiki.qemu.org/Features/LiveBlockMigration

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Ruben Kerkhof]

On Tue, Aug 14, 2012 at 4:42 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Tue, Aug 14, 2012 at 08:32:31AM -0500, Anthony Liguori wrote:
>> To be replaced with live block copy.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  migration.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 653a3c1..babccf4 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>      MigrationParams params;
>>      const char *p;
>>      int ret;
>> +    static bool suppress_deprecation_message;
>>
>>      params.blk = blk;
>>      params.shared = inc;
>>
>> +    if (blk && !suppress_deprecation_message) {
>> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
>
> qerror_report_once() would be nice :).
>
>> +                      "Block migration is deprecated.  "
>> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
>
> The page doesn't exist, I think it should be:
> http://wiki.qemu.org/Features/LiveBlockMigration

Can the new live block copy method still use tcp just like the current
block migration? The wiki page only mentions iscsi.
I make extensive use of block migration over tcp, which works fine and
is handled by libvirt. I'd rather not introduce iscsi in my
environment.

Kind regards,

Ruben

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Stefan Hajnoczi]

On Wed, Aug 15, 2012 at 10:12:42AM +0200, Ruben Kerkhof wrote:
> On Tue, Aug 14, 2012 at 4:42 PM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
> > On Tue, Aug 14, 2012 at 08:32:31AM -0500, Anthony Liguori wrote:
> >> To be replaced with live block copy.
> >>
> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> ---
> >>  migration.c |    9 +++++++++
> >>  1 files changed, 9 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/migration.c b/migration.c
> >> index 653a3c1..babccf4 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >>      MigrationParams params;
> >>      const char *p;
> >>      int ret;
> >> +    static bool suppress_deprecation_message;
> >>
> >>      params.blk = blk;
> >>      params.shared = inc;
> >>
> >> +    if (blk && !suppress_deprecation_message) {
> >> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> >
> > qerror_report_once() would be nice :).
> >
> >> +                      "Block migration is deprecated.  "
> >> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> >
> > The page doesn't exist, I think it should be:
> > http://wiki.qemu.org/Features/LiveBlockMigration
> 
> Can the new live block copy method still use tcp just like the current
> block migration? The wiki page only mentions iscsi.
> I make extensive use of block migration over tcp, which works fine and
> is handled by libvirt. I'd rather not introduce iscsi in my
> environment.

The new live block copy approach is different and that's why classic
block migration is only deprecated but not dropped:

Live block copy doesn't transfer data in-band during live migration.
Instead it currently requires storage access from both hosts, for
example:
1. NFS or CIFS
2. iSCSI or NBD

I think when classic block migration is removed for good it should be
just as easy to use through libvirt using TCP because that's still a
valid use case and probably the simplest one to get started.  (Libvirt
could orchestrate an NBD connection behind the scenes, for example.)

Stefan

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Ruben Kerkhof]

Hi Stefan,

Thanks for your explanation.

On Wed, Aug 15, 2012 at 2:38 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> I think when classic block migration is removed for good it should be
> just as easy to use through libvirt using TCP because that's still a
> valid use case and probably the simplest one to get started.  (Libvirt
> could orchestrate an NBD connection behind the scenes, for example.)

I really don't care which protocol is used, as long as libvirt handles
it for me.

Kind regards,

Ruben

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Paolo Bonzini]

Il 15/08/2012 14:38, Stefan Hajnoczi ha scritto:
> The new live block copy approach is different and that's why classic
> block migration is only deprecated but not dropped:
> 
> Live block copy doesn't transfer data in-band during live migration.
> Instead it currently requires storage access from both hosts, for
> example:
> 1. NFS or CIFS
> 2. iSCSI or NBD
> 
> I think when classic block migration is removed for good it should be
> just as easy to use through libvirt using TCP because that's still a
> valid use case and probably the simplest one to get started.  (Libvirt
> could orchestrate an NBD connection behind the scenes, for example.)

That's correct.  Live block migration will use two ports, both served by
the destination, one using NBD and one for RAM/device migration data.
It will be a little more complicated than just migrate -b, but nothing
that libvirt cannot orchestrate.

Paolo

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Ruben Kerkhof]

On Sat, Aug 18, 2012 at 9:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> That's correct.  Live block migration will use two ports, both served by
> the destination, one using NBD and one for RAM/device migration data.
> It will be a little more complicated than just migrate -b, but nothing
> that libvirt cannot orchestrate.

Will I still be able to cap the migration bandwidth?
And how about security? Libvirt can tunnel the data, I believe by
passing an fd to qemu, and encrypting the connection with TLS.

Thanks,

Ruben

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Paolo Bonzini]

Il 19/08/2012 10:13, Ruben Kerkhof ha scritto:
>> > That's correct.  Live block migration will use two ports, both served by
>> > the destination, one using NBD and one for RAM/device migration data.
>> > It will be a little more complicated than just migrate -b, but nothing
>> > that libvirt cannot orchestrate.
> Will I still be able to cap the migration bandwidth?

Yes, though you will have to cap the two bandwidths separately.

> And how about security? Libvirt can tunnel the data, I believe by
> passing an fd to qemu, and encrypting the connection with TLS.

Libvirt can do the same on the NBD connection.

Paolo

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Ruben Kerkhof]

On Mon, Aug 20, 2012 at 10:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 19/08/2012 10:13, Ruben Kerkhof ha scritto:
>>> > That's correct.  Live block migration will use two ports, both served by
>>> > the destination, one using NBD and one for RAM/device migration data.
>>> > It will be a little more complicated than just migrate -b, but nothing
>>> > that libvirt cannot orchestrate.
>> Will I still be able to cap the migration bandwidth?
>
> Yes, though you will have to cap the two bandwidths separately.
>
>> And how about security? Libvirt can tunnel the data, I believe by
>> passing an fd to qemu, and encrypting the connection with TLS.
>
> Libvirt can do the same on the NBD connection.
>
> Paolo

Thanks, that's good to know.

Ruben

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Paolo Bonzini]

Il 14/08/2012 16:42, Stefan Hajnoczi ha scritto:
>> > 
>> > +    if (blk && !suppress_deprecation_message) {
>> > +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> qerror_report_once() would be nice :).
> 
>> > +                      "Block migration is deprecated.  "
>> > +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> The page doesn't exist, I think it should be:
> http://wiki.qemu.org/Features/LiveBlockMigration
> 

Since the plan (NBD in-process server + mirroring job) is a bit
different from what is in that page, perhaps we could just ask to tell
qemu-devel@nongnu.org about their use case and useful information
(performance, guest load etc.).

paolo

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Luiz Capitulino]

On Tue, 14 Aug 2012 08:32:31 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> To be replaced with live block copy.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  migration.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 653a3c1..babccf4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      MigrationParams params;
>      const char *p;
>      int ret;
> +    static bool suppress_deprecation_message;
>  
>      params.blk = blk;
>      params.shared = inc;
>  
> +    if (blk && !suppress_deprecation_message) {
> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> +                      "Block migration is deprecated.  "
> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> +                      "for an alternative syntax.");

Why not error_set()?

> +        suppress_deprecation_message = true;
> +    }
> +
>      if (s->state == MIG_STATE_ACTIVE) {
>          error_set(errp, QERR_MIGRATION_ACTIVE);
>          return;

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 14 Aug 2012 08:32:31 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> To be replaced with live block copy.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  migration.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>> 
>> diff --git a/migration.c b/migration.c
>> index 653a3c1..babccf4 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>      MigrationParams params;
>>      const char *p;
>>      int ret;
>> +    static bool suppress_deprecation_message;
>>  
>>      params.blk = blk;
>>      params.shared = inc;
>>  
>> +    if (blk && !suppress_deprecation_message) {
>> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
>> +                      "Block migration is deprecated.  "
>> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
>> +                      "for an alternative syntax.");
>
> Why not error_set()?

Because we fall through (we don't fail).  This is just a warning.

If you error_set(), then the migration command fails.  We don't want it
to fail.

I guess qerror_report will do that too :-(

I'll change it to an fprintf :-((

Regards,

Anthony Liguori

>
>> +        suppress_deprecation_message = true;
>> +    }
>> +
>>      if (s->state == MIG_STATE_ACTIVE) {
>>          error_set(errp, QERR_MIGRATION_ACTIVE);
>>          return;

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Markus Armbruster]

Anthony Liguori <aliguori@us.ibm.com> writes:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> On Tue, 14 Aug 2012 08:32:31 -0500
>> Anthony Liguori <aliguori@us.ibm.com> wrote:
>>
>>> To be replaced with live block copy.
>>> 
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> ---
>>>  migration.c |    9 +++++++++
>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/migration.c b/migration.c
>>> index 653a3c1..babccf4 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>>      MigrationParams params;
>>>      const char *p;
>>>      int ret;
>>> +    static bool suppress_deprecation_message;
>>>  
>>>      params.blk = blk;
>>>      params.shared = inc;
>>>  
>>> +    if (blk && !suppress_deprecation_message) {
>>> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
>>> +                      "Block migration is deprecated.  "
>>> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
>>> +                      "for an alternative syntax.");
>>
>> Why not error_set()?
>
> Because we fall through (we don't fail).  This is just a warning.
>
> If you error_set(), then the migration command fails.  We don't want it
> to fail.
>
> I guess qerror_report will do that too :-(
>
> I'll change it to an fprintf :-((

Please use error_report(), to ensure it's visible in the monitor.

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Luiz Capitulino]

On Tue, 14 Aug 2012 22:25:40 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Anthony Liguori <aliguori@us.ibm.com> writes:
> 
> > Luiz Capitulino <lcapitulino@redhat.com> writes:
> >
> >> On Tue, 14 Aug 2012 08:32:31 -0500
> >> Anthony Liguori <aliguori@us.ibm.com> wrote:
> >>
> >>> To be replaced with live block copy.
> >>> 
> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >>> ---
> >>>  migration.c |    9 +++++++++
> >>>  1 files changed, 9 insertions(+), 0 deletions(-)
> >>> 
> >>> diff --git a/migration.c b/migration.c
> >>> index 653a3c1..babccf4 100644
> >>> --- a/migration.c
> >>> +++ b/migration.c
> >>> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >>>      MigrationParams params;
> >>>      const char *p;
> >>>      int ret;
> >>> +    static bool suppress_deprecation_message;
> >>>  
> >>>      params.blk = blk;
> >>>      params.shared = inc;
> >>>  
> >>> +    if (blk && !suppress_deprecation_message) {
> >>> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> >>> +                      "Block migration is deprecated.  "
> >>> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> >>> +                      "for an alternative syntax.");
> >>
> >> Why not error_set()?
> >
> > Because we fall through (we don't fail).  This is just a warning.
> >
> > If you error_set(), then the migration command fails.  We don't want it
> > to fail.
> >
> > I guess qerror_report will do that too :-(
> >
> > I'll change it to an fprintf :-((
> 
> Please use error_report(), to ensure it's visible in the monitor.

The user targeted warning should be added to hmp_migrate(), where we can
use monitor_printf(). The fprintf() call is fine here, for those who use
qmp and get stuff written to stderr logged.

Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release

[Luiz Capitulino]

On Tue, 14 Aug 2012 14:45:17 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Tue, 14 Aug 2012 08:32:31 -0500
> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> >
> >> To be replaced with live block copy.
> >> 
> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> ---
> >>  migration.c |    9 +++++++++
> >>  1 files changed, 9 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/migration.c b/migration.c
> >> index 653a3c1..babccf4 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >>      MigrationParams params;
> >>      const char *p;
> >>      int ret;
> >> +    static bool suppress_deprecation_message;
> >>  
> >>      params.blk = blk;
> >>      params.shared = inc;
> >>  
> >> +    if (blk && !suppress_deprecation_message) {
> >> +        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> >> +                      "Block migration is deprecated.  "
> >> +                      "See http://wiki.qemu.org/Features/LiveBlockCopy "
> >> +                      "for an alternative syntax.");
> >
> > Why not error_set()?
> 
> Because we fall through (we don't fail).  This is just a warning.
> 
> If you error_set(), then the migration command fails.  We don't want it
> to fail.
> 
> I guess qerror_report will do that too :-(
> 
> I'll change it to an fprintf :-((

Yes, I'd also add a monitor_printf() call to hmp_migrate() and a note to
the schema.