From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40296)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1aPAy0-0005Tr-0i
	for qemu-devel@nongnu.org; Fri, 29 Jan 2016 10:28:09 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1aPAxw-0004Cv-Qj
	for qemu-devel@nongnu.org; Fri, 29 Jan 2016 10:28:07 -0500
Received: from mx1.redhat.com ([209.132.183.28]:41682)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1aPAxw-0004Bp-IM
	for qemu-devel@nongnu.org; Fri, 29 Jan 2016 10:28:04 -0500
Date: Fri, 29 Jan 2016 13:28:01 -0200
From: Eduardo Habkost <ehabkost@redhat.com>
Message-ID: <20160129152801.GZ3869@thinpad.lan.raisama.net>
References: <1453710287-12706-1-git-send-email-valentin.rakush@gmail.com>
	<20160126153538.GN3869@thinpad.lan.raisama.net>
	<20160126155121.GI15172@redhat.com>
	<20160126172635.GO3869@thinpad.lan.raisama.net>
	<20160126221913.GA13460@redhat.com>
	<20160127150937.GQ3869@thinpad.lan.raisama.net>
	<20160127152359.GC14935@redhat.com>
	<CAL0Arcyge92sBD1dxv2pNmo5jiXE8yC6oyM4rqYKtjd_Dg2q=g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAL0Arcyge92sBD1dxv2pNmo5jiXE8yC6oyM4rqYKtjd_Dg2q=g@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp,
 qapi: create qom-type-prop-list for class properties
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Valentin Rakush <valentin.rakush@gmail.com>
Cc: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org, lcapitulino@redhat.com, asmetanin@virtuozzo.com, "Denis V. Lunev" <den@openvz.org>, Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>

On Fri, Jan 29, 2016 at 01:03:38PM +0300, Valentin Rakush wrote:
> Hi Eduardo, hi Daniel,
> 
> I checked most of the classes that are used for x86_64 qemu simulation with
> this command line:
> x86_64-softmmu/qemu-system-x86_64 -qmp tcp:localhost:4444,server,nowait
> -machine pc -cpu core2duo
> 
> Here are some of the classes that cannot provide properties with
> device_list_properties call:
> /object/machine/generic-pc-machine/pc-0.13-machine
> /object/bus/i2c-bus
> /interface/user-creatable
> /object/tls-creds/tls-creds-anon
> /object/memory-backend/memory-backend-file
> /object/qemu:memory-region
> /object/rng-backend/rng-random
> /object/tpm-backend/tpm-passthrough
> /object/tls-creds/tls-creds-x509
> /object/secret
> 
> They cannot provide properties because these classes cannot be casted to
> TYPE_DEVICE. This is done intentionally because TYPE_DEVICE has its own
> properties.

Can you clarify what you mean by "TYPE_DEVICE has its own
properties"? TYPE_DEVICE properties are registered as normal QOM
properties.

We can still add a new command that's not specific for
TYPE_DEVICE (if necessary). The point is that it shouldn't return
arbitrarily different (and incomplete) data from the existing
mechanism to list properties.

In other words, I don't see why the output of "qom-type-prop-list
<type>" can't be as good as the output of "device-list-properties
<type>". If we make return only class-properties, it will be less
complete and less useful.


> Also TYPE_MACHINE has own properties of type GlobalProperty.

I don't understand what you mean, here. GlobalProperties are not
machine properties, they are just property=value pairs to be
registered as global properties. They are unrelated to the
properties TYPE_MACHINE actually has.

> Here are two ways (AFAICS):
> - we refactor TYPE_DEVICE and TYPE_MACHINE so they store their properties
> in the ObjectClass properties.

Too many classes need to be converted. We would still need
something to use during the transiation.

> - we change device_list_properties so it process different classes
> differently.

Could you clarify what you mean by "process different classes
differently"?

A third option is to just use object_new(), like
qmp_device_list_properties() already does.

> 
> The disadvantage of the second approach, is that it is complicating code in
> favor of simplifying qapi interface. I like first approach with
> refactoring, although it is more complex. The first approach should put all
> properties in the base classes and then use this properties everywhere
> (command line help, qmp etc.) The simplest way the refactoring can be done,
> is by moving TYPE_DEVICE properties to ObjectClass and merging them somehow
> with TYPE_MACHINE GlobalProperty. Then we will use these properties for all
> other types of classes.
> 
> Of course, we can leave device_list_properties as it is and use
> qom-type-prop-list instead.
> 
> What do you think? Does these design options make sense for you?

We can add a new command if we don't want to change how
device-list-properties work. But first I would like to understand
the actual reasons for the new command, because we can't argue
about it if we don't know what the command output will be used
for. How exactly would callers qom-type-prop-list use that
information?

I see 3 cases where property names are used:

1) QMP QOM commands (qom-get/qom-set):

These properties are available using qom-list, already.

2) -device/device_add options:

These properties are available in device-list-properties,
already.

3) -object/object-add options:

In this case, if you want to return complete data, you only have
two options: a) convert all TYPE_USER_CREATABLE classes to use
class-properties; or b) use the same approach used by
qmp_device_list_properties() (object_new() followed by
enumeration of properteis).

4) -machine options:

This is similar to -object: the list will be incomplete unless
all machine subclasses are converted to use only
class-properties, or the new command uses object_new().

5) -cpu options:

Ditto. the list will be incomplete unless all CPU subclasses are
converted to use only class-properties, or the new command uses
object_new().


That doesn't mean we don't want to convert other classes to use
class-properties later to simplify internal QEMU code. But if you
want to propose a new QMP command, let's make one that returns
useful data for real use cases.

I am not sure the list above is complete, so I would like to
understand how exactly the data you want to return will be used.
So for each of the classes you mentioned, I would like to know:

> /object/machine/generic-pc-machine/pc-0.13-machine

What exactly do you think the caller use the output of
"qom-type-prop-list pc-0.13-machine" for? How exactly? Would it
use them in the QEMU command-line? In other QMP commands? Which
ones?

> /object/bus/i2c-bus

Ditto, what exactly do you tink the caller would do with the
output of "qom-type-prop-list i2c-bus"?

> /interface/user-creatable

user-creatable is an interface. If you want to allow interfaces
to register class-properties, you probably need special code for
them during object creation.

Also, see my question about abstract classes in my previous reply
to Daniel. We can deal with interfaces and abstract classes
later, as we don't even expose class hierarchy information to the
outside (AFAIK).

> /object/tls-creds/tls-creds-anon

What exactly do you tink the caller would do with the output of
"qom-type-prop-list tls-creds-anon"?

> /object/memory-backend/memory-backend-file
> /object/qemu:memory-region
> /object/rng-backend/rng-random
> /object/tpm-backend/tpm-passthrough
> /object/tls-creds/tls-creds-x509
> /object/secret

Ditto for each of the classes above.

-- 
Eduardo