[Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Anthony Liguori]

Hi,

I wanted to start a central thread as this discussion is spread in two places 
right now.

I think I'm becoming convinced that realized belongs in DeviceState and that 
BlockDriverState does not have a realized equivalent.

To me, realized represents Vcc.  When realized=true, the guest has power and is 
active.  When realized=false, the guest has lost power.  The realize() event is 
the rising edge of Vcc, unrealized() is the falling edge.

realize() should be used to take any actions that require all parameters to be 
set that need to happen before the guest has power.  This later clause is 
extremely important.  unrealize() should be used to unset anything setup in 
realize().

The destructor being invoked does not imply that unrealize() has happened.

I think BlockDriverState wants two separate signals: an opened signal and an 
attached signal.

With BDS, there are a few phases:

opened = false
attached = false

All properties can be set here and no side effects occur.

It is not valid to do a read/write request to a BDS in this state.

opened = true
attached = false

Properties that affect file opening (flags and filename) cannot be changed after 
this point.  Depending on the contents of the file, a backing_file property may 
be created after opened = true.  The backing_file that gets created does *not* 
automatically have opened = true.  A user explicitly needs to set that.

This behavior is necessary to allow overriding backing files.

It is not valid to do a read/write request to a BDS in this state.

opened = true
attached = true

This means that the device is in-use by something in QEMU.  Requests may be in 
flight as read/write requests are now allowed to be issued.

Most properties cannot be changed in this state (presumably).

opened = false
attached = true

Invalid state.

So there are three valid states and none of them map directly to the concept of 
realized for devices.

I think this argues pretty clearly for realize() to not live in Object and 
instead to allow base classes to implement whatever properties make sense to them.

Regard,

Anthony Liguori

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Andreas Färber]

Am 12.06.2012 00:05, schrieb Anthony Liguori:
> I think I'm becoming convinced that realized belongs in DeviceState and
> that BlockDriverState does not have a realized equivalent.

> To me, realized represents Vcc.  When realized=true, the guest has power
> and is active.  When realized=false, the guest has lost power.  The
> realize() event is the rising edge of Vcc, unrealized() is the falling
> edge.

Then please name it appropriately: "powered" and "unpowered".

Realization has nothing to do with power, it's an OOP term that
distinguishes from instantiation.

The way this discussion has headed is very unfortunate for me since I
need such a hook for the CPUs today and not in a far future when the
whole of qdev has been refactored to match the QOM type / inheritance
model and qdev has been integrated into linux-user / bsd-user. So if I
don't get ObjectClass::realize for the realize functions we've already
grown (arm, i386) then I'll need some CPUClass::realize mechanism as
(unnecessary?) interim solution to solve the cpu_init() design issue...

Regards,
Andreas

> realize() should be used to take any actions that require all parameters
> to be set that need to happen before the guest has power.  This later
> clause is extremely important.  unrealize() should be used to unset
> anything setup in realize().
> 
> The destructor being invoked does not imply that unrealize() has happened.
[...]
> I think this argues pretty clearly for realize() to not live in Object
> and instead to allow base classes to implement whatever properties make
> sense to them.
> 
> Regard,
> 
> Anthony Liguori

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Anthony Liguori]

On 06/11/2012 07:29 PM, Andreas Färber wrote:
> Am 12.06.2012 00:05, schrieb Anthony Liguori:
>> I think I'm becoming convinced that realized belongs in DeviceState and
>> that BlockDriverState does not have a realized equivalent.
>
>> To me, realized represents Vcc.  When realized=true, the guest has power
>> and is active.  When realized=false, the guest has lost power.  The
>> realize() event is the rising edge of Vcc, unrealized() is the falling
>> edge.
>
> Then please name it appropriately: "powered" and "unpowered".
>
> Realization has nothing to do with power, it's an OOP term that
> distinguishes from instantiation.

Eh?  I'm not familiar with it as a OOP term.

> The way this discussion has headed is very unfortunate for me since I
> need such a hook for the CPUs today and not in a far future when the
> whole of qdev has been refactored to match the QOM type / inheritance

I warned you early on that CPUs were not the best place to start...

I think the fundamental problem is that CPU wants to be a TYPE_DEVICE.  We 
probably shouldn't have started with it as TYPE_OBJECT.

I think that's really the immediate problem that needs to be solved.  I think 
avoiding making it TYPE_DEVICE is what's caused us to attempt to push realized 
into Object in the first place.

Regards,

Anthony Liguori

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Paolo Bonzini]

Il 12/06/2012 00:05, Anthony Liguori ha scritto:
> 
> opened = true
> attached = true
> 
> This means that the device is in-use by something in QEMU.  Requests may
> be in flight as read/write requests are now allowed to be issued.
> 
> Most properties cannot be changed in this state (presumably).

So this is "realized".

Or Object needs to provide support for multiple states, and (static)
properties can have a bitmap of which states let them be modified.

Paolo

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Kevin Wolf]

Am 12.06.2012 00:05, schrieb Anthony Liguori:
> To me, realized represents Vcc.  When realized=true, the guest has power and is 
> active.  When realized=false, the guest has lost power.  The realize() event is 
> the rising edge of Vcc, unrealized() is the falling edge.
> 
> realize() should be used to take any actions that require all parameters to be 
> set that need to happen before the guest has power.  This later clause is 
> extremely important.  unrealize() should be used to unset anything setup in 
> realize().

The one thing I'm really unsure about is whether you're combining two
different things into one concept: This Vcc concept and Andreas' OOP
concept (which is btw the same as I understood realize before we
discussed yesterday on IRC - I guess you must have explained it as such
at some point).

Is it really always the power-on event that causes the final check of
properties, setup of device state and makes some properties read-only?
Or is that just a coincidence and happens to be true only with some devices?

> The destructor being invoked does not imply that unrealize() has happened.

Which makes some sense in your Vcc model, but doesn't at all in the OOP
concept model.

Kevin

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Kevin Wolf]

Am 12.06.2012 08:10, schrieb Paolo Bonzini:
> Il 12/06/2012 00:05, Anthony Liguori ha scritto:
>>
>> opened = true
>> attached = true
>>
>> This means that the device is in-use by something in QEMU.  Requests may
>> be in flight as read/write requests are now allowed to be issued.
>>
>> Most properties cannot be changed in this state (presumably).
> 
> So this is "realized".

I tend to agree, but it's not quite clear to me. It could also be the
intermediate state, but I guess that in practice that would be less useful.

> Or Object needs to provide support for multiple states, and (static)
> properties can have a bitmap of which states let them be modified.

That would probably be the cleanest solution. Not sure if it's worth the
effort, though.

Kevin

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> Hi,
>
> I wanted to start a central thread as this discussion is spread in two
> places right now.
>
> I think I'm becoming convinced that realized belongs in DeviceState
> and that BlockDriverState does not have a realized equivalent.
>
> To me, realized represents Vcc.  When realized=true, the guest has
> power and is active.  When realized=false, the guest has lost power.
> The realize() event is the rising edge of Vcc, unrealized() is the
> falling edge.
>
> realize() should be used to take any actions that require all
> parameters to be set that need to happen before the guest has power.
> This later clause is extremely important.  unrealize() should be used
> to unset anything setup in realize().
>
> The destructor being invoked does not imply that unrealize() has happened.
>
> I think BlockDriverState wants two separate signals: an opened signal
> and an attached signal.
>
> With BDS, there are a few phases:
>
> opened = false
> attached = false
>
> All properties can be set here and no side effects occur.
>
> It is not valid to do a read/write request to a BDS in this state.
>
> opened = true
> attached = false
>
> Properties that affect file opening (flags and filename) cannot be
> changed after this point.  Depending on the contents of the file, a
> backing_file property may be created after opened = true.  The
> backing_file that gets created does *not* automatically have opened =
> true.  A user explicitly needs to set that.
>
> This behavior is necessary to allow overriding backing files.

Could you explain why we need to override backing files between open and
attach?

Can you give another example of something we need to be able to do
between open and attach?

> It is not valid to do a read/write request to a BDS in this state.
>
> opened = true
> attached = true
>
> This means that the device is in-use by something in QEMU.  Requests
> may be in flight as read/write requests are now allowed to be issued.
>
> Most properties cannot be changed in this state (presumably).
>
> opened = false
> attached = true
>
> Invalid state.
>
> So there are three valid states and none of them map directly to the
> concept of realized for devices.
>
> I think this argues pretty clearly for realize() to not live in Object
> and instead to allow base classes to implement whatever properties
> make sense to them.

I'm not yet convinced.

QOM design started with two states: unrealized and realized.

For block backends, you suggest we need to split the unrealized state.
I'm not sure that's actually necessary.  But even if it is, the result
still satisfies the unrealized/realized contract:

1. While unrealized, all you can do is set properties.  Setting a
   property may fail.

2. While realized, the object "can do stuff", but its properties are
   typically immutable.

Yes, that's a pretty vague contract.  But TYPE_OBJECT is a pretty
abstract thing, so some vagueness may be unavoidable.  If you can come
up with a tighter one, excellent.

Note that a BDS satisfies 1. whether it's opened or not.

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Paolo Bonzini]

Il 13/06/2012 14:53, Markus Armbruster ha scritto:
>> > This behavior is necessary to allow overriding backing files.
> Could you explain why we need to override backing files between open and
> attach?

Open tells us that you need a backing file.

After attach, state should be finalized.

Hence, between open and attach is the only time when you can override
backing files.

> Can you give another example of something we need to be able to do
> between open and attach?

I can't think of anything.

Paolo

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Anthony Liguori]

On 06/13/2012 07:53 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>> Hi,
>>
>> I wanted to start a central thread as this discussion is spread in two
>> places right now.
>>
>> I think I'm becoming convinced that realized belongs in DeviceState
>> and that BlockDriverState does not have a realized equivalent.
>>
>> To me, realized represents Vcc.  When realized=true, the guest has
>> power and is active.  When realized=false, the guest has lost power.
>> The realize() event is the rising edge of Vcc, unrealized() is the
>> falling edge.
>>
>> realize() should be used to take any actions that require all
>> parameters to be set that need to happen before the guest has power.
>> This later clause is extremely important.  unrealize() should be used
>> to unset anything setup in realize().
>>
>> The destructor being invoked does not imply that unrealize() has happened.
>>
>> I think BlockDriverState wants two separate signals: an opened signal
>> and an attached signal.
>>
>> With BDS, there are a few phases:
>>
>> opened = false
>> attached = false
>>
>> All properties can be set here and no side effects occur.
>>
>> It is not valid to do a read/write request to a BDS in this state.
>>
>> opened = true
>> attached = false
>>
>> Properties that affect file opening (flags and filename) cannot be
>> changed after this point.  Depending on the contents of the file, a
>> backing_file property may be created after opened = true.  The
>> backing_file that gets created does *not* automatically have opened =
>> true.  A user explicitly needs to set that.
>>
>> This behavior is necessary to allow overriding backing files.
>
> Could you explain why we need to override backing files between open and
> attach?

We may eventually support multiple backing files (this is supported by VMDK, for 
instance).

So you need to explicitly open the file before you can even discover how many 
backing files there are.

> Can you give another example of something we need to be able to do
> between open and attach?

Depends on the image format.  For something like QED/QCOW3 where there's an 
explicit meta-data repair phase, you may want to expose that flag.  That's an 
operation that must happen after open but almost certainly before attach.

>> It is not valid to do a read/write request to a BDS in this state.
>>
>> opened = true
>> attached = true
>>
>> This means that the device is in-use by something in QEMU.  Requests
>> may be in flight as read/write requests are now allowed to be issued.
>>
>> Most properties cannot be changed in this state (presumably).
>>
>> opened = false
>> attached = true
>>
>> Invalid state.
>>
>> So there are three valid states and none of them map directly to the
>> concept of realized for devices.
>>
>> I think this argues pretty clearly for realize() to not live in Object
>> and instead to allow base classes to implement whatever properties
>> make sense to them.
>
> I'm not yet convinced.
>
> QOM design started with two states: unrealized and realized.

No, that was never part of the QOM design actually.  That was always in Device 
actually.

> For block backends, you suggest we need to split the unrealized state.

I think it's just a coincidence that opened=false, attached=true is an invalid 
state.  Consider CharDriverState as a counter example.

The equivalent of opened=false for CDS is a disconnected TCP CDS.  While 
requests during opened=false is an error for BDS, for CDS, the data is simply 
discarded.  So opened=false, attached=true is a perfectly valid state for CDS.

So it's not splitting unrealize.  It's really two independent parameters.

> I'm not sure that's actually necessary.  But even if it is, the result
> still satisfies the unrealized/realized contract:
>
> 1. While unrealized, all you can do is set properties.  Setting a
>     property may fail.

When attached=true, you cannot set properties.  There are many concrete states 
where this is true.

And this isn't even universally true.  Some properties *can* be set when 
attached=true.  For example, open flags is likely to be a good example of this.

So we really can make broad generalizations like the above.

> 2. While realized, the object "can do stuff", but its properties are
>     typically immutable.

'typically' is the give-away word here.

My view of the world is that an Object may have 0 or more bits of state 
associated with it.  Properties in general need to implement checks to determine 
whether setting the property is valid given the current arrangement of state.

I think 95% of the cases, this state will be in super classes and property 
permissions will fit into large classes (read-only when realized=true).

Regards,

Anthony Liguori

Re: [Qemu-devel] Semantics of DeviceState::realized and BlockDriverState

[Kevin Wolf]

Am 13.06.2012 14:53, schrieb Markus Armbruster:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>> Properties that affect file opening (flags and filename) cannot be
>> changed after this point.  Depending on the contents of the file, a
>> backing_file property may be created after opened = true.  The
>> backing_file that gets created does *not* automatically have opened =
>> true.  A user explicitly needs to set that.
>>
>> This behavior is necessary to allow overriding backing files.
> 
> Could you explain why we need to override backing files between open and
> attach?
> 
> Can you give another example of something we need to be able to do
> between open and attach?

It's just one example of an option that can get a default value from the
image file, but can we overridden by the user (e.g. by libvirt in order
to pass a file descriptor instead).

The same is true for the backing file format, for an option whether QED
mode may be used and probably quite a few more.

> I'm not yet convinced.
> 
> QOM design started with two states: unrealized and realized.
> 
> For block backends, you suggest we need to split the unrealized state.
> I'm not sure that's actually necessary.  But even if it is, the result
> still satisfies the unrealized/realized contract:
> 
> 1. While unrealized, all you can do is set properties.  Setting a
>    property may fail.
> 
> 2. While realized, the object "can do stuff", but its properties are
>    typically immutable.
> 
> Yes, that's a pretty vague contract.  But TYPE_OBJECT is a pretty
> abstract thing, so some vagueness may be unavoidable.  If you can come
> up with a tighter one, excellent.
> 
> Note that a BDS satisfies 1. whether it's opened or not.

There's a difference concerning which properties can be written.
Initially you can write to all properties, after opening the image file
some (like the filename) become read-only, and after
attach/realize/whatever you call it more of them become read-only.

Kevin