Re: QMP (without OOB) function running in thread different from the main thread as part of aio_poll

[Juan Quintela <quintela@redhat.com>]

Kevin Wolf <kwolf@redhat.com> wrote:
> Am 28.04.2023 um 10:38 hat Juan Quintela geschrieben:
>> Kevin Wolf <kwolf@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