All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Maxim Levitsky <mlevitsk@redhat.com>
Subject: Re: [PATCH v6 04/10] device_core: use drain_call_rcu in in qmp_device_add
Date: Fri, 2 Oct 2020 23:22:52 +0200	[thread overview]
Message-ID: <0f261cf8-63d8-d0dd-b717-1b43b01e6801@redhat.com> (raw)
In-Reply-To: <20201002173558.232960-5-pbonzini@redhat.com>

On 02/10/20 19:35, Paolo Bonzini wrote:
> From: Maxim Levitsky <mlevitsk@redhat.com>
> 
> Soon, a device removal might only happen on RCU callback execution.
> This is okay for device-del which provides a DEVICE_DELETED event,
> but not for the failure case of device-add.  To avoid changing
> monitor semantics, just drain all pending RCU callbacks on error.

Nope, this is not enough...  qemu-iotests test 067 still breaks; if I
leave the drain_call_rcu() in qmp_device_del, on the other hand, the
hotplug test in qtest-ppc64:qos-test hangs because DEVICE_DELETED comes
before the {'return'}.

One way to handle this is:

1) handle events properly in libqtest.  For example, rename
qtest_qmp_receive to qtest_qmp_receive_dict (most callers can keep on
calling qtest_qmp_receive, though some have to switch if they look for
events); a new qtest_qmp_receive that calls qtest_qmp_receive_dict until
it doesn't get an event, and in the meanwhile stashes the events in a
GList.  Then qtest_qmp_eventwait_ref can look for events in the GList
prior to calling qtest_qmp_receive_any if it doesn't find any.

2) with (1) in place we can probably keep the drain_call_rcu(), but
perhaps we can drop it if we remove test 067 and add equivalent tests to
tests/qtest/drive_del.c.

The reason I'm not fond of the drain_call_rcu() in device_del is that
there is already a DEVICE_DELETED callback, and device-del is (at least
for some buses) an advisory operation that the guest needs to attend to.
 So there's no guarantee that you get the DEVICE_DELETED event before
drain_call_rcu().

I can look at this next week.

Paolo


  reply	other threads:[~2020-10-02 21:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 17:35 [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 01/10] qdev: add "check if address free" callback for buses Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 02/10] scsi: switch to bus->check_address Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 03/10] scsi/scsi_bus: switch search direction in scsi_device_find Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 04/10] device_core: use drain_call_rcu in in qmp_device_add Paolo Bonzini
2020-10-02 21:22   ` Paolo Bonzini [this message]
2020-10-02 17:35 ` [PATCH v6 05/10] device-core: use RCU for list of children of a bus Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 06/10] device-core: use atomic_set on .realized property Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 07/10] scsi/scsi-bus: scsi_device_find: don't return unrealized devices Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 08/10] scsi/scsi_bus: Add scsi_device_get Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 09/10] virtio-scsi: use scsi_device_get Paolo Bonzini
2020-10-02 17:35 ` [PATCH v6 10/10] scsi/scsi_bus: fix races in REPORT LUNS Paolo Bonzini
2020-10-03  9:31 ` [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0f261cf8-63d8-d0dd-b717-1b43b01e6801@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.