Re: [PATCH for-6.2 02/12] qom: Use DEVICE_*CLASS instead of OBJECT_*CLASS

From: Eduardo Habkost <ehabkost@redhat.com>
To: Juan Quintela <quintela@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH for-6.2 02/12] qom: Use DEVICE_*CLASS instead of OBJECT_*CLASS
Date: Tue, 10 Aug 2021 10:22:01 -0400	[thread overview]
Message-ID: <20210810142201.4pvwejjniuz4xfug@habkost.net> (raw)
In-Reply-To: <87eeb11pjq.fsf@secure.mitica>

On Tue, Aug 10, 2021 at 01:56:25PM +0200, Juan Quintela wrote:
> > -        DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
> > -                                             TYPE_DEVICE);
> > +        DeviceClass *dc = DEVICE_CLASS(list->data);
> 
> Finding where DEVICE_CLASS is defined is .... interesting.

That's a valid concern, but I wonder what we can do to address
this.  The existing practice of defining all macros manually
leads to a high number of mistakes and inconsistencies[1].

Now, once all QOM types are converted to the new macros (which is
work in progress), maybe we could replace:

  DEVICE_CLASS(oc)
  DEVICE_GET_CLASS(dev)

with more grep-friendly expressions like:

  CLASS(DEVICE, oc)
  GET_CLASS(DEVICE, dev)

The type of those expressions would still be (DeviceClass*).

---

[1] These are some of the fixes for bugs or inconsistencies that were
already merged to qemu.git:

6a567fbcf0b8 nubus: Delete unused NUBUS_BRIDGE macro
98b49b2bea15 spapr: Remove unnecessary DRC type-checker macros
08e14bb7e060 platform-bus: Delete macros for non-existing typedef
5c8b0f2cc799 can_emu: Delete macros for non-existing typedef
f58b770fbbd9 virtio-ccw: Fix definition of VIRTIO_CCW_BUS_GET_CLASS

These are fixes for broken QOM macros I submitted recently:

[PATCH for-6.2 1/6] acpi: Delete broken ACPI_GED_X86 macro
[PATCH for-6.2 2/6] sbsa_gwdt: Delete broken SBSA_*CLASS macros

And these are some other inconsistencies that are still in the
current tree, that need to be addressed:

hw/i386/kvm/i8254.c:45:1: type name mismatch: TYPE_KVM_I8254 vs KVM_PIT
hw/net/e1000.c:158:1: type name mismatch: TYPE_E1000_BASE vs E1000
hw/rtc/m48t59-isa.c:38:1: mismatching class type for M48TXX_ISA (M48txxISADeviceClass)
hw/rtc/m48t59-isa.c:131:1: class type declared here (None)
hw/rtc/m48t59.c:47:1: mismatching class type for M48TXX_SYS_BUS (M48txxSysBusDeviceClass)
hw/rtc/m48t59.c:654:1: class type declared here (None)
hw/s390x/virtio-ccw.h:63:1: typedef name mismatch: VirtioCcwBusState is defined as struct VirtioBusState
hw/s390x/virtio-ccw.h:59:1: typedef is here
hw/scsi/megasas.c:137:1: type name mismatch: TYPE_MEGASAS_BASE vs MEGASAS
hw/virtio/virtio-pci.h:29:1: typedef name mismatch: VirtioPCIBusState is defined as struct VirtioBusState
hw/virtio/virtio-pci.h:25:1: typedef is here
include/exec/memory.h:48:1: mismatching instance type for RAM_DISCARD_MANAGER (RamDiscardManager)
softmmu/memory.c:3418:1: instance type declared here (None)
include/hw/isa/superio.h:20:1: mismatching instance type for ISA_SUPERIO (ISASuperIODevice)
hw/isa/isa-superio.c:180:1: instance type declared here (None)
include/hw/s390x/event-facility.h:197:1: type name mismatch: TYPE_SCLP_EVENT_FACILITY vs EVENT_FACILITY
include/hw/s390x/s390-ccw.h:22:1: type name mismatch: TYPE_S390_CCW vs S390_CCW_DEVICE
include/hw/vfio/vfio-amd-xgbe.h:43:1: type name mismatch: TYPE_VFIO_AMD_XGBE vs VFIO_AMD_XGBE_DEVICE
include/hw/vfio/vfio-calxeda-xgmac.h:40:1: type name mismatch: TYPE_VFIO_CALXEDA_XGMAC vs VFIO_CALXEDA_XGMAC_DEVICE
include/hw/vfio/vfio-platform.h:73:1: type name mismatch: TYPE_VFIO_PLATFORM vs VFIO_PLATFORM_DEVICE
include/hw/watchdog/wdt_diag288.h:10:1: type name mismatch: TYPE_WDT_DIAG288 vs DIAG288
migration/migration.h:141:1: type name mismatch: TYPE_MIGRATION vs MIGRATION_OBJ
target/ppc/cpu.h:1253:1: mismatching instance type for PPC_VIRTUAL_HYPERVISOR (PPCVirtualHypervisor)
target/ppc/cpu_init.c:9090:1: instance type declared here (None)

-- 
Eduardo