Gleb Natapov <gleb@xxxxxxxxxx> writes:
> Add "deriver_name" to DeviceInfo to use in device path building. In
Typo "deriver". Same in subject.
> contrast to "name" "driver_name" should refer to functionality device
> provides instead of particular device model like "name" does.
Why is that useful in a device path?
I'm afraid "driver_name" is too confusing, because we already use
"driver" and "name" for the name of the device model: DeviceInfo member
name, and qemu_device_opts option name "driver".
Perhaps something like "class"?
> Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
> ---
> hw/fdc.c | 1 +
> hw/ide/isa.c | 1 +
> hw/ide/piix.c | 1 +
> hw/ide/qdev.c | 1 +
> hw/isa-bus.c | 1 +
> hw/piix_pci.c | 2 ++
> hw/qdev.h | 6 ++++++
> hw/virtio-pci.c | 2 ++
> 8 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index c159dcb..b4217a3 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -2040,6 +2040,7 @@ static const VMStateDescription vmstate_isa_fdc ={
> static ISADeviceInfo isa_fdc_info = {
> .init = isabus_fdc_init1,
> .qdev.name = "isa-fdc",
> + .qdev.driver_name = "fdc",
> .qdev.size = sizeof(FDCtrlISABus),
> .qdev.no_user = 1,
> .qdev.vmsd = &vmstate_isa_fdc,
> diff --git a/hw/ide/isa.c b/hw/ide/isa.c
> index 6b57e0d..489cc99 100644
> --- a/hw/ide/isa.c
> +++ b/hw/ide/isa.c
> @@ -98,6 +98,7 @@ ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
>
> static ISADeviceInfo isa_ide_info = {
> .qdev.name = "isa-ide",
> + .qdev.driver_name = "ata",
> .qdev.size = sizeof(ISAIDEState),
> .init = isa_ide_initfn,
> .qdev.reset = isa_ide_reset,
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 07483e8..6206201 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -182,6 +182,7 @@ PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> static PCIDeviceInfo piix_ide_info[] = {
> {
> .qdev.name = "piix3-ide",
> + .qdev.driver_name = "ata",
> .qdev.size = sizeof(PCIIDEState),
> .qdev.no_user = 1,
> .init = pci_piix3_ide_initfn,
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 0808760..341548e 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -134,6 +134,7 @@ static int ide_drive_initfn(IDEDevice *dev)
>
> static IDEDeviceInfo ide_drive_info = {
> .qdev.name = "ide-drive",
> + .qdev.driver_name = "ata-disk",
> .qdev.size = sizeof(IDEDrive),
> .init = ide_drive_initfn,
> .qdev.props = (Property[]) {
"ata-disk" is misleading, as it doesn't have to be a disk. "ata-drive"?
Hmm, can't we stick to "ide-drive" just as well?
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 4e306de..2c42b80 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -153,6 +153,7 @@ static int isabus_bridge_init(SysBusDevice *dev)
> static SysBusDeviceInfo isabus_bridge_info = {
> .init = isabus_bridge_init,
> .qdev.name = "isabus-bridge",
> + .qdev.driver_name = "isa",
> .qdev.size = sizeof(SysBusDevice),
> .qdev.no_user = 1,
> };
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 00060f3..4e5e5df 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -364,6 +364,7 @@ static PCIDeviceInfo i440fx_info[] = {
> .config_write = i440fx_write_config,
> },{
> .qdev.name = "PIIX3",
> + .qdev.driver_name = "pci-isa-bridge",
> .qdev.desc = "ISA bridge",
> .qdev.size = sizeof(PIIX3State),
> .qdev.vmsd = &vmstate_piix3,
> @@ -377,6 +378,7 @@ static PCIDeviceInfo i440fx_info[] = {
> static SysBusDeviceInfo i440fx_pcihost_info = {
> .init = i440fx_pcihost_initfn,
> .qdev.name = "i440FX-pcihost",
> + .qdev.driver_name = "pci",
> .qdev.size = sizeof(I440FXState),
> .qdev.no_user = 1,
> };
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 579328a..a9a98f8 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -139,6 +139,7 @@ typedef void (*qdev_resetfn)(DeviceState *dev);
>
> struct DeviceInfo {
> const char *name;
> + const char *driver_name;
> const char *alias;
> const char *desc;
> size_t size;
> @@ -288,6 +289,11 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props);
> void qdev_prop_register_global_list(GlobalProperty *props);
> void qdev_prop_set_globals(DeviceState *dev);
>
> +static inline const char *qdev_driver_name(DeviceState *dev)
> +{
> + return dev->info->driver_name ? : dev->info->name;
> +}
> +
Aha, it defaults to the device model name dev->info->name. That's why
you get away with not defining it in most DeviceInfo.
> /* This is a nasty hack to allow passing a NULL bus to qdev_create. */
> extern struct BusInfo system_bus_info;
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 2d78ca6..b67ded6 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -769,6 +769,7 @@ static int virtio_9p_init_pci(PCIDevice *pci_dev)
> static PCIDeviceInfo virtio_info[] = {
> {
> .qdev.name = "virtio-blk-pci",
> + .qdev.driver_name = "virtio-blk",
> .qdev.size = sizeof(VirtIOPCIProxy),
> .init = virtio_blk_init_pci,
> .exit = virtio_blk_exit_pci,
> @@ -782,6 +783,7 @@ static PCIDeviceInfo virtio_info[] = {
> .qdev.reset = virtio_pci_reset,
> },{
> .qdev.name = "virtio-net-pci",
> + .qdev.driver_name = "ethernet",
> .qdev.size = sizeof(VirtIOPCIProxy),
> .init = virtio_net_init_pci,
> .exit = virtio_net_exit_pci,
Do we want a free-for-all ad hoc set of values for driver_name? The
values become ABI instantly... Can we adopt some existing set of names
for device classes? Else, can we define our own with a bit more
control?
Note that a single device could implement more than one device class. I
guess a sane PCI device would do that with a separate function each, so
we'd be fine there. Do we want to hardcode "single class per qdev"?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
