From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45239) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF4tm-0000Jh-Pn for qemu-devel@nongnu.org; Fri, 21 Sep 2012 11:12:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TF4tg-0007TA-5n for qemu-devel@nongnu.org; Fri, 21 Sep 2012 11:12:10 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:33440) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF4tf-0007Rs-VB for qemu-devel@nongnu.org; Fri, 21 Sep 2012 11:12:04 -0400 Received: by pbbrp12 with SMTP id rp12so7892153pbb.4 for ; Fri, 21 Sep 2012 08:12:02 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <505C83BA.1090100@redhat.com> Date: Fri, 21 Sep 2012 17:11:54 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1348236500-2565-1-git-send-email-quintela@redhat.com> <1348236500-2565-14-git-send-email-quintela@redhat.com> In-Reply-To: <1348236500-2565-14-git-send-email-quintela@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 13/14] savevm: New save live migration method: pending List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org Il 21/09/2012 16:08, Juan Quintela ha scritto: > 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. Technically, the one in qemu_savevm_iterate may not translate into actual downtime if the guest does not need I/O, as it is "just" stolen time. But this is just a nit in the commit message. Very good catch! Another nit: > > +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; > +} Here you may want to pass max_size - ret for the last argument of se->ops->save_live_pending. But in practice this loop will be executed exactly once, so: Reviewed-by: Paolo Bonzini Paolo > 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).