Kevin Wolf <kw...@redhat.com> wrote: > Am 28.04.2023 um 10:38 hat Juan Quintela geschrieben: >> Kevin Wolf <kw...@redhat.com> wrote: >> >> I am perhaps a bit ingenuous here, but it is there a way to convince >> >> qemu that snapshot_save_job_bh *HAS* to run on the main thread? >> > >> > I believe we're talking about a technicality here. I asked another more >> > fundamental question that nobody has answered yet: >> > >> > Why do you think that it's ok to call bdrv_writev_vmstate() without >> > holding the BQL? >> >> I will say this function starts by bdrv_ (i.e. block layer people) and >> endes with _vmstate (i.e. migration people). >> >> To be honest, I don't know. That is why I _supposed_ you have an idea. > > My idea is that bdrv_*() can only be called when you hold the BQL, or > for BlockDriverStates in an iothread the AioContext lock. > > Apparently dropping the BQL in migration code was introduced in Paolo's > commit 9b095037527.
Damn. I reviewed it, so I am as guilty as the author. 10 years later without problems I will not blame that patch. I guess we changed something else that broke doing it without the lock. But no, I still don't have suggestions/ideas. > I'm not sure what this was supposed to improve in > the case of snapshots because the VM is stopped anyway. > > Would anything bad happen if we removed the BQL unlock/lock section in > qemu_savevm_state() again? Dunno. For what is worth, I can say that it survives migration-test, but don't ask me why/how/... Fiona, can you check if it fixes your troubles? Later, Juan. > Kevin