Patchwork [22/22] migration: Make state definitions local

login
register
mail settings
Submitter Juan Quintela
Date Feb. 23, 2011, 12:44 a.m.
Message ID <83ec39769be88d8e9916e2180830765c722eebaa.1298421307.git.quintela@redhat.com>
Download mbox | patch
Permalink /patch/84027/
State New
Headers show

Comments

Juan Quintela - Feb. 23, 2011, 12:44 a.m.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 ++++++
 migration.h |    6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)
Yoshiaki Tamura - Feb. 23, 2011, 8:35 a.m.
2011/2/23 Juan Quintela <quintela@redhat.com>:
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c |    6 ++++++
>  migration.h |    6 ------
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 383ebaf..90fc2a0 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -31,6 +31,12 @@
>     do { } while (0)
>  #endif
>
> +#define MIG_STATE_ERROR                -1
> +#define MIG_STATE_NONE         0
> +#define MIG_STATE_CANCELLED    1
> +#define MIG_STATE_ACTIVE       2
> +#define MIG_STATE_COMPLETED    3
> +
>  static MigrationState current_migration = {
>     .state = MIG_STATE_NONE,
>      /* Migration speed throttling */
> diff --git a/migration.h b/migration.h
> index 9457807..493fbe5 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -18,12 +18,6 @@
>  #include "qemu-common.h"
>  #include "notify.h"
>
> -#define MIG_STATE_ERROR                -1
> -#define MIG_STATE_NONE         0
> -#define MIG_STATE_CANCELLED    1
> -#define MIG_STATE_ACTIVE       2
> -#define MIG_STATE_COMPLETED    3
> -

Although you're right, I would prefer to keep it so that somebody
outside of migration may understand the status in the future if
there are no harms.

Yoshi

>  typedef struct MigrationState MigrationState;
>
>  struct MigrationState
> --
> 1.7.4
>
>
>
Juan Quintela - Feb. 23, 2011, 9:21 a.m.
Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
> 2011/2/23 Juan Quintela <quintela@redhat.com>:
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration.c |    6 ++++++
>>  migration.h |    6 ------
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 383ebaf..90fc2a0 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -31,6 +31,12 @@
>>     do { } while (0)
>>  #endif
>>
>> +#define MIG_STATE_ERROR                -1
>> +#define MIG_STATE_NONE         0
>> +#define MIG_STATE_CANCELLED    1
>> +#define MIG_STATE_ACTIVE       2
>> +#define MIG_STATE_COMPLETED    3
>> +
>>  static MigrationState current_migration = {
>>     .state = MIG_STATE_NONE,
>>      /* Migration speed throttling */
>> diff --git a/migration.h b/migration.h
>> index 9457807..493fbe5 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -18,12 +18,6 @@
>>  #include "qemu-common.h"
>>  #include "notify.h"
>>
>> -#define MIG_STATE_ERROR                -1
>> -#define MIG_STATE_NONE         0
>> -#define MIG_STATE_CANCELLED    1
>> -#define MIG_STATE_ACTIVE       2
>> -#define MIG_STATE_COMPLETED    3
>> -
>
> Although you're right, I would prefer to keep it so that somebody
> outside of migration may understand the status in the future if
> there are no harms.

my plan is to move MigrationState inside migration.c, and then decide
what to export/not export.  Next thing to do is move migration to its
own thread.  Before doing that, I need to know what parts are used/not
used outside migration.c.  Removing it now means that nothing gets to
use it without needing a patch.

Later, Juan..
Yoshiaki Tamura - Feb. 24, 2011, 4:19 a.m.
2011/2/23 Juan Quintela <quintela@redhat.com>:
> Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
>> 2011/2/23 Juan Quintela <quintela@redhat.com>:
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> ---
>>>  migration.c |    6 ++++++
>>>  migration.h |    6 ------
>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/migration.c b/migration.c
>>> index 383ebaf..90fc2a0 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -31,6 +31,12 @@
>>>     do { } while (0)
>>>  #endif
>>>
>>> +#define MIG_STATE_ERROR                -1
>>> +#define MIG_STATE_NONE         0
>>> +#define MIG_STATE_CANCELLED    1
>>> +#define MIG_STATE_ACTIVE       2
>>> +#define MIG_STATE_COMPLETED    3
>>> +
>>>  static MigrationState current_migration = {
>>>     .state = MIG_STATE_NONE,
>>>      /* Migration speed throttling */
>>> diff --git a/migration.h b/migration.h
>>> index 9457807..493fbe5 100644
>>> --- a/migration.h
>>> +++ b/migration.h
>>> @@ -18,12 +18,6 @@
>>>  #include "qemu-common.h"
>>>  #include "notify.h"
>>>
>>> -#define MIG_STATE_ERROR                -1
>>> -#define MIG_STATE_NONE         0
>>> -#define MIG_STATE_CANCELLED    1
>>> -#define MIG_STATE_ACTIVE       2
>>> -#define MIG_STATE_COMPLETED    3
>>> -
>>
>> Although you're right, I would prefer to keep it so that somebody
>> outside of migration may understand the status in the future if
>> there are no harms.
>
> my plan is to move MigrationState inside migration.c, and then decide
> what to export/not export.

Well, it may be just a policy, but it's already exported, and I
would like to keep it unless it bothers your plan.  IIUC, I don't
think it does.

> Next thing to do is move migration to its
> own thread.  Before doing that, I need to know what parts are used/not
> used outside migration.c.  Removing it now means that nothing gets to
> use it without needing a patch.

I've once asked Anthony whether it's possible to make migration
to different threads, but his answer was no due to hard
dependency of qemu's internal code, and making migration to
different threads are bad design.

Thanks,

Yoshi

>
> Later, Juan..
>
>
Juan Quintela - Feb. 24, 2011, 12:23 p.m.
Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
> 2011/2/23 Juan Quintela <quintela@redhat.com>:
>> Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
>>> 2011/2/23 Juan Quintela <quintela@redhat.com>:

>>> Although you're right, I would prefer to keep it so that somebody
>>> outside of migration may understand the status in the future if
>>> there are no harms.
>>
>> my plan is to move MigrationState inside migration.c, and then decide
>> what to export/not export.
>
> Well, it may be just a policy, but it's already exported, and I
> would like to keep it unless it bothers your plan.  IIUC, I don't
> think it does.
>
>> Next thing to do is move migration to its
>> own thread.  Before doing that, I need to know what parts are used/not
>> used outside migration.c.  Removing it now means that nothing gets to
>> use it without needing a patch.
>
> I've once asked Anthony whether it's possible to make migration
> to different threads, but his answer was no due to hard
> dependency of qemu's internal code, and making migration to
> different threads are bad design.

I know.  But Anthony is seeing the light O:-)

Basically, without an own thread we are not able to:
- do anything else while on incoming migration
  (namely using the monitor)
- do anything else than migration.  We can try hard and let vcpus to
  run, but we would still clog the io_thread.
- We are not able to saturate 10Gbit networking (basically we are doing
  2/3 level of bufferering (depending on how you count).

So, once code is there, I guess we will convince Anthony to commit it.

Later, Juan.
Anthony Liguori - Feb. 24, 2011, 2:46 p.m.
On 02/24/2011 06:23 AM, Juan Quintela wrote:
> Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>  wrote:
>    
>> 2011/2/23 Juan Quintela<quintela@redhat.com>:
>>      
>>> Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>  wrote:
>>>        
>>>> 2011/2/23 Juan Quintela<quintela@redhat.com>:
>>>>          
>    
>>>> Although you're right, I would prefer to keep it so that somebody
>>>> outside of migration may understand the status in the future if
>>>> there are no harms.
>>>>          
>>> my plan is to move MigrationState inside migration.c, and then decide
>>> what to export/not export.
>>>        
>> Well, it may be just a policy, but it's already exported, and I
>> would like to keep it unless it bothers your plan.  IIUC, I don't
>> think it does.
>>
>>      
>>> Next thing to do is move migration to its
>>> own thread.  Before doing that, I need to know what parts are used/not
>>> used outside migration.c.  Removing it now means that nothing gets to
>>> use it without needing a patch.
>>>        
>> I've once asked Anthony whether it's possible to make migration
>> to different threads, but his answer was no due to hard
>> dependency of qemu's internal code, and making migration to
>> different threads are bad design.
>>      
> I know.  But Anthony is seeing the light O:-)
>    

Let's be very careful about quoting Anthony as he's known to be 
incoherent 90% of the time :-)

I don't quite recall the context of the discussion with Yoshi, but I'm 
not quite there in terms of advocating that we throw a bucket full of 
threads at migration.  I think we should move the ram migration to 
another I/O thread that doesn't hold a lock against the main I/O 
thread.  That's all.

Regards,

Anthony Liguori

> Basically, without an own thread we are not able to:
> - do anything else while on incoming migration
>    (namely using the monitor)
> - do anything else than migration.  We can try hard and let vcpus to
>    run, but we would still clog the io_thread.
> - We are not able to saturate 10Gbit networking (basically we are doing
>    2/3 level of bufferering (depending on how you count).
>
> So, once code is there, I guess we will convince Anthony to commit it.
>
> Later, Juan.
>
>
Yoshiaki Tamura - Feb. 25, 2011, 1:20 a.m.
2011/2/24 Juan Quintela <quintela@redhat.com>:
> Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
>> 2011/2/23 Juan Quintela <quintela@redhat.com>:
>>> Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
>>>> 2011/2/23 Juan Quintela <quintela@redhat.com>:
>
>>>> Although you're right, I would prefer to keep it so that somebody
>>>> outside of migration may understand the status in the future if
>>>> there are no harms.
>>>
>>> my plan is to move MigrationState inside migration.c, and then decide
>>> what to export/not export.
>>
>> Well, it may be just a policy, but it's already exported, and I
>> would like to keep it unless it bothers your plan.  IIUC, I don't
>> think it does.
>>
>>> Next thing to do is move migration to its
>>> own thread.  Before doing that, I need to know what parts are used/not
>>> used outside migration.c.  Removing it now means that nothing gets to
>>> use it without needing a patch.
>>
>> I've once asked Anthony whether it's possible to make migration
>> to different threads, but his answer was no due to hard
>> dependency of qemu's internal code, and making migration to
>> different threads are bad design.
>
> I know.  But Anthony is seeing the light O:-)

I'm with you at making live migration fast, but let me comment a
few.

> Basically, without an own thread we are not able to:
> - do anything else while on incoming migration
>  (namely using the monitor)

It's not true.  Just adding fixed headers to buffered file should
be enough for that.  I've done it for Kemari as you can see
this (http://www.osrg.net/kemari/download/kemari-v0.2-fedora11.mov).

> - do anything else than migration.  We can try hard and let vcpus to
>  run, but we would still clog the io_thread.
> - We are not able to saturate 10Gbit networking (basically we are doing
>  2/3 level of bufferering (depending on how you count).

I think current byte-based dirty bitmap for sending rams are
responsible for this too.  I've converted it to bit-based dirty
and made traversing 100x faster.  Also bypassing QEMUFile buffer
in case of rams would boost to some degrees.

Thanks,

Yoshi

> So, once code is there, I guess we will convince Anthony to commit it.
>
> Later, Juan.
>
>
Yoshiaki Tamura - Feb. 25, 2011, 1:27 a.m.
2011/2/24 Anthony Liguori <anthony@codemonkey.ws>:
> On 02/24/2011 06:23 AM, Juan Quintela wrote:
>>
>> Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>  wrote:
>>
>>>
>>> 2011/2/23 Juan Quintela<quintela@redhat.com>:
>>>
>>>>
>>>> Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>  wrote:
>>>>
>>>>>
>>>>> 2011/2/23 Juan Quintela<quintela@redhat.com>:
>>>>>
>>
>>
>>>>>
>>>>> Although you're right, I would prefer to keep it so that somebody
>>>>> outside of migration may understand the status in the future if
>>>>> there are no harms.
>>>>>
>>>>
>>>> my plan is to move MigrationState inside migration.c, and then decide
>>>> what to export/not export.
>>>>
>>>
>>> Well, it may be just a policy, but it's already exported, and I
>>> would like to keep it unless it bothers your plan.  IIUC, I don't
>>> think it does.
>>>
>>>
>>>>
>>>> Next thing to do is move migration to its
>>>> own thread.  Before doing that, I need to know what parts are used/not
>>>> used outside migration.c.  Removing it now means that nothing gets to
>>>> use it without needing a patch.
>>>>
>>>
>>> I've once asked Anthony whether it's possible to make migration
>>> to different threads, but his answer was no due to hard
>>> dependency of qemu's internal code, and making migration to
>>> different threads are bad design.
>>>
>>
>> I know.  But Anthony is seeing the light O:-)
>>
>
> Let's be very careful about quoting Anthony as he's known to be incoherent
> 90% of the time :-)

There you are :)

> I don't quite recall the context of the discussion with Yoshi, but I'm not
> quite there in terms of advocating that we throw a bucket full of threads at
> migration.  I think we should move the ram migration to another I/O thread
> that doesn't hold a lock against the main I/O thread.  That's all.

Let's just forget about the old discussion and have a new one.
Why do you want to have a new thread only for ram migration?  I
know that it's the majority of the migration, but how do you
serialize it with other device migration for single QEMUFile?
IIUC, it seems to get mixed up easily or lose paralism.

Thanks,

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> Basically, without an own thread we are not able to:
>> - do anything else while on incoming migration
>>   (namely using the monitor)
>> - do anything else than migration.  We can try hard and let vcpus to
>>   run, but we would still clog the io_thread.
>> - We are not able to saturate 10Gbit networking (basically we are doing
>>   2/3 level of bufferering (depending on how you count).
>>
>> So, once code is there, I guess we will convince Anthony to commit it.
>>
>> Later, Juan.
>>
>>
>
>
>

Patch

diff --git a/migration.c b/migration.c
index 383ebaf..90fc2a0 100644
--- a/migration.c
+++ b/migration.c
@@ -31,6 +31,12 @@ 
     do { } while (0)
 #endif

+#define MIG_STATE_ERROR		-1
+#define MIG_STATE_NONE		0
+#define MIG_STATE_CANCELLED	1
+#define MIG_STATE_ACTIVE	2
+#define MIG_STATE_COMPLETED	3
+
 static MigrationState current_migration = {
     .state = MIG_STATE_NONE,
      /* Migration speed throttling */
diff --git a/migration.h b/migration.h
index 9457807..493fbe5 100644
--- a/migration.h
+++ b/migration.h
@@ -18,12 +18,6 @@ 
 #include "qemu-common.h"
 #include "notify.h"

-#define MIG_STATE_ERROR		-1
-#define MIG_STATE_NONE		0
-#define MIG_STATE_CANCELLED	1
-#define MIG_STATE_ACTIVE	2
-#define MIG_STATE_COMPLETED	3
-
 typedef struct MigrationState MigrationState;

 struct MigrationState