Re: [Qemu-devel] [PULL 7/7] vl: introduce vm_shutdown()

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 03/12/2018 08:05 PM, John Snow wrote:
> 
> 
> On 03/09/2018 08:19 AM, Stefan Hajnoczi wrote:
>> Commit 00d09fdbbae5f7864ce754913efc84c12fdf9f1a ("vl: pause vcpus before
>> stopping iothreads") and commit dce8921b2baaf95974af8176406881872067adfa
>> ("iothread: Stop threads before main() quits") tried to work around the
>> fact that emulation was still active during termination by stopping
>> iothreads.  They suffer from race conditions:
>> 1. virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the
>>    virtio_scsi_ctx_check() assertion failure because the BDS AioContext
>>    has been modified by iothread_stop_all().
>> 2. Guest vq kick racing with main loop termination leaves a readable
>>    ioeventfd that is handled by the next aio_poll() when external
>>    clients are enabled again, resulting in unwanted emulation activity.
>>
>> This patch obsoletes those commits by fully disabling emulation activity
>> when vcpus are stopped.
>>
>> Use the new vm_shutdown() function instead of pause_all_vcpus() so that
>> vm change state handlers are invoked too.  Virtio devices will now stop
>> their ioeventfds, preventing further emulation activity after vm_stop().
>>
>> Note that vm_stop(RUN_STATE_SHUTDOWN) cannot be used because it emits a
>> QMP STOP event that may affect existing clients.
>>
>> It is no longer necessary to call replay_disable_events() directly since
>> vm_shutdown() does so already.
>>
>> Drop iothread_stop_all() since it is no longer used.
>>
>> Cc: Fam Zheng <famz@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>> Message-id: 20180307144205.20619-5-stefanha@redhat.com
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Stefan, I see the same iotest regression that was reported by John Snow also
on s390. I can confirm that it works with this patch reverted.

Christian