[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS r
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers |
Date: |
Thu, 30 Mar 2017 10:32:48 -0300 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Thu, Mar 30, 2017 at 10:59:10AM +0200, Paolo Bonzini wrote:
>
>
> On 29/03/2017 21:41, Eduardo Habkost wrote:
> > QOM has a data model where class struct data is static: class
> > structs are initialized at class_init, and never changed again.
>
> Isn't that just the way class data is being used? There's no reason for
> class data to be static. It happens to be that way because our
> hierarchies are pretty shallow and global static variables are used
> instead of class data. But it doesn't _have_ to be like that.
I disagree. I believe this is how class data is meant to be used.
if we change per-class data at runtime outside class_init, we
have a hard time providing querying and introspection mechanisms
for types. If something really needs to be changed at runtime
outside class_init, it belongs to object instance fields, not
class data fields.
e.g. the main benefit we got from eliminating the compat_* global
variables in machine/pc code wasn't simply the global variable
elimination: it was making the compat data static and available
at the MachineClass structs, instead of burying it inside
machine_init functions. If we kept the old model where
class-specific data was initialized at machine_init-time,
extending query-machines with that info, and writing new machine
data queries would still be impossible.
For reference, this is how I think we could have dealt with the
existing cases of non-const class variables:
> > qbus_realize(), code added by:
> > commit 61de36761b565a4138d8ad7ec75489ab28fe84b6
> > Date: Thu Feb 6 16:08:15 2014 +0100
> > qdev: Keep global allocation counter per bus
This one might make sense: it's easier to keep data in a BusClass
field than a automatic_id[bus_type] dictionary somewhere else.
But I would still prefer to have a automatic_id[bus_type]
dictionary inside MachineState instead of this.
> >
> > mips_jazz_init(), code added by:
> > commit 54e755588cf1e90f0b1460c4e8e6b6a54b6d3a32
> > Date: Mon Nov 4 23:26:17 2013 +0100
> > mips jazz: do not raise data bus exception when accessing invalid
> > addresses
IMO, a simple MIPSCPU::ignore_invalid_exec_access boolean flag
would be better than hijacking TYPE_MIPS_CPU's
do_unassigned_access() method on the fly.
> >
> > xen_set_dynamic_sysbus(), code added by:
> > commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894
> > Date: Tue Nov 22 07:10:58 2016 +0100
> > xen: create qdev for each backend device
Here, setting has_dynamic_sysbus for all PC classes would be
better, so a future supported-device-types/device-slot querying
mechanism wouldn't incorrectly report xen-backend as unsupported.
But there are additional problems I want to sove regarding sysbus
before doing that.
BTW, this is the case that prompted me to write this series in
the first place. I believe changing
MachineClass::has_dynamic_sysbus at runtime was a mistake, and a
const object_get_class() would have helped us catch that.
> >
> > s390_cpu_realizefn(), code added by:
> > commit c6644fc88bed1176f67b4b7a9df691bcf25c0c5b
> > Date: Fri Mar 4 12:34:31 2016 -0500
> > s390x/cpu: Get rid of side effects when creating a vcpu
Here, I see no reason for a per-cpu-class field at all. Even a
static next_cpu_id variable wouldn't hurt.
Also, this is the only remaining code still using cpu_exists(),
and the same logic probably can be implemented using
MachineClass::possible_cpu_arch_ids() instead.
>
> That said, the QUALIFIED_CAST concept is very interesting and I'd even
> extend it to OBJECT_CHECK, object_dynamic_cast and
> object_dynamic_cast_assert. I'm just not sure about returning const
> from object_get_class()/*_GET_CLASS, which ironically is the thing that
> prompted you to write the series. :)
Makes sense. I will send extra patches to change the object cast
macros, too.
>
> Paolo
>
--
Eduardo
- [Qemu-devel] [RFC v2 4/9] qom: Make object_class_get_parent() const-aware, (continued)
- [Qemu-devel] [RFC v2 4/9] qom: Make object_class_get_parent() const-aware, Eduardo Habkost, 2017/03/29
- [Qemu-devel] [RFC v2 5/9] Make class parameter const at some functions, Eduardo Habkost, 2017/03/29
- [Qemu-devel] [RFC v2 6/9] Explicitly cast the *_GET_CLASS() value when we break the rules, Eduardo Habkost, 2017/03/29
- [Qemu-devel] [RFC v2 8/9] qom: Make class cast macros/functions const-aware, Eduardo Habkost, 2017/03/29
- [Qemu-devel] [RFC v2 9/9] qom: Make object_get_class() return const pointer, Eduardo Habkost, 2017/03/29
- Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers, Laszlo Ersek, 2017/03/29
- Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers, Eduardo Habkost, 2017/03/29
- [Qemu-devel] [RFC v2 7/9] Use const variables for *_GET_CLASS values, Eduardo Habkost, 2017/03/29
- Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers, Paolo Bonzini, 2017/03/30
- Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers,
Eduardo Habkost <=