All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: peter.maydell@linaro.org, ehabkost@redhat.com, mst@redhat.com,
	marcel.a@redhat.com, qemu-devel@nongnu.org, blauwirbel@gmail.com,
	alex.williamson@redhat.com, anthony@codemonkey.ws,
	afaerber@suse.de
Subject: Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface.
Date: Mon, 09 Dec 2013 18:18:42 +0100	[thread overview]
Message-ID: <52A5FB72.8080307@redhat.com> (raw)
In-Reply-To: <20131209174856.5609760d@nial.usersys.redhat.com>

Il 09/12/2013 17:48, Igor Mammedov ha scritto:
>> > 
>> > We could have separate check/plug methods.  Only check can fail, it must
>> > be idempotent, and it can be invoked while the device is still unrealized.
> Reasons I've stated before apply to 'check' as well, for only specific device knows
> when it's fine to call it. That would just replace "plug" with "check" in realize()
> for not much of benefit.

Check is idempotent, and can be called before realize makes any change
(it could also be called after the device is added to
/machine/unattached, it's not a big difference).

Plug is called after realize.

>> > 
>> > The reason I liked the interface, is because it removes the need for
>> > each bus to add its own plug/unplug handling.
>        ^^^
> Have you meant device instead of bus?

I meant each bus-specific abstract class (PCIDevice, SCSIDevice, etc.).

> It's still improvement over current PCI hotplug code and allows to simplify
> other buses as well by removing callbacks from them.

Exactly.  But so far you don't get any benefit: no removal of PCI
hotplug code, no removal of allow_hotunplug.  What I'm proposing, I
believe, has a small cost and already starts the transition (which I
believe we can complete for 2.0).

> The only way to call callbacks from DEVICE.realize()/unplug(), I see, is if we make
> them all nofail, then it would be safe to call them in "realize" property setter.
> But we have at least 2 callbacks that can fail:
>  pcie_cap_slot_hotplug() and shpc_device_hotplug()

Both of them can be handled by a "check" step in the handler.

> Goal of this series was to add and demonstrate reusable hotplug interface as
> opposed to PCI specific or SCSI-bus specific ones, so it could be used for memory
> hotplug as well. It might not do everything we would like but it looks like a move
> the right direction.
> If it's wrong direction, I could drop the idea and fallback to an original
> less intrusive approach, taking in account your comment to move type definitions
> into separate header.

No, absolutely.  I think it's the right direction, I just think more
aspects of it should be made generic.

Paolo

  reply	other threads:[~2013-12-09 17:19 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 17:03 [Qemu-devel] [PATCH 0/7] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 1/7] define hotplug interface Igor Mammedov
2013-12-06 17:26   ` Paolo Bonzini
2013-12-09 12:42     ` Igor Mammedov
2013-12-09 13:15       ` Paolo Bonzini
2013-12-09  5:40   ` Li Guang
2013-12-09  9:11     ` Marcel Apfelbaum
2013-12-09 12:44     ` Igor Mammedov
2013-12-09  8:58   ` Marcel Apfelbaum
2013-12-09 12:52     ` Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 2/7] qdev: add to BusState "hotplug-device" link Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 3/7] hw/acpi: move typeinfo to the file end Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 4/7] acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-device interface Igor Mammedov
2013-12-09  9:02   ` Marcel Apfelbaum
2013-12-09 13:24     ` Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 5/7] pci/shpc: convert SHPC " Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 6/7] pci/pcie: convert PCIE " Igor Mammedov
2013-12-06 17:03 ` [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface Igor Mammedov
2013-12-06 17:29   ` Paolo Bonzini
2013-12-09 13:41     ` Igor Mammedov
2013-12-09 13:47       ` Paolo Bonzini
2013-12-09 14:14         ` Igor Mammedov
2013-12-09 14:36           ` Paolo Bonzini
2013-12-09 15:08             ` Igor Mammedov
2013-12-09 15:16               ` Paolo Bonzini
2013-12-09 16:48                 ` Igor Mammedov
2013-12-09 17:18                   ` Paolo Bonzini [this message]
2013-12-09 21:15                     ` Igor Mammedov
2013-12-09 22:28                       ` Paolo Bonzini
2013-12-09  9:09   ` Marcel Apfelbaum
2013-12-09 13:55     ` Igor Mammedov
2013-12-09 14:01       ` Marcel Apfelbaum
2013-12-06 17:31 ` [Qemu-devel] [PATCH 0/7] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Paolo Bonzini
2013-12-09 14:15   ` Igor Mammedov

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=52A5FB72.8080307@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=alex.williamson@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=blauwirbel@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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.