All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Vivek Goyal <vgoyal@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH v1] pc: Support coldplugging of virtio-pmem-pci devices on all buses
Date: Tue, 9 Jun 2020 11:47:30 -0400	[thread overview]
Message-ID: <20200609114608-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20200525084511.51379-1-david@redhat.com>

On Mon, May 25, 2020 at 10:45:11AM +0200, David Hildenbrand wrote:
> E.g., with "pc-q35-4.2", trying to coldplug a virtio-pmem-pci devices
> results in
>     "virtio-pmem-pci not supported on this bus"
> 
> Reasons is, that the bus does not support hotplug and, therefore, does
> not have a hotplug handler. Let's allow coldplugging virtio-pmem devices
> on such buses. The hotplug order is only relevant for virtio-pmem-pci
> when the guest is already alive and the device is visible before
> memory_device_plug() wired up the memory device bits.
> 
> Hotplug attempts will still fail with:
>     "Error: Bus 'pcie.0' does not support hotplugging"
> 
> Hotunplug attempts will still fail with:
>     "Error: Bus 'pcie.0' does not support hotplugging"
> 
> Reported-by: Vivek Goyal <vgoyal@redhat.com>
> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

I assume you are still debugging Vivek's issues, right?
Let me know when you feel it's time to merge this ...

> ---
>  hw/i386/pc.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2128f3d6fe..c740495eb6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1663,13 +1663,13 @@ static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
>      HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>      Error *local_err = NULL;
>  
> -    if (!hotplug_dev2) {
> +    if (!hotplug_dev2 && dev->hotplugged) {
>          /*
>           * Without a bus hotplug handler, we cannot control the plug/unplug
> -         * order. This should never be the case on x86, however better add
> -         * a safety net.
> +         * order. We should never reach this point when hotplugging on x86,
> +         * however, better add a safety net.
>           */
> -        error_setg(errp, "virtio-pmem-pci not supported on this bus.");
> +        error_setg(errp, "virtio-pmem-pci hotplug not supported on this bus.");
>          return;
>      }
>      /*
> @@ -1678,7 +1678,7 @@ static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
>       */
>      memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
>                             &local_err);
> -    if (!local_err) {
> +    if (!local_err && hotplug_dev2) {
>          hotplug_handler_pre_plug(hotplug_dev2, dev, &local_err);
>      }
>      error_propagate(errp, local_err);
> @@ -1696,9 +1696,11 @@ static void pc_virtio_pmem_pci_plug(HotplugHandler *hotplug_dev,
>       * device bits.
>       */
>      memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
> -    hotplug_handler_plug(hotplug_dev2, dev, &local_err);
> -    if (local_err) {
> -        memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
> +    if (hotplug_dev2) {
> +        hotplug_handler_plug(hotplug_dev2, dev, &local_err);
> +        if (local_err) {
> +            memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
> +        }
>      }
>      error_propagate(errp, local_err);
>  }
> -- 
> 2.25.4



  parent reply	other threads:[~2020-06-09 15:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25  8:45 [PATCH v1] pc: Support coldplugging of virtio-pmem-pci devices on all buses David Hildenbrand
2020-05-25 10:06 ` Pankaj Gupta
2020-05-26 13:28 ` Vivek Goyal
2020-05-26 13:44   ` David Hildenbrand
2020-05-26 14:22     ` Vivek Goyal
2020-05-26 14:43       ` David Hildenbrand
2020-05-26 14:56         ` Daniel P. Berrangé
2020-05-27 19:51         ` Pankaj Gupta
2020-06-09 15:47 ` Michael S. Tsirkin [this message]
2020-06-09 15:51   ` David Hildenbrand

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=20200609114608-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=david@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pankaj.gupta.linux@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=vgoyal@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.