On 08/11/2011 12:18 PM, Paolo Bonzini wrote:
Now, migrate_fd_cleanup, buffured_close is just executed by the migration thread. I am not letting iothread call any migration cancellation related functions. In stead it just submits the request and waits for the migration thread to terminate itself in the next iteration. The reason is to avoid the call to qemu_fflush, qemu_savevm_state_cancel (to carry out migrate_cancel) from iothread while migration thread is transferring data without holding the locks.@@ -175,20 +170,20 @@ static int buffered_close(void *opaque) while (!s->has_error&& s->buffer_size) { buffered_flush(s); - if (s->freeze_output) + if (s->freeze_output) { s->wait_for_unfreeze(s); + } }This is racy; you might end up calling buffered_put_buffer twice from two different threads.
s->fd is closed in migrate_fd_cleanup (which calls buffered_close). So I flush the buffer in buffered_close before closing the descriptor, and then migration thread simply exits because s->closed is set.- ret = s->close(s->opaque); + s->closed = 1; - qemu_del_timer(s->timer); - qemu_free_timer(s->timer); + ret = s->close(s->opaque); qemu_free(s->buffer); - qemu_free(s);... similarly, here the migration thread might end up using the buffer. Just set s->closed here and wait for thread completion; the migration thread can handle the flushes free the buffer etc. Let the migration thread do as much as possible, it will simplify your life.return ret; } + static int buffered_rate_limit(void *opaque) { QEMUFileBuffered *s = opaque;@@ -228,34 +223,55 @@ static int64_t buffered_get_rate_limit(void *opaque)return s->xfer_limit; } -static void buffered_rate_tick(void *opaque) +static void *migrate_vm(void *opaque) { QEMUFileBuffered *s = opaque;+ int64_t current_time, expire_time = qemu_get_clock_ms(rt_clock) + 100;+ struct timeval tv = { .tv_sec = 0, .tv_usec = 100000}; - if (s->has_error) { - buffered_close(s); - return; - } + qemu_mutex_lock_iothread(); - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100); + while (!s->closed) {... This can be in fact while (!s->closed || s->buffered_size) and that alone will subsume the loop in buffered_close, no?
- Umesh -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
