[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread |
Date: |
Thu, 18 Oct 2012 10:39:20 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121009 Thunderbird/16.0 |
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> This will allow us finer control in next patches.
>
> Signed-off-by: Juan Quintela <address@hidden>
> ---
> 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:
>
- [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void, (continued)
- [Qemu-devel] [PATCH 05/30] migration: make qemu_fopen_ops_buffered() return void, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 17/30] migration: move migration_fd_put_ready(), Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 19/30] migration: move migration notifier, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 15/30] migration-fd: remove duplicate include, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 23/30] migration: print times for end phase, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 16/30] migration: move buffered_file.c code into migration.c, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 24/30] ram: rename last_block to last_seen_block, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 22/30] migration: unfold rest of migrate_fd_put_ready() into thread, Juan Quintela, 2012/10/18
- [Qemu-devel] [PATCH 21/30] migration: move exit condition to migration thread, Juan Quintela, 2012/10/18
[Qemu-devel] [PATCH 20/30] migration: move begining stage to the migration thread, Juan Quintela, 2012/10/18
[Qemu-devel] [PATCH 29/30] migration: Only go to the iterate stage if there is anything to send, Juan Quintela, 2012/10/18
[Qemu-devel] [PATCH 27/30] ram: Use memory_region_test_and_clear_dirty, Juan Quintela, 2012/10/18
[Qemu-devel] [PATCH 25/30] ram: Add last_sent_block, Juan Quintela, 2012/10/18
[Qemu-devel] [PATCH 26/30] memory: introduce memory_region_test_and_clear_dirty, Juan Quintela, 2012/10/18