Hi, Manish, On Mon, May 16, 2022 at 07:01:35PM +0530, manish.mishra wrote: > > On 26/04/22 5:08 am, Peter Xu wrote: > LGTM, > Peter, I wanted to give review-tag for this and ealier patch too. I am new > to qemu > review process so not sure how give review-tag, did not find any reference > on > google too. So if you please let me know how to do it.
It's here: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/submitting-a-patch.rst;hb=HEAD#l492 Since afaict QEMU is mostly following what Linux does, you can also reference to this one with more context: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes But since you're still having question regarding this patch, no rush on providing your R-bs; let's finish the discussion first. [...] > > +static void postcopy_pause_ram_fast_load(MigrationIncomingState *mis) > > +{ > > + trace_postcopy_pause_fast_load(); > > + qemu_mutex_unlock(&mis->postcopy_prio_thread_mutex); > > I may have misunderstood synchronisation here but very very rare chance, > > as both threads are working independently is it possible qemu_sem_post on > > postcopy_pause_sem_fast_load is called by main thread even before we go > > to qemu_sem_wait in next line, causing some kind of deadlock. That's should > > not be possible as i understand it requires manually calling > qmp_migration_recover > > so chances are almost impossible. Just wanted to confirm it. Sorry I don't quite get the question, could you elaborate? E.g., when the described deadlock happened, what is both main thread and preempt load thread doing? What are they waiting at? Note: we have already released mutex before waiting on sem. > > > + qemu_sem_wait(&mis->postcopy_pause_sem_fast_load); > > Just wanted to confirm why postcopy_pause_incoming is not called here > itself. postcopy_pause_incoming() is only used in the main ram load thread, while this function (postcopy_pause_ram_fast_load) is only called by the preempt load thread. > > Is it based on assumption that if there is error in any of the channel it > will > > eventually be paused on source side, closing both channels, resulting > > postcopy_pause_incoming will be called from main thread on destination? Yes. > > Usually it should be good to call as early as possible. It is left to main > > thread default path so that we do not have any synchronisation overhead? What's the sync overhead you mentioned? What we want to do here is simply to put all the dest QEMU migration threads into a halted state rather than quitting, so that they can be continued when necessary. > > Also Peter, i was trying to understand postcopy recovery model so is use > case > > of qmp_migrate_pause just for debugging purpose? Yes. It's also a way to cleanly stop using the network (comparing to force unplug the nic ports?) for whatever reason with a shutdown() syscall upon the socket. I just don't know whether there's any real use case of that in reality. Thanks, -- Peter Xu
