Re: [PATCH 0/3] hw: Fix abuse of QOM class internals modified by their instances

[Philippe Mathieu-Daudé]

(posted too quickly)

On 23/5/23 08:44, Philippe Mathieu-Daudé wrote:
> Bernhard warned for QOM class abuse here [*]:
>
> > A realize method is supposed to modify a single instance only
> > while we're modifying the behavior of whole classes here, i.e.
> > will affect every instance of these classes. This goes against
> > QOM design principles and will therefore be confusing for people
> > who are familiar with QOM in particular and OOP in general.
>
> This series fixes the cases I found while auditing.

Audited but not yet fixed:

- accel/xen
  xen_init() sets 'default_ram_id'
  -> need to figure migration compatibility (because using shared
     pc_machine_class_init and DEFINE_PC_MACHINE), will be posted
     separately.

- hw/core/bus
  qbus_init_internal() increments 'automatic_ids'
  -> need more thought.

- hw/core/
  machine_parse_smp_config() sets 'smp_props.has_clusters'
  -> dubious

- hw/remote/
  remote_object_init() and probe_pci_info() set 'vendor_id', 'nr_devs'
  -> need more thought.

- qom/object/
  object_dynamic_cast_assert() populates object_cast_cache[]
  -> QOM internal, likely OK.

- softmmu/vl
  configure_blockdev() overwrites 'machine_class->block_default_type'
  -> dubious

- gdbstub
  ppc_gdb_gen_spr_xml() sets 'gdb_num_sprs' and 'gdb_spr_xml'
  -> dubious, likely fixable (maybe like patch #3 of this series)

- tests/unit/
  smp_parse_test() set 'smp_props.prefer_sockets'
  -> dubious, probably tolerable.

> [*] 
> https://lore.kernel.org/qemu-devel/0DAAC63B-0C0F-44C4-B7EB-ACD6C9A36BF1@gmail.com/