* [Qemu-devel] [PATCH 01/30] split MRU ram list
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
@ 2012-10-18 7:29 ` Juan Quintela
2012-10-21 11:58 ` Orit Wasserman
2012-10-21 17:02 ` Peter Maydell
2012-10-18 7:29 ` [Qemu-devel] [PATCH 02/30] add a version number to ram_list Juan Quintela
` (30 subsequent siblings)
31 siblings, 2 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
From: Paolo Bonzini <pbonzini@redhat.com>
Outside the execution threads the normal, non-MRU-ized order of
the RAM blocks should always be enough. So manage two separate
lists, which will have separate locking rules.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 1 +
cpu-all.h | 4 +++-
exec.c | 18 +++++++++++++-----
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index e6effe8..4293557 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -48,6 +48,7 @@
#include "qemu/page_cache.h"
#include "qmp-commands.h"
#include "trace.h"
+#include "cpu-all.h"
#ifdef DEBUG_ARCH_INIT
#define DPRINTF(fmt, ...) \
diff --git a/cpu-all.h b/cpu-all.h
index 6aa7e58..6558a6f 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -490,8 +490,9 @@ typedef struct RAMBlock {
ram_addr_t offset;
ram_addr_t length;
uint32_t flags;
- char idstr[256];
+ QLIST_ENTRY(RAMBlock) next_mru;
QLIST_ENTRY(RAMBlock) next;
+ char idstr[256];
#if defined(__linux__) && !defined(TARGET_S390X)
int fd;
#endif
@@ -499,6 +500,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
uint8_t *phys_dirty;
+ QLIST_HEAD(, RAMBlock) blocks_mru;
QLIST_HEAD(, RAMBlock) blocks;
} RAMList;
extern RAMList ram_list;
diff --git a/exec.c b/exec.c
index e63ad0d..718bbc2 100644
--- a/exec.c
+++ b/exec.c
@@ -56,6 +56,7 @@
#include "xen-mapcache.h"
#include "trace.h"
#endif
+#include "cpu-all.h"
#include "cputlb.h"
@@ -112,7 +113,10 @@ static uint8_t *code_gen_ptr;
int phys_ram_fd;
static int in_migration;
-RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
+RAMList ram_list = {
+ .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks),
+ .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
+};
static MemoryRegion *system_memory;
static MemoryRegion *system_io;
@@ -633,6 +637,7 @@ bool tcg_enabled(void)
void cpu_exec_init_all(void)
{
#if !defined(CONFIG_USER_ONLY)
+ qemu_mutex_init(&ram_list.mutex);
memory_map_init();
io_mem_init();
#endif
@@ -2568,6 +2573,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
new_block->length = size;
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2595,6 +2601,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
g_free(block);
return;
}
@@ -2608,6 +2615,7 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
@@ -2713,12 +2721,12 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
{
RAMBlock *block;
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
if (addr - block->offset < block->length) {
/* Move this entry to to start of the list. */
if (block != QLIST_FIRST(&ram_list.blocks)) {
- QLIST_REMOVE(block, next);
- QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
+ QLIST_REMOVE(block, next_mru);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, block, next_mru);
}
if (xen_enabled()) {
/* We need to check if the requested address is in the RAM
@@ -2813,7 +2821,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
return 0;
}
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
/* This case append when the block is not mapped. */
if (block->host == NULL) {
continue;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 01/30] split MRU ram list
2012-10-18 7:29 ` [Qemu-devel] [PATCH 01/30] split MRU ram list Juan Quintela
@ 2012-10-21 11:58 ` Orit Wasserman
2012-10-21 17:02 ` Peter Maydell
1 sibling, 0 replies; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 11:58 UTC (permalink / raw)
To: Juan Quintela; +Cc: Paolo Bonzini, qemu-devel
On 10/18/2012 09:29 AM, Juan Quintela wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Outside the execution threads the normal, non-MRU-ized order of
> the RAM blocks should always be enough. So manage two separate
> lists, which will have separate locking rules.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> arch_init.c | 1 +
> cpu-all.h | 4 +++-
> exec.c | 18 +++++++++++++-----
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index e6effe8..4293557 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -48,6 +48,7 @@
> #include "qemu/page_cache.h"
> #include "qmp-commands.h"
> #include "trace.h"
> +#include "cpu-all.h"
>
> #ifdef DEBUG_ARCH_INIT
> #define DPRINTF(fmt, ...) \
> diff --git a/cpu-all.h b/cpu-all.h
> index 6aa7e58..6558a6f 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -490,8 +490,9 @@ typedef struct RAMBlock {
> ram_addr_t offset;
> ram_addr_t length;
> uint32_t flags;
> - char idstr[256];
> + QLIST_ENTRY(RAMBlock) next_mru;
> QLIST_ENTRY(RAMBlock) next;
> + char idstr[256];
> #if defined(__linux__) && !defined(TARGET_S390X)
> int fd;
> #endif
> @@ -499,6 +500,7 @@ typedef struct RAMBlock {
>
> typedef struct RAMList {
> uint8_t *phys_dirty;
> + QLIST_HEAD(, RAMBlock) blocks_mru;
> QLIST_HEAD(, RAMBlock) blocks;
> } RAMList;
> extern RAMList ram_list;
> diff --git a/exec.c b/exec.c
> index e63ad0d..718bbc2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -56,6 +56,7 @@
> #include "xen-mapcache.h"
> #include "trace.h"
> #endif
> +#include "cpu-all.h"
>
> #include "cputlb.h"
>
> @@ -112,7 +113,10 @@ static uint8_t *code_gen_ptr;
> int phys_ram_fd;
> static int in_migration;
>
> -RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
> +RAMList ram_list = {
> + .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks),
> + .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
> +};
>
> static MemoryRegion *system_memory;
> static MemoryRegion *system_io;
> @@ -633,6 +637,7 @@ bool tcg_enabled(void)
> void cpu_exec_init_all(void)
> {
> #if !defined(CONFIG_USER_ONLY)
> + qemu_mutex_init(&ram_list.mutex);
> memory_map_init();
> io_mem_init();
> #endif
> @@ -2568,6 +2573,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> new_block->length = size;
>
> QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
> + QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
>
> ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
> last_ram_offset() >> TARGET_PAGE_BITS);
> @@ -2595,6 +2601,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
> QLIST_FOREACH(block, &ram_list.blocks, next) {
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> + QLIST_REMOVE(block, next_mru);
> g_free(block);
> return;
> }
> @@ -2608,6 +2615,7 @@ void qemu_ram_free(ram_addr_t addr)
> QLIST_FOREACH(block, &ram_list.blocks, next) {
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> + QLIST_REMOVE(block, next_mru);
> if (block->flags & RAM_PREALLOC_MASK) {
> ;
> } else if (mem_path) {
> @@ -2713,12 +2721,12 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
> {
> RAMBlock *block;
>
> - QLIST_FOREACH(block, &ram_list.blocks, next) {
> + QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
> if (addr - block->offset < block->length) {
> /* Move this entry to to start of the list. */
> if (block != QLIST_FIRST(&ram_list.blocks)) {
> - QLIST_REMOVE(block, next);
> - QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
> + QLIST_REMOVE(block, next_mru);
> + QLIST_INSERT_HEAD(&ram_list.blocks_mru, block, next_mru);
> }
> if (xen_enabled()) {
> /* We need to check if the requested address is in the RAM
> @@ -2813,7 +2821,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
> return 0;
> }
>
> - QLIST_FOREACH(block, &ram_list.blocks, next) {
> + QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
> /* This case append when the block is not mapped. */
> if (block->host == NULL) {
> continue;
>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 01/30] split MRU ram list
2012-10-18 7:29 ` [Qemu-devel] [PATCH 01/30] split MRU ram list Juan Quintela
2012-10-21 11:58 ` Orit Wasserman
@ 2012-10-21 17:02 ` Peter Maydell
1 sibling, 0 replies; 58+ messages in thread
From: Peter Maydell @ 2012-10-21 17:02 UTC (permalink / raw)
To: Juan Quintela; +Cc: Paolo Bonzini, qemu-devel
On 18 October 2012 08:29, Juan Quintela <quintela@redhat.com> wrote:
> -RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
> +RAMList ram_list = {
> + .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks),
> + .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
> +};
Maybe worth adding a comment explaining why we have the two lists,
to save people having to ferret around in the revision history later?
-- PMM
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 02/30] add a version number to ram_list
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
2012-10-18 7:29 ` [Qemu-devel] [PATCH 01/30] split MRU ram list Juan Quintela
@ 2012-10-18 7:29 ` Juan Quintela
2012-10-21 12:00 ` Orit Wasserman
2012-10-18 7:29 ` [Qemu-devel] [PATCH 03/30] protect the ramlist with a separate mutex Juan Quintela
` (29 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Umesh Deshpande
From: Umesh Deshpande <udeshpan@redhat.com>
This will be used to detect if last_block might have become invalid
across different calls to ram_save_live.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 7 ++++++-
cpu-all.h | 1 +
exec.c | 5 ++++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 4293557..b47313d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -336,6 +336,7 @@ static RAMBlock *last_block;
static ram_addr_t last_offset;
static unsigned long *migration_bitmap;
static uint64_t migration_dirty_pages;
+static uint32_t last_version;
static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
ram_addr_t offset)
@@ -406,7 +407,6 @@ static void migration_bitmap_sync(void)
}
}
-
/*
* ram_save_block: Writes a page of memory to the stream f
*
@@ -558,6 +558,7 @@ static void reset_ram_globals(void)
{
last_block = NULL;
last_offset = 0;
+ last_version = ram_list.version;
sort_ram_list();
}
@@ -613,6 +614,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
uint64_t expected_downtime;
MigrationState *s = migrate_get_current();
+ if (ram_list.version != last_version) {
+ reset_ram_globals();
+ }
+
bytes_transferred_last = bytes_transferred;
bwidth = qemu_get_clock_ns(rt_clock);
diff --git a/cpu-all.h b/cpu-all.h
index 6558a6f..e07c91c 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -500,6 +500,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
uint8_t *phys_dirty;
+ uint32_t version;
QLIST_HEAD(, RAMBlock) blocks_mru;
QLIST_HEAD(, RAMBlock) blocks;
} RAMList;
diff --git a/exec.c b/exec.c
index 718bbc2..1e04711 100644
--- a/exec.c
+++ b/exec.c
@@ -637,7 +637,6 @@ bool tcg_enabled(void)
void cpu_exec_init_all(void)
{
#if !defined(CONFIG_USER_ONLY)
- qemu_mutex_init(&ram_list.mutex);
memory_map_init();
io_mem_init();
#endif
@@ -2575,6 +2574,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
+ ram_list.version++;
+
ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
last_ram_offset() >> TARGET_PAGE_BITS);
memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
@@ -2602,6 +2603,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
if (addr == block->offset) {
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ ram_list.version++;
g_free(block);
return;
}
@@ -2616,6 +2618,7 @@ void qemu_ram_free(ram_addr_t addr)
if (addr == block->offset) {
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ ram_list.version++;
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 02/30] add a version number to ram_list
2012-10-18 7:29 ` [Qemu-devel] [PATCH 02/30] add a version number to ram_list Juan Quintela
@ 2012-10-21 12:00 ` Orit Wasserman
0 siblings, 0 replies; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 12:00 UTC (permalink / raw)
To: Juan Quintela; +Cc: Paolo Bonzini, Umesh Deshpande, qemu-devel
On 10/18/2012 09:29 AM, Juan Quintela wrote:
> From: Umesh Deshpande <udeshpan@redhat.com>
>
> This will be used to detect if last_block might have become invalid
> across different calls to ram_save_live.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> arch_init.c | 7 ++++++-
> cpu-all.h | 1 +
> exec.c | 5 ++++-
> 3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 4293557..b47313d 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -336,6 +336,7 @@ static RAMBlock *last_block;
> static ram_addr_t last_offset;
> static unsigned long *migration_bitmap;
> static uint64_t migration_dirty_pages;
> +static uint32_t last_version;
>
> static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
> ram_addr_t offset)
> @@ -406,7 +407,6 @@ static void migration_bitmap_sync(void)
> }
> }
>
> -
> /*
> * ram_save_block: Writes a page of memory to the stream f
> *
> @@ -558,6 +558,7 @@ static void reset_ram_globals(void)
> {
> last_block = NULL;
> last_offset = 0;
> + last_version = ram_list.version;
> sort_ram_list();
> }
>
> @@ -613,6 +614,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> uint64_t expected_downtime;
> MigrationState *s = migrate_get_current();
>
> + if (ram_list.version != last_version) {
> + reset_ram_globals();
> + }
> +
> bytes_transferred_last = bytes_transferred;
> bwidth = qemu_get_clock_ns(rt_clock);
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 6558a6f..e07c91c 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -500,6 +500,7 @@ typedef struct RAMBlock {
>
> typedef struct RAMList {
> uint8_t *phys_dirty;
> + uint32_t version;
> QLIST_HEAD(, RAMBlock) blocks_mru;
> QLIST_HEAD(, RAMBlock) blocks;
> } RAMList;
> diff --git a/exec.c b/exec.c
> index 718bbc2..1e04711 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -637,7 +637,6 @@ bool tcg_enabled(void)
> void cpu_exec_init_all(void)
> {
> #if !defined(CONFIG_USER_ONLY)
> - qemu_mutex_init(&ram_list.mutex);
> memory_map_init();
> io_mem_init();
> #endif
> @@ -2575,6 +2574,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
> QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
>
> + ram_list.version++;
> +
> ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
> last_ram_offset() >> TARGET_PAGE_BITS);
> memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
> @@ -2602,6 +2603,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + ram_list.version++;
> g_free(block);
> return;
> }
> @@ -2616,6 +2618,7 @@ void qemu_ram_free(ram_addr_t addr)
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + ram_list.version++;
> if (block->flags & RAM_PREALLOC_MASK) {
> ;
> } else if (mem_path) {
>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 03/30] protect the ramlist with a separate mutex
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
2012-10-18 7:29 ` [Qemu-devel] [PATCH 01/30] split MRU ram list Juan Quintela
2012-10-18 7:29 ` [Qemu-devel] [PATCH 02/30] add a version number to ram_list Juan Quintela
@ 2012-10-18 7:29 ` Juan Quintela
2012-10-21 12:05 ` Orit Wasserman
2012-10-18 7:30 ` [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread Juan Quintela
` (28 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Umesh Deshpande
From: Umesh Deshpande <udeshpan@redhat.com>
Add the new mutex that protects shared state between ram_save_live
and the iothread. If the iothread mutex has to be taken together
with the ramlist mutex, the iothread shall always be _outside_.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 9 ++++++++-
cpu-all.h | 8 ++++++++
exec.c | 23 +++++++++++++++++++++--
3 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index b47313d..2d29828 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -553,7 +553,6 @@ static void ram_migration_cancel(void *opaque)
migration_end();
}
-
static void reset_ram_globals(void)
{
last_block = NULL;
@@ -573,6 +572,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
bitmap_set(migration_bitmap, 1, ram_pages);
migration_dirty_pages = ram_pages;
+ qemu_mutex_lock_ramlist();
bytes_transferred = 0;
reset_ram_globals();
@@ -600,6 +600,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
qemu_put_be64(f, block->length);
}
+ qemu_mutex_unlock_ramlist();
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
return 0;
@@ -614,6 +615,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
uint64_t expected_downtime;
MigrationState *s = migrate_get_current();
+ qemu_mutex_lock_ramlist();
+
if (ram_list.version != last_version) {
reset_ram_globals();
}
@@ -662,6 +665,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
bwidth = 0.000001;
}
+ qemu_mutex_unlock_ramlist();
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
@@ -682,6 +686,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
{
migration_bitmap_sync();
+ qemu_mutex_lock_ramlist();
+
/* try transferring iterative blocks of memory */
/* flush all remaining blocks regardless of rate limiting */
@@ -697,6 +703,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
}
memory_global_dirty_log_stop();
+ qemu_mutex_unlock_ramlist();
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
g_free(migration_bitmap);
diff --git a/cpu-all.h b/cpu-all.h
index e07c91c..2bafcdd 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -22,6 +22,7 @@
#include "qemu-common.h"
#include "qemu-tls.h"
#include "cpu-common.h"
+#include "qemu-thread.h"
/* some important defines:
*
@@ -490,7 +491,9 @@ typedef struct RAMBlock {
ram_addr_t offset;
ram_addr_t length;
uint32_t flags;
+ /* Protected by the iothread lock. */
QLIST_ENTRY(RAMBlock) next_mru;
+ /* Protected by the ramlist lock. */
QLIST_ENTRY(RAMBlock) next;
char idstr[256];
#if defined(__linux__) && !defined(TARGET_S390X)
@@ -499,9 +502,12 @@ typedef struct RAMBlock {
} RAMBlock;
typedef struct RAMList {
+ QemuMutex mutex;
+ /* Protected by the iothread lock. */
uint8_t *phys_dirty;
uint32_t version;
QLIST_HEAD(, RAMBlock) blocks_mru;
+ /* Protected by the ramlist lock. */
QLIST_HEAD(, RAMBlock) blocks;
} RAMList;
extern RAMList ram_list;
@@ -521,6 +527,8 @@ extern int mem_prealloc;
void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
ram_addr_t last_ram_offset(void);
+void qemu_mutex_lock_ramlist(void);
+void qemu_mutex_unlock_ramlist(void);
#endif /* !CONFIG_USER_ONLY */
int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
diff --git a/exec.c b/exec.c
index 1e04711..cf9de92 100644
--- a/exec.c
+++ b/exec.c
@@ -637,6 +637,7 @@ bool tcg_enabled(void)
void cpu_exec_init_all(void)
{
#if !defined(CONFIG_USER_ONLY)
+ qemu_mutex_init(&ram_list.mutex);
memory_map_init();
io_mem_init();
#endif
@@ -2329,6 +2330,16 @@ void qemu_flush_coalesced_mmio_buffer(void)
kvm_flush_coalesced_mmio_buffer();
}
+void qemu_mutex_lock_ramlist(void)
+{
+ qemu_mutex_lock(&ram_list.mutex);
+}
+
+void qemu_mutex_unlock_ramlist(void)
+{
+ qemu_mutex_unlock(&ram_list.mutex);
+}
+
#if defined(__linux__) && !defined(TARGET_S390X)
#include <sys/vfs.h>
@@ -2510,6 +2521,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
}
pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
+ qemu_mutex_lock_ramlist();
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
@@ -2517,6 +2529,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
abort();
}
}
+ qemu_mutex_unlock_ramlist();
}
static int memory_try_enable_merging(void *addr, size_t len)
@@ -2540,6 +2553,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
size = TARGET_PAGE_ALIGN(size);
new_block = g_malloc0(sizeof(*new_block));
+ qemu_mutex_lock_ramlist();
new_block->mr = mr;
new_block->offset = find_ram_offset(size);
if (host) {
@@ -2575,6 +2589,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
ram_list.version++;
+ qemu_mutex_unlock_ramlist();
ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2599,21 +2614,24 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
{
RAMBlock *block;
+ qemu_mutex_lock_ramlist();
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
ram_list.version++;
g_free(block);
- return;
+ break;
}
}
+ qemu_mutex_unlock_ramlist();
}
void qemu_ram_free(ram_addr_t addr)
{
RAMBlock *block;
+ qemu_mutex_lock_ramlist();
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
@@ -2644,9 +2662,10 @@ void qemu_ram_free(ram_addr_t addr)
#endif
}
g_free(block);
- return;
+ break;
}
}
+ qemu_mutex_unlock_ramlist();
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 03/30] protect the ramlist with a separate mutex
2012-10-18 7:29 ` [Qemu-devel] [PATCH 03/30] protect the ramlist with a separate mutex Juan Quintela
@ 2012-10-21 12:05 ` Orit Wasserman
0 siblings, 0 replies; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 12:05 UTC (permalink / raw)
To: Juan Quintela; +Cc: Paolo Bonzini, Umesh Deshpande, qemu-devel
On 10/18/2012 09:29 AM, Juan Quintela wrote:
> From: Umesh Deshpande <udeshpan@redhat.com>
>
> Add the new mutex that protects shared state between ram_save_live
> and the iothread. If the iothread mutex has to be taken together
> with the ramlist mutex, the iothread shall always be _outside_.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> arch_init.c | 9 ++++++++-
> cpu-all.h | 8 ++++++++
> exec.c | 23 +++++++++++++++++++++--
> 3 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index b47313d..2d29828 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -553,7 +553,6 @@ static void ram_migration_cancel(void *opaque)
> migration_end();
> }
>
> -
> static void reset_ram_globals(void)
> {
> last_block = NULL;
> @@ -573,6 +572,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> bitmap_set(migration_bitmap, 1, ram_pages);
> migration_dirty_pages = ram_pages;
>
> + qemu_mutex_lock_ramlist();
> bytes_transferred = 0;
> reset_ram_globals();
>
> @@ -600,6 +600,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> qemu_put_be64(f, block->length);
> }
>
> + qemu_mutex_unlock_ramlist();
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>
> return 0;
> @@ -614,6 +615,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> uint64_t expected_downtime;
> MigrationState *s = migrate_get_current();
>
> + qemu_mutex_lock_ramlist();
> +
> if (ram_list.version != last_version) {
> reset_ram_globals();
> }
> @@ -662,6 +665,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> bwidth = 0.000001;
> }
>
> + qemu_mutex_unlock_ramlist();
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>
> expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
> @@ -682,6 +686,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> {
> migration_bitmap_sync();
>
> + qemu_mutex_lock_ramlist();
> +
> /* try transferring iterative blocks of memory */
>
> /* flush all remaining blocks regardless of rate limiting */
> @@ -697,6 +703,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> }
> memory_global_dirty_log_stop();
>
> + qemu_mutex_unlock_ramlist();
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>
> g_free(migration_bitmap);
> diff --git a/cpu-all.h b/cpu-all.h
> index e07c91c..2bafcdd 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -22,6 +22,7 @@
> #include "qemu-common.h"
> #include "qemu-tls.h"
> #include "cpu-common.h"
> +#include "qemu-thread.h"
>
> /* some important defines:
> *
> @@ -490,7 +491,9 @@ typedef struct RAMBlock {
> ram_addr_t offset;
> ram_addr_t length;
> uint32_t flags;
> + /* Protected by the iothread lock. */
> QLIST_ENTRY(RAMBlock) next_mru;
> + /* Protected by the ramlist lock. */
> QLIST_ENTRY(RAMBlock) next;
> char idstr[256];
> #if defined(__linux__) && !defined(TARGET_S390X)
> @@ -499,9 +502,12 @@ typedef struct RAMBlock {
> } RAMBlock;
>
> typedef struct RAMList {
> + QemuMutex mutex;
> + /* Protected by the iothread lock. */
> uint8_t *phys_dirty;
> uint32_t version;
> QLIST_HEAD(, RAMBlock) blocks_mru;
> + /* Protected by the ramlist lock. */
> QLIST_HEAD(, RAMBlock) blocks;
> } RAMList;
> extern RAMList ram_list;
> @@ -521,6 +527,8 @@ extern int mem_prealloc;
>
> void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
> ram_addr_t last_ram_offset(void);
> +void qemu_mutex_lock_ramlist(void);
> +void qemu_mutex_unlock_ramlist(void);
> #endif /* !CONFIG_USER_ONLY */
>
> int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
> diff --git a/exec.c b/exec.c
> index 1e04711..cf9de92 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -637,6 +637,7 @@ bool tcg_enabled(void)
> void cpu_exec_init_all(void)
> {
> #if !defined(CONFIG_USER_ONLY)
> + qemu_mutex_init(&ram_list.mutex);
> memory_map_init();
> io_mem_init();
> #endif
> @@ -2329,6 +2330,16 @@ void qemu_flush_coalesced_mmio_buffer(void)
> kvm_flush_coalesced_mmio_buffer();
> }
>
> +void qemu_mutex_lock_ramlist(void)
> +{
> + qemu_mutex_lock(&ram_list.mutex);
> +}
> +
> +void qemu_mutex_unlock_ramlist(void)
> +{
> + qemu_mutex_unlock(&ram_list.mutex);
> +}
> +
> #if defined(__linux__) && !defined(TARGET_S390X)
>
> #include <sys/vfs.h>
> @@ -2510,6 +2521,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
> }
> pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
>
> + qemu_mutex_lock_ramlist();
> QLIST_FOREACH(block, &ram_list.blocks, next) {
> if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
> fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
> @@ -2517,6 +2529,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
> abort();
> }
> }
> + qemu_mutex_unlock_ramlist();
> }
>
> static int memory_try_enable_merging(void *addr, size_t len)
> @@ -2540,6 +2553,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> size = TARGET_PAGE_ALIGN(size);
> new_block = g_malloc0(sizeof(*new_block));
>
> + qemu_mutex_lock_ramlist();
> new_block->mr = mr;
> new_block->offset = find_ram_offset(size);
> if (host) {
> @@ -2575,6 +2589,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
>
> ram_list.version++;
> + qemu_mutex_unlock_ramlist();
>
> ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
> last_ram_offset() >> TARGET_PAGE_BITS);
> @@ -2599,21 +2614,24 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
> {
> RAMBlock *block;
>
> + qemu_mutex_lock_ramlist();
> QLIST_FOREACH(block, &ram_list.blocks, next) {
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> ram_list.version++;
> g_free(block);
> - return;
> + break;
> }
> }
> + qemu_mutex_unlock_ramlist();
> }
>
> void qemu_ram_free(ram_addr_t addr)
> {
> RAMBlock *block;
>
> + qemu_mutex_lock_ramlist();
> QLIST_FOREACH(block, &ram_list.blocks, next) {
> if (addr == block->offset) {
> QLIST_REMOVE(block, next);
> @@ -2644,9 +2662,10 @@ void qemu_ram_free(ram_addr_t addr)
> #endif
> }
> g_free(block);
> - return;
> + break;
> }
> }
> + qemu_mutex_unlock_ramlist();
>
> }
>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (2 preceding siblings ...)
2012-10-18 7:29 ` [Qemu-devel] [PATCH 03/30] protect the ramlist with a separate mutex Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 8:56 ` Paolo Bonzini
` (2 more replies)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void Juan Quintela
` (27 subsequent siblings)
31 siblings, 3 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
We still protect everything except the wait with the iothread lock.
But we moved from a timer to a thread. Steps one by one.
We also need to detect when we have finished with a variable "complete".
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 58 +++++++++++++++++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index ed92df1..4b90d54 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -18,6 +18,7 @@
#include "qemu-timer.h"
#include "qemu-char.h"
#include "buffered_file.h"
+#include "qemu-thread.h"
//#define DEBUG_BUFFERED_FILE
@@ -31,7 +32,8 @@ typedef struct QEMUFileBuffered
uint8_t *buffer;
size_t buffer_size;
size_t buffer_capacity;
- QEMUTimer *timer;
+ QemuThread thread;
+ bool complete;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -160,11 +162,8 @@ static int buffered_close(void *opaque)
if (ret >= 0) {
ret = ret2;
}
- qemu_del_timer(s->timer);
- qemu_free_timer(s->timer);
- g_free(s->buffer);
- g_free(s);
-
+ ret = migrate_fd_close(s->migration_state);
+ s->complete = true;
return ret;
}
@@ -215,23 +214,38 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static void buffered_rate_tick(void *opaque)
+/* 10ms xfer_limit is the limit that we should write each 10ms */
+#define BUFFER_DELAY 100
+
+static void *buffered_file_thread(void *opaque)
{
QEMUFileBuffered *s = opaque;
+ int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;
- if (qemu_file_get_error(s->file)) {
- buffered_close(s);
- return;
- }
-
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
-
- if (s->freeze_output)
- return;
-
- s->bytes_xfer = 0;
+ while (true) {
+ int64_t current_time = qemu_get_clock_ms(rt_clock);
- buffered_put_buffer(s, NULL, 0, 0);
+ if (s->complete) {
+ break;
+ }
+ if (s->freeze_output) {
+ continue;
+ }
+ if (current_time >= expire_time) {
+ s->bytes_xfer = 0;
+ expire_time = current_time + BUFFER_DELAY;
+ }
+ if (s->bytes_xfer >= s->xfer_limit) {
+ /* usleep expects microseconds */
+ usleep((expire_time - current_time)*1000);
+ }
+ qemu_mutex_lock_iothread();
+ buffered_put_buffer(s, NULL, 0, 0);
+ qemu_mutex_unlock_iothread();
+ }
+ g_free(s->buffer);
+ g_free(s);
+ return NULL;
}
QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
@@ -242,15 +256,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
s->migration_state = migration_state;
s->xfer_limit = migration_state->bandwidth_limit / 10;
+ s->complete = false;
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
buffered_get_rate_limit);
- s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
-
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+ qemu_thread_create(&s->thread, buffered_file_thread, s,
+ QEMU_THREAD_DETACHED);
return s->file;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread
2012-10-18 7:30 ` [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread Juan Quintela
@ 2012-10-18 8:56 ` Paolo Bonzini
2012-10-21 12:09 ` Orit Wasserman
2012-11-12 11:42 ` Paolo Bonzini
2 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:56 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> + }
> + if (s->bytes_xfer >= s->xfer_limit) {
> + /* usleep expects microseconds */
> + usleep((expire_time - current_time)*1000);
> + }
g_usleep please.
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread
2012-10-18 7:30 ` [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread Juan Quintela
2012-10-18 8:56 ` Paolo Bonzini
@ 2012-10-21 12:09 ` Orit Wasserman
2012-11-12 11:42 ` Paolo Bonzini
2 siblings, 0 replies; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 12:09 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 10/18/2012 09:30 AM, Juan Quintela wrote:
> We still protect everything except the wait with the iothread lock.
> But we moved from a timer to a thread. Steps one by one.
>
> We also need to detect when we have finished with a variable "complete".
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> buffered_file.c | 58 +++++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index ed92df1..4b90d54 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -18,6 +18,7 @@
> #include "qemu-timer.h"
> #include "qemu-char.h"
> #include "buffered_file.h"
> +#include "qemu-thread.h"
>
> //#define DEBUG_BUFFERED_FILE
>
> @@ -31,7 +32,8 @@ typedef struct QEMUFileBuffered
> uint8_t *buffer;
> size_t buffer_size;
> size_t buffer_capacity;
> - QEMUTimer *timer;
> + QemuThread thread;
> + bool complete;
> } QEMUFileBuffered;
>
> #ifdef DEBUG_BUFFERED_FILE
> @@ -160,11 +162,8 @@ static int buffered_close(void *opaque)
> if (ret >= 0) {
> ret = ret2;
> }
> - qemu_del_timer(s->timer);
> - qemu_free_timer(s->timer);
> - g_free(s->buffer);
> - g_free(s);
> -
> + ret = migrate_fd_close(s->migration_state);
> + s->complete = true;
> return ret;
> }
>
> @@ -215,23 +214,38 @@ static int64_t buffered_get_rate_limit(void *opaque)
> return s->xfer_limit;
> }
>
> -static void buffered_rate_tick(void *opaque)
> +/* 10ms xfer_limit is the limit that we should write each 10ms */
maybe 100, please fix the comment
Orit
> +#define BUFFER_DELAY 100
> +
> +static void *buffered_file_thread(void *opaque)
> {
> QEMUFileBuffered *s = opaque;
> + int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;
>
> - if (qemu_file_get_error(s->file)) {
> - buffered_close(s);
> - return;
> - }
> -
> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
> -
> - if (s->freeze_output)
> - return;
> -
> - s->bytes_xfer = 0;
> + while (true) {
> + int64_t current_time = qemu_get_clock_ms(rt_clock);
>
> - buffered_put_buffer(s, NULL, 0, 0);
> + if (s->complete) {
> + break;
> + }
> + if (s->freeze_output) {
> + continue;
> + }
> + if (current_time >= expire_time) {
> + s->bytes_xfer = 0;
> + expire_time = current_time + BUFFER_DELAY;
> + }
> + if (s->bytes_xfer >= s->xfer_limit) {
> + /* usleep expects microseconds */
> + usleep((expire_time - current_time)*1000);
> + }
> + qemu_mutex_lock_iothread();
> + buffered_put_buffer(s, NULL, 0, 0);
> + qemu_mutex_unlock_iothread();
> + }
> + g_free(s->buffer);
> + g_free(s);
> + return NULL;
> }
>
> QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
> @@ -242,15 +256,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
>
> s->migration_state = migration_state;
> s->xfer_limit = migration_state->bandwidth_limit / 10;
> + s->complete = false;
>
> s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
> buffered_close, buffered_rate_limit,
> buffered_set_rate_limit,
> buffered_get_rate_limit);
>
> - s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
> -
> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
> + qemu_thread_create(&s->thread, buffered_file_thread, s,
> + QEMU_THREAD_DETACHED);
>
> return s->file;
> }
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread
2012-10-18 7:30 ` [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread Juan Quintela
2012-10-18 8:56 ` Paolo Bonzini
2012-10-21 12:09 ` Orit Wasserman
@ 2012-11-12 11:42 ` Paolo Bonzini
2 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-11-12 11:42 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> @@ -160,11 +162,8 @@ static int buffered_close(void *opaque)
> if (ret >= 0) {
> ret = ret2;
> }
> - qemu_del_timer(s->timer);
> - qemu_free_timer(s->timer);
> - g_free(s->buffer);
> - g_free(s);
> -
> + ret = migrate_fd_close(s->migration_state);
> + s->complete = true;
> return ret;
Just above these lines there is
ret2 = migrate_fd_close(s->migration_state);
I believe the second call to migrate_fd_close should be removed.
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (3 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 04/30] buffered_file: Move from using a timer to use a thread Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-21 12:10 ` Orit Wasserman
2012-10-18 7:30 ` [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly Juan Quintela
` (26 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
We want the file assignment to happen before the thread is created to
avoid locking, so we just do it before creating the thread.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 13 ++++++-------
buffered_file.h | 2 +-
migration.c | 2 +-
migration.h | 1 +
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 4b90d54..6395b37 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -33,7 +33,6 @@ typedef struct QEMUFileBuffered
size_t buffer_size;
size_t buffer_capacity;
QemuThread thread;
- bool complete;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -163,7 +162,7 @@ static int buffered_close(void *opaque)
ret = ret2;
}
ret = migrate_fd_close(s->migration_state);
- s->complete = true;
+ s->migration_state->complete = true;
return ret;
}
@@ -225,7 +224,7 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
- if (s->complete) {
+ if (s->migration_state->complete) {
break;
}
if (s->freeze_output) {
@@ -248,7 +247,7 @@ static void *buffered_file_thread(void *opaque)
return NULL;
}
-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
+void qemu_fopen_ops_buffered(MigrationState *migration_state)
{
QEMUFileBuffered *s;
@@ -256,15 +255,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
s->migration_state = migration_state;
s->xfer_limit = migration_state->bandwidth_limit / 10;
- s->complete = false;
+ s->migration_state->complete = false;
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
buffered_get_rate_limit);
+ migration_state->file = s->file;
+
qemu_thread_create(&s->thread, buffered_file_thread, s,
QEMU_THREAD_DETACHED);
-
- return s->file;
}
diff --git a/buffered_file.h b/buffered_file.h
index ef010fe..8a246fd 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,6 +17,6 @@
#include "hw/hw.h"
#include "migration.h"
-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state);
+void qemu_fopen_ops_buffered(MigrationState *migration_state);
#endif
diff --git a/migration.c b/migration.c
index 62e0304..02f4ffa 100644
--- a/migration.c
+++ b/migration.c
@@ -431,7 +431,7 @@ void migrate_fd_connect(MigrationState *s)
int ret;
s->state = MIG_STATE_ACTIVE;
- s->file = qemu_fopen_ops_buffered(s);
+ qemu_fopen_ops_buffered(s);
DPRINTF("beginning savevm\n");
ret = qemu_savevm_state_begin(s->file, &s->params);
diff --git a/migration.h b/migration.h
index 1c3e9b7..a63c5d5 100644
--- a/migration.h
+++ b/migration.h
@@ -45,6 +45,7 @@ struct MigrationState
int64_t dirty_pages_rate;
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
int64_t xbzrle_cache_size;
+ bool complete;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void
2012-10-18 7:30 ` [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void Juan Quintela
@ 2012-10-21 12:10 ` Orit Wasserman
0 siblings, 0 replies; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 12:10 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 10/18/2012 09:30 AM, Juan Quintela wrote:
> We want the file assignment to happen before the thread is created to
> avoid locking, so we just do it before creating the thread.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> buffered_file.c | 13 ++++++-------
> buffered_file.h | 2 +-
> migration.c | 2 +-
> migration.h | 1 +
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index 4b90d54..6395b37 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -33,7 +33,6 @@ typedef struct QEMUFileBuffered
> size_t buffer_size;
> size_t buffer_capacity;
> QemuThread thread;
> - bool complete;
> } QEMUFileBuffered;
>
> #ifdef DEBUG_BUFFERED_FILE
> @@ -163,7 +162,7 @@ static int buffered_close(void *opaque)
> ret = ret2;
> }
> ret = migrate_fd_close(s->migration_state);
> - s->complete = true;
> + s->migration_state->complete = true;
> return ret;
> }
>
> @@ -225,7 +224,7 @@ static void *buffered_file_thread(void *opaque)
> while (true) {
> int64_t current_time = qemu_get_clock_ms(rt_clock);
>
> - if (s->complete) {
> + if (s->migration_state->complete) {
> break;
> }
> if (s->freeze_output) {
> @@ -248,7 +247,7 @@ static void *buffered_file_thread(void *opaque)
> return NULL;
> }
>
> -QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
> +void qemu_fopen_ops_buffered(MigrationState *migration_state)
> {
> QEMUFileBuffered *s;
>
> @@ -256,15 +255,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
>
> s->migration_state = migration_state;
> s->xfer_limit = migration_state->bandwidth_limit / 10;
> - s->complete = false;
> + s->migration_state->complete = false;
>
> s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
> buffered_close, buffered_rate_limit,
> buffered_set_rate_limit,
> buffered_get_rate_limit);
>
> + migration_state->file = s->file;
> +
> qemu_thread_create(&s->thread, buffered_file_thread, s,
> QEMU_THREAD_DETACHED);
> -
> - return s->file;
> }
> diff --git a/buffered_file.h b/buffered_file.h
> index ef010fe..8a246fd 100644
> --- a/buffered_file.h
> +++ b/buffered_file.h
> @@ -17,6 +17,6 @@
> #include "hw/hw.h"
> #include "migration.h"
>
> -QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state);
> +void qemu_fopen_ops_buffered(MigrationState *migration_state);
>
> #endif
> diff --git a/migration.c b/migration.c
> index 62e0304..02f4ffa 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -431,7 +431,7 @@ void migrate_fd_connect(MigrationState *s)
> int ret;
>
> s->state = MIG_STATE_ACTIVE;
> - s->file = qemu_fopen_ops_buffered(s);
> + qemu_fopen_ops_buffered(s);
>
> DPRINTF("beginning savevm\n");
> ret = qemu_savevm_state_begin(s->file, &s->params);
> diff --git a/migration.h b/migration.h
> index 1c3e9b7..a63c5d5 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -45,6 +45,7 @@ struct MigrationState
> int64_t dirty_pages_rate;
> bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
> int64_t xbzrle_cache_size;
> + bool complete;
> };
>
> void process_incoming_migration(QEMUFile *f);
>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (4 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-11-12 11:44 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 07/30] migration: make writes blocking Juan Quintela
` (25 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
You can only stop all cpus from the iothread or an vcpu. As we want
to do it from the migration_thread, we need to do this dance with the
botton handlers.
This patch is a request for ideas. I can move this function to cpus.c, but
wondered if there is an easy way of doing this?
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/migration.c b/migration.c
index 02f4ffa..05ef1a3 100644
--- a/migration.c
+++ b/migration.c
@@ -20,6 +20,7 @@
#include "sysemu.h"
#include "block.h"
#include "qemu_socket.h"
+#include "qemu-thread.h"
#include "block-migration.h"
#include "qmp-commands.h"
@@ -322,11 +323,22 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
void migrate_fd_put_ready(MigrationState *s)
{
int ret;
+ static bool first_time = true;
if (s->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
return;
}
+ if (first_time) {
+ first_time = false;
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(s->file, &s->params);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ migrate_fd_error(s);
+ return;
+ }
+ }
DPRINTF("iterate\n");
ret = qemu_savevm_state_iterate(s->file);
@@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
DPRINTF("done iterating\n");
start_time = qemu_get_clock_ms(rt_clock);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
- vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ if (old_vm_running) {
+ vm_stop(RUN_STATE_FINISH_MIGRATE);
+ } else {
+ vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ }
if (qemu_savevm_state_complete(s->file) < 0) {
migrate_fd_error(s);
@@ -428,19 +444,8 @@ bool migration_has_failed(MigrationState *s)
void migrate_fd_connect(MigrationState *s)
{
- int ret;
-
s->state = MIG_STATE_ACTIVE;
qemu_fopen_ops_buffered(s);
-
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->file, &s->params);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- return;
- }
- migrate_fd_put_ready(s);
}
static MigrationState *migrate_init(const MigrationParams *params)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly
2012-10-18 7:30 ` [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly Juan Quintela
@ 2012-11-12 11:44 ` Paolo Bonzini
2012-11-14 15:21 ` Paolo Bonzini
0 siblings, 1 reply; 58+ messages in thread
From: Paolo Bonzini @ 2012-11-12 11:44 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> You can only stop all cpus from the iothread or an vcpu. As we want
> to do it from the migration_thread, we need to do this dance with the
> botton handlers.
>
> This patch is a request for ideas. I can move this function to cpus.c, but
> wondered if there is an easy way of doing this?
The commit message here is stale (I think I had pointed this out
already). Also, first_time should be a member of MigrationState,
initialized in migrate_fd_connect.
Paolo
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 02f4ffa..05ef1a3 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -20,6 +20,7 @@
> #include "sysemu.h"
> #include "block.h"
> #include "qemu_socket.h"
> +#include "qemu-thread.h"
> #include "block-migration.h"
> #include "qmp-commands.h"
>
> @@ -322,11 +323,22 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
> void migrate_fd_put_ready(MigrationState *s)
> {
> int ret;
> + static bool first_time = true;
>
> if (s->state != MIG_STATE_ACTIVE) {
> DPRINTF("put_ready returning because of non-active state\n");
> return;
> }
> + if (first_time) {
> + first_time = false;
> + DPRINTF("beginning savevm\n");
> + ret = qemu_savevm_state_begin(s->file, &s->params);
> + if (ret < 0) {
> + DPRINTF("failed, %d\n", ret);
> + migrate_fd_error(s);
> + return;
> + }
> + }
>
> DPRINTF("iterate\n");
> ret = qemu_savevm_state_iterate(s->file);
> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
> DPRINTF("done iterating\n");
> start_time = qemu_get_clock_ms(rt_clock);
> qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> + if (old_vm_running) {
> + vm_stop(RUN_STATE_FINISH_MIGRATE);
> + } else {
> + vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> + }
>
> if (qemu_savevm_state_complete(s->file) < 0) {
> migrate_fd_error(s);
> @@ -428,19 +444,8 @@ bool migration_has_failed(MigrationState *s)
>
> void migrate_fd_connect(MigrationState *s)
> {
> - int ret;
> -
> s->state = MIG_STATE_ACTIVE;
> qemu_fopen_ops_buffered(s);
> -
> - DPRINTF("beginning savevm\n");
> - ret = qemu_savevm_state_begin(s->file, &s->params);
> - if (ret < 0) {
> - DPRINTF("failed, %d\n", ret);
> - migrate_fd_error(s);
> - return;
> - }
> - migrate_fd_put_ready(s);
> }
>
> static MigrationState *migrate_init(const MigrationParams *params)
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly
2012-11-12 11:44 ` Paolo Bonzini
@ 2012-11-14 15:21 ` Paolo Bonzini
2012-12-14 12:36 ` Juan Quintela
0 siblings, 1 reply; 58+ messages in thread
From: Paolo Bonzini @ 2012-11-14 15:21 UTC (permalink / raw)
Cc: qemu-devel, Juan Quintela
Il 12/11/2012 12:44, Paolo Bonzini ha scritto:
>> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
>> DPRINTF("done iterating\n");
>> start_time = qemu_get_clock_ms(rt_clock);
>> qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>> - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>> + if (old_vm_running) {
>> + vm_stop(RUN_STATE_FINISH_MIGRATE);
>> + } else {
>> + vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>> + }
>>
>> if (qemu_savevm_state_complete(s->file) < 0) {
>> migrate_fd_error(s);
This hunk also seems to be useless nowadays.
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly
2012-11-14 15:21 ` Paolo Bonzini
@ 2012-12-14 12:36 ` Juan Quintela
2012-12-14 13:53 ` Paolo Bonzini
0 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-12-14 12:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 12/11/2012 12:44, Paolo Bonzini ha scritto:
>>> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
>>> DPRINTF("done iterating\n");
>>> start_time = qemu_get_clock_ms(rt_clock);
>>> qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>>> - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>> + if (old_vm_running) {
>>> + vm_stop(RUN_STATE_FINISH_MIGRATE);
>>> + } else {
>>> + vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>> + }
>>>
>>> if (qemu_savevm_state_complete(s->file) < 0) {
>>> migrate_fd_error(s);
>
> This hunk also seems to be useless nowadays.
it is needed for when we are migrating in suspended state. We need to
fix it some other way, but it needs to remaing for now.
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly
2012-12-14 12:36 ` Juan Quintela
@ 2012-12-14 13:53 ` Paolo Bonzini
0 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-12-14 13:53 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
Il 14/12/2012 13:36, Juan Quintela ha scritto:
>>>> >>> - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>>> >>> + if (old_vm_running) {
>>>> >>> + vm_stop(RUN_STATE_FINISH_MIGRATE);
>>>> >>> + } else {
>>>> >>> + vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>>> >>> + }
>>>> >>>
>>>> >>> if (qemu_savevm_state_complete(s->file) < 0) {
>>>> >>> migrate_fd_error(s);
>> >
>> > This hunk also seems to be useless nowadays.
> it is needed for when we are migrating in suspended state.
That was the case when the line read
vm_stop(RUN_STATE_FINISH_MIGRATE);
but it reads vm_stop_force_state. So you're being kind and not forcing
the change if running, but that's not necessary. Just always using
vm_stop_force_state works now, and would work with the migration thread too.
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 07/30] migration: make writes blocking
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (5 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 06/30] migration: stop all cpus correctly Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-11-12 11:52 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 08/30] migration: remove unfreeze logic Juan Quintela
` (24 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Move all the writes to the migration_thread, and make writings
blocking. Notice that are still using the iothread for everything
that we do.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 2 --
migration-fd.c | 6 ------
migration-tcp.c | 19 ++-----------------
migration-unix.c | 2 --
migration.c | 21 ---------------------
qemu-file.h | 5 -----
savevm.c | 5 -----
7 files changed, 2 insertions(+), 58 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 6c97db9..908f22e 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -76,8 +76,6 @@ int exec_start_outgoing_migration(MigrationState *s, const char *command)
goto err_after_open;
}
- socket_set_nonblock(s->fd);
-
s->opaque = qemu_popen(f, "w");
s->close = exec_close;
diff --git a/migration-fd.c b/migration-fd.c
index 7335167..aba1be7 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -81,11 +81,6 @@ int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
goto err_after_get_fd;
}
- if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
- DPRINTF("Unable to set nonblocking mode on file descriptor\n");
- goto err_after_open;
- }
-
s->get_error = fd_errno;
s->write = fd_write;
s->close = fd_close;
@@ -93,7 +88,6 @@ int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
migrate_fd_connect(s);
return 0;
-err_after_open:
close(s->fd);
err_after_get_fd:
return -1;
diff --git a/migration-tcp.c b/migration-tcp.c
index a15c2b8..812ae22 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -53,21 +53,6 @@ static int tcp_close(MigrationState *s)
return r;
}
-static void tcp_wait_for_connect(int fd, void *opaque)
-{
- MigrationState *s = opaque;
-
- if (fd < 0) {
- DPRINTF("migrate connect error\n");
- s->fd = -1;
- migrate_fd_error(s);
- } else {
- DPRINTF("migrate connect success\n");
- s->fd = fd;
- migrate_fd_connect(s);
- }
-}
-
int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
{
@@ -75,12 +60,12 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
s->write = socket_write;
s->close = tcp_close;
- s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s,
- errp);
+ s->fd = inet_connect(host_port, errp);
if (error_is_set(errp)) {
migrate_fd_error(s);
return -1;
}
+ migrate_fd_connect(s);
return 0;
}
diff --git a/migration-unix.c b/migration-unix.c
index 169de88..bb41bac 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -96,8 +96,6 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
return -errno;
}
- socket_set_nonblock(s->fd);
-
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
if (ret == -1) {
diff --git a/migration.c b/migration.c
index 05ef1a3..a8b2f4a 100644
--- a/migration.c
+++ b/migration.c
@@ -247,10 +247,6 @@ static int migrate_fd_cleanup(MigrationState *s)
{
int ret = 0;
- if (s->fd != -1) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
- }
-
if (s->file) {
DPRINTF("closing file\n");
ret = qemu_fclose(s->file);
@@ -285,18 +281,6 @@ static void migrate_fd_completed(MigrationState *s)
notifier_list_notify(&migration_state_notifiers, s);
}
-static void migrate_fd_put_notify(void *opaque)
-{
- MigrationState *s = opaque;
- int ret;
-
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
- ret = qemu_file_put_notify(s->file);
- if (ret) {
- migrate_fd_error(s);
- }
-}
-
ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
size_t size)
{
@@ -313,10 +297,6 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
if (ret == -1)
ret = -(s->get_error(s));
- if (ret == -EAGAIN) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
- }
-
return ret;
}
@@ -412,7 +392,6 @@ int migrate_fd_wait_for_unfreeze(MigrationState *s)
int migrate_fd_close(MigrationState *s)
{
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
return s->close(s);
}
diff --git a/qemu-file.h b/qemu-file.h
index 9c8985b..e88892c 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -104,11 +104,6 @@ int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
int64_t qemu_file_get_rate_limit(QEMUFile *f);
int qemu_file_get_error(QEMUFile *f);
-/* Try to send any outstanding data. This function is useful when output is
- * halted due to rate limiting or EAGAIN errors occur as it can be used to
- * resume output. */
-int qemu_file_put_notify(QEMUFile *f);
-
static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
{
qemu_put_be64(f, *pv);
diff --git a/savevm.c b/savevm.c
index b080d37..69f1768 100644
--- a/savevm.c
+++ b/savevm.c
@@ -523,11 +523,6 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
-int qemu_file_put_notify(QEMUFile *f)
-{
- return f->put_buffer(f->opaque, NULL, 0, 0);
-}
-
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 07/30] migration: make writes blocking
2012-10-18 7:30 ` [Qemu-devel] [PATCH 07/30] migration: make writes blocking Juan Quintela
@ 2012-11-12 11:52 ` Paolo Bonzini
0 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-11-12 11:52 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> @@ -247,10 +247,6 @@ static int migrate_fd_cleanup(MigrationState *s)
> {
> int ret = 0;
>
> - if (s->fd != -1) {
> - qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
> - }
> -
> if (s->file) {
> DPRINTF("closing file\n");
> ret = qemu_fclose(s->file);
While reviewing this patch, I noticed that migrate_fd_cleanup calls
migrate_fd_close twice. The first via qemu_fclose's own call to
buffered_close, the second directly.
Perhaps you can include something like
diff --git a/migration.c b/migration.c
--- a/migration.c
+++ b/migration.c
@@ -274,5 +274,5 @@ static int migrate_fd_cleanup(MigrationState *s)
}
- migrate_fd_close(s);
+ assert(s->fd == -1);
return ret;
}
somewhere in the series?
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 08/30] migration: remove unfreeze logic
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (6 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 07/30] migration: make writes blocking Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 09/30] migration: take finer locking Juan Quintela
` (23 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Now that we have a thread, and blocking writes, we don't need it.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 24 +-----------------------
migration.c | 23 -----------------------
migration.h | 1 -
3 files changed, 1 insertion(+), 47 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 6395b37..876cc8c 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -26,7 +26,6 @@ typedef struct QEMUFileBuffered
{
MigrationState *migration_state;
QEMUFile *file;
- int freeze_output;
size_t bytes_xfer;
size_t xfer_limit;
uint8_t *buffer;
@@ -70,13 +69,6 @@ static ssize_t buffered_flush(QEMUFileBuffered *s)
ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
s->buffer_size - offset);
- if (ret == -EAGAIN) {
- DPRINTF("backend not ready, freezing\n");
- ret = 0;
- s->freeze_output = 1;
- break;
- }
-
if (ret <= 0) {
DPRINTF("error flushing data, %zd\n", ret);
break;
@@ -110,9 +102,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
return error;
}
- DPRINTF("unfreezing output\n");
- s->freeze_output = 0;
-
if (size > 0) {
DPRINTF("buffering %d bytes\n", size - offset);
buffered_append(s, buf, size);
@@ -126,7 +115,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
if (pos == 0 && size == 0) {
DPRINTF("file is ready\n");
- if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) {
+ if (s->bytes_xfer < s->xfer_limit) {
DPRINTF("notifying client\n");
migrate_fd_put_ready(s->migration_state);
}
@@ -149,12 +138,6 @@ static int buffered_close(void *opaque)
if (ret < 0) {
break;
}
- if (s->freeze_output) {
- ret = migrate_fd_wait_for_unfreeze(s->migration_state);
- if (ret < 0) {
- break;
- }
- }
}
ret2 = migrate_fd_close(s->migration_state);
@@ -181,8 +164,6 @@ static int buffered_rate_limit(void *opaque)
if (ret) {
return ret;
}
- if (s->freeze_output)
- return 1;
if (s->bytes_xfer > s->xfer_limit)
return 1;
@@ -227,9 +208,6 @@ static void *buffered_file_thread(void *opaque)
if (s->migration_state->complete) {
break;
}
- if (s->freeze_output) {
- continue;
- }
if (current_time >= expire_time) {
s->bytes_xfer = 0;
expire_time = current_time + BUFFER_DELAY;
diff --git a/migration.c b/migration.c
index a8b2f4a..29ee710 100644
--- a/migration.c
+++ b/migration.c
@@ -367,29 +367,6 @@ static void migrate_fd_cancel(MigrationState *s)
migrate_fd_cleanup(s);
}
-int migrate_fd_wait_for_unfreeze(MigrationState *s)
-{
- int ret;
-
- DPRINTF("wait for unfreeze\n");
- if (s->state != MIG_STATE_ACTIVE)
- return -EINVAL;
-
- do {
- fd_set wfds;
-
- FD_ZERO(&wfds);
- FD_SET(s->fd, &wfds);
-
- ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
- } while (ret == -1 && (s->get_error(s)) == EINTR);
-
- if (ret == -1) {
- return -s->get_error(s);
- }
- return 0;
-}
-
int migrate_fd_close(MigrationState *s)
{
return s->close(s);
diff --git a/migration.h b/migration.h
index a63c5d5..505f792 100644
--- a/migration.h
+++ b/migration.h
@@ -82,7 +82,6 @@ void migrate_fd_connect(MigrationState *s);
ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
size_t size);
void migrate_fd_put_ready(MigrationState *s);
-int migrate_fd_wait_for_unfreeze(MigrationState *s);
int migrate_fd_close(MigrationState *s);
void add_migration_state_change_notifier(Notifier *notify);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 09/30] migration: take finer locking
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (7 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 08/30] migration: remove unfreeze logic Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 10/30] buffered_file: Unfold the trick to restart generating migration data Juan Quintela
` (22 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Instead of locking the whole migration_thread inside loop, just lock
migration_fd_put_notify, that is what interacts with the rest of the
world.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 2 --
migration.c | 5 +++++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 876cc8c..a3aba2a 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -216,9 +216,7 @@ static void *buffered_file_thread(void *opaque)
/* usleep expects microseconds */
usleep((expire_time - current_time)*1000);
}
- qemu_mutex_lock_iothread();
buffered_put_buffer(s, NULL, 0, 0);
- qemu_mutex_unlock_iothread();
}
g_free(s->buffer);
g_free(s);
diff --git a/migration.c b/migration.c
index 29ee710..82c2663 100644
--- a/migration.c
+++ b/migration.c
@@ -305,8 +305,10 @@ void migrate_fd_put_ready(MigrationState *s)
int ret;
static bool first_time = true;
+ qemu_mutex_lock_iothread();
if (s->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
+ qemu_mutex_unlock_iothread();
return;
}
if (first_time) {
@@ -316,6 +318,7 @@ void migrate_fd_put_ready(MigrationState *s)
if (ret < 0) {
DPRINTF("failed, %d\n", ret);
migrate_fd_error(s);
+ qemu_mutex_unlock_iothread();
return;
}
}
@@ -351,6 +354,8 @@ void migrate_fd_put_ready(MigrationState *s)
}
}
}
+ qemu_mutex_unlock_iothread();
+
}
static void migrate_fd_cancel(MigrationState *s)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 10/30] buffered_file: Unfold the trick to restart generating migration data
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (8 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 09/30] migration: take finer locking Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 11/30] buffered_file: don't flush on put buffer Juan Quintela
` (21 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
This was needed before due to the way that the callbacks worked.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index a3aba2a..7692950 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -113,14 +113,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
return error;
}
- if (pos == 0 && size == 0) {
- DPRINTF("file is ready\n");
- if (s->bytes_xfer < s->xfer_limit) {
- DPRINTF("notifying client\n");
- migrate_fd_put_ready(s->migration_state);
- }
- }
-
return size;
}
@@ -216,8 +208,15 @@ static void *buffered_file_thread(void *opaque)
/* usleep expects microseconds */
usleep((expire_time - current_time)*1000);
}
- buffered_put_buffer(s, NULL, 0, 0);
+ buffered_flush(s);
+
+ DPRINTF("file is ready\n");
+ if (s->bytes_xfer < s->xfer_limit) {
+ DPRINTF("notifying client\n");
+ migrate_fd_put_ready(s->migration_state);
+ }
}
+
g_free(s->buffer);
g_free(s);
return NULL;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 11/30] buffered_file: don't flush on put buffer
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (9 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 10/30] buffered_file: Unfold the trick to restart generating migration data Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 12/30] buffered_file: unfold buffered_append in buffered_put_buffer Juan Quintela
` (20 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
We call buffered_put_buffer with iothread held, and buffered_flush() does
synchronous writes. We only want to do the synchronous writes outside.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 7692950..f508026 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -107,12 +107,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
buffered_append(s, buf, size);
}
- error = buffered_flush(s);
- if (error < 0) {
- DPRINTF("buffered flush error. bailing: %s\n", strerror(-error));
- return error;
- }
-
return size;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 12/30] buffered_file: unfold buffered_append in buffered_put_buffer
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (10 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 11/30] buffered_file: don't flush on put buffer Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 13/30] savevm: New save live migration method: pending Juan Quintela
` (19 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
It was the only user, and now buffered_put_buffer just do the append
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index f508026..fcb3907 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -42,22 +42,6 @@ typedef struct QEMUFileBuffered
do { } while (0)
#endif
-static void buffered_append(QEMUFileBuffered *s,
- const uint8_t *buf, size_t size)
-{
- if (size > (s->buffer_capacity - s->buffer_size)) {
- DPRINTF("increasing buffer capacity from %zu by %zu\n",
- s->buffer_capacity, size + 1024);
-
- s->buffer_capacity += size + 1024;
-
- s->buffer = g_realloc(s->buffer, s->buffer_capacity);
- }
-
- memcpy(s->buffer + s->buffer_size, buf, size);
- s->buffer_size += size;
-}
-
static ssize_t buffered_flush(QEMUFileBuffered *s)
{
size_t offset = 0;
@@ -102,11 +86,22 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
return error;
}
- if (size > 0) {
- DPRINTF("buffering %d bytes\n", size - offset);
- buffered_append(s, buf, size);
+ if (size <= 0) {
+ return size;
}
+ if (size > (s->buffer_capacity - s->buffer_size)) {
+ DPRINTF("increasing buffer capacity from %zu by %zu\n",
+ s->buffer_capacity, size + 1024);
+
+ s->buffer_capacity += size + 1024;
+
+ s->buffer = g_realloc(s->buffer, s->buffer_capacity);
+ }
+
+ memcpy(s->buffer + s->buffer_size, buf, size);
+ s->buffer_size += size;
+
return size;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 13/30] savevm: New save live migration method: pending
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (11 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 12/30] buffered_file: unfold buffered_append in buffered_put_buffer Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 14/30] migration: include qemu-file.h Juan Quintela
` (18 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Code just now does (simplified for clarity)
if (qemu_savevm_state_iterate(s->file) == 1) {
vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
qemu_savevm_state_complete(s->file);
}
Problem here is that qemu_savevm_state_iterate() returns 1 when it
knows that remaining memory to sent takes less than max downtime.
But this means that we could end spending 2x max_downtime, one
downtime in qemu_savevm_iterate, and the other in
qemu_savevm_state_complete.
Changed code to:
pending_size = qemu_savevm_state_pending(s->file, max_size);
DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
if (pending_size >= max_size) {
ret = qemu_savevm_state_iterate(s->file);
} else {
vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
qemu_savevm_state_complete(s->file);
}
So what we do is: at current network speed, we calculate the maximum
number of bytes we can sent: max_size.
Then we ask every save_live section how much they have pending. If
they are less than max_size, we move to complete phase, otherwise we
do an iterate one.
This makes things much simpler, because now individual sections don't
have to caluclate the bandwidth (it was implossible to do right from
there).
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 48 ++++++++++++++++++------------------------------
block-migration.c | 49 ++++++++++---------------------------------------
buffered_file.c | 25 ++++++++++++++++++-------
migration.c | 22 +++++++++++++++-------
migration.h | 2 +-
savevm.c | 19 +++++++++++++++++++
sysemu.h | 1 +
vmstate.h | 1 +
8 files changed, 83 insertions(+), 84 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 2d29828..1eefef8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -608,12 +608,9 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
static int ram_save_iterate(QEMUFile *f, void *opaque)
{
- uint64_t bytes_transferred_last;
- double bwidth = 0;
int ret;
int i;
- uint64_t expected_downtime;
- MigrationState *s = migrate_get_current();
+ int64_t t0;
qemu_mutex_lock_ramlist();
@@ -621,9 +618,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
reset_ram_globals();
}
- bytes_transferred_last = bytes_transferred;
- bwidth = qemu_get_clock_ns(rt_clock);
-
+ t0 = qemu_get_clock_ns(rt_clock);
i = 0;
while ((ret = qemu_file_rate_limit(f)) == 0) {
int bytes_sent;
@@ -641,7 +636,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
iterations
*/
if ((i & 63) == 0) {
- uint64_t t1 = (qemu_get_clock_ns(rt_clock) - bwidth) / 1000000;
+ uint64_t t1 = (qemu_get_clock_ns(rt_clock) - t0) / 1000000;
if (t1 > MAX_WAIT) {
DPRINTF("big wait: %" PRIu64 " milliseconds, %d iterations\n",
t1, i);
@@ -655,31 +650,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
return ret;
}
- bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
- bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
-
- /* if we haven't transferred anything this round, force
- * expected_downtime to a very high value, but without
- * crashing */
- if (bwidth == 0) {
- bwidth = 0.000001;
- }
-
qemu_mutex_unlock_ramlist();
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
- DPRINTF("ram_save_live: expected(%" PRIu64 ") <= max(" PRIu64 ")?\n",
- expected_downtime, migrate_max_downtime());
-
- if (expected_downtime <= migrate_max_downtime()) {
- migration_bitmap_sync();
- expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
- s->expected_downtime = expected_downtime / 1000000; /* ns -> ms */
-
- return expected_downtime <= migrate_max_downtime();
- }
- return 0;
+ return i;
}
static int ram_save_complete(QEMUFile *f, void *opaque)
@@ -712,6 +686,19 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
return 0;
}
+static uint64_t ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+{
+ uint64_t remaining_size;
+
+ remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE;
+
+ if (remaining_size < max_size) {
+ migration_bitmap_sync();
+ remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE;
+ }
+ return remaining_size;
+}
+
static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
{
int ret, rc = 0;
@@ -897,6 +884,7 @@ SaveVMHandlers savevm_ram_handlers = {
.save_live_setup = ram_save_setup,
.save_live_iterate = ram_save_iterate,
.save_live_complete = ram_save_complete,
+ .save_live_pending = ram_save_pending,
.load_state = ram_load,
.cancel = ram_migration_cancel,
};
diff --git a/block-migration.c b/block-migration.c
index 71b9601..5db01fe 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -77,9 +77,7 @@ typedef struct BlkMigState {
int64_t total_sector_sum;
int prev_progress;
int bulk_completed;
- long double total_time;
long double prev_time_offset;
- int reads;
} BlkMigState;
static BlkMigState block_mig_state;
@@ -132,12 +130,6 @@ uint64_t blk_mig_bytes_total(void)
return sum << BDRV_SECTOR_BITS;
}
-static inline long double compute_read_bwidth(void)
-{
- assert(block_mig_state.total_time != 0);
- return (block_mig_state.reads / block_mig_state.total_time) * BLOCK_SIZE;
-}
-
static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t sector)
{
int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
@@ -191,8 +183,6 @@ static void blk_mig_read_cb(void *opaque, int ret)
blk->ret = ret;
- block_mig_state.reads++;
- block_mig_state.total_time += (curr_time - block_mig_state.prev_time_offset);
block_mig_state.prev_time_offset = curr_time;
QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
@@ -310,8 +300,6 @@ static void init_blk_migration(QEMUFile *f)
block_mig_state.total_sector_sum = 0;
block_mig_state.prev_progress = -1;
block_mig_state.bulk_completed = 0;
- block_mig_state.total_time = 0;
- block_mig_state.reads = 0;
bdrv_iterate(init_blk_migration_it, NULL);
}
@@ -493,32 +481,6 @@ static int64_t get_remaining_dirty(void)
return dirty * BLOCK_SIZE;
}
-static int is_stage2_completed(void)
-{
- int64_t remaining_dirty;
- long double bwidth;
-
- if (block_mig_state.bulk_completed == 1) {
-
- remaining_dirty = get_remaining_dirty();
- if (remaining_dirty == 0) {
- return 1;
- }
-
- bwidth = compute_read_bwidth();
-
- if ((remaining_dirty / bwidth) <=
- migrate_max_downtime()) {
- /* finish stage2 because we think that we can finish remaining work
- below max_downtime */
-
- return 1;
- }
- }
-
- return 0;
-}
-
static void blk_mig_cleanup(void)
{
BlkMigDevState *bmds;
@@ -619,7 +581,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
- return is_stage2_completed();
+ return 0;
}
static int block_save_complete(QEMUFile *f, void *opaque)
@@ -659,6 +621,14 @@ static int block_save_complete(QEMUFile *f, void *opaque)
return 0;
}
+static uint64_t block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+{
+
+ DPRINTF("Enter save live pending %ld\n", get_remaining_dirty());
+
+ return get_remaining_dirty();
+}
+
static int block_load(QEMUFile *f, void *opaque, int version_id)
{
static int banner_printed;
@@ -755,6 +725,7 @@ SaveVMHandlers savevm_block_handlers = {
.save_live_setup = block_save_setup,
.save_live_iterate = block_save_iterate,
.save_live_complete = block_save_complete,
+ .save_live_pending = block_save_pending,
.load_state = block_load,
.cancel = block_migration_cancel,
.is_active = block_is_active,
diff --git a/buffered_file.c b/buffered_file.c
index fcb3907..c21f847 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -175,13 +175,15 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-/* 10ms xfer_limit is the limit that we should write each 10ms */
+/* 100ms xfer_limit is the limit that we should write each 100ms */
#define BUFFER_DELAY 100
static void *buffered_file_thread(void *opaque)
{
QEMUFileBuffered *s = opaque;
- int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;
+ int64_t initial_time = qemu_get_clock_ms(rt_clock);
+ int64_t max_size = 0;
+ bool last_round = false;
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
@@ -189,20 +191,29 @@ static void *buffered_file_thread(void *opaque)
if (s->migration_state->complete) {
break;
}
- if (current_time >= expire_time) {
+ if (current_time >= initial_time + BUFFER_DELAY) {
+ uint64_t transferred_bytes = s->bytes_xfer;
+ uint64_t time_spent = current_time - initial_time;
+ double bandwidth = transferred_bytes / time_spent;
+ max_size = bandwidth * migrate_max_downtime() / 1000000;
+
+ DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
+ " bandwidth %g max_size %" PRId64 "\n",
+ transferred_bytes, time_spent, bandwidth, max_size);
+
s->bytes_xfer = 0;
- expire_time = current_time + BUFFER_DELAY;
+ initial_time = current_time;
}
- if (s->bytes_xfer >= s->xfer_limit) {
+ if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
/* usleep expects microseconds */
- usleep((expire_time - current_time)*1000);
+ usleep((initial_time + BUFFER_DELAY - current_time)*1000);
}
buffered_flush(s);
DPRINTF("file is ready\n");
if (s->bytes_xfer < s->xfer_limit) {
DPRINTF("notifying client\n");
- migrate_fd_put_ready(s->migration_state);
+ last_round = migrate_fd_put_ready(s->migration_state, max_size);
}
}
diff --git a/migration.c b/migration.c
index 82c2663..8054a77 100644
--- a/migration.c
+++ b/migration.c
@@ -300,16 +300,18 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
return ret;
}
-void migrate_fd_put_ready(MigrationState *s)
+bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
{
int ret;
static bool first_time = true;
+ uint64_t pending_size;
+ bool last_round = false;
qemu_mutex_lock_iothread();
if (s->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
qemu_mutex_unlock_iothread();
- return;
+ return false;
}
if (first_time) {
first_time = false;
@@ -319,15 +321,19 @@ void migrate_fd_put_ready(MigrationState *s)
DPRINTF("failed, %d\n", ret);
migrate_fd_error(s);
qemu_mutex_unlock_iothread();
- return;
+ return false;
}
}
DPRINTF("iterate\n");
- ret = qemu_savevm_state_iterate(s->file);
- if (ret < 0) {
- migrate_fd_error(s);
- } else if (ret == 1) {
+ pending_size = qemu_savevm_state_pending(s->file, max_size);
+ DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+ if (pending_size >= max_size) {
+ ret = qemu_savevm_state_iterate(s->file);
+ if (ret < 0) {
+ migrate_fd_error(s);
+ }
+ } else {
int old_vm_running = runstate_is_running();
int64_t start_time, end_time;
@@ -353,9 +359,11 @@ void migrate_fd_put_ready(MigrationState *s)
vm_start();
}
}
+ last_round = true;
}
qemu_mutex_unlock_iothread();
+ return last_round;
}
static void migrate_fd_cancel(MigrationState *s)
diff --git a/migration.h b/migration.h
index 505f792..1f2baed 100644
--- a/migration.h
+++ b/migration.h
@@ -81,7 +81,7 @@ void migrate_fd_connect(MigrationState *s);
ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
size_t size);
-void migrate_fd_put_ready(MigrationState *s);
+bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size);
int migrate_fd_close(MigrationState *s);
void add_migration_state_change_notifier(Notifier *notify);
diff --git a/savevm.c b/savevm.c
index 69f1768..a68c37b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1681,6 +1681,25 @@ int qemu_savevm_state_complete(QEMUFile *f)
return qemu_file_get_error(f);
}
+uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
+{
+ SaveStateEntry *se;
+ uint64_t ret = 0;
+
+ QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+ if (!se->ops || !se->ops->save_live_pending) {
+ continue;
+ }
+ if (se->ops && se->ops->is_active) {
+ if (!se->ops->is_active(se->opaque)) {
+ continue;
+ }
+ }
+ ret += se->ops->save_live_pending(f, se->opaque, max_size);
+ }
+ return ret;
+}
+
void qemu_savevm_state_cancel(QEMUFile *f)
{
SaveStateEntry *se;
diff --git a/sysemu.h b/sysemu.h
index 0c39a3a..0276205 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -83,6 +83,7 @@ int qemu_savevm_state_begin(QEMUFile *f,
int qemu_savevm_state_iterate(QEMUFile *f);
int qemu_savevm_state_complete(QEMUFile *f);
void qemu_savevm_state_cancel(QEMUFile *f);
+uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size);
int qemu_loadvm_state(QEMUFile *f);
/* SLIRP */
diff --git a/vmstate.h b/vmstate.h
index c9c320e..241d962 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -35,6 +35,7 @@ typedef struct SaveVMHandlers {
int (*save_live_setup)(QEMUFile *f, void *opaque);
int (*save_live_iterate)(QEMUFile *f, void *opaque);
int (*save_live_complete)(QEMUFile *f, void *opaque);
+ uint64_t (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size);
void (*cancel)(void *opaque);
LoadStateHandler *load_state;
bool (*is_active)(void *opaque);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 14/30] migration: include qemu-file.h
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (12 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 13/30] savevm: New save live migration method: pending Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 15/30] migration-fd: remove duplicate include Juan Quintela
` (17 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
They don't use/know anything about buffered-file.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 2 +-
migration-fd.c | 2 +-
migration-tcp.c | 2 +-
migration-unix.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 908f22e..c451545 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -19,7 +19,7 @@
#include "qemu_socket.h"
#include "migration.h"
#include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
#include "block.h"
#include <sys/types.h>
#include <sys/wait.h>
diff --git a/migration-fd.c b/migration-fd.c
index aba1be7..25f0245 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -18,7 +18,7 @@
#include "migration.h"
#include "monitor.h"
#include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
#include "block.h"
#include "qemu_socket.h"
diff --git a/migration-tcp.c b/migration-tcp.c
index 812ae22..fdf8b0f 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -17,7 +17,7 @@
#include "qemu_socket.h"
#include "migration.h"
#include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
#include "block.h"
//#define DEBUG_MIGRATION_TCP
diff --git a/migration-unix.c b/migration-unix.c
index bb41bac..64bc0c3 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -17,7 +17,7 @@
#include "qemu_socket.h"
#include "migration.h"
#include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
#include "block.h"
//#define DEBUG_MIGRATION_UNIX
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 15/30] migration-fd: remove duplicate include
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (13 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 14/30] migration: include qemu-file.h Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c Juan Quintela
` (16 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-fd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/migration-fd.c b/migration-fd.c
index 25f0245..8f6b55f 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -20,7 +20,6 @@
#include "qemu-char.h"
#include "qemu-file.h"
#include "block.h"
-#include "qemu_socket.h"
//#define DEBUG_MIGRATION_FD
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (14 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 15/30] migration-fd: remove duplicate include Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 8:57 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 17/30] migration: move migration_fd_put_ready() Juan Quintela
` (15 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
This only moves the code (also from buffered_file.h to migration.h).
Fix whitespace until checkpatch is happy.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
Makefile.objs | 2 +-
buffered_file.c | 244 --------------------------------------------------------
buffered_file.h | 22 -----
migration.c | 218 +++++++++++++++++++++++++++++++++++++++++++++++++-
migration.h | 1 +
5 files changed, 219 insertions(+), 268 deletions(-)
delete mode 100644 buffered_file.c
delete mode 100644 buffered_file.h
diff --git a/Makefile.objs b/Makefile.objs
index 74b3542..3de8f27 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -73,7 +73,7 @@ extra-obj-$(CONFIG_LINUX) += fsdev/
common-obj-y += tcg-runtime.o host-utils.o main-loop.o
common-obj-y += input.o
-common-obj-y += buffered_file.o migration.o migration-tcp.o
+common-obj-y += migration.o migration-tcp.o
common-obj-y += qemu-char.o #aio.o
common-obj-y += block-migration.o iohandler.o
common-obj-y += pflib.o
diff --git a/buffered_file.c b/buffered_file.c
deleted file mode 100644
index c21f847..0000000
--- a/buffered_file.c
+++ /dev/null
@@ -1,244 +0,0 @@
-/*
- * QEMU buffered QEMUFile
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- * Anthony Liguori <aliguori@us.ibm.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- *
- * Contributions after 2012-01-13 are licensed under the terms of the
- * GNU GPL, version 2 or (at your option) any later version.
- */
-
-#include "qemu-common.h"
-#include "hw/hw.h"
-#include "qemu-timer.h"
-#include "qemu-char.h"
-#include "buffered_file.h"
-#include "qemu-thread.h"
-
-//#define DEBUG_BUFFERED_FILE
-
-typedef struct QEMUFileBuffered
-{
- MigrationState *migration_state;
- QEMUFile *file;
- size_t bytes_xfer;
- size_t xfer_limit;
- uint8_t *buffer;
- size_t buffer_size;
- size_t buffer_capacity;
- QemuThread thread;
-} QEMUFileBuffered;
-
-#ifdef DEBUG_BUFFERED_FILE
-#define DPRINTF(fmt, ...) \
- do { printf("buffered-file: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
-#endif
-
-static ssize_t buffered_flush(QEMUFileBuffered *s)
-{
- size_t offset = 0;
- ssize_t ret = 0;
-
- DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);
-
- while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
-
- ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
- s->buffer_size - offset);
- if (ret <= 0) {
- DPRINTF("error flushing data, %zd\n", ret);
- break;
- } else {
- DPRINTF("flushed %zd byte(s)\n", ret);
- offset += ret;
- s->bytes_xfer += ret;
- }
- }
-
- DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
- memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
- s->buffer_size -= offset;
-
- if (ret < 0) {
- return ret;
- }
- return offset;
-}
-
-static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
-{
- QEMUFileBuffered *s = opaque;
- ssize_t error;
-
- DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
-
- error = qemu_file_get_error(s->file);
- if (error) {
- DPRINTF("flush when error, bailing: %s\n", strerror(-error));
- return error;
- }
-
- if (size <= 0) {
- return size;
- }
-
- if (size > (s->buffer_capacity - s->buffer_size)) {
- DPRINTF("increasing buffer capacity from %zu by %zu\n",
- s->buffer_capacity, size + 1024);
-
- s->buffer_capacity += size + 1024;
-
- s->buffer = g_realloc(s->buffer, s->buffer_capacity);
- }
-
- memcpy(s->buffer + s->buffer_size, buf, size);
- s->buffer_size += size;
-
- return size;
-}
-
-static int buffered_close(void *opaque)
-{
- QEMUFileBuffered *s = opaque;
- ssize_t ret = 0;
- int ret2;
-
- DPRINTF("closing\n");
-
- s->xfer_limit = INT_MAX;
- while (!qemu_file_get_error(s->file) && s->buffer_size) {
- ret = buffered_flush(s);
- if (ret < 0) {
- break;
- }
- }
-
- ret2 = migrate_fd_close(s->migration_state);
- if (ret >= 0) {
- ret = ret2;
- }
- ret = migrate_fd_close(s->migration_state);
- s->migration_state->complete = true;
- return ret;
-}
-
-/*
- * The meaning of the return values is:
- * 0: We can continue sending
- * 1: Time to stop
- * negative: There has been an error
- */
-static int buffered_rate_limit(void *opaque)
-{
- QEMUFileBuffered *s = opaque;
- int ret;
-
- ret = qemu_file_get_error(s->file);
- if (ret) {
- return ret;
- }
-
- if (s->bytes_xfer > s->xfer_limit)
- return 1;
-
- return 0;
-}
-
-static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
-{
- QEMUFileBuffered *s = opaque;
- if (qemu_file_get_error(s->file)) {
- goto out;
- }
- if (new_rate > SIZE_MAX) {
- new_rate = SIZE_MAX;
- }
-
- s->xfer_limit = new_rate / 10;
-
-out:
- return s->xfer_limit;
-}
-
-static int64_t buffered_get_rate_limit(void *opaque)
-{
- QEMUFileBuffered *s = opaque;
-
- return s->xfer_limit;
-}
-
-/* 100ms xfer_limit is the limit that we should write each 100ms */
-#define BUFFER_DELAY 100
-
-static void *buffered_file_thread(void *opaque)
-{
- QEMUFileBuffered *s = opaque;
- int64_t initial_time = qemu_get_clock_ms(rt_clock);
- int64_t max_size = 0;
- bool last_round = false;
-
- while (true) {
- int64_t current_time = qemu_get_clock_ms(rt_clock);
-
- if (s->migration_state->complete) {
- break;
- }
- if (current_time >= initial_time + BUFFER_DELAY) {
- uint64_t transferred_bytes = s->bytes_xfer;
- uint64_t time_spent = current_time - initial_time;
- double bandwidth = transferred_bytes / time_spent;
- max_size = bandwidth * migrate_max_downtime() / 1000000;
-
- DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
- " bandwidth %g max_size %" PRId64 "\n",
- transferred_bytes, time_spent, bandwidth, max_size);
-
- s->bytes_xfer = 0;
- initial_time = current_time;
- }
- if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
- /* usleep expects microseconds */
- usleep((initial_time + BUFFER_DELAY - current_time)*1000);
- }
- buffered_flush(s);
-
- DPRINTF("file is ready\n");
- if (s->bytes_xfer < s->xfer_limit) {
- DPRINTF("notifying client\n");
- last_round = migrate_fd_put_ready(s->migration_state, max_size);
- }
- }
-
- g_free(s->buffer);
- g_free(s);
- return NULL;
-}
-
-void qemu_fopen_ops_buffered(MigrationState *migration_state)
-{
- QEMUFileBuffered *s;
-
- s = g_malloc0(sizeof(*s));
-
- s->migration_state = migration_state;
- s->xfer_limit = migration_state->bandwidth_limit / 10;
- s->migration_state->complete = false;
-
- s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
- buffered_close, buffered_rate_limit,
- buffered_set_rate_limit,
- buffered_get_rate_limit);
-
- migration_state->file = s->file;
-
- qemu_thread_create(&s->thread, buffered_file_thread, s,
- QEMU_THREAD_DETACHED);
-}
diff --git a/buffered_file.h b/buffered_file.h
deleted file mode 100644
index 8a246fd..0000000
--- a/buffered_file.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * QEMU buffered QEMUFile
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- * Anthony Liguori <aliguori@us.ibm.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_BUFFERED_FILE_H
-#define QEMU_BUFFERED_FILE_H
-
-#include "hw/hw.h"
-#include "migration.h"
-
-void qemu_fopen_ops_buffered(MigrationState *migration_state);
-
-#endif
diff --git a/migration.c b/migration.c
index 8054a77..b1567f3 100644
--- a/migration.c
+++ b/migration.c
@@ -16,7 +16,7 @@
#include "qemu-common.h"
#include "migration.h"
#include "monitor.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
#include "sysemu.h"
#include "block.h"
#include "qemu_socket.h"
@@ -569,3 +569,219 @@ int64_t migrate_xbzrle_cache_size(void)
return s->xbzrle_cache_size;
}
+
+/* migration thread support */
+
+typedef struct QEMUFileBuffered {
+ MigrationState *migration_state;
+ QEMUFile *file;
+ size_t bytes_xfer;
+ size_t xfer_limit;
+ uint8_t *buffer;
+ size_t buffer_size;
+ size_t buffer_capacity;
+ QemuThread thread;
+} QEMUFileBuffered;
+
+static ssize_t buffered_flush(QEMUFileBuffered *s)
+{
+ size_t offset = 0;
+ ssize_t ret = 0;
+
+ DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);
+
+ while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
+
+ ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
+ s->buffer_size - offset);
+ if (ret <= 0) {
+ DPRINTF("error flushing data, %zd\n", ret);
+ break;
+ } else {
+ DPRINTF("flushed %zd byte(s)\n", ret);
+ offset += ret;
+ s->bytes_xfer += ret;
+ }
+ }
+
+ DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
+ memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
+ s->buffer_size -= offset;
+
+ if (ret < 0) {
+ return ret;
+ }
+ return offset;
+}
+
+static int buffered_put_buffer(void *opaque, const uint8_t *buf,
+ int64_t pos, int size)
+{
+ QEMUFileBuffered *s = opaque;
+ ssize_t error;
+
+ DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
+
+ error = qemu_file_get_error(s->file);
+ if (error) {
+ DPRINTF("flush when error, bailing: %s\n", strerror(-error));
+ return error;
+ }
+
+ if (size <= 0) {
+ return size;
+ }
+
+ if (size > (s->buffer_capacity - s->buffer_size)) {
+ DPRINTF("increasing buffer capacity from %zu by %zu\n",
+ s->buffer_capacity, size + 1024);
+
+ s->buffer_capacity += size + 1024;
+
+ s->buffer = g_realloc(s->buffer, s->buffer_capacity);
+ }
+
+ memcpy(s->buffer + s->buffer_size, buf, size);
+ s->buffer_size += size;
+
+ return size;
+}
+
+static int buffered_close(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+ ssize_t ret = 0;
+ int ret2;
+
+ DPRINTF("closing\n");
+
+ s->xfer_limit = INT_MAX;
+ while (!qemu_file_get_error(s->file) && s->buffer_size) {
+ ret = buffered_flush(s);
+ if (ret < 0) {
+ break;
+ }
+ }
+
+ ret2 = migrate_fd_close(s->migration_state);
+ if (ret >= 0) {
+ ret = ret2;
+ }
+ ret = migrate_fd_close(s->migration_state);
+ s->migration_state->complete = true;
+ return ret;
+}
+
+/*
+ * The meaning of the return values is:
+ * 0: We can continue sending
+ * 1: Time to stop
+ * negative: There has been an error
+ */
+static int buffered_rate_limit(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+ int ret;
+
+ ret = qemu_file_get_error(s->file);
+ if (ret) {
+ return ret;
+ }
+
+ if (s->bytes_xfer > s->xfer_limit) {
+ return 1;
+ }
+
+ return 0;
+}
+
+static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
+{
+ QEMUFileBuffered *s = opaque;
+ if (qemu_file_get_error(s->file)) {
+ goto out;
+ }
+ if (new_rate > SIZE_MAX) {
+ new_rate = SIZE_MAX;
+ }
+
+ s->xfer_limit = new_rate / 10;
+
+out:
+ return s->xfer_limit;
+}
+
+static int64_t buffered_get_rate_limit(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+
+ return s->xfer_limit;
+}
+
+/* 100ms xfer_limit is the limit that we should write each 100ms */
+#define BUFFER_DELAY 100
+
+static void *buffered_file_thread(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+ int64_t initial_time = qemu_get_clock_ms(rt_clock);
+ int64_t max_size = 0;
+ bool last_round = false;
+
+ while (true) {
+ int64_t current_time = qemu_get_clock_ms(rt_clock);
+
+ if (s->migration_state->complete) {
+ break;
+ }
+ if (current_time >= initial_time + BUFFER_DELAY) {
+ uint64_t transferred_bytes = s->bytes_xfer;
+ uint64_t time_spent = current_time - initial_time;
+ double bandwidth = transferred_bytes / time_spent;
+ max_size = bandwidth * migrate_max_downtime() / 1000000;
+
+ DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
+ " bandwidth %g max_size %" PRId64 "\n",
+ transferred_bytes, time_spent, bandwidth, max_size);
+
+ s->bytes_xfer = 0;
+ initial_time = current_time;
+ }
+ if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
+ /* usleep expects microseconds */
+ usleep((initial_time + BUFFER_DELAY - current_time)*1000);
+ }
+ buffered_flush(s);
+
+ DPRINTF("file is ready\n");
+ if (s->bytes_xfer < s->xfer_limit) {
+ DPRINTF("notifying client\n");
+ last_round = migrate_fd_put_ready(s->migration_state, max_size);
+ }
+ }
+
+ g_free(s->buffer);
+ g_free(s);
+ return NULL;
+}
+
+void qemu_fopen_ops_buffered(MigrationState *migration_state)
+{
+ QEMUFileBuffered *s;
+
+ s = g_malloc0(sizeof(*s));
+
+ s->migration_state = migration_state;
+ s->xfer_limit = migration_state->bandwidth_limit / 10;
+ s->migration_state->complete = false;
+
+ s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
+ buffered_close, buffered_rate_limit,
+ buffered_set_rate_limit,
+ buffered_get_rate_limit);
+
+ migration_state->file = s->file;
+
+ qemu_thread_create(&s->thread, buffered_file_thread, s,
+ QEMU_THREAD_DETACHED);
+}
diff --git a/migration.h b/migration.h
index 1f2baed..40c8e9c 100644
--- a/migration.h
+++ b/migration.h
@@ -129,4 +129,5 @@ int64_t migrate_xbzrle_cache_size(void);
int64_t xbzrle_cache_resize(int64_t new_size);
+void qemu_fopen_ops_buffered(MigrationState *migration_state);
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c
2012-10-18 7:30 ` [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c Juan Quintela
@ 2012-10-18 8:57 ` Paolo Bonzini
0 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:57 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> This only moves the code (also from buffered_file.h to migration.h).
> Fix whitespace until checkpatch is happy.
While I agree with this patch, it is also a conflict magnet. My
migration-in-a-coroutine cleanups will touch buffered_file.c, you're
warned...
Paolo
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> Makefile.objs | 2 +-
> buffered_file.c | 244 --------------------------------------------------------
> buffered_file.h | 22 -----
> migration.c | 218 +++++++++++++++++++++++++++++++++++++++++++++++++-
> migration.h | 1 +
> 5 files changed, 219 insertions(+), 268 deletions(-)
> delete mode 100644 buffered_file.c
> delete mode 100644 buffered_file.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 74b3542..3de8f27 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -73,7 +73,7 @@ extra-obj-$(CONFIG_LINUX) += fsdev/
>
> common-obj-y += tcg-runtime.o host-utils.o main-loop.o
> common-obj-y += input.o
> -common-obj-y += buffered_file.o migration.o migration-tcp.o
> +common-obj-y += migration.o migration-tcp.o
> common-obj-y += qemu-char.o #aio.o
> common-obj-y += block-migration.o iohandler.o
> common-obj-y += pflib.o
> diff --git a/buffered_file.c b/buffered_file.c
> deleted file mode 100644
> index c21f847..0000000
> --- a/buffered_file.c
> +++ /dev/null
> @@ -1,244 +0,0 @@
> -/*
> - * QEMU buffered QEMUFile
> - *
> - * Copyright IBM, Corp. 2008
> - *
> - * Authors:
> - * Anthony Liguori <aliguori@us.ibm.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2. See
> - * the COPYING file in the top-level directory.
> - *
> - * Contributions after 2012-01-13 are licensed under the terms of the
> - * GNU GPL, version 2 or (at your option) any later version.
> - */
> -
> -#include "qemu-common.h"
> -#include "hw/hw.h"
> -#include "qemu-timer.h"
> -#include "qemu-char.h"
> -#include "buffered_file.h"
> -#include "qemu-thread.h"
> -
> -//#define DEBUG_BUFFERED_FILE
> -
> -typedef struct QEMUFileBuffered
> -{
> - MigrationState *migration_state;
> - QEMUFile *file;
> - size_t bytes_xfer;
> - size_t xfer_limit;
> - uint8_t *buffer;
> - size_t buffer_size;
> - size_t buffer_capacity;
> - QemuThread thread;
> -} QEMUFileBuffered;
> -
> -#ifdef DEBUG_BUFFERED_FILE
> -#define DPRINTF(fmt, ...) \
> - do { printf("buffered-file: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> - do { } while (0)
> -#endif
> -
> -static ssize_t buffered_flush(QEMUFileBuffered *s)
> -{
> - size_t offset = 0;
> - ssize_t ret = 0;
> -
> - DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);
> -
> - while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
> -
> - ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
> - s->buffer_size - offset);
> - if (ret <= 0) {
> - DPRINTF("error flushing data, %zd\n", ret);
> - break;
> - } else {
> - DPRINTF("flushed %zd byte(s)\n", ret);
> - offset += ret;
> - s->bytes_xfer += ret;
> - }
> - }
> -
> - DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
> - memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
> - s->buffer_size -= offset;
> -
> - if (ret < 0) {
> - return ret;
> - }
> - return offset;
> -}
> -
> -static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
> -{
> - QEMUFileBuffered *s = opaque;
> - ssize_t error;
> -
> - DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
> -
> - error = qemu_file_get_error(s->file);
> - if (error) {
> - DPRINTF("flush when error, bailing: %s\n", strerror(-error));
> - return error;
> - }
> -
> - if (size <= 0) {
> - return size;
> - }
> -
> - if (size > (s->buffer_capacity - s->buffer_size)) {
> - DPRINTF("increasing buffer capacity from %zu by %zu\n",
> - s->buffer_capacity, size + 1024);
> -
> - s->buffer_capacity += size + 1024;
> -
> - s->buffer = g_realloc(s->buffer, s->buffer_capacity);
> - }
> -
> - memcpy(s->buffer + s->buffer_size, buf, size);
> - s->buffer_size += size;
> -
> - return size;
> -}
> -
> -static int buffered_close(void *opaque)
> -{
> - QEMUFileBuffered *s = opaque;
> - ssize_t ret = 0;
> - int ret2;
> -
> - DPRINTF("closing\n");
> -
> - s->xfer_limit = INT_MAX;
> - while (!qemu_file_get_error(s->file) && s->buffer_size) {
> - ret = buffered_flush(s);
> - if (ret < 0) {
> - break;
> - }
> - }
> -
> - ret2 = migrate_fd_close(s->migration_state);
> - if (ret >= 0) {
> - ret = ret2;
> - }
> - ret = migrate_fd_close(s->migration_state);
> - s->migration_state->complete = true;
> - return ret;
> -}
> -
> -/*
> - * The meaning of the return values is:
> - * 0: We can continue sending
> - * 1: Time to stop
> - * negative: There has been an error
> - */
> -static int buffered_rate_limit(void *opaque)
> -{
> - QEMUFileBuffered *s = opaque;
> - int ret;
> -
> - ret = qemu_file_get_error(s->file);
> - if (ret) {
> - return ret;
> - }
> -
> - if (s->bytes_xfer > s->xfer_limit)
> - return 1;
> -
> - return 0;
> -}
> -
> -static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
> -{
> - QEMUFileBuffered *s = opaque;
> - if (qemu_file_get_error(s->file)) {
> - goto out;
> - }
> - if (new_rate > SIZE_MAX) {
> - new_rate = SIZE_MAX;
> - }
> -
> - s->xfer_limit = new_rate / 10;
> -
> -out:
> - return s->xfer_limit;
> -}
> -
> -static int64_t buffered_get_rate_limit(void *opaque)
> -{
> - QEMUFileBuffered *s = opaque;
> -
> - return s->xfer_limit;
> -}
> -
> -/* 100ms xfer_limit is the limit that we should write each 100ms */
> -#define BUFFER_DELAY 100
> -
> -static void *buffered_file_thread(void *opaque)
> -{
> - QEMUFileBuffered *s = opaque;
> - int64_t initial_time = qemu_get_clock_ms(rt_clock);
> - int64_t max_size = 0;
> - bool last_round = false;
> -
> - while (true) {
> - int64_t current_time = qemu_get_clock_ms(rt_clock);
> -
> - if (s->migration_state->complete) {
> - break;
> - }
> - if (current_time >= initial_time + BUFFER_DELAY) {
> - uint64_t transferred_bytes = s->bytes_xfer;
> - uint64_t time_spent = current_time - initial_time;
> - double bandwidth = transferred_bytes / time_spent;
> - max_size = bandwidth * migrate_max_downtime() / 1000000;
> -
> - DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
> - " bandwidth %g max_size %" PRId64 "\n",
> - transferred_bytes, time_spent, bandwidth, max_size);
> -
> - s->bytes_xfer = 0;
> - initial_time = current_time;
> - }
> - if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
> - /* usleep expects microseconds */
> - usleep((initial_time + BUFFER_DELAY - current_time)*1000);
> - }
> - buffered_flush(s);
> -
> - DPRINTF("file is ready\n");
> - if (s->bytes_xfer < s->xfer_limit) {
> - DPRINTF("notifying client\n");
> - last_round = migrate_fd_put_ready(s->migration_state, max_size);
> - }
> - }
> -
> - g_free(s->buffer);
> - g_free(s);
> - return NULL;
> -}
> -
> -void qemu_fopen_ops_buffered(MigrationState *migration_state)
> -{
> - QEMUFileBuffered *s;
> -
> - s = g_malloc0(sizeof(*s));
> -
> - s->migration_state = migration_state;
> - s->xfer_limit = migration_state->bandwidth_limit / 10;
> - s->migration_state->complete = false;
> -
> - s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
> - buffered_close, buffered_rate_limit,
> - buffered_set_rate_limit,
> - buffered_get_rate_limit);
> -
> - migration_state->file = s->file;
> -
> - qemu_thread_create(&s->thread, buffered_file_thread, s,
> - QEMU_THREAD_DETACHED);
> -}
> diff --git a/buffered_file.h b/buffered_file.h
> deleted file mode 100644
> index 8a246fd..0000000
> --- a/buffered_file.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/*
> - * QEMU buffered QEMUFile
> - *
> - * Copyright IBM, Corp. 2008
> - *
> - * Authors:
> - * Anthony Liguori <aliguori@us.ibm.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2. See
> - * the COPYING file in the top-level directory.
> - *
> - */
> -
> -#ifndef QEMU_BUFFERED_FILE_H
> -#define QEMU_BUFFERED_FILE_H
> -
> -#include "hw/hw.h"
> -#include "migration.h"
> -
> -void qemu_fopen_ops_buffered(MigrationState *migration_state);
> -
> -#endif
> diff --git a/migration.c b/migration.c
> index 8054a77..b1567f3 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -16,7 +16,7 @@
> #include "qemu-common.h"
> #include "migration.h"
> #include "monitor.h"
> -#include "buffered_file.h"
> +#include "qemu-file.h"
> #include "sysemu.h"
> #include "block.h"
> #include "qemu_socket.h"
> @@ -569,3 +569,219 @@ int64_t migrate_xbzrle_cache_size(void)
>
> return s->xbzrle_cache_size;
> }
> +
> +/* migration thread support */
> +
> +typedef struct QEMUFileBuffered {
> + MigrationState *migration_state;
> + QEMUFile *file;
> + size_t bytes_xfer;
> + size_t xfer_limit;
> + uint8_t *buffer;
> + size_t buffer_size;
> + size_t buffer_capacity;
> + QemuThread thread;
> +} QEMUFileBuffered;
> +
> +static ssize_t buffered_flush(QEMUFileBuffered *s)
> +{
> + size_t offset = 0;
> + ssize_t ret = 0;
> +
> + DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);
> +
> + while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
> +
> + ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
> + s->buffer_size - offset);
> + if (ret <= 0) {
> + DPRINTF("error flushing data, %zd\n", ret);
> + break;
> + } else {
> + DPRINTF("flushed %zd byte(s)\n", ret);
> + offset += ret;
> + s->bytes_xfer += ret;
> + }
> + }
> +
> + DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
> + memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
> + s->buffer_size -= offset;
> +
> + if (ret < 0) {
> + return ret;
> + }
> + return offset;
> +}
> +
> +static int buffered_put_buffer(void *opaque, const uint8_t *buf,
> + int64_t pos, int size)
> +{
> + QEMUFileBuffered *s = opaque;
> + ssize_t error;
> +
> + DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
> +
> + error = qemu_file_get_error(s->file);
> + if (error) {
> + DPRINTF("flush when error, bailing: %s\n", strerror(-error));
> + return error;
> + }
> +
> + if (size <= 0) {
> + return size;
> + }
> +
> + if (size > (s->buffer_capacity - s->buffer_size)) {
> + DPRINTF("increasing buffer capacity from %zu by %zu\n",
> + s->buffer_capacity, size + 1024);
> +
> + s->buffer_capacity += size + 1024;
> +
> + s->buffer = g_realloc(s->buffer, s->buffer_capacity);
> + }
> +
> + memcpy(s->buffer + s->buffer_size, buf, size);
> + s->buffer_size += size;
> +
> + return size;
> +}
> +
> +static int buffered_close(void *opaque)
> +{
> + QEMUFileBuffered *s = opaque;
> + ssize_t ret = 0;
> + int ret2;
> +
> + DPRINTF("closing\n");
> +
> + s->xfer_limit = INT_MAX;
> + while (!qemu_file_get_error(s->file) && s->buffer_size) {
> + ret = buffered_flush(s);
> + if (ret < 0) {
> + break;
> + }
> + }
> +
> + ret2 = migrate_fd_close(s->migration_state);
> + if (ret >= 0) {
> + ret = ret2;
> + }
> + ret = migrate_fd_close(s->migration_state);
> + s->migration_state->complete = true;
> + return ret;
> +}
> +
> +/*
> + * The meaning of the return values is:
> + * 0: We can continue sending
> + * 1: Time to stop
> + * negative: There has been an error
> + */
> +static int buffered_rate_limit(void *opaque)
> +{
> + QEMUFileBuffered *s = opaque;
> + int ret;
> +
> + ret = qemu_file_get_error(s->file);
> + if (ret) {
> + return ret;
> + }
> +
> + if (s->bytes_xfer > s->xfer_limit) {
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
> +{
> + QEMUFileBuffered *s = opaque;
> + if (qemu_file_get_error(s->file)) {
> + goto out;
> + }
> + if (new_rate > SIZE_MAX) {
> + new_rate = SIZE_MAX;
> + }
> +
> + s->xfer_limit = new_rate / 10;
> +
> +out:
> + return s->xfer_limit;
> +}
> +
> +static int64_t buffered_get_rate_limit(void *opaque)
> +{
> + QEMUFileBuffered *s = opaque;
> +
> + return s->xfer_limit;
> +}
> +
> +/* 100ms xfer_limit is the limit that we should write each 100ms */
> +#define BUFFER_DELAY 100
> +
> +static void *buffered_file_thread(void *opaque)
> +{
> + QEMUFileBuffered *s = opaque;
> + int64_t initial_time = qemu_get_clock_ms(rt_clock);
> + int64_t max_size = 0;
> + bool last_round = false;
> +
> + while (true) {
> + int64_t current_time = qemu_get_clock_ms(rt_clock);
> +
> + if (s->migration_state->complete) {
> + break;
> + }
> + if (current_time >= initial_time + BUFFER_DELAY) {
> + uint64_t transferred_bytes = s->bytes_xfer;
> + uint64_t time_spent = current_time - initial_time;
> + double bandwidth = transferred_bytes / time_spent;
> + max_size = bandwidth * migrate_max_downtime() / 1000000;
> +
> + DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
> + " bandwidth %g max_size %" PRId64 "\n",
> + transferred_bytes, time_spent, bandwidth, max_size);
> +
> + s->bytes_xfer = 0;
> + initial_time = current_time;
> + }
> + if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
> + /* usleep expects microseconds */
> + usleep((initial_time + BUFFER_DELAY - current_time)*1000);
> + }
> + buffered_flush(s);
> +
> + DPRINTF("file is ready\n");
> + if (s->bytes_xfer < s->xfer_limit) {
> + DPRINTF("notifying client\n");
> + last_round = migrate_fd_put_ready(s->migration_state, max_size);
> + }
> + }
> +
> + g_free(s->buffer);
> + g_free(s);
> + return NULL;
> +}
> +
> +void qemu_fopen_ops_buffered(MigrationState *migration_state)
> +{
> + QEMUFileBuffered *s;
> +
> + s = g_malloc0(sizeof(*s));
> +
> + s->migration_state = migration_state;
> + s->xfer_limit = migration_state->bandwidth_limit / 10;
> + s->migration_state->complete = false;
> +
> + s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
> + buffered_close, buffered_rate_limit,
> + buffered_set_rate_limit,
> + buffered_get_rate_limit);
> +
> + migration_state->file = s->file;
> +
> + qemu_thread_create(&s->thread, buffered_file_thread, s,
> + QEMU_THREAD_DETACHED);
> +}
> diff --git a/migration.h b/migration.h
> index 1f2baed..40c8e9c 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -129,4 +129,5 @@ int64_t migrate_xbzrle_cache_size(void);
>
> int64_t xbzrle_cache_resize(int64_t new_size);
>
> +void qemu_fopen_ops_buffered(MigrationState *migration_state);
> #endif
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 17/30] migration: move migration_fd_put_ready()
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (15 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 18/30] migration: Inline qemu_fopen_ops_buffered into migrate_fd_connect Juan Quintela
` (14 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Put it near its use and un-export it.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 132 ++++++++++++++++++++++++++++++------------------------------
migration.h | 1 -
2 files changed, 66 insertions(+), 67 deletions(-)
diff --git a/migration.c b/migration.c
index b1567f3..a99653e 100644
--- a/migration.c
+++ b/migration.c
@@ -300,72 +300,6 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
return ret;
}
-bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
-{
- int ret;
- static bool first_time = true;
- uint64_t pending_size;
- bool last_round = false;
-
- qemu_mutex_lock_iothread();
- if (s->state != MIG_STATE_ACTIVE) {
- DPRINTF("put_ready returning because of non-active state\n");
- qemu_mutex_unlock_iothread();
- return false;
- }
- if (first_time) {
- first_time = false;
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->file, &s->params);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- qemu_mutex_unlock_iothread();
- return false;
- }
- }
-
- DPRINTF("iterate\n");
- pending_size = qemu_savevm_state_pending(s->file, max_size);
- DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
- if (pending_size >= max_size) {
- ret = qemu_savevm_state_iterate(s->file);
- if (ret < 0) {
- migrate_fd_error(s);
- }
- } else {
- int old_vm_running = runstate_is_running();
- int64_t start_time, end_time;
-
- DPRINTF("done iterating\n");
- start_time = qemu_get_clock_ms(rt_clock);
- qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
- if (old_vm_running) {
- vm_stop(RUN_STATE_FINISH_MIGRATE);
- } else {
- vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
- }
-
- if (qemu_savevm_state_complete(s->file) < 0) {
- migrate_fd_error(s);
- } else {
- migrate_fd_completed(s);
- }
- end_time = qemu_get_clock_ms(rt_clock);
- s->total_time = end_time - s->total_time;
- s->downtime = end_time - start_time;
- if (s->state != MIG_STATE_COMPLETED) {
- if (old_vm_running) {
- vm_start();
- }
- }
- last_round = true;
- }
- qemu_mutex_unlock_iothread();
-
- return last_round;
-}
-
static void migrate_fd_cancel(MigrationState *s)
{
if (s->state != MIG_STATE_ACTIVE)
@@ -718,6 +652,72 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
+static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
+{
+ int ret;
+ static bool first_time = true;
+ uint64_t pending_size;
+ bool last_round = false;
+
+ qemu_mutex_lock_iothread();
+ if (s->state != MIG_STATE_ACTIVE) {
+ DPRINTF("put_ready returning because of non-active state\n");
+ qemu_mutex_unlock_iothread();
+ return false;
+ }
+ if (first_time) {
+ first_time = false;
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(s->file, &s->params);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ migrate_fd_error(s);
+ qemu_mutex_unlock_iothread();
+ return false;
+ }
+ }
+
+ DPRINTF("iterate\n");
+ pending_size = qemu_savevm_state_pending(s->file, max_size);
+ DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+ if (pending_size >= max_size) {
+ ret = qemu_savevm_state_iterate(s->file);
+ if (ret < 0) {
+ migrate_fd_error(s);
+ }
+ } else {
+ int old_vm_running = runstate_is_running();
+ int64_t start_time, end_time;
+
+ DPRINTF("done iterating\n");
+ start_time = qemu_get_clock_ms(rt_clock);
+ qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+ if (old_vm_running) {
+ vm_stop(RUN_STATE_FINISH_MIGRATE);
+ } else {
+ vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ }
+
+ if (qemu_savevm_state_complete(s->file) < 0) {
+ migrate_fd_error(s);
+ } else {
+ migrate_fd_completed(s);
+ }
+ end_time = qemu_get_clock_ms(rt_clock);
+ s->total_time = end_time - s->total_time;
+ s->downtime = end_time - start_time;
+ if (s->state != MIG_STATE_COMPLETED) {
+ if (old_vm_running) {
+ vm_start();
+ }
+ }
+ last_round = true;
+ }
+ qemu_mutex_unlock_iothread();
+
+ return last_round;
+}
+
/* 100ms xfer_limit is the limit that we should write each 100ms */
#define BUFFER_DELAY 100
diff --git a/migration.h b/migration.h
index 40c8e9c..f4c4d1e 100644
--- a/migration.h
+++ b/migration.h
@@ -81,7 +81,6 @@ void migrate_fd_connect(MigrationState *s);
ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
size_t size);
-bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size);
int migrate_fd_close(MigrationState *s);
void add_migration_state_change_notifier(Notifier *notify);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 18/30] migration: Inline qemu_fopen_ops_buffered into migrate_fd_connect
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (16 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 17/30] migration: move migration_fd_put_ready() Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 19/30] migration: move migration notifier Juan Quintela
` (13 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 9 ++-------
migration.h | 2 --
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/migration.c b/migration.c
index a99653e..1f783e1 100644
--- a/migration.c
+++ b/migration.c
@@ -345,12 +345,6 @@ bool migration_has_failed(MigrationState *s)
s->state == MIG_STATE_ERROR);
}
-void migrate_fd_connect(MigrationState *s)
-{
- s->state = MIG_STATE_ACTIVE;
- qemu_fopen_ops_buffered(s);
-}
-
static MigrationState *migrate_init(const MigrationParams *params)
{
MigrationState *s = migrate_get_current();
@@ -765,10 +759,11 @@ static void *buffered_file_thread(void *opaque)
return NULL;
}
-void qemu_fopen_ops_buffered(MigrationState *migration_state)
+void migrate_fd_connect(MigrationState *migration_state)
{
QEMUFileBuffered *s;
+ migration_state->state = MIG_STATE_ACTIVE;
s = g_malloc0(sizeof(*s));
s->migration_state = migration_state;
diff --git a/migration.h b/migration.h
index f4c4d1e..60a2335 100644
--- a/migration.h
+++ b/migration.h
@@ -127,6 +127,4 @@ int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
int64_t xbzrle_cache_resize(int64_t new_size);
-
-void qemu_fopen_ops_buffered(MigrationState *migration_state);
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 19/30] migration: move migration notifier
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (17 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 18/30] migration: Inline qemu_fopen_ops_buffered into migrate_fd_connect Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 20/30] migration: move begining stage to the migration thread Juan Quintela
` (12 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
At this point, it is waranteed that state is ACTIVE. Old position
didn't assured hat.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/migration.c b/migration.c
index 1f783e1..65f96b7 100644
--- a/migration.c
+++ b/migration.c
@@ -432,8 +432,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
}
return;
}
-
- notifier_list_notify(&migration_state_notifiers, s);
}
void qmp_migrate_cancel(Error **errp)
@@ -779,4 +777,5 @@ void migrate_fd_connect(MigrationState *migration_state)
qemu_thread_create(&s->thread, buffered_file_thread, s,
QEMU_THREAD_DETACHED);
+ notifier_list_notify(&migration_state_notifiers, s);
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 20/30] migration: move begining stage to the migration thread
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (18 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 19/30] migration: move migration notifier Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 21/30] migration: move exit condition to " Juan Quintela
` (11 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/migration.c b/migration.c
index 65f96b7..8cacbc3 100644
--- a/migration.c
+++ b/migration.c
@@ -647,7 +647,6 @@ static int64_t buffered_get_rate_limit(void *opaque)
static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
{
int ret;
- static bool first_time = true;
uint64_t pending_size;
bool last_round = false;
@@ -657,17 +656,6 @@ static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
qemu_mutex_unlock_iothread();
return false;
}
- if (first_time) {
- first_time = false;
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->file, &s->params);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- qemu_mutex_unlock_iothread();
- return false;
- }
- }
DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(s->file, max_size);
@@ -716,9 +704,21 @@ static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
static void *buffered_file_thread(void *opaque)
{
QEMUFileBuffered *s = opaque;
+ MigrationState *m = s->migration_state;
int64_t initial_time = qemu_get_clock_ms(rt_clock);
int64_t max_size = 0;
bool last_round = false;
+ int ret;
+
+ qemu_mutex_lock_iothread();
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(m->file, &m->params);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ qemu_mutex_unlock_iothread();
+ goto out;
+ }
+ qemu_mutex_unlock_iothread();
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
@@ -748,10 +748,14 @@ static void *buffered_file_thread(void *opaque)
DPRINTF("file is ready\n");
if (s->bytes_xfer < s->xfer_limit) {
DPRINTF("notifying client\n");
- last_round = migrate_fd_put_ready(s->migration_state, max_size);
+ last_round = migrate_fd_put_ready(m, max_size);
}
}
+out:
+ if (ret < 0) {
+ migrate_fd_error(m);
+ }
g_free(s->buffer);
g_free(s);
return NULL;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 21/30] migration: move exit condition to migration thread
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (19 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 20/30] migration: move begining stage to the migration thread Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 8:34 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread Juan Quintela
` (10 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/migration.c b/migration.c
index 8cacbc3..7206866 100644
--- a/migration.c
+++ b/migration.c
@@ -651,12 +651,6 @@ static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
bool last_round = false;
qemu_mutex_lock_iothread();
- if (s->state != MIG_STATE_ACTIVE) {
- DPRINTF("put_ready returning because of non-active state\n");
- qemu_mutex_unlock_iothread();
- return false;
- }
-
DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(s->file, max_size);
DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
@@ -723,9 +717,18 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
- if (s->migration_state->complete) {
+ qemu_mutex_lock_iothread();
+ if (m->state != MIG_STATE_ACTIVE) {
+ DPRINTF("put_ready returning because of non-active state\n");
+ qemu_mutex_unlock_iothread();
break;
}
+ if (m->complete) {
+ qemu_mutex_unlock_iothread();
+ break;
+ }
+ qemu_mutex_unlock_iothread();
+
if (current_time >= initial_time + BUFFER_DELAY) {
uint64_t transferred_bytes = s->bytes_xfer;
uint64_t time_spent = current_time - initial_time;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 21/30] migration: move exit condition to migration thread
2012-10-18 7:30 ` [Qemu-devel] [PATCH 21/30] migration: move exit condition to " Juan Quintela
@ 2012-10-18 8:34 ` Paolo Bonzini
2012-10-26 11:43 ` Juan Quintela
0 siblings, 1 reply; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:34 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> - if (s->migration_state->complete) {
> + qemu_mutex_lock_iothread();
So, was it a bug that we were accessing ->complete without the BQL?
> + if (m->state != MIG_STATE_ACTIVE) {
> + DPRINTF("put_ready returning because of non-active state\n");
The contents of the debug message obsolete. Besides, I would just put
the two branches in the same "if (m->state != MIG_STATE_ACTIVE ||
m->complete)".
> + qemu_mutex_unlock_iothread();
> break;
> }
> + if (m->complete) {
> + qemu_mutex_unlock_iothread();
> + break;
> + }
> + qemu_mutex_unlock_iothread();
> +
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 21/30] migration: move exit condition to migration thread
2012-10-18 8:34 ` Paolo Bonzini
@ 2012-10-26 11:43 ` Juan Quintela
0 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-26 11:43 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/10/2012 09:30, Juan Quintela ha scritto:
>> - if (s->migration_state->complete) {
>> + qemu_mutex_lock_iothread();
>
> So, was it a bug that we were accessing ->complete without the BQL?
>
>> + if (m->state != MIG_STATE_ACTIVE) {
>> + DPRINTF("put_ready returning because of non-active state\n");
>
> The contents of the debug message obsolete. Besides, I would just put
> the two branches in the same "if (m->state != MIG_STATE_ACTIVE ||
> m->complete)".
BQL is not needed. Complete is only used by the migration thread. but
maintaing it outside of the iothread lock, makes locking even more complicated.
>
>> + qemu_mutex_unlock_iothread();
>> break;
>> }
>> + if (m->complete) {
>> + qemu_mutex_unlock_iothread();
>> + break;
>> + }
>> + qemu_mutex_unlock_iothread();
>> +
>
> Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (20 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 21/30] migration: move exit condition to " Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 8:39 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 23/30] migration: print times for end phase Juan Quintela
` (9 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
This will allow us finer control in next patches.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 95 ++++++++++++++++++++++++++-----------------------------------
1 file changed, 41 insertions(+), 54 deletions(-)
diff --git a/migration.c b/migration.c
index 7206866..e6ff1f1 100644
--- a/migration.c
+++ b/migration.c
@@ -644,54 +644,6 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
-{
- int ret;
- uint64_t pending_size;
- bool last_round = false;
-
- qemu_mutex_lock_iothread();
- DPRINTF("iterate\n");
- pending_size = qemu_savevm_state_pending(s->file, max_size);
- DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
- if (pending_size >= max_size) {
- ret = qemu_savevm_state_iterate(s->file);
- if (ret < 0) {
- migrate_fd_error(s);
- }
- } else {
- int old_vm_running = runstate_is_running();
- int64_t start_time, end_time;
-
- DPRINTF("done iterating\n");
- start_time = qemu_get_clock_ms(rt_clock);
- qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
- if (old_vm_running) {
- vm_stop(RUN_STATE_FINISH_MIGRATE);
- } else {
- vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
- }
-
- if (qemu_savevm_state_complete(s->file) < 0) {
- migrate_fd_error(s);
- } else {
- migrate_fd_completed(s);
- }
- end_time = qemu_get_clock_ms(rt_clock);
- s->total_time = end_time - s->total_time;
- s->downtime = end_time - start_time;
- if (s->state != MIG_STATE_COMPLETED) {
- if (old_vm_running) {
- vm_start();
- }
- }
- last_round = true;
- }
- qemu_mutex_unlock_iothread();
-
- return last_round;
-}
-
/* 100ms xfer_limit is the limit that we should write each 100ms */
#define BUFFER_DELAY 100
@@ -716,6 +668,7 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
+ uint64_t pending_size;
qemu_mutex_lock_iothread();
if (m->state != MIG_STATE_ACTIVE) {
@@ -727,6 +680,46 @@ static void *buffered_file_thread(void *opaque)
qemu_mutex_unlock_iothread();
break;
}
+ if (s->bytes_xfer < s->xfer_limit) {
+ DPRINTF("iterate\n");
+ pending_size = qemu_savevm_state_pending(m->file, max_size);
+ DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+ if (pending_size >= max_size) {
+ ret = qemu_savevm_state_iterate(m->file);
+ if (ret < 0) {
+ qemu_mutex_unlock_iothread();
+ break;
+ }
+ } else {
+ int old_vm_running = runstate_is_running();
+ int64_t start_time, end_time;
+
+ DPRINTF("done iterating\n");
+ start_time = qemu_get_clock_ms(rt_clock);
+ qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+ if (old_vm_running) {
+ vm_stop(RUN_STATE_FINISH_MIGRATE);
+ } else {
+ vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ }
+ ret = qemu_savevm_state_complete(m->file);
+ if (ret < 0) {
+ qemu_mutex_unlock_iothread();
+ break;
+ } else {
+ migrate_fd_completed(m);
+ }
+ end_time = qemu_get_clock_ms(rt_clock);
+ m->total_time = end_time - m->total_time;
+ m->downtime = end_time - start_time;
+ if (m->state != MIG_STATE_COMPLETED) {
+ if (old_vm_running) {
+ vm_start();
+ }
+ }
+ last_round = true;
+ }
+ }
qemu_mutex_unlock_iothread();
if (current_time >= initial_time + BUFFER_DELAY) {
@@ -747,12 +740,6 @@ static void *buffered_file_thread(void *opaque)
usleep((initial_time + BUFFER_DELAY - current_time)*1000);
}
buffered_flush(s);
-
- DPRINTF("file is ready\n");
- if (s->bytes_xfer < s->xfer_limit) {
- DPRINTF("notifying client\n");
- last_round = migrate_fd_put_ready(m, max_size);
- }
}
out:
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread
2012-10-18 7:30 ` [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread Juan Quintela
@ 2012-10-18 8:39 ` Paolo Bonzini
2012-10-18 8:55 ` Paolo Bonzini
0 siblings, 1 reply; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:39 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> This will allow us finer control in next patches.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration.c | 95 ++++++++++++++++++++++++++-----------------------------------
> 1 file changed, 41 insertions(+), 54 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 7206866..e6ff1f1 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -644,54 +644,6 @@ static int64_t buffered_get_rate_limit(void *opaque)
> return s->xfer_limit;
> }
>
> -static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
> -{
> - int ret;
> - uint64_t pending_size;
> - bool last_round = false;
> -
> - qemu_mutex_lock_iothread();
> - DPRINTF("iterate\n");
> - pending_size = qemu_savevm_state_pending(s->file, max_size);
> - DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
> - if (pending_size >= max_size) {
> - ret = qemu_savevm_state_iterate(s->file);
> - if (ret < 0) {
> - migrate_fd_error(s);
> - }
> - } else {
> - int old_vm_running = runstate_is_running();
> - int64_t start_time, end_time;
> -
> - DPRINTF("done iterating\n");
> - start_time = qemu_get_clock_ms(rt_clock);
> - qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> - if (old_vm_running) {
> - vm_stop(RUN_STATE_FINISH_MIGRATE);
> - } else {
> - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> - }
> -
> - if (qemu_savevm_state_complete(s->file) < 0) {
> - migrate_fd_error(s);
> - } else {
> - migrate_fd_completed(s);
> - }
> - end_time = qemu_get_clock_ms(rt_clock);
> - s->total_time = end_time - s->total_time;
> - s->downtime = end_time - start_time;
> - if (s->state != MIG_STATE_COMPLETED) {
> - if (old_vm_running) {
> - vm_start();
> - }
> - }
> - last_round = true;
> - }
> - qemu_mutex_unlock_iothread();
> -
> - return last_round;
> -}
> -
> /* 100ms xfer_limit is the limit that we should write each 100ms */
> #define BUFFER_DELAY 100
>
> @@ -716,6 +668,7 @@ static void *buffered_file_thread(void *opaque)
>
> while (true) {
> int64_t current_time = qemu_get_clock_ms(rt_clock);
> + uint64_t pending_size;
>
> qemu_mutex_lock_iothread();
> if (m->state != MIG_STATE_ACTIVE) {
> @@ -727,6 +680,46 @@ static void *buffered_file_thread(void *opaque)
> qemu_mutex_unlock_iothread();
> break;
> }
> + if (s->bytes_xfer < s->xfer_limit) {
> + DPRINTF("iterate\n");
> + pending_size = qemu_savevm_state_pending(m->file, max_size);
> + DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
> + if (pending_size >= max_size) {
> + ret = qemu_savevm_state_iterate(m->file);
So RAM migration is still being run inside the BQL, isn't it?
> + if (ret < 0) {
> + qemu_mutex_unlock_iothread();
> + break;
There's a lot of
qemu_mutex_unlock_iothread();
break;
in this function. Perhaps it is better if you make an invariant that
the loop is entered and exited with the BQL taken, and it is only
unlocked in the middle. It makes sense once you fold everything in
migration.c.
It can be a separate patch though.
> + }
> + } else {
> + int old_vm_running = runstate_is_running();
> + int64_t start_time, end_time;
> +
> + DPRINTF("done iterating\n");
> + start_time = qemu_get_clock_ms(rt_clock);
> + qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> + if (old_vm_running) {
> + vm_stop(RUN_STATE_FINISH_MIGRATE);
> + } else {
> + vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> + }
> + ret = qemu_savevm_state_complete(m->file);
> + if (ret < 0) {
> + qemu_mutex_unlock_iothread();
> + break;
> + } else {
> + migrate_fd_completed(m);
> + }
> + end_time = qemu_get_clock_ms(rt_clock);
> + m->total_time = end_time - m->total_time;
> + m->downtime = end_time - start_time;
> + if (m->state != MIG_STATE_COMPLETED) {
> + if (old_vm_running) {
> + vm_start();
> + }
> + }
> + last_round = true;
> + }
> + }
> qemu_mutex_unlock_iothread();
>
> if (current_time >= initial_time + BUFFER_DELAY) {
> @@ -747,12 +740,6 @@ static void *buffered_file_thread(void *opaque)
> usleep((initial_time + BUFFER_DELAY - current_time)*1000);
> }
> buffered_flush(s);
> -
> - DPRINTF("file is ready\n");
> - if (s->bytes_xfer < s->xfer_limit) {
> - DPRINTF("notifying client\n");
> - last_round = migrate_fd_put_ready(m, max_size);
> - }
> }
>
> out:
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread
2012-10-18 8:39 ` Paolo Bonzini
@ 2012-10-18 8:55 ` Paolo Bonzini
0 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:55 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 10:39, Paolo Bonzini ha scritto:
> in this function. Perhaps it is better if you make an invariant that
> the loop is entered and exited with the BQL taken, and it is only
> unlocked in the middle. It makes sense once you fold everything in
> migration.c.
i.e. this:
diff --git a/migration.c b/migration.c
index 554d79a..bd9fe2d 100644
--- a/migration.c
+++ b/migration.c
@@ -661,23 +661,18 @@ static void *buffered_file_thread(void *opaque)
ret = qemu_savevm_state_begin(m->file, &m->params);
if (ret < 0) {
DPRINTF("failed, %d\n", ret);
- qemu_mutex_unlock_iothread();
goto out;
}
- qemu_mutex_unlock_iothread();
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
uint64_t pending_size;
- qemu_mutex_lock_iothread();
if (m->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
- qemu_mutex_unlock_iothread();
break;
}
if (m->complete) {
- qemu_mutex_unlock_iothread();
break;
}
if (s->bytes_xfer < s->xfer_limit) {
@@ -687,7 +682,6 @@ static void *buffered_file_thread(void *opaque)
if (pending_size && pending_size >= max_size) {
ret = qemu_savevm_state_iterate(m->file);
if (ret < 0) {
- qemu_mutex_unlock_iothread();
break;
}
} else {
@@ -708,7 +702,6 @@ static void *buffered_file_thread(void *opaque)
printf("vm_stop 2 %ld\n", end_time - start_time);
ret = qemu_savevm_state_complete(m->file);
if (ret < 0) {
- qemu_mutex_unlock_iothread();
break;
} else {
end_time = qemu_get_clock_ms(rt_clock);
@@ -729,8 +722,8 @@ static void *buffered_file_thread(void *opaque)
last_round = true;
}
}
- qemu_mutex_unlock_iothread();
+ qemu_mutex_unlock_iothread();
if (current_time >= initial_time + BUFFER_DELAY) {
uint64_t transferred_bytes = s->bytes_xfer;
uint64_t time_spent = current_time - initial_time;
@@ -749,9 +742,11 @@ static void *buffered_file_thread(void *opaque)
usleep((initial_time + BUFFER_DELAY - current_time)*1000);
}
buffered_flush(s);
+ qemu_mutex_lock_iothread();
}
out:
+ qemu_mutex_unlock_iothread();
if (ret < 0) {
migrate_fd_error(m);
}
BTW the completion (basically the "else" starting at "int old_vm_running =
runstate_is_running()" looks much nicer in a separate function, like:
if (pending_size && pending_size >= max_size) {
ret = qemu_savevm_state_iterate(m->file);
} else {
ret = migration_complete(m);
last_round = true;
}
if (ret < 0) {
break;
}
Paolo
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 23/30] migration: print times for end phase
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (21 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 24/30] ram: rename last_block to last_seen_block Juan Quintela
` (8 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block.c | 6 ++++++
cpus.c | 17 +++++++++++++++++
migration.c | 9 +++++++++
savevm.c | 13 +++++++++++++
4 files changed, 45 insertions(+)
diff --git a/block.c b/block.c
index e95f613..9cd6ffe 100644
--- a/block.c
+++ b/block.c
@@ -2648,9 +2648,15 @@ int bdrv_get_flags(BlockDriverState *bs)
void bdrv_flush_all(void)
{
BlockDriverState *bs;
+ int64_t start_time, end_time;
+
+ start_time = qemu_get_clock_ms(rt_clock);
QTAILQ_FOREACH(bs, &bdrv_states, list) {
bdrv_flush(bs);
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time flush device %s: %ld\n", bs->filename,
+ end_time - start_time);
}
}
diff --git a/cpus.c b/cpus.c
index 191cbf5..63e51f1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -435,14 +435,31 @@ int cpu_is_stopped(CPUArchState *env)
static void do_vm_stop(RunState state)
{
+ int64_t start_time, end_time;
+
if (runstate_is_running()) {
+ start_time = qemu_get_clock_ms(rt_clock);
cpu_disable_ticks();
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time cpu_disable_ticks %ld\n", end_time - start_time);
pause_all_vcpus();
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time pause_all_vcpus %ld\n", end_time - start_time);
runstate_set(state);
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time runstate_set %ld\n", end_time - start_time);
vm_state_notify(0, state);
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time vmstate_notify %ld\n", end_time - start_time);
bdrv_drain_all();
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time bdrv_drain_all %ld\n", end_time - start_time);
bdrv_flush_all();
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time bdrv_flush_all %ld\n", end_time - start_time);
monitor_protocol_event(QEVENT_STOP, NULL);
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("time monitor_protocol_event %ld\n", end_time - start_time);
}
}
diff --git a/migration.c b/migration.c
index e6ff1f1..3856a99 100644
--- a/migration.c
+++ b/migration.c
@@ -697,19 +697,26 @@ static void *buffered_file_thread(void *opaque)
DPRINTF("done iterating\n");
start_time = qemu_get_clock_ms(rt_clock);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("wakeup_request %ld\n", end_time - start_time);
if (old_vm_running) {
vm_stop(RUN_STATE_FINISH_MIGRATE);
} else {
vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
}
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("vm_stop 2 %ld\n", end_time - start_time);
ret = qemu_savevm_state_complete(m->file);
if (ret < 0) {
qemu_mutex_unlock_iothread();
break;
} else {
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("complete without error 3a %ld\n", end_time - start_time);
migrate_fd_completed(m);
}
end_time = qemu_get_clock_ms(rt_clock);
+ printf("completed %ld\n", end_time - start_time);
m->total_time = end_time - m->total_time;
m->downtime = end_time - start_time;
if (m->state != MIG_STATE_COMPLETED) {
@@ -717,6 +724,8 @@ static void *buffered_file_thread(void *opaque)
vm_start();
}
}
+ end_time = qemu_get_clock_ms(rt_clock);
+ printf("end completed stage %ld\n", end_time - start_time);
last_round = true;
}
}
diff --git a/savevm.c b/savevm.c
index a68c37b..3ebde5c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1629,9 +1629,14 @@ int qemu_savevm_state_complete(QEMUFile *f)
{
SaveStateEntry *se;
int ret;
+ int64_t t1;
+ int64_t t0 = qemu_get_clock_ms(rt_clock);
cpu_synchronize_all_states();
+ t1 = qemu_get_clock_ms(rt_clock);
+ printf("synchronize_all_states %ld\n", t1 - t0);
+ t0 = t1;
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
if (!se->ops || !se->ops->save_live_complete) {
continue;
@@ -1652,6 +1657,11 @@ int qemu_savevm_state_complete(QEMUFile *f)
return ret;
}
}
+ t1 = qemu_get_clock_ms(rt_clock);
+
+ printf("migrate RAM %ld\n", t1 - t0);
+
+ t0 = t1;
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
int len;
@@ -1676,6 +1686,9 @@ int qemu_savevm_state_complete(QEMUFile *f)
trace_savevm_section_end(se->section_id);
}
+ t1 = qemu_get_clock_ms(rt_clock);
+
+ printf("migrate rest devices %ld\n", t1 - t0);
qemu_put_byte(f, QEMU_VM_EOF);
return qemu_file_get_error(f);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 24/30] ram: rename last_block to last_seen_block
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (22 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 23/30] migration: print times for end phase Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 25/30] ram: Add last_sent_block Juan Quintela
` (7 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 1eefef8..8ac4c94 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -332,7 +332,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
return bytes_sent;
}
-static RAMBlock *last_block;
+
+/* This is the last block that we have visited serching for dirty pages
+ */
+static RAMBlock *last_seen_block;
static ram_addr_t last_offset;
static unsigned long *migration_bitmap;
static uint64_t migration_dirty_pages;
@@ -417,7 +420,7 @@ static void migration_bitmap_sync(void)
static int ram_save_block(QEMUFile *f, bool last_stage)
{
- RAMBlock *block = last_block;
+ RAMBlock *block = last_seen_block;
ram_addr_t offset = last_offset;
int bytes_sent = -1;
MemoryRegion *mr;
@@ -430,7 +433,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
mr = block->mr;
if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
uint8_t *p;
- int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
+ int cont = (block == last_seen_block) ?
+ RAM_SAVE_FLAG_CONTINUE : 0;
p = memory_region_get_ram_ptr(mr) + offset;
@@ -469,9 +473,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
if (!block)
block = QLIST_FIRST(&ram_list.blocks);
}
- } while (block != last_block || offset != last_offset);
+ } while (block != last_seen_block || offset != last_offset);
- last_block = block;
+ last_seen_block = block;
last_offset = offset;
return bytes_sent;
@@ -555,7 +559,7 @@ static void ram_migration_cancel(void *opaque)
static void reset_ram_globals(void)
{
- last_block = NULL;
+ last_seen_block = NULL;
last_offset = 0;
last_version = ram_list.version;
sort_ram_list();
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 25/30] ram: Add last_sent_block
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (23 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 24/30] ram: rename last_block to last_seen_block Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 26/30] memory: introduce memory_region_test_and_clear_dirty Juan Quintela
` (6 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Orit Wasserman
This is the last block from where we have sent data.
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch_init.c b/arch_init.c
index 8ac4c94..6f39ebd 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -336,6 +336,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
/* This is the last block that we have visited serching for dirty pages
*/
static RAMBlock *last_seen_block;
+/* This is the last block from where we have sent data */
+static RAMBlock *last_sent_block;
static ram_addr_t last_offset;
static unsigned long *migration_bitmap;
static uint64_t migration_dirty_pages;
@@ -433,7 +435,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
mr = block->mr;
if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
uint8_t *p;
- int cont = (block == last_seen_block) ?
+ int cont = (block == last_sent_block) ?
RAM_SAVE_FLAG_CONTINUE : 0;
p = memory_region_get_ram_ptr(mr) + offset;
@@ -462,6 +464,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
/* if page is unmodified, continue to the next */
if (bytes_sent != 0) {
+ last_sent_block = block;
break;
}
}
@@ -560,6 +563,7 @@ static void ram_migration_cancel(void *opaque)
static void reset_ram_globals(void)
{
last_seen_block = NULL;
+ last_sent_block = NULL;
last_offset = 0;
last_version = ram_list.version;
sort_ram_list();
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 26/30] memory: introduce memory_region_test_and_clear_dirty
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (24 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 25/30] ram: Add last_sent_block Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 27/30] ram: Use memory_region_test_and_clear_dirty Juan Quintela
` (5 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
This function avoids having to do two calls, one to test the dirty bit, and
other to reset it.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
memory.c | 15 +++++++++++++++
memory.h | 17 +++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/memory.c b/memory.c
index 4f3ade0..126fb63 100644
--- a/memory.c
+++ b/memory.c
@@ -1080,6 +1080,21 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
}
+bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
+ target_phys_addr_t addr,
+ target_phys_addr_t size, unsigned client)
+{
+ bool ret;
+ assert(mr->terminates);
+ ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
+ 1 << client);
+ if (ret) {
+ cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
+ }
+ return ret;
+}
+
+
void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
{
FlatRange *fr;
diff --git a/memory.h b/memory.h
index 37ce151..08af012 100644
--- a/memory.h
+++ b/memory.h
@@ -434,6 +434,23 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
target_phys_addr_t size);
/**
+ * memory_region_test_and_clear_dirty: Check whether a range of bytes is dirty
+ * for a specified client. It clears them.
+ *
+ * Checks whether a range of bytes has been written to since the last
+ * call to memory_region_reset_dirty() with the same @client. Dirty logging
+ * must be enabled.
+ *
+ * @mr: the memory region being queried.
+ * @addr: the address (relative to the start of the region) being queried.
+ * @size: the size of the range being queried.
+ * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or
+ * %DIRTY_MEMORY_VGA.
+ */
+bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
+ target_phys_addr_t addr,
+ target_phys_addr_t size, unsigned client)
+/**
* memory_region_sync_dirty_bitmap: Synchronize a region's dirty bitmap with
* any external TLBs (e.g. kvm)
*
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 27/30] ram: Use memory_region_test_and_clear_dirty
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (25 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 26/30] memory: introduce memory_region_test_and_clear_dirty Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 12:52 ` Eric Blake
2012-10-18 7:30 ` [Qemu-devel] [PATCH 28/30] fix memory.c Juan Quintela
` (4 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
This avoids having to do two walks over the dirty bitmap, once reading
the dirty bits, and anthoer cleaning them.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 6f39ebd..8391375 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -390,13 +390,12 @@ static void migration_bitmap_sync(void)
QLIST_FOREACH(block, &ram_list.blocks, next) {
for (addr = 0; addr < block->length; addr += TARGET_PAGE_SIZE) {
- if (memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE,
- DIRTY_MEMORY_MIGRATION)) {
+ if (memory_region_test_and_clear_dirty(block->mr,
+ addr, TARGET_PAGE_SIZE,
+ DIRTY_MEMORY_MIGRATION)) {
migration_bitmap_set_dirty(block->mr, addr);
}
}
- memory_region_reset_dirty(block->mr, 0, block->length,
- DIRTY_MEMORY_MIGRATION);
}
trace_migration_bitmap_sync_end(migration_dirty_pages
- num_dirty_pages_init);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 28/30] fix memory.c
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (26 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 27/30] ram: Use memory_region_test_and_clear_dirty Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 8:43 ` Paolo Bonzini
2012-10-18 7:30 ` [Qemu-devel] [PATCH 29/30] migration: Only go to the iterate stage if there is anything to send Juan Quintela
` (3 subsequent siblings)
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
memory.c | 4 +++-
memory.h | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index 126fb63..4d0fa96 100644
--- a/memory.c
+++ b/memory.c
@@ -1089,7 +1089,9 @@ bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
1 << client);
if (ret) {
- cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
+ cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
+ mr->ram_addr + addr + size,
+ 1 << client);
}
return ret;
}
diff --git a/memory.h b/memory.h
index 08af012..0dcc0f4 100644
--- a/memory.h
+++ b/memory.h
@@ -449,7 +449,8 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
*/
bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
target_phys_addr_t addr,
- target_phys_addr_t size, unsigned client)
+ target_phys_addr_t size,
+ unsigned client);
/**
* memory_region_sync_dirty_bitmap: Synchronize a region's dirty bitmap with
* any external TLBs (e.g. kvm)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 28/30] fix memory.c
2012-10-18 7:30 ` [Qemu-devel] [PATCH 28/30] fix memory.c Juan Quintela
@ 2012-10-18 8:43 ` Paolo Bonzini
0 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 8:43 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> memory.c | 4 +++-
> memory.h | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index 126fb63..4d0fa96 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1089,7 +1089,9 @@ bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
> ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
> 1 << client);
> if (ret) {
> - cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
> + cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
> + mr->ram_addr + addr + size,
> + 1 << client);
> }
> return ret;
> }
> diff --git a/memory.h b/memory.h
> index 08af012..0dcc0f4 100644
> --- a/memory.h
> +++ b/memory.h
> @@ -449,7 +449,8 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
> */
> bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
> target_phys_addr_t addr,
> - target_phys_addr_t size, unsigned client)
> + target_phys_addr_t size,
> + unsigned client);
> /**
> * memory_region_sync_dirty_bitmap: Synchronize a region's dirty bitmap with
> * any external TLBs (e.g. kvm)
>
This should be squashed in patch 26.
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH 29/30] migration: Only go to the iterate stage if there is anything to send
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (27 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 28/30] fix memory.c Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-18 7:30 ` [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking Juan Quintela
` (2 subsequent siblings)
31 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Orit Wasserman
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration.c b/migration.c
index 3856a99..554d79a 100644
--- a/migration.c
+++ b/migration.c
@@ -684,7 +684,7 @@ static void *buffered_file_thread(void *opaque)
DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(m->file, max_size);
DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
- if (pending_size >= max_size) {
+ if (pending_size && pending_size >= max_size) {
ret = qemu_savevm_state_iterate(m->file);
if (ret < 0) {
qemu_mutex_unlock_iothread();
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (28 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 29/30] migration: Only go to the iterate stage if there is anything to send Juan Quintela
@ 2012-10-18 7:30 ` Juan Quintela
2012-10-21 13:01 ` Orit Wasserman
2012-10-18 9:00 ` [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Paolo Bonzini
2012-10-26 13:04 ` Paolo Bonzini
31 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-18 7:30 UTC (permalink / raw)
To: qemu-devel
Instead of testing each page individually, we search what is the next
dirty page with a bitmap operation. We have to reorganize the code to
move from a "for" loop, to a while(dirty) loop.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8391375..cd6ebef 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -343,18 +343,21 @@ static unsigned long *migration_bitmap;
static uint64_t migration_dirty_pages;
static uint32_t last_version;
-static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
- ram_addr_t offset)
+static inline
+ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
+ ram_addr_t start)
{
- bool ret;
- int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;
+ unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS;
+ unsigned long nr = base + (start >> TARGET_PAGE_BITS);
+ unsigned long size = base + (int128_get64(mr->size) >> TARGET_PAGE_BITS);
- ret = test_and_clear_bit(nr, migration_bitmap);
+ unsigned long next = find_next_bit(migration_bitmap, size, nr);
- if (ret) {
+ if (next < size) {
+ clear_bit(next, migration_bitmap);
migration_dirty_pages--;
}
- return ret;
+ return (next - base) << TARGET_PAGE_BITS;
}
static inline bool migration_bitmap_set_dirty(MemoryRegion *mr,
@@ -423,6 +426,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
{
RAMBlock *block = last_seen_block;
ram_addr_t offset = last_offset;
+ bool complete_round = false;
int bytes_sent = -1;
MemoryRegion *mr;
ram_addr_t current_addr;
@@ -430,9 +434,21 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
if (!block)
block = QLIST_FIRST(&ram_list.blocks);
- do {
+ while(true) {
mr = block->mr;
- if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
+ offset = migration_bitmap_find_and_reset_dirty(mr, offset);
+ if (complete_round && block == last_seen_block &&
+ offset >= last_offset) {
+ break;
+ }
+ if (offset >= block->length) {
+ offset = 0;
+ block = QLIST_NEXT(block, next);
+ if (!block) {
+ block = QLIST_FIRST(&ram_list.blocks);
+ complete_round = true;
+ }
+ } else {
uint8_t *p;
int cont = (block == last_sent_block) ?
RAM_SAVE_FLAG_CONTINUE : 0;
@@ -467,16 +483,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
break;
}
}
-
- offset += TARGET_PAGE_SIZE;
- if (offset >= block->length) {
- offset = 0;
- block = QLIST_NEXT(block, next);
- if (!block)
- block = QLIST_FIRST(&ram_list.blocks);
- }
- } while (block != last_seen_block || offset != last_offset);
-
+ }
last_seen_block = block;
last_offset = offset;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-18 7:30 ` [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking Juan Quintela
@ 2012-10-21 13:01 ` Orit Wasserman
2012-10-26 11:39 ` Juan Quintela
0 siblings, 1 reply; 58+ messages in thread
From: Orit Wasserman @ 2012-10-21 13:01 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 10/18/2012 09:30 AM, Juan Quintela wrote:
> Instead of testing each page individually, we search what is the next
> dirty page with a bitmap operation. We have to reorganize the code to
> move from a "for" loop, to a while(dirty) loop.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> arch_init.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 8391375..cd6ebef 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -343,18 +343,21 @@ static unsigned long *migration_bitmap;
> static uint64_t migration_dirty_pages;
> static uint32_t last_version;
>
> -static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
> - ram_addr_t offset)
> +static inline
> +ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
> + ram_addr_t start)
> {
> - bool ret;
> - int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;
> + unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS;
> + unsigned long nr = base + (start >> TARGET_PAGE_BITS);
> + unsigned long size = base + (int128_get64(mr->size) >> TARGET_PAGE_BITS);
>
> - ret = test_and_clear_bit(nr, migration_bitmap);
> + unsigned long next = find_next_bit(migration_bitmap, size, nr);
>
> - if (ret) {
> + if (next < size) {
> + clear_bit(next, migration_bitmap);
> migration_dirty_pages--;
> }
> - return ret;
> + return (next - base) << TARGET_PAGE_BITS;
> }
>
> static inline bool migration_bitmap_set_dirty(MemoryRegion *mr,
> @@ -423,6 +426,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
> {
> RAMBlock *block = last_seen_block;
> ram_addr_t offset = last_offset;
> + bool complete_round = false;
> int bytes_sent = -1;
> MemoryRegion *mr;
> ram_addr_t current_addr;
> @@ -430,9 +434,21 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
> if (!block)
> block = QLIST_FIRST(&ram_list.blocks);
>
> - do {
> + while(true) {
> mr = block->mr;
> - if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
> + offset = migration_bitmap_find_and_reset_dirty(mr, offset);
> + if (complete_round && block == last_seen_block &&
> + offset >= last_offset) {
> + break;
> + }
Juan,
You need to exchange those line, first check to see you did a full round than
calculate and reset the offset, in the way it is written now you may reset a bit and than break of the loop
without sending it.
Orit
> + if (offset >= block->length) {
> + offset = 0;
> + block = QLIST_NEXT(block, next);
> + if (!block) {
> + block = QLIST_FIRST(&ram_list.blocks);
> + complete_round = true;
> + }
> + } else {
> uint8_t *p;
> int cont = (block == last_sent_block) ?
> RAM_SAVE_FLAG_CONTINUE : 0;
> @@ -467,16 +483,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
> break;
> }
> }
> -
> - offset += TARGET_PAGE_SIZE;
> - if (offset >= block->length) {
> - offset = 0;
> - block = QLIST_NEXT(block, next);
> - if (!block)
> - block = QLIST_FIRST(&ram_list.blocks);
> - }
> - } while (block != last_seen_block || offset != last_offset);
> -
> + }
> last_seen_block = block;
> last_offset = offset;
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-21 13:01 ` Orit Wasserman
@ 2012-10-26 11:39 ` Juan Quintela
2012-10-28 8:35 ` Orit Wasserman
0 siblings, 1 reply; 58+ messages in thread
From: Juan Quintela @ 2012-10-26 11:39 UTC (permalink / raw)
To: Orit Wasserman; +Cc: qemu-devel
Orit Wasserman <owasserm@redhat.com> wrote:
> On 10/18/2012 09:30 AM, Juan Quintela wrote:
>> Instead of testing each page individually, we search what is the next
>> dirty page with a bitmap operation. We have to reorganize the code to
>> move from a "for" loop, to a while(dirty) loop.
>>
>>
>> - do {
>> + while(true) {
>> mr = block->mr;
>> - if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
>> + offset = migration_bitmap_find_and_reset_dirty(mr, offset);
>> + if (complete_round && block == last_seen_block &&
>> + offset >= last_offset) {
>> + break;
>> + }
> Juan,
> You need to exchange those line, first check to see you did a full round than
> calculate and reset the offset, in the way it is written now you may
> reset a bit and than break of the loop
> without sending it.
How?
if complete_round == true, it means that we are in the second round.
block == last_seen_block means that we are back at the 1st block that we
have looked.
if offset >= last_offset, there are two options:
a- == last_offset: that was the 1st one that we checked, so it can't be
true.
b- >= last_offset: it means tat we have already passed that bit, it
_has_ to be zero, otherwise somebody has changed the bitmap under or
foot.
Or have I missed something?
Notice that at some point we should allow for concurrent dirty of the
bitmap, but we need to do yet more things.
Later, Juan.
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-26 11:39 ` Juan Quintela
@ 2012-10-28 8:35 ` Orit Wasserman
2012-10-30 10:15 ` Orit Wasserman
0 siblings, 1 reply; 58+ messages in thread
From: Orit Wasserman @ 2012-10-28 8:35 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
On 10/26/2012 01:39 PM, Juan Quintela wrote:
> Orit Wasserman <owasserm@redhat.com> wrote:
>> On 10/18/2012 09:30 AM, Juan Quintela wrote:
>>> Instead of testing each page individually, we search what is the next
>>> dirty page with a bitmap operation. We have to reorganize the code to
>>> move from a "for" loop, to a while(dirty) loop.
>>>
>
>>>
>>> - do {
>>> + while(true) {
>>> mr = block->mr;
>>> - if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
>>> + offset = migration_bitmap_find_and_reset_dirty(mr, offset);
>>> + if (complete_round && block == last_seen_block &&
>>> + offset >= last_offset) {
>>> + break;
>>> + }
>> Juan,
>> You need to exchange those line, first check to see you did a full round than
>> calculate and reset the offset, in the way it is written now you may
>> reset a bit and than break of the loop
>> without sending it.
>
> How?
>
> if complete_round == true, it means that we are in the second round.
>
> block == last_seen_block means that we are back at the 1st block that we
> have looked.
>
> if offset >= last_offset, there are two options:
> a- == last_offset: that was the 1st one that we checked, so it can't be
> true.
> b- >= last_offset: it means tat we have already passed that bit, it
> _has_ to be zero, otherwise somebody has changed the bitmap under or
> foot.
>
> Or have I missed something?
If we are in iterate state this means the bitmap is changing all the time,
even when we didn't finish a complete cycle (for example we get to the bandwidth limit, exit ram_save_iterate and sync the bitmap in pending).
This means that bits in part of the bitmap we already walked can get dirty again.
In that case the if condition can be true for a dirty bit,
in the current code we reset it and than break for the loop, this means it is set clean without sending it, which is a problem.
Changing the line order will fix it (the way it was before).
Regards,
Orit
> Notice that at some point we should allow for concurrent dirty of the
> bitmap, but we need to do yet more things.
>
> Later, Juan.
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-28 8:35 ` Orit Wasserman
@ 2012-10-30 10:15 ` Orit Wasserman
2012-10-30 15:33 ` Juan Quintela
0 siblings, 1 reply; 58+ messages in thread
From: Orit Wasserman @ 2012-10-30 10:15 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
On 10/28/2012 10:35 AM, Orit Wasserman wrote:
> On 10/26/2012 01:39 PM, Juan Quintela wrote:
>> Orit Wasserman <owasserm@redhat.com> wrote:
>>> On 10/18/2012 09:30 AM, Juan Quintela wrote:
>>>> Instead of testing each page individually, we search what is the next
>>>> dirty page with a bitmap operation. We have to reorganize the code to
>>>> move from a "for" loop, to a while(dirty) loop.
>>>>
>>
>>>>
>>>> - do {
>>>> + while(true) {
>>>> mr = block->mr;
>>>> - if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
>>>> + offset = migration_bitmap_find_and_reset_dirty(mr, offset);
>>>> + if (complete_round && block == last_seen_block &&
>>>> + offset >= last_offset) {
>>>> + break;
>>>> + }
>>> Juan,
>>> You need to exchange those line, first check to see you did a full round than
>>> calculate and reset the offset, in the way it is written now you may
>>> reset a bit and than break of the loop
>>> without sending it.
>>
>> How?
>>
>> if complete_round == true, it means that we are in the second round.
>>
>> block == last_seen_block means that we are back at the 1st block that we
>> have looked.
>>
>> if offset >= last_offset, there are two options:
>> a- == last_offset: that was the 1st one that we checked, so it can't be
>> true.
>> b- >= last_offset: it means tat we have already passed that bit, it
>> _has_ to be zero, otherwise somebody has changed the bitmap under or
>> foot.
>>
>> Or have I missed something?
> If we are in iterate state this means the bitmap is changing all the time,
> even when we didn't finish a complete cycle (for example we get to the bandwidth limit, exit ram_save_iterate and sync the bitmap in pending).
> This means that bits in part of the bitmap we already walked can get dirty again.
> In that case the if condition can be true for a dirty bit,
> in the current code we reset it and than break for the loop, this means it is set clean without sending it, which is a problem.
> Changing the line order will fix it (the way it was before).
>
OK read the code more thoroughly, there is no way that the bitmap will if we do a full cycle in ram_save_block,
We can actually simplify and the function by checking the number of dirty pages. If it is zero we can return
immediately.
If it is not zero it means we will exit the loop when we will find the dirty page.
Cheers,
Orit
> Regards,
> Orit
>
>> Notice that at some point we should allow for concurrent dirty of the
>> bitmap, but we need to do yet more things.
>>
>> Later, Juan.
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking
2012-10-30 10:15 ` Orit Wasserman
@ 2012-10-30 15:33 ` Juan Quintela
0 siblings, 0 replies; 58+ messages in thread
From: Juan Quintela @ 2012-10-30 15:33 UTC (permalink / raw)
To: Orit Wasserman; +Cc: qemu-devel
Orit Wasserman <owasserm@redhat.com> wrote:
>> If we are in iterate state this means the bitmap is changing all the time,
>> even when we didn't finish a complete cycle (for example we get to
>> the bandwidth limit, exit ram_save_iterate and sync the bitmap in
>> pending).
>> This means that bits in part of the bitmap we already walked can get
>> dirty again.
>> In that case the if condition can be true for a dirty bit,
>> in the current code we reset it and than break for the loop, this
>> means it is set clean without sending it, which is a problem.
>> Changing the line order will fix it (the way it was before).
>>
> OK read the code more thoroughly, there is no way that the bitmap will
> if we do a full cycle in ram_save_block,
> We can actually simplify and the function by checking the number of
> dirty pages. If it is zero we can return
> immediately.
> If it is not zero it means we will exit the loop when we will find the
> dirty page.
Very good idea. Thanks.
Later, Juan.
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (29 preceding siblings ...)
2012-10-18 7:30 ` [Qemu-devel] [PATCH 30/30] ram: optimize migration bitmap walking Juan Quintela
@ 2012-10-18 9:00 ` Paolo Bonzini
2012-10-26 13:04 ` Paolo Bonzini
31 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-18 9:00 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
Il 18/10/2012 09:29, Juan Quintela ha scritto:
> v3:
>
> This is work in progress on top of the previous migration series just sent.
>
> - Introduces a thread for migration instead of using a timer and callback
> - remove the writting to the fd from the iothread lock
> - make the writes synchronous
> - Introduce a new pending method that returns how many bytes are pending for
> one save live section
> - last patch just shows printfs to see where the time is being spent
> on the migration complete phase.
> (yes it pollutes all uses of stop on the monitor)
>
> So far I have found that we spent a lot of time on bdrv_flush_all() It
> can take from 1ms to 600ms (yes, it is not a typo). That dwarfs the
> migration default downtime time (30ms).
>
> Stop all vcpus:
>
> - it works now (after the changes on qemu_cpu_is_vcpu on the previous
> series) caveat is that the time that brdv_flush_all() takes is
> "unpredictable". Any silver bullets?
You could reuse the "block" live migration item. In block_save_pending,
start a bdrv_aio_flush() on all block devices that have already
completed the previous one.
But that's not a regression in the migration thread, isn't it?
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition
2012-10-18 7:29 [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Juan Quintela
` (30 preceding siblings ...)
2012-10-18 9:00 ` [Qemu-devel] [PATCH 00/30] Migration thread 20121017 edition Paolo Bonzini
@ 2012-10-26 13:04 ` Paolo Bonzini
31 siblings, 0 replies; 58+ messages in thread
From: Paolo Bonzini @ 2012-10-26 13:04 UTC (permalink / raw)
To: Juan Quintela; +Cc: Anthony Liguori, qemu-devel
Il 18/10/2012 09:29, Juan Quintela ha scritto:
> Hi
>
> This series apply on top of the refactoring that I sent yesterday.
> Changes from the last version include:
>
> - buffered_file.c is gone, its functionality is merged in migration.c
> special attention to the megre of buffered_file_thread() &
> migration_file_put_notify().
>
> - Some more bitmap handling optimizations (thanks to Orit & Paolo for
> suggestions and code and Vinod for testing)
>
> Please review. Included is the pointer to the full tree.
Anthony, I think patches 1-13 are ready to go (but only if Juan reviews
my incoming-migration-in-a-coroutine first ;)).
Paolo
^ permalink raw reply [flat|nested] 58+ messages in thread