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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

(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/