Merging KVM live migration upstream

Merging KVM live migration upstream

[Jan Kiszka]

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

Avi or Uri,

could you explain the first and third hunk? Why are they needed in
qemu-kvm, and will we also need something comparable upstream? They do
not look very beautiful.

The second hunk, I guess, should become a kvm hook to
cpu_physical_memory_get_dirty - or is this too costly for other users of
this inline function?

And does anyone knows further migration-related hunks that are missing
upstream (except for the KVM hook in
cpu_physical_memory_set_dirty_tracking)?

Jan

--- qemu/vl.c
+++ qemu-kvm/vl.c
@@ -3097,6 +3204,8 @@ static int ram_load_v1(QEMUFile *f, void
     if (qemu_get_be32(f) != last_ram_offset)
         return -EINVAL;
     for(i = 0; i < last_ram_offset; i+= TARGET_PAGE_SIZE) {
+        if (kvm_enabled() && (i>=0xa0000) && (i<0xc0000)) /* do not access video-addresses */
+            continue;
         ret = ram_get_page(f, qemu_get_ram_ptr(i), TARGET_PAGE_SIZE);
         if (ret)
             return ret;
@@ -3183,6 +3292,15 @@ static int ram_save_block(QEMUFile *f)
     int found = 0;

     while (addr < last_ram_offset) {
+        if (kvm_enabled() && current_addr == 0) {
+            int r;
+            r = kvm_update_dirty_pages_log();
+            if (r) {
+                fprintf(stderr, "%s: update dirty pages log failed %d\n", __FUNCTION__, r);
+                qemu_file_set_error(f);
+                return 0;
+            }
+        }
         if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
             uint8_t *p;

@@ -3273,6 +3391,8 @@ static int ram_load_dead(QEMUFile *f, vo
     if (ram_decompress_open(s, f) < 0)
         return -EINVAL;
     for(i = 0; i < last_ram_offset; i+= BDRV_HASH_BLOCK_SIZE) {
+        if (kvm_enabled() && (i>=0xa0000) && (i<0xc0000)) /* do not access video-addresses */
+            continue;
         if (ram_decompress_buf(s, buf, 1) < 0) {
             fprintf(stderr, "Error while reading ram block header\n");
             goto error;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: Merging KVM live migration upstream

[Avi Kivity]

Jan Kiszka wrote:
> Avi or Uri,
>
> could you explain the first and third hunk? Why are they needed in
> qemu-kvm, and will we also need something comparable upstream? They do
> not look very beautiful.
>
>   

These date from the bad old days where we relied on phys_ram_base.  I 
think this is obsolete.

Since we reserve these memory addresses, it won't do any harm, but we 
can certainly remove them.

> The second hunk, I guess, should become a kvm hook to
> cpu_physical_memory_get_dirty - or is this too costly for other users of
> this inline function?
>   

Note the dependency on current_addr: this is meant to happen every time 
we cycle through all memory, so it doesn't fit in 
cpu_physical_memory_get_dirty().

> And does anyone knows further migration-related hunks that are missing
> upstream (except for the KVM hook in
> cpu_physical_memory_set_dirty_tracking)?
>
>   

We need to make sure vga plays well with this, like we discussed.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: Merging KVM live migration upstream

[Jan Kiszka]

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

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Avi or Uri,
>>
>> could you explain the first and third hunk? Why are they needed in
>> qemu-kvm, and will we also need something comparable upstream? They do
>> not look very beautiful.
>>
>>   
> 
> These date from the bad old days where we relied on phys_ram_base.  I
> think this is obsolete.
> 
> Since we reserve these memory addresses, it won't do any harm, but we
> can certainly remove them.

That's great, will drop it.

> 
>> The second hunk, I guess, should become a kvm hook to
>> cpu_physical_memory_get_dirty - or is this too costly for other users of
>> this inline function?
>>   
> 
> Note the dependency on current_addr: this is meant to happen every time
> we cycle through all memory, so it doesn't fit in
> cpu_physical_memory_get_dirty().

Yes, I realized that there is already cpu_physical_sync_dirty_bitmap(),
and I will simply use this.

> 
>> And does anyone knows further migration-related hunks that are missing
>> upstream (except for the KVM hook in
>> cpu_physical_memory_set_dirty_tracking)?
>>
>>   
> 
> We need to make sure vga plays well with this, like we discussed.

Already implemented in ~30 additional LOC. Just have to test the whole
series (found some more things to clean up)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: Merging KVM live migration upstream

[Anthony Liguori]

Jan Kiszka wrote:
> Avi or Uri,
>
> could you explain the first and third hunk? Why are they needed in
> qemu-kvm, and will we also need something comparable upstream? They do
> not look very beautiful.
>   

This is leftovers from v1 loading.  It used to be that we saved the vga 
buffer independently before slot tracking was as sophisticated in qemu 
as it is today.  We don't restore v1 vga that would contain this section 
though so it's basically dead code.

I'd just remove it personally.

> The second hunk, I guess, should become a kvm hook to
> cpu_physical_memory_get_dirty - or is this too costly for other users of
> this inline function?
>
> And does anyone knows further migration-related hunks that are missing
> upstream (except for the KVM hook in
> cpu_physical_memory_set_dirty_tracking)?
>   

The cpu_save/cpu_load additions.

> Jan
>
> --- qemu/vl.c
> +++ qemu-kvm/vl.c
> @@ -3097,6 +3204,8 @@ static int ram_load_v1(QEMUFile *f, void
>      if (qemu_get_be32(f) != last_ram_offset)
>          return -EINVAL;
>      for(i = 0; i < last_ram_offset; i+= TARGET_PAGE_SIZE) {
> +        if (kvm_enabled() && (i>=0xa0000) && (i<0xc0000)) /* do not access video-addresses */
> +            continue;
>          ret = ram_get_page(f, qemu_get_ram_ptr(i), TARGET_PAGE_SIZE);
>          if (ret)
>              return ret;
> @@ -3183,6 +3292,15 @@ static int ram_save_block(QEMUFile *f)
>      int found = 0;
>
>      while (addr < last_ram_offset) {
> +        if (kvm_enabled() && current_addr == 0) {
> +            int r;
> +            r = kvm_update_dirty_pages_log();
> +            if (r) {
> +                fprintf(stderr, "%s: update dirty pages log failed %d\n", __FUNCTION__, r);
> +                qemu_file_set_error(f);
> +                return 0;
> +            }
> +        }
>          if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
>              uint8_t *p;
>
> @@ -3273,6 +3391,8 @@ static int ram_load_dead(QEMUFile *f, vo
>      if (ram_decompress_open(s, f) < 0)
>          return -EINVAL;
>      for(i = 0; i < last_ram_offset; i+= BDRV_HASH_BLOCK_SIZE) {
> +        if (kvm_enabled() && (i>=0xa0000) && (i<0xc0000)) /* do not access video-addresses */
> +            continue;
>          if (ram_decompress_buf(s, buf, 1) < 0) {
>              fprintf(stderr, "Error while reading ram block header\n");
>              goto error;
>
>