qom device lifecycle interaction with hotplug/hotunplug ?

qom device lifecycle interaction with hotplug/hotunplug ?

[Peter Maydell]

Hi; this is a question which came up in Damien's reset series
which I don't know the answer to:

What is the interaction of the QOM device lifecycle (instance_init/realize/
unrealize/instance_finalize) with hotplug and hot-unplug ? I couldn't
find any documentation of this but maybe I was looking in the wrong
place...

Looking at device_set_realized() it seems like we treat "realize"
as meaning "and also do the hot-plug if this is a device we're
trying to hotplug". On the other hand hot-unplug is I think the
other way around: when we get a hot-unplug event we assume that
it should also imply an "unrealize" (but just unrealizing doesn't
auto-hot-unplug) ?

Once a device is hot-unplugged (and thus unrealized) is it valid
for it to be re-hot-plugged, or is the assumption that it's then
destroyed and a fresh device is created if the user wants to plug
something in again later ? Put another way, is it valid for a qdev
device to see state transitions realize -> unrealize -> realize ?

thanks
-- PMM

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

On Thu, Nov 28, 2019 at 04:00:06PM +0000, Peter Maydell wrote:
> Hi; this is a question which came up in Damien's reset series
> which I don't know the answer to:
> 
> What is the interaction of the QOM device lifecycle (instance_init/realize/
> unrealize/instance_finalize) with hotplug and hot-unplug ? I couldn't
> find any documentation of this but maybe I was looking in the wrong
> place...
> 
> Looking at device_set_realized() it seems like we treat "realize"
> as meaning "and also do the hot-plug if this is a device we're
> trying to hotplug". On the other hand hot-unplug is I think the
> other way around: when we get a hot-unplug event we assume that
> it should also imply an "unrealize" (but just unrealizing doesn't
> auto-hot-unplug) ?

Your description seems accurate, and I agree it is confusing.

It would be more consistent if realized=true didn't plug the
device automatically, and qdev_device_add() asked the hotplug
handler to plug the device instead.

> 
> Once a device is hot-unplugged (and thus unrealized) is it valid
> for it to be re-hot-plugged, or is the assumption that it's then
> destroyed and a fresh device is created if the user wants to plug
> something in again later ? Put another way, is it valid for a qdev
> device to see state transitions realize -> unrealize -> realize ?

My interpretation is that this is valid in theory, but likely to
crash a large portion of our devices if we tried it.

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Igor Mammedov]

On Thu, 28 Nov 2019 16:00:06 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> Hi; this is a question which came up in Damien's reset series
> which I don't know the answer to:
> 
> What is the interaction of the QOM device lifecycle (instance_init/realize/
> unrealize/instance_finalize) with hotplug and hot-unplug ? I couldn't
> find any documentation of this but maybe I was looking in the wrong
> place...
> 
> Looking at device_set_realized() it seems like we treat "realize"
> as meaning "and also do the hot-plug if this is a device we're
> trying to hotplug". On the other hand hot-unplug is I think the
> other way around: when we get a hot-unplug event we assume that
> it should also imply an "unrealize" (but just unrealizing doesn't
> auto-hot-unplug) ?

Let me try to describe it.

device 'hotplug' interface is poorly named nowadays as it's
just 'plug' interface which checks/wires device into existing machine.
'hotplug' attribute is just informs plug controller that it might
wish to take additional actions to complete device initialization
and notify guest.

plug workflow is as follow:

  DeviceState::realize()
     hotplug_ctrl = qdev_get_hotplug_handler(dev);
     hotplug_handler_pre_plug(hotplug_ctrl, dev) // check / prepare / reserve resources, can fail
     // call concrete device realize_fn()
     hotplug_handler_plug(hotplug_ctrl, dev) // wire device up to board/bus/notify guest, shouldn't fail

 * now old bus based qdev hotplug are tied to _plug callback that
   controller (hotplug_ctrl) that owns bus sets during bus creation.
   (Ideally we should split that on _pre_plug and _plug parts)
 * also any other QOM object could be controller, to allow buss-less
   hotplug (ex: arm-virt machine or pc/q35 machines)

Unplug is a different beast, it could be originated from mgmt side
device_del() and/or from guest side.

device_del() can go 2 ways: qdev_unplug()
 * devices that support surprise removal (i.e. does not require guest cooperation)
   go directly to
       hotplug_handler_unplug() // tear down device from machine
       object_unparent(); -> unrealize() + finalize() // device gone
   essentially it's old qdev code behavior as is.
      
 * async removal a bit different, instead of removal it asks
   controller to process unplug request hotplug_handler_unplug_request()
   and does nothing else.
   After guest is prepared to device removal it notifies QEMU in some way
   to eject device. That calls the same sequence
     hotplug_handler_unplug()
     object_unparent()

> Once a device is hot-unplugged (and thus unrealized) is it valid
> for it to be re-hot-plugged, or is the assumption that it's then
> destroyed and a fresh device is created if the user wants to plug
> something in again later ? Put another way, is it valid for a qdev
> device to see state transitions realize -> unrealize -> realize ?

I don't think we do it currently (or maybe we do with failover but
I missed that train), but I don't see why it can't be done.

I theory it's upto the place where actual eject request is originated from,
it could do unrealize -> realize instead of unparent as far as it calls
hotplug_handler_unplug().

> 
> thanks
> -- PMM
> 

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Peter Maydell]

On Thu, 28 Nov 2019 at 17:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Thu, 28 Nov 2019 16:00:06 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> > Once a device is hot-unplugged (and thus unrealized) is it valid
> > for it to be re-hot-plugged, or is the assumption that it's then
> > destroyed and a fresh device is created if the user wants to plug
> > something in again later ? Put another way, is it valid for a qdev
> > device to see state transitions realize -> unrealize -> realize ?
>
> I don't think we do it currently (or maybe we do with failover but
> I missed that train), but I don't see why it can't be done.

Well, as Eduardo says, if we don't currently do it then
we probably have a lot of subtly buggy code. Requiring it to work
imposes a requirement on the 'unrealize' function that it
doesn't just do required cleanup/resource releasing actions,
but also returns the device back to exactly the state it was in
after instance_init, so that 'realize' will work correctly.
That's quite a lot of code auditing/effort if we don't actually
have a current or future use for making this work, rather than
just requiring that an unrealized device object is immediately
finalized without possibility of resurrection.

If we do have a plausible usecase then I think we should document
that unrealize needs to handle this, and also have a basic
smoke test of the realize->unrealize->realize.

thanks
-- PMM

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Igor Mammedov]

On Thu, 28 Nov 2019 17:57:56 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 28 Nov 2019 at 17:27, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Thu, 28 Nov 2019 16:00:06 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:  
> > > Once a device is hot-unplugged (and thus unrealized) is it valid
> > > for it to be re-hot-plugged, or is the assumption that it's then
> > > destroyed and a fresh device is created if the user wants to plug
> > > something in again later ? Put another way, is it valid for a qdev
> > > device to see state transitions realize -> unrealize -> realize ?  
> >
> > I don't think we do it currently (or maybe we do with failover but
> > I missed that train), but I don't see why it can't be done.  
> 
> Well, as Eduardo says, if we don't currently do it then
> we probably have a lot of subtly buggy code. Requiring it to work
> imposes a requirement on the 'unrealize' function that it
> doesn't just do required cleanup/resource releasing actions,
> but also returns the device back to exactly the state it was in
> after instance_init, so that 'realize' will work correctly.
> That's quite a lot of code auditing/effort if we don't actually
> have a current or future use for making this work, rather than
> just requiring that an unrealized device object is immediately
> finalized without possibility of resurrection.
> 
> If we do have a plausible usecase then I think we should document
> that unrealize needs to handle this, and also have a basic
> smoke test of the realize->unrealize->realize.
yep, if we talk about generic approach, it's a problem.
But if a concrete combo of device/hotplug controller is considered
where such flow were necessary, it should be possible.

But from the my very limited understanding, on real hardware,
once device is uplugged it's gone (finalized) from machine
perspective, so it's unclear to my why someone would use
realize->unrealize->realize hotplug scenario.

> 
> thanks
> -- PMM
> 

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Igor Mammedov]

On Thu, 28 Nov 2019 13:33:58 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Thu, Nov 28, 2019 at 04:00:06PM +0000, Peter Maydell wrote:
> > Hi; this is a question which came up in Damien's reset series
> > which I don't know the answer to:
> > 
> > What is the interaction of the QOM device lifecycle (instance_init/realize/
> > unrealize/instance_finalize) with hotplug and hot-unplug ? I couldn't
> > find any documentation of this but maybe I was looking in the wrong
> > place...
> > 
> > Looking at device_set_realized() it seems like we treat "realize"
> > as meaning "and also do the hot-plug if this is a device we're
> > trying to hotplug". On the other hand hot-unplug is I think the
> > other way around: when we get a hot-unplug event we assume that
> > it should also imply an "unrealize" (but just unrealizing doesn't
> > auto-hot-unplug) ?  
> 
> Your description seems accurate, and I agree it is confusing.
> 
> It would be more consistent if realized=true didn't plug the
> device automatically, and qdev_device_add() asked the hotplug
> handler to plug the device instead.
agreed, it's confusing. But that would not allow to
  o = object_new()
  set props
  o.realize()
reuse the same plug handlers.

we potentially can convert it to device_add input arguments
and then call qdev_device_add() instead, which would then
handle plug handlers, not sure it's doable though.
Other than that I don't have any ideas how to make it less confusing.

> > Once a device is hot-unplugged (and thus unrealized) is it valid
> > for it to be re-hot-plugged, or is the assumption that it's then
> > destroyed and a fresh device is created if the user wants to plug
> > something in again later ? Put another way, is it valid for a qdev
> > device to see state transitions realize -> unrealize -> realize ?  
> 
> My interpretation is that this is valid in theory, but likely to
> crash a large portion of our devices if we tried it.
> 

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Peter Maydell]

On Fri, 29 Nov 2019 at 12:26, Igor Mammedov <imammedo@redhat.com> wrote:
> But from the my very limited understanding, on real hardware,
> once device is uplugged it's gone (finalized) from machine
> perspective, so it's unclear to my why someone would use
> realize->unrealize->realize hotplug scenario.

Well, on real hardware 'unplug' is different from 'unrealize'.
So I think for QEMU if we wanted to allow this sort of 'unplug
and replug the same device' we should do it by:

 instance_init -> realize -> plug -> unplug -> plug -> unplug ->
   unrealize -> finalize

So unrealize/finalize is when the device is actually destroyed,
and if you're going to replug the device you don't destroy it
on unplug.

thanks
-- PMM

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Damien Hedde]

On 11/29/19 1:45 PM, Peter Maydell wrote:
> On Fri, 29 Nov 2019 at 12:26, Igor Mammedov <imammedo@redhat.com> wrote:
>> But from the my very limited understanding, on real hardware,
>> once device is uplugged it's gone (finalized) from machine
>> perspective, so it's unclear to my why someone would use
>> realize->unrealize->realize hotplug scenario.
> 
> Well, on real hardware 'unplug' is different from 'unrealize'.
> So I think for QEMU if we wanted to allow this sort of 'unplug
> and replug the same device' we should do it by:
> 
>  instance_init -> realize -> plug -> unplug -> plug -> unplug ->
>    unrealize -> finalize
> 
> So unrealize/finalize is when the device is actually destroyed,
> and if you're going to replug the device you don't destroy it
> on unplug.
> 

Hello everybody,

What I was initially wondering (or afraid of) when this question/problem
comes to me is;
Are there some cases where QEMU does the following (in the context of an
hotplugged device):

instance_init -> realize (and plug) -> unrealize -> change some
properties -> realize
with no unplug / plug in between

because I have the impression, the realize was here to allow setting
properties. But it may be pure nonsense as I do not know well the
underlying mechanisms there.

Regards,
--
Damien

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Igor Mammedov]

On Fri, 29 Nov 2019 14:05:23 +0100
Damien Hedde <damien.hedde@greensocs.com> wrote:

> On 11/29/19 1:45 PM, Peter Maydell wrote:
> > On Fri, 29 Nov 2019 at 12:26, Igor Mammedov <imammedo@redhat.com> wrote:  
> >> But from the my very limited understanding, on real hardware,
> >> once device is uplugged it's gone (finalized) from machine
> >> perspective, so it's unclear to my why someone would use
> >> realize->unrealize->realize hotplug scenario.  
> > 
> > Well, on real hardware 'unplug' is different from 'unrealize'.
> > So I think for QEMU if we wanted to allow this sort of 'unplug
> > and replug the same device' we should do it by:
> > 
> >  instance_init -> realize -> plug -> unplug -> plug -> unplug ->
> >    unrealize -> finalize
> > 
> > So unrealize/finalize is when the device is actually destroyed,
> > and if you're going to replug the device you don't destroy it
> > on unplug.
> >   
> 
> Hello everybody,
> 
> What I was initially wondering (or afraid of) when this question/problem
> comes to me is;
> Are there some cases where QEMU does the following (in the context of an
> hotplugged device):
> 
> instance_init -> realize (and plug) -> unrealize -> change some
> properties -> realize
> with no unplug / plug in between

not that I know of (at least not with device_add/del)

> because I have the impression, the realize was here to allow setting
> properties.

in theory it should be
 instance_init -> set properties -> realize
and ping pong realize <-> unrealize, should work as far the device
in question takes that in consideration.

In practice for generic arbitrary device it probably won't work
out of the box since people tended to put too much in realize
and didn't care about proper cleanup since device in question
typically is destroyed right after unrealize.

So it's probably implementable for some internal device
if it doesn't use device_add/del, otherwise millage might vary.

> But it may be pure nonsense as I do not know well the
> underlying mechanisms there.
> 
> Regards,
> --
> Damien
> 

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

On Fri, Nov 29, 2019 at 01:40:55PM +0100, Igor Mammedov wrote:
> On Thu, 28 Nov 2019 13:33:58 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Thu, Nov 28, 2019 at 04:00:06PM +0000, Peter Maydell wrote:
> > > Hi; this is a question which came up in Damien's reset series
> > > which I don't know the answer to:
> > > 
> > > What is the interaction of the QOM device lifecycle (instance_init/realize/
> > > unrealize/instance_finalize) with hotplug and hot-unplug ? I couldn't
> > > find any documentation of this but maybe I was looking in the wrong
> > > place...
> > > 
> > > Looking at device_set_realized() it seems like we treat "realize"
> > > as meaning "and also do the hot-plug if this is a device we're
> > > trying to hotplug". On the other hand hot-unplug is I think the
> > > other way around: when we get a hot-unplug event we assume that
> > > it should also imply an "unrealize" (but just unrealizing doesn't
> > > auto-hot-unplug) ?  
> > 
> > Your description seems accurate, and I agree it is confusing.
> > 
> > It would be more consistent if realized=true didn't plug the
> > device automatically, and qdev_device_add() asked the hotplug
> > handler to plug the device instead.
> agreed, it's confusing. But that would not allow to
>   o = object_new()
>   set props
>   o.realize()
> reuse the same plug handlers.
> 

I thought we had very few places that set realized=true directly,
so changing this behavior would be easy.

I was mistaken.  Grepping for 'set_bool.*"realized"' found more
than 300 matches.

> we potentially can convert it to device_add input arguments
> and then call qdev_device_add() instead, which would then
> handle plug handlers, not sure it's doable though.
> Other than that I don't have any ideas how to make it less confusing.

We could introduce a "plugged" property which implicitly calls
the hotplug handler, and run a global s/"realized"/"plugged"/
substitution in the whole tree.  Would it be worth the trouble,
though?

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

On Fri, Nov 29, 2019 at 12:45:09PM +0000, Peter Maydell wrote:
> On Fri, 29 Nov 2019 at 12:26, Igor Mammedov <imammedo@redhat.com> wrote:
> > But from the my very limited understanding, on real hardware,
> > once device is uplugged it's gone (finalized) from machine
> > perspective, so it's unclear to my why someone would use
> > realize->unrealize->realize hotplug scenario.
> 
> Well, on real hardware 'unplug' is different from 'unrealize'.
> So I think for QEMU if we wanted to allow this sort of 'unplug
> and replug the same device' we should do it by:
> 
>  instance_init -> realize -> plug -> unplug -> plug -> unplug ->
>    unrealize -> finalize
> 
> So unrealize/finalize is when the device is actually destroyed,
> and if you're going to replug the device you don't destroy it
> on unplug.

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?

Similarly, do you expect use cases that will require us to
implement unplug-without-unrealize?

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Peter Maydell]

On Fri, 29 Nov 2019 at 20:05, Eduardo Habkost <ehabkost@redhat.com> 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.

> 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.

thanks
-- PMM

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

+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 <ehabkost@redhat.com> 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?

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Jens Freimann]

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 <ehabkost@redhat.com> 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.

regards
Jens

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

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 <ehabkost@redhat.com> 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?

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Jens Freimann]

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 <ehabkost@redhat.com> 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?

should_be_hidden is a mechanism for implementing
realize-without-plug: kind of. It's a mechanism that ensures
qdev_device_add() returns early as long as the condition to hide the
device is true. You could to the realize-without-plug in the handler
function that decides if the device should be "hidden".  

partially_hotplugged is a mechanism for implementing
unplug-without-unrealize: yes. 

regards
Jens

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Eduardo Habkost]

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 <ehabkost@redhat.com> 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?
> 
> should_be_hidden is a mechanism for implementing
> realize-without-plug: kind of. It's a mechanism that ensures
> qdev_device_add() returns early as long as the condition to hide the
> device is true. You could to the realize-without-plug in the handler
> function that decides if the device should be "hidden".

Oh, right.  I thought "qdev_device_add() returns early" meant
"return after realize, before plug".  Now I see it returns before
object_new().  This means we have another user-visible device
state: "defined (in QemuOpts), but not created".

> 
> partially_hotplugged is a mechanism for implementing
> unplug-without-unrealize: yes.

Thanks!

-- 
Eduardo

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Damien Hedde]

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 <ehabkost@redhat.com> 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
failover_replug_primary():
> 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 ?

Thanks,
--
Damien

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Igor Mammedov]

On Wed, 4 Dec 2019 15:51:06 -0300
Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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?  
> > 
> > should_be_hidden is a mechanism for implementing
> > realize-without-plug: kind of. It's a mechanism that ensures
> > qdev_device_add() returns early as long as the condition to hide the
> > device is true. You could to the realize-without-plug in the handler
> > function that decides if the device should be "hidden".  
> 
> Oh, right.  I thought "qdev_device_add() returns early" meant
> "return after realize, before plug".  Now I see it returns before
> object_new().  This means we have another user-visible device
> state: "defined (in QemuOpts), but not created".

If I'm not mistaken this new behavior was introduced because
vfio-pci is not split on backend and frontend like majority
of pluggable devices, so the only option (apart from doing split)
was to fake unplug to avoid releasing resources that should be
owned y backend.

> > 
> > partially_hotplugged is a mechanism for implementing
> > unplug-without-unrealize: yes.  
> 
> Thanks!
> 

Re: qom device lifecycle interaction with hotplug/hotunplug ?

[Jens Freimann]

On Wed, Dec 11, 2019 at 01:52:33PM +0100, Damien Hedde wrote:
>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
>And if migration fails this same device is plugged back using
>failover_replug_primary():
>> 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).

I tested this now and it really is a no-op. It was required in a
earlier version of the patches and I missed to remove it when I
reworked the re-plug functionality.

I will send a patch to remove it.

regards
Jens