Re: [Qemu-devel] [PATCH] scripts: Add qom-tree script as modern equivalent of info qtree

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 07.02.2014 14:06, schrieb Paolo Bonzini:
>> Il 07/02/2014 13:44, Andreas Färber ha scritto:
>>> Am 07.02.2014 12:21, schrieb Paolo Bonzini:
>>>> Let's stop talking about theory and let's look at the actual ccode,
>>>> please.
>>>
>>> I have posted actual patches, you haven't.
>> 
>> I have reviewed those, and said that we can apply all three.  It's
>> certainly better than reverting.  That doesn't mean that keeping broken
>> code would have been better than reverting.
>
> Right, my preferred way would've been a) Peter C. noticing in the first
> place or b) Peter C. supplying the one-line NAND fix that I had to write
> myself after no one else managed to come up with a real fix. :(
>
>>  And let me repeat that
>> *reverting a broken patch should always be one of the alternatives*.
>
> Yes. But it has never been *mandatory* to revert things first before
> fixing them. We usually apply incremental fixes referencing the commit
> hash that broke things.

You're attacking a strawman here.

Nobody has demanded revert first, fix later.  The regression got
reported, possible fixes (including a revert) were discussed, Peter
volunteered a follow-up fix, we accepted his offer, and when he couldn't
complete the job in time, he recommended to fall back to revert for the
time being.

This is as standard operating procedure as it gets.  Except for the part
where you get all excited, I must say.

>> But my request to "look at the actual code" was not related to
>> contribution of patches.  It referred to _all_ of QEMU device hierarchy.
>> Your assertion that "qdev is dead" seems quite an exaggeration; having
>> contributed quite a few patches to "kill" qdev-specific mechanisms in
>> favor of generic ones, it seems very much alive to me.
>
> Negative, all the device registration, structs, etc. of qdev apart from
> the actual devices' *State is gone, no longer exists. See Anthony's
> large commits in Git history if you don't believe me. You do know better
> than what you are saying here. I certainly agree that there were a
> number of oversights and bugs the two of us and others have been fixing.

Much of qdev has been reimplemented on top of QOM.  It's not "gone" in
any sense of the word I'd understand.  Perhaps it'll be gone some day,
but we're not there, yet.

> So you could argue that QOM was applied to fast with insufficient
> testing and thinking-through - but also in this case I will rather fix
> the fallout than reverting QOM. ;)

"Reverting QOM" is an even bigger strawman.

> So yes, we still have buses, but yet qdev is a thing of the past. That's
> why I have always asked to use device_ rather than qdev_ in new qdev.c
> code. Scott openly said he didn't realize we had switched to QOM because
> as a device author he was still using qdev_create(), qdev_init() and
> other APIs with qdev in their name - that's the battle I'm fighting:
> getting qdev out of people's heads and stopping to think the old way
> just because it's been like that for a long time.

It's a good fight, but lashing out at people won't do squat for winning
it.

> However, based on Anthony's vision/dogma/whatever I do not consider it
> worth to rename qdev_set_parent_bus() to device_set_parent_bus() iff
> Anthony wants to get rid of buses. And in that case i wouldn't want to
> replace qdev_create() with object_new() + qdev_set_parent_bus() since
> that's replacing Satan with Lucifer. ;)
>
>> Let's look at qdev.  Ask ourselves what useful functionality of Device
>> has nothing to do with devices.  Ask ourselves where it clashes with the
>> design of Object, and which of the two is better.  Make a design that is
>> consistent with what we need, not with a generic 2-year old vision that
>> sometimes borders on dogma.  Then we can write code.
>> 
>> This is all totally unrelated from which interesting relationships are
>> useful to extract and visualize from the QOM tree---and my point there
>> is that both parent-child ("qom-tree") and controller-controlled ("info
>> qtree") are useful relationships.
>
> I'm not arguing against that. I have rather been telling you that trying
> to *shoehorn* QOM objects into "info qtree" cannot work well.
>
> The claim has been that info qtree *should* include all devices (nand in
> particular) and that claim I strongly dispute. And I reject you giving

Strawman#3.

Whether nand belongs into "info qtree" or not can be debated.  Whether
it's okay that "info qtree" crashes when a nand device is around cannot.

> me instructions for how I should implement "info qtree" for you in your
> opinion.

Please consider taking a break and cool down a bit.  That's what I do
when I catch myself writing such stuff.  Or my friends catch me.

> I intend to post patches offering new, alternative displays of the
> composition tree (info qom-composition) since apparently there is a new
> demand for HMP (I was actually under the impression HMP was deprecated
> by QMP ever since we started added QMP-only commands like qom-* and
> cpu-add, so this script would've been all to make use of the existing
> QMP interface to get property values printed!) and I have been
> considering a cpu-add like HMP wrapper around qom-list for property
> discovery.

A welcome contribution!

Demand for HMP isn't new, and HMP is deprecated only as stable interface
for tools.  We've told people to use QMP so many times that folks
getting the impression we want to deprecate HMP in general is quite
understandable.  It's incorrect all the same.

New functionality for HMP must be also available in QMP, unless it's
something that doesn't make sense in QMP.

HMP commands should be implemented so that the HMP code only deals with
the user interface.  The real work should be done by the same code that
does it for QMP.

[...]