[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: qom device lifecycle interaction with hotplug/hotunplug ?

From: Damien Hedde
Subject: Re: qom device lifecycle interaction with hotplug/hotunplug ?
Date: Wed, 11 Dec 2019 13:52:33 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0

On 12/4/19 7:51 PM, Eduardo Habkost wrote:
> On Wed, Dec 04, 2019 at 05:21:25PM +0100, Jens Freimann wrote:
>> On Wed, Dec 04, 2019 at 11:35:37AM -0300, Eduardo Habkost wrote:
>>> On Wed, Dec 04, 2019 at 10:18:24AM +0100, Jens Freimann wrote:
>>>> On Tue, Dec 03, 2019 at 06:40:04PM -0300, Eduardo Habkost wrote:
>>>>> +jfreimann, +mst
>>>>> On Sat, Nov 30, 2019 at 11:10:19AM +0000, Peter Maydell wrote:
>>>>>> On Fri, 29 Nov 2019 at 20:05, Eduardo Habkost <address@hidden> wrote:
>>>>>>> So, to summarize the current issues:
>>>>>>> 1) realize triggers a plug operation implicitly.
>>>>>>> 2) unplug triggers unrealize implicitly.
>>>>>>> Do you expect to see use cases that will require us to implement
>>>>>>> realize-without-plug?
>>>>>> I don't think so, but only because of the oddity that
>>>>>> we put lots of devices on the 'sysbus' and claim that
>>>>>> that's plugging them into the bus. The common case of
>>>>>> 'realize' is where one device (say an SoC) has a bunch of child
>>>>>> devices (like UARTs); the SoC's realize method realizes its child
>>>>>> devices. Those devices all end up plugged into the 'sysbus'
>>>>>> but there's no actual bus there, it's fictional and about
>>>>>> the only thing it matters for is reset propagation (which
>>>>>> we don't model right either). A few devices don't live on
>>>>>> buses at all.
>>>>> That's my impression as well.
>>>>>>> Similarly, do you expect use cases that will require us to
>>>>>>> implement unplug-without-unrealize?
>>>>>> I don't know enough about hotplug to answer this one:
>>>>>> it's essentially what I'm hoping you'd be able to answer.
>>>>>> I vaguely had in mind that eg the user might be able to
>>>>>> create a 'disk' object, plug it into a SCSI bus, then
>>>>>> unplug it from the bus without the disk and all its data
>>>>>> evaporating, and maybe plug it back into the SCSI
>>>>>> bus (or some other SCSI bus) later ? But I don't know
>>>>>> anything about how we expose that kind of thing to the
>>>>>> user via QMP/HMP.
>>>>> This ability isn't exposed to the user at all.  Our existing
>>>>> interfaces are -device, device_add and device_del.
>>>>> We do have something new that sounds suspiciously similar to
>>>>> "unplugged but not unrealized", though: the new hidden device
>>>>> API, added by commit f3a850565693 ("qdev/qbus: add hidden device
>>>>> support").
>>>>> Jens, Michael, what exactly is the difference between a "hidden"
>>>>> device and a "unplugged" device?
>>>> "hidden" the way we use it for virtio-net failover is actually unplugged. 
>>>> But it
>>>> doesn't have to be that way. You can register a function that decides
>>>> if the device should be hidden, i.e. plugged now, or do something else
>>>> with it (in the virtio-net failover case we just save everything we
>>>> need to plug the device later).
>>>> We did introduce a "unplugged but not unrealized" function too as part
>>>> of the failover feature. See "a99c4da9fc pci: mark devices partially
>>>> unplugged"
>>>> This was needed so we would be able to re-plug the device in case a
>>>> migration failed and we need to hotplug the primary device back to the
>>>> guest. To avoid the risk of not getting the resources the device needs
>>>> we don't unrealize but just trigger the unplug from the guest OS.
>>> Thanks for the explanation.  Let me confirm if I understand the
>>> purpose of the new mechanisms: should_be_hidden is a mechanism
>>> for implementing realize-without-plug.  partially_hotplugged is a
>>> mechanism for implementing unplug-without-unrealize.  Is that
>>> correct?
>> partially_hotplugged is a mechanism for implementing
>> unplug-without-unrealize: yes.

I'm currently trying to understand if my multi-phase reset series has
some interference with this new mechanism and I have some question.

IIUC when migration starts. the vfio-pci device is partially unplugged
using failover_unplug_primary():
> static bool failover_unplug_primary(VirtIONet *n)
> {
>     [...]
>         pci_dev->partially_hotplugged = true;
>         hotplug_handler_unplug_request(hotplug_ctrl, n->primary_dev,
>     [...]
> }

And if migration fails this same device is plugged back using
> static bool failover_replug_primary(VirtIONet *n, Error **errp)
> {
>     [...]
>     qdev_set_parent_bus(n->primary_dev, n->primary_bus);
>     [...]
>     hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
>     if (hotplug_ctrl) {
>         hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, &err);
>         if (err) {
>             goto out;
>         }
>         hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
>     }
>     [...]
> }

My concern is about the qdev_set_parent_bus() call above (because I
touch this function in my series) and don't want to have side effects there.

I looked at the code and thought the partial unplug has the effect of
cutting off the unplug procedure just before doing the qdev real unplug.
In pcie_unplug_device() we return before doing the object_unparent():
> static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, ...
>  {
>      [...]
>      if (dev->partially_hotplugged) {
>          dev->qdev.pending_deleted_event = false;
>          return;
>      }
>      hotplug_handler_unplug(hotplug_ctrl, DEVICE(dev), &error_abort);
>      object_unparent(OBJECT(dev));
>  }

>From my understanding, object_unparent() is the only way to really
unplug a device from a bus regarding qdev (and it also unrealizes the
device). So I have the feeling that qdev_set_parent_bus() here is a
no-op (because primary_dev is already on primary_bus).

Am I wrong ? Is there any other cases I missed where the primary_dev is
not already on the primary_bus when failover_replug_primary() is called ?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]