On 07/29/2011 10:57 PM, Umesh Deshpande wrote:
+ qemu_mutex_unlock_iothread();while (s->state == MIG_STATE_ACTIVE) { if (migrate_fd_check_expire()) { + qemu_mutex_lock_iothread(); buffered_rate_tick(s->file); + qemu_mutex_unlock_iothread(); } if (s->state != MIG_STATE_ACTIVE) { @@ -392,12 +396,11 @@ void migrate_fd_begin(void *arg) if (s->callback) { migrate_fd_wait_for_unfreeze(s); + qemu_mutex_lock_iothread(); s->callback(s); + qemu_mutex_unlock_iothread(); } } - -out: - qemu_mutex_unlock_iothread();
I think it's clearer to unlock explicitly around the waiting points (see review of 1/3). In fact, I think you're working around the busy wait by accessing s->state outside the lock, right? I don't think this is provably safe; moving the knowledge of the thread entirely within buffered_file.c also fixes this, because then the lifetimes of the thread and the QEMUFile are much clearer.
Thanks, Paolo
