Merging KVM live migration upstream
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
vl.c +13 -0
@@ -3097,6 +3204,8 @@ static int ram_load_v1(QEMUFile *f, voidif (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, voif (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;
Quoted 7 lines · Jan Kiszka
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.
Quoted 4 lines
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().
Quoted 5 lines
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.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Quoted 9 lines · Avi Kivity
Quoted 7 lines · Jan Kiszka
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.
Quoted 5 lines
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.
Quoted 9 lines
Quoted 5 lines
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 incpu_physical_memory_get_dirty().
Yes, I realized that there is already cpu_physical_sync_dirty_bitmap(), and I will simply use this.
Quoted 8 lines
Quoted 6 lines
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
Quoted 6 lines · Jan Kiszka
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.
Quoted 8 lines
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.
Quoted 40 lines
Jan
vl.c +13
-0@@ -3097,6 +3204,8 @@ static int ram_load_v1(QEMUFile *f, voidif (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, voif (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;