Patchwork [v6,4/6] Add support for marking memory to not be migrated. On migration, memory is checked for the NO_MIGRATION_FLAG.

login
register
mail settings
Submitter Cam Macdonell
Date June 4, 2010, 9:45 p.m.
Message ID <1275687942-12312-5-git-send-email-cam@cs.ualberta.ca>
Download mbox | patch
Permalink /patch/54719/
State New
Headers show

Comments

Cam Macdonell - June 4, 2010, 9:45 p.m.
This is useful for devices that do not want to take memory regions data with them on migration.
---
 arch_init.c  |   28 ++++++++++++++++------------
 cpu-all.h    |    2 ++
 cpu-common.h |    2 ++
 exec.c       |   12 ++++++++++++
 4 files changed, 32 insertions(+), 12 deletions(-)
Anthony Liguori - June 14, 2010, 3:51 p.m.
On 06/04/2010 04:45 PM, Cam Macdonell wrote:
> This is useful for devices that do not want to take memory regions data with them on migration.
> ---
>   arch_init.c  |   28 ++++++++++++++++------------
>   cpu-all.h    |    2 ++
>   cpu-common.h |    2 ++
>   exec.c       |   12 ++++++++++++
>   4 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index cfc03ea..7a234fa 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -118,18 +118,21 @@ static int ram_save_block(QEMUFile *f)
>                                               current_addr + TARGET_PAGE_SIZE,
>                                               MIGRATION_DIRTY_FLAG);
>
> -            p = qemu_get_ram_ptr(current_addr);
> -
> -            if (is_dup_page(p, *p)) {
> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
> -                qemu_put_byte(f, *p);
> -            } else {
> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
> -                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> -            }
> +            if (!cpu_physical_memory_get_dirty(current_addr,
> +                                                    NO_MIGRATION_FLAG)) {
> +                p = qemu_get_ram_ptr(current_addr);
> +
> +                if (is_dup_page(p, *p)) {
> +                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
> +                    qemu_put_byte(f, *p);
> +                } else {
> +                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
> +                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> +                }
>
> -            found = 1;
> -            break;
> +                found = 1;
> +                break;
> +            }
>           }
>    

Shouldn't we just disable live migration out right?

I would rather that the device mark migration as impossible having the 
user hot remove the device before migration and then add it again after 
migration.  Device assignment could also use this functionality.

What this does is make migration possible but fundamentally broken which 
is not a good thing.

Regards,

Anthony Liguori

>           addr += TARGET_PAGE_SIZE;
>           current_addr = (saved_addr + addr) % last_ram_offset;
> @@ -146,7 +149,8 @@ static ram_addr_t ram_save_remaining(void)
>       ram_addr_t count = 0;
>
>       for (addr = 0; addr<  last_ram_offset; addr += TARGET_PAGE_SIZE) {
> -        if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
> +        if (!cpu_physical_memory_get_dirty(addr, NO_MIGRATION_FLAG)&&
> +                cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
>               count++;
>           }
>       }
> diff --git a/cpu-all.h b/cpu-all.h
> index 9080cc7..4df00ab 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -887,6 +887,8 @@ extern int mem_prealloc;
>   #define CODE_DIRTY_FLAG      0x02
>   #define MIGRATION_DIRTY_FLAG 0x08
>
> +#define NO_MIGRATION_FLAG 0x10
> +
>   #define DIRTY_ALL_FLAG  (VGA_DIRTY_FLAG | CODE_DIRTY_FLAG | MIGRATION_DIRTY_FLAG)
>
>   /* read dirty bit (return 0 or 1) */
> diff --git a/cpu-common.h b/cpu-common.h
> index 4b0ba60..a1ebbbe 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -39,6 +39,8 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
>       cpu_register_physical_memory_offset(start_addr, size, phys_offset, 0);
>   }
>
> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t size);
> +
>   ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
>   ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>   ram_addr_t qemu_ram_alloc(ram_addr_t);
> diff --git a/exec.c b/exec.c
> index 39c18a7..c11d22f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2786,6 +2786,18 @@ static void *file_ram_alloc(ram_addr_t memory, const char *path)
>   }
>   #endif
>
> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t length)
> +{
> +    int i, len;
> +    uint8_t *p;
> +
> +    len = length>>  TARGET_PAGE_BITS;
> +    p = phys_ram_flags + (start>>  TARGET_PAGE_BITS);
> +    for (i = 0; i<  len; i++) {
> +        p[i] |= NO_MIGRATION_FLAG;
> +    }
> +}
> +
>   ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
>   {
>       RAMBlock *new_block;
>
Cam Macdonell - June 14, 2010, 4:08 p.m.
On Mon, Jun 14, 2010 at 9:51 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/04/2010 04:45 PM, Cam Macdonell wrote:
>>
>> This is useful for devices that do not want to take memory regions data
>> with them on migration.
>> ---
>>  arch_init.c  |   28 ++++++++++++++++------------
>>  cpu-all.h    |    2 ++
>>  cpu-common.h |    2 ++
>>  exec.c       |   12 ++++++++++++
>>  4 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index cfc03ea..7a234fa 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -118,18 +118,21 @@ static int ram_save_block(QEMUFile *f)
>>                                              current_addr +
>> TARGET_PAGE_SIZE,
>>                                              MIGRATION_DIRTY_FLAG);
>>
>> -            p = qemu_get_ram_ptr(current_addr);
>> -
>> -            if (is_dup_page(p, *p)) {
>> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
>> -                qemu_put_byte(f, *p);
>> -            } else {
>> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
>> -                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
>> -            }
>> +            if (!cpu_physical_memory_get_dirty(current_addr,
>> +                                                    NO_MIGRATION_FLAG)) {
>> +                p = qemu_get_ram_ptr(current_addr);
>> +
>> +                if (is_dup_page(p, *p)) {
>> +                    qemu_put_be64(f, current_addr |
>> RAM_SAVE_FLAG_COMPRESS);
>> +                    qemu_put_byte(f, *p);
>> +                } else {
>> +                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
>> +                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
>> +                }
>>
>> -            found = 1;
>> -            break;
>> +                found = 1;
>> +                break;
>> +            }
>>          }
>>
>
> Shouldn't we just disable live migration out right?

I'm confused, as you seemed insistent on migration before.  Do you
want to support static migration (suspend/resume), but not live
migration?  What information do the master/peer roles represent then?

>
> I would rather that the device mark migration as impossible having the user
> hot remove the device before migration and then add it again after
> migration.  Device assignment could also use this functionality.

Would marking migration impossible be a new mechanism or are there
other devices that mark migration impossible? or something added to
QMP "Sorry, you can't migrate with device 'x' attached"?

Cam

>>          addr += TARGET_PAGE_SIZE;
>>          current_addr = (saved_addr + addr) % last_ram_offset;
>> @@ -146,7 +149,8 @@ static ram_addr_t ram_save_remaining(void)
>>      ram_addr_t count = 0;
>>
>>      for (addr = 0; addr<  last_ram_offset; addr += TARGET_PAGE_SIZE) {
>> -        if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
>> +        if (!cpu_physical_memory_get_dirty(addr, NO_MIGRATION_FLAG)&&
>> +                cpu_physical_memory_get_dirty(addr,
>> MIGRATION_DIRTY_FLAG)) {
>>              count++;
>>          }
>>      }
>> diff --git a/cpu-all.h b/cpu-all.h
>> index 9080cc7..4df00ab 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -887,6 +887,8 @@ extern int mem_prealloc;
>>  #define CODE_DIRTY_FLAG      0x02
>>  #define MIGRATION_DIRTY_FLAG 0x08
>>
>> +#define NO_MIGRATION_FLAG 0x10
>> +
>>  #define DIRTY_ALL_FLAG  (VGA_DIRTY_FLAG | CODE_DIRTY_FLAG |
>> MIGRATION_DIRTY_FLAG)
>>
>>  /* read dirty bit (return 0 or 1) */
>> diff --git a/cpu-common.h b/cpu-common.h
>> index 4b0ba60..a1ebbbe 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -39,6 +39,8 @@ static inline void
>> cpu_register_physical_memory(target_phys_addr_t start_addr,
>>      cpu_register_physical_memory_offset(start_addr, size, phys_offset,
>> 0);
>>  }
>>
>> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t size);
>> +
>>  ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
>>  ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>>  ram_addr_t qemu_ram_alloc(ram_addr_t);
>> diff --git a/exec.c b/exec.c
>> index 39c18a7..c11d22f 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2786,6 +2786,18 @@ static void *file_ram_alloc(ram_addr_t memory,
>> const char *path)
>>  }
>>  #endif
>>
>> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t length)
>> +{
>> +    int i, len;
>> +    uint8_t *p;
>> +
>> +    len = length>>  TARGET_PAGE_BITS;
>> +    p = phys_ram_flags + (start>>  TARGET_PAGE_BITS);
>> +    for (i = 0; i<  len; i++) {
>> +        p[i] |= NO_MIGRATION_FLAG;
>> +    }
>> +}
>> +
>>  ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
>>  {
>>      RAMBlock *new_block;
>>
>
>
Anthony Liguori - June 14, 2010, 4:15 p.m.
On 06/14/2010 11:08 AM, Cam Macdonell wrote:
> On Mon, Jun 14, 2010 at 9:51 AM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>    
>> On 06/04/2010 04:45 PM, Cam Macdonell wrote:
>>      
>>> This is useful for devices that do not want to take memory regions data
>>> with them on migration.
>>> ---
>>>   arch_init.c  |   28 ++++++++++++++++------------
>>>   cpu-all.h    |    2 ++
>>>   cpu-common.h |    2 ++
>>>   exec.c       |   12 ++++++++++++
>>>   4 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch_init.c b/arch_init.c
>>> index cfc03ea..7a234fa 100644
>>> --- a/arch_init.c
>>> +++ b/arch_init.c
>>> @@ -118,18 +118,21 @@ static int ram_save_block(QEMUFile *f)
>>>                                               current_addr +
>>> TARGET_PAGE_SIZE,
>>>                                               MIGRATION_DIRTY_FLAG);
>>>
>>> -            p = qemu_get_ram_ptr(current_addr);
>>> -
>>> -            if (is_dup_page(p, *p)) {
>>> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
>>> -                qemu_put_byte(f, *p);
>>> -            } else {
>>> -                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
>>> -                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
>>> -            }
>>> +            if (!cpu_physical_memory_get_dirty(current_addr,
>>> +                                                    NO_MIGRATION_FLAG)) {
>>> +                p = qemu_get_ram_ptr(current_addr);
>>> +
>>> +                if (is_dup_page(p, *p)) {
>>> +                    qemu_put_be64(f, current_addr |
>>> RAM_SAVE_FLAG_COMPRESS);
>>> +                    qemu_put_byte(f, *p);
>>> +                } else {
>>> +                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
>>> +                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
>>> +                }
>>>
>>> -            found = 1;
>>> -            break;
>>> +                found = 1;
>>> +                break;
>>> +            }
>>>           }
>>>
>>>        
>> Shouldn't we just disable live migration out right?
>>      
> I'm confused, as you seemed insistent on migration before.  Do you
> want to support static migration (suspend/resume), but not live
> migration?  What information do the master/peer roles represent then?
>    

When role=master, you should not disable live migration and you should 
still migrate the contents of the data.  Otherwise, when you migrate, 
you lose the contents of shared memory and since the role is master, 
it's the one responsible for the data.

>> I would rather that the device mark migration as impossible having the user
>> hot remove the device before migration and then add it again after
>> migration.  Device assignment could also use this functionality.
>>      
> Would marking migration impossible be a new mechanism or are there
> other devices that mark migration impossible? or something added to
> QMP "Sorry, you can't migrate with device 'x' attached"?
>    

We don't have such a mechanism today.

Regards,

Anthony Liguori

> Cam
>
>    
>>>           addr += TARGET_PAGE_SIZE;
>>>           current_addr = (saved_addr + addr) % last_ram_offset;
>>> @@ -146,7 +149,8 @@ static ram_addr_t ram_save_remaining(void)
>>>       ram_addr_t count = 0;
>>>
>>>       for (addr = 0; addr<    last_ram_offset; addr += TARGET_PAGE_SIZE) {
>>> -        if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
>>> +        if (!cpu_physical_memory_get_dirty(addr, NO_MIGRATION_FLAG)&&
>>> +                cpu_physical_memory_get_dirty(addr,
>>> MIGRATION_DIRTY_FLAG)) {
>>>               count++;
>>>           }
>>>       }
>>> diff --git a/cpu-all.h b/cpu-all.h
>>> index 9080cc7..4df00ab 100644
>>> --- a/cpu-all.h
>>> +++ b/cpu-all.h
>>> @@ -887,6 +887,8 @@ extern int mem_prealloc;
>>>   #define CODE_DIRTY_FLAG      0x02
>>>   #define MIGRATION_DIRTY_FLAG 0x08
>>>
>>> +#define NO_MIGRATION_FLAG 0x10
>>> +
>>>   #define DIRTY_ALL_FLAG  (VGA_DIRTY_FLAG | CODE_DIRTY_FLAG |
>>> MIGRATION_DIRTY_FLAG)
>>>
>>>   /* read dirty bit (return 0 or 1) */
>>> diff --git a/cpu-common.h b/cpu-common.h
>>> index 4b0ba60..a1ebbbe 100644
>>> --- a/cpu-common.h
>>> +++ b/cpu-common.h
>>> @@ -39,6 +39,8 @@ static inline void
>>> cpu_register_physical_memory(target_phys_addr_t start_addr,
>>>       cpu_register_physical_memory_offset(start_addr, size, phys_offset,
>>> 0);
>>>   }
>>>
>>> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t size);
>>> +
>>>   ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
>>>   ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>>>   ram_addr_t qemu_ram_alloc(ram_addr_t);
>>> diff --git a/exec.c b/exec.c
>>> index 39c18a7..c11d22f 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -2786,6 +2786,18 @@ static void *file_ram_alloc(ram_addr_t memory,
>>> const char *path)
>>>   }
>>>   #endif
>>>
>>> +void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t length)
>>> +{
>>> +    int i, len;
>>> +    uint8_t *p;
>>> +
>>> +    len = length>>    TARGET_PAGE_BITS;
>>> +    p = phys_ram_flags + (start>>    TARGET_PAGE_BITS);
>>> +    for (i = 0; i<    len; i++) {
>>> +        p[i] |= NO_MIGRATION_FLAG;
>>> +    }
>>> +}
>>> +
>>>   ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
>>>   {
>>>       RAMBlock *new_block;
>>>
>>>        
>>
>>

Patch

diff --git a/arch_init.c b/arch_init.c
index cfc03ea..7a234fa 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -118,18 +118,21 @@  static int ram_save_block(QEMUFile *f)
                                             current_addr + TARGET_PAGE_SIZE,
                                             MIGRATION_DIRTY_FLAG);
 
-            p = qemu_get_ram_ptr(current_addr);
-
-            if (is_dup_page(p, *p)) {
-                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
-                qemu_put_byte(f, *p);
-            } else {
-                qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
-                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
-            }
+            if (!cpu_physical_memory_get_dirty(current_addr,
+                                                    NO_MIGRATION_FLAG)) {
+                p = qemu_get_ram_ptr(current_addr);
+
+                if (is_dup_page(p, *p)) {
+                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
+                    qemu_put_byte(f, *p);
+                } else {
+                    qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
+                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+                }
 
-            found = 1;
-            break;
+                found = 1;
+                break;
+            }
         }
         addr += TARGET_PAGE_SIZE;
         current_addr = (saved_addr + addr) % last_ram_offset;
@@ -146,7 +149,8 @@  static ram_addr_t ram_save_remaining(void)
     ram_addr_t count = 0;
 
     for (addr = 0; addr < last_ram_offset; addr += TARGET_PAGE_SIZE) {
-        if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
+        if (!cpu_physical_memory_get_dirty(addr, NO_MIGRATION_FLAG) &&
+                cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
             count++;
         }
     }
diff --git a/cpu-all.h b/cpu-all.h
index 9080cc7..4df00ab 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -887,6 +887,8 @@  extern int mem_prealloc;
 #define CODE_DIRTY_FLAG      0x02
 #define MIGRATION_DIRTY_FLAG 0x08
 
+#define NO_MIGRATION_FLAG 0x10
+
 #define DIRTY_ALL_FLAG  (VGA_DIRTY_FLAG | CODE_DIRTY_FLAG | MIGRATION_DIRTY_FLAG)
 
 /* read dirty bit (return 0 or 1) */
diff --git a/cpu-common.h b/cpu-common.h
index 4b0ba60..a1ebbbe 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -39,6 +39,8 @@  static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
     cpu_register_physical_memory_offset(start_addr, size, phys_offset, 0);
 }
 
+void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t size);
+
 ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
 ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
 ram_addr_t qemu_ram_alloc(ram_addr_t);
diff --git a/exec.c b/exec.c
index 39c18a7..c11d22f 100644
--- a/exec.c
+++ b/exec.c
@@ -2786,6 +2786,18 @@  static void *file_ram_alloc(ram_addr_t memory, const char *path)
 }
 #endif
 
+void cpu_mark_pages_no_migrate(ram_addr_t start, uint64_t length)
+{
+    int i, len;
+    uint8_t *p;
+
+    len = length >> TARGET_PAGE_BITS;
+    p = phys_ram_flags + (start >> TARGET_PAGE_BITS);
+    for (i = 0; i < len; i++) {
+        p[i] |= NO_MIGRATION_FLAG;
+    }
+}
+
 ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
 {
     RAMBlock *new_block;