diff mbox

block-migration: deprecate block migration for the 1.2 release

Message ID 1344951151-26387-1-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Aug. 14, 2012, 1:32 p.m. UTC
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(-)

Comments

Kevin Wolf Aug. 14, 2012, 1:48 p.m. UTC | #1
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;
>
Stefan Hajnoczi Aug. 14, 2012, 2:42 p.m. UTC | #2
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
Luiz Capitulino Aug. 14, 2012, 2:49 p.m. UTC | #3
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;
Luiz Capitulino Aug. 14, 2012, 2:52 p.m. UTC | #4
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;
> > 
>
Kevin Wolf Aug. 14, 2012, 3:02 p.m. UTC | #5
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
Luiz Capitulino Aug. 14, 2012, 3:48 p.m. UTC | #6
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.
Kevin Wolf Aug. 14, 2012, 3:59 p.m. UTC | #7
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
Anthony Liguori Aug. 14, 2012, 7:45 p.m. UTC | #8
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;
Markus Armbruster Aug. 14, 2012, 8:25 p.m. UTC | #9
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.
Luiz Capitulino Aug. 14, 2012, 8:26 p.m. UTC | #10
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.
Luiz Capitulino Aug. 14, 2012, 8:32 p.m. UTC | #11
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.
Ruben Kerkhof Aug. 15, 2012, 8:12 a.m. UTC | #12
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
Stefan Hajnoczi Aug. 15, 2012, 12:38 p.m. UTC | #13
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
Ruben Kerkhof Aug. 16, 2012, 7:37 a.m. UTC | #14
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
Paolo Bonzini Aug. 18, 2012, 7:08 p.m. UTC | #15
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
Paolo Bonzini Aug. 18, 2012, 7:10 p.m. UTC | #16
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
Ruben Kerkhof Aug. 19, 2012, 8:13 a.m. UTC | #17
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
Paolo Bonzini Aug. 20, 2012, 8:23 a.m. UTC | #18
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
Ruben Kerkhof Aug. 20, 2012, 8:47 a.m. UTC | #19
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
diff mbox

Patch

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;