[Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

  Hi,

This patch series brings driver classes to qdev.

DeviceInfo gets a new field specifying what kind of device this is.
For starters there are Sound cards and Network cards.  The number
of device classes will probably grow over time.

The device class will be shown in the listing printed by '-device ?', so
users and management apps can figure what kinds of -- say -- network
cards are supported by that particular qemu binary.

Last patch in the series makes pci_nic_init() use this as well.  The
hard-coded driver name tables are gone.  Compiling out drivers for
certain pci network cards is easy now, just lose the object file.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/5] qdev/class: core

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   29 ++++++++++++++++++++++++-----
 hw/qdev.h |    9 +++++++++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 009e68d..67cca3b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -46,7 +46,19 @@ void qdev_register(DeviceInfo *info)
     device_info_list = info;
 }
 
-static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
+const char *qdev_class_name(DeviceClass class)
+{
+    static const char *names[] = {
+        [ DEV_CLASS_UNSPECIFIED ] = "<unspecified>",
+        [ DEV_CLASS_NETWORK ]     = "network",
+        [ DEV_CLASS_SOUND ]       = "sound",
+    };
+    if (class < ARRAY_SIZE(names))
+        return names[class];
+    return "<invalid>";
+}
+
+DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name, DeviceClass class)
 {
     DeviceInfo *info;
 
@@ -54,6 +66,8 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
     for (info = device_info_list; info != NULL; info = info->next) {
         if (bus_info && info->bus_info != bus_info)
             continue;
+        if (class && info->class != class)
+            continue;
         if (strcmp(info->name, name) != 0)
             continue;
         return info;
@@ -63,6 +77,8 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
     for (info = device_info_list; info != NULL; info = info->next) {
         if (bus_info && info->bus_info != bus_info)
             continue;
+        if (class && info->class != class)
+            continue;
         if (!info->alias)
             continue;
         if (strcmp(info->alias, name) != 0)
@@ -87,7 +103,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
         bus = main_system_bus;
     }
 
-    info = qdev_find_info(bus->info, name);
+    info = qdev_find_info(bus->info, name, 0);
     if (!info) {
         hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);
     }
@@ -121,6 +137,8 @@ DeviceState *qdev_device_add(const char *cmdline)
                 fprintf(stderr, ", alias \"%s\"", info->alias);
             if (info->desc)
                 fprintf(stderr, ", desc \"%s\"", info->desc);
+            if (info->class)
+                fprintf(stderr, ", class \"%s\"", qdev_class_name(info->class));
             if (info->no_user)
                 fprintf(stderr, ", no-user");
             fprintf(stderr, "\n");
@@ -131,7 +149,7 @@ DeviceState *qdev_device_add(const char *cmdline)
         params = cmdline + n;
         get_param_value(addr, sizeof(addr), "addr", params);
     }
-    info = qdev_find_info(NULL, driver);
+    info = qdev_find_info(NULL, driver, 0);
 
     if (!info) {
         fprintf(stderr, "Device \"%s\" not found.  Try -device '?' for a list.\n",
@@ -378,7 +396,8 @@ void do_info_qdrv(Monitor *mon)
     DeviceInfo *info;
 
     for (info = device_info_list; info != NULL; info = info->next) {
-        monitor_printf(mon, "name \"%s\", bus %s\n",
-                       info->name, info->bus_info->name);
+        monitor_printf(mon, "name \"%s\", bus %s, class %s\n",
+                       info->name, info->bus_info->name,
+                       qdev_class_name(info->class));
     }
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index cf6083f..a35b712 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -79,10 +79,17 @@ typedef void (*qdev_initfn)(DeviceState *dev, DeviceInfo *info);
 typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
               int unit);
 
+typedef enum DeviceClass {
+    DEV_CLASS_UNSPECIFIED = 0,
+    DEV_CLASS_NETWORK,
+    DEV_CLASS_SOUND,
+} DeviceClass;
+
 struct DeviceInfo {
     const char *name;
     const char *alias;
     const char *desc;
+    DeviceClass class;
     size_t size;
     Property *props;
     int no_user;
@@ -94,6 +101,8 @@ struct DeviceInfo {
 };
 
 void qdev_register(DeviceInfo *info);
+const char *qdev_class_name(DeviceClass class);
+DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name, DeviceClass class);
 
 /* Register device properties.  */
 /* GPIO inputs also double as IRQ sinks.  */
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/5] qdev/class: tag sound

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ac97.c   |    1 +
 hw/es1370.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index 873469d..7f4ab23 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1367,6 +1367,7 @@ int ac97_init (PCIBus *bus)
 static PCIDeviceInfo ac97_info = {
     .qdev.name    = "AC97",
     .qdev.size    = sizeof (PCIAC97LinkState),
+    .qdev.class   = DEV_CLASS_SOUND,
     .init         = ac97_initfn,
 };
 
diff --git a/hw/es1370.c b/hw/es1370.c
index 0cfdb76..d882d66 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -1056,6 +1056,7 @@ static PCIDeviceInfo es1370_info = {
     .qdev.name    = "ES1370",
     .qdev.desc    = "Ensoniq AudioPCI Sound Card",
     .qdev.size    = sizeof (PCIES1370State),
+    .qdev.class   = DEV_CLASS_SOUND,
     .init         = es1370_initfn,
 };
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/5] qdev/class: tag network

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/e1000.c      |    7 ++++---
 hw/eepro100.c   |   21 ++++++++++++---------
 hw/ne2000.c     |    7 ++++---
 hw/pcnet.c      |    8 +++++---
 hw/rtl8139.c    |    7 ++++---
 hw/virtio-pci.c |    7 ++++---
 6 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 4ac8918..8f53e00 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -1126,9 +1126,10 @@ static void pci_e1000_init(PCIDevice *pci_dev)
 }
 
 static PCIDeviceInfo e1000_info = {
-    .qdev.name = "e1000",
-    .qdev.size = sizeof(E1000State),
-    .init      = pci_e1000_init,
+    .qdev.name  = "e1000",
+    .qdev.size  = sizeof(E1000State),
+    .qdev.class = DEV_CLASS_NETWORK,
+    .init       = pci_e1000_init,
 };
 
 static void e1000_register_devices(void)
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 85446ed..a64c997 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1794,17 +1794,20 @@ static void pci_i82559er_init(PCIDevice *dev)
 
 static PCIDeviceInfo eepro100_info[] = {
     {
-        .qdev.name = "i82551",
-        .qdev.size = sizeof(PCIEEPRO100State),
-        .init      = pci_i82551_init,
+        .qdev.name  = "i82551",
+        .qdev.size  = sizeof(PCIEEPRO100State),
+        .qdev.class = DEV_CLASS_NETWORK,
+        .init       = pci_i82551_init,
     },{
-        .qdev.name = "i82557b",
-        .qdev.size = sizeof(PCIEEPRO100State),
-        .init      = pci_i82557b_init,
+        .qdev.name  = "i82557b",
+        .qdev.size  = sizeof(PCIEEPRO100State),
+        .qdev.class = DEV_CLASS_NETWORK,
+        .init       = pci_i82557b_init,
     },{
-        .qdev.name = "i82559er",
-        .qdev.size = sizeof(PCIEEPRO100State),
-        .init      = pci_i82559er_init,
+        .qdev.name  = "i82559er",
+        .qdev.size  = sizeof(PCIEEPRO100State),
+        .qdev.class = DEV_CLASS_NETWORK,
+        .init       = pci_i82559er_init,
     },{
         /* end of list */
     }
diff --git a/hw/ne2000.c b/hw/ne2000.c
index 66ff29d..a532ae3 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -833,9 +833,10 @@ static void pci_ne2000_init(PCIDevice *pci_dev)
 }
 
 static PCIDeviceInfo ne2000_info = {
-    .qdev.name = "ne2k_pci",
-    .qdev.size = sizeof(PCINE2000State),
-    .init      = pci_ne2000_init,
+    .qdev.name  = "ne2k_pci",
+    .qdev.size  = sizeof(PCINE2000State),
+    .qdev.class = DEV_CLASS_NETWORK,
+    .init       = pci_ne2000_init,
 };
 
 static void ne2000_register_devices(void)
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 22ab6be..fc84bc0 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -2144,6 +2144,7 @@ static SysBusDeviceInfo lance_info = {
     .init = lance_init,
     .qdev.name  = "lance",
     .qdev.size  = sizeof(SysBusPCNetState),
+    .qdev.class = DEV_CLASS_NETWORK,
     .qdev.props = (Property[]) {
         {
             .name   = "dma",
@@ -2157,9 +2158,10 @@ static SysBusDeviceInfo lance_info = {
 #endif /* TARGET_SPARC */
 
 static PCIDeviceInfo pcnet_info = {
-    .qdev.name = "pcnet",
-    .qdev.size = sizeof(PCIPCNetState),
-    .init      = pci_pcnet_init,
+    .qdev.name  = "pcnet",
+    .qdev.size  = sizeof(PCIPCNetState),
+    .qdev.class = DEV_CLASS_NETWORK,
+    .init       = pci_pcnet_init,
 };
 
 static void pcnet_register_devices(void)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 91165db..d16269c 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3500,9 +3500,10 @@ static void pci_rtl8139_init(PCIDevice *dev)
 }
 
 static PCIDeviceInfo rtl8139_info = {
-    .qdev.name = "rtl8139",
-    .qdev.size = sizeof(PCIRTL8139State),
-    .init      = pci_rtl8139_init,
+    .qdev.name  = "rtl8139",
+    .qdev.size  = sizeof(PCIRTL8139State),
+    .qdev.class = DEV_CLASS_NETWORK,
+    .init       = pci_rtl8139_init,
 };
 
 static void rtl8139_register_devices(void)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 39e290d..d28d7b3 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -472,9 +472,10 @@ static PCIDeviceInfo virtio_info[] = {
         .qdev.size = sizeof(VirtIOPCIProxy),
         .init      = virtio_blk_init_pci,
     },{
-        .qdev.name = "virtio-net-pci",
-        .qdev.size = sizeof(VirtIOPCIProxy),
-        .init      = virtio_net_init_pci,
+        .qdev.name  = "virtio-net-pci",
+        .qdev.size  = sizeof(VirtIOPCIProxy),
+        .qdev.class = DEV_CLASS_NETWORK,
+        .init       = virtio_net_init_pci,
     },{
         .qdev.name = "virtio-console-pci",
         .qdev.size = sizeof(VirtIOPCIProxy),
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/5] qdev/class: helper function to get a list of drivers.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   17 +++++++++++++++++
 hw/qdev.h |    2 ++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 67cca3b..baa7d16 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -88,6 +88,23 @@ DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name, DeviceClass clas
     return NULL;
 }
 
+size_t qdev_list_devices(BusInfo *bus_info, DeviceClass class,
+                         char *dest, size_t len)
+{
+    DeviceInfo *info;
+    size_t pos = 0;
+
+    for (info = device_info_list; info != NULL; info = info->next) {
+        if (bus_info && info->bus_info != bus_info)
+            continue;
+        if (class && info->class != class)
+            continue;
+        pos += snprintf(dest+pos, len-pos, "%s%s", pos ? "," : "",
+                        info->alias ? info->alias : info->name);
+    }
+    return pos;
+}
+
 /* Create a new device.  This only initializes the device state structure
    and allows properties to be set.  qdev_init should be called to
    initialize the actual device emulation.  */
diff --git a/hw/qdev.h b/hw/qdev.h
index a35b712..3203a3e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -103,6 +103,8 @@ struct DeviceInfo {
 void qdev_register(DeviceInfo *info);
 const char *qdev_class_name(DeviceClass class);
 DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name, DeviceClass class);
+size_t qdev_list_devices(BusInfo *bus_info, DeviceClass class,
+                         char *dest, size_t len);
 
 /* Register device properties.  */
 /* GPIO inputs also double as IRQ sinks.  */
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/5] qdev/class: make pci_nic_init() use qdev's device list.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c        |   58 +++++++++++++++++++-----------------------------------
 hw/virtio-pci.c |    1 +
 2 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index cc4882e..948611e 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -784,53 +784,37 @@ DeviceState *pci_create(const char *name, const char *devaddr)
     return dev;
 }
 
-static const char * const pci_nic_models[] = {
-    "ne2k_pci",
-    "i82551",
-    "i82557b",
-    "i82559er",
-    "rtl8139",
-    "e1000",
-    "pcnet",
-    "virtio",
-    NULL
-};
-
-static const char * const pci_nic_names[] = {
-    "ne2k_pci",
-    "i82551",
-    "i82557b",
-    "i82559er",
-    "rtl8139",
-    "e1000",
-    "pcnet",
-    "virtio-net-pci",
-    NULL
-};
-
 /* Initialize a PCI NIC.  */
 PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
                         const char *default_devaddr)
 {
     const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
+    char buf[256];
     DeviceState *dev;
-    int i;
+    DeviceInfo *info;
+    int ret = 0;
 
-    qemu_check_nic_model_list(nd, pci_nic_models, default_model);
-
-    for (i = 0; pci_nic_models[i]; i++) {
-        if (strcmp(nd->model, pci_nic_models[i]) == 0) {
-            dev = pci_create(pci_nic_names[i], devaddr);
-            if (nd->id)
-                snprintf(dev->id, sizeof(dev->id), "%s", nd->id);
-            dev->nd = nd;
-            qdev_init(dev);
-            nd->private = dev;
-            return DO_UPCAST(PCIDevice, qdev, dev);
+    if (!nd->model)
+        nd->model = strdup(default_model);
+
+    info = qdev_find_info(&pci_bus_info, nd->model, DEV_CLASS_NETWORK);
+    if (!info) {
+        if (strcmp(nd->model, "?") != 0) {
+            fprintf(stderr, "qemu: Unsupported NIC model: %s\n", nd->model);
+            ret = 1;
         }
+        qdev_list_devices(&pci_bus_info, DEV_CLASS_NETWORK, buf, sizeof(buf));
+        fprintf(stderr, "qemu: Supported NIC models: %s\n", buf);
+        exit(ret);
     }
 
-    return NULL;
+    dev = pci_create(info->name, devaddr);
+    if (nd->id)
+        snprintf(dev->id, sizeof(dev->id), "%s", nd->id);
+    dev->nd = nd;
+    qdev_init(dev);
+    nd->private = dev;
+    return DO_UPCAST(PCIDevice, qdev, dev);
 }
 
 typedef struct {
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d28d7b3..0b3f41f 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -473,6 +473,7 @@ static PCIDeviceInfo virtio_info[] = {
         .init      = virtio_blk_init_pci,
     },{
         .qdev.name  = "virtio-net-pci",
+        .qdev.alias = "virtio",
         .qdev.size  = sizeof(VirtIOPCIProxy),
         .qdev.class = DEV_CLASS_NETWORK,
         .init       = virtio_net_init_pci,
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Paul Brook]

On Thursday 09 July 2009, Gerd Hoffmann wrote:
>   Hi,
>
> This patch series brings driver classes to qdev.
>
> DeviceInfo gets a new field specifying what kind of device this is.
> For starters there are Sound cards and Network cards.  The number
> of device classes will probably grow over time.
>
> The device class will be shown in the listing printed by '-device ?', so
> users and management apps can figure what kinds of -- say -- network
> cards are supported by that particular qemu binary.

I'm not sure this is a good idea. You should be able to figure it out from the 
device properties. Plus there's no reason why a device can't implement 
multiple "classes" of device.

Paul

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

On 07/09/09 15:19, Paul Brook wrote:
> On Thursday 09 July 2009, Gerd Hoffmann wrote:
>>    Hi,
>>
>> This patch series brings driver classes to qdev.
>
> I'm not sure this is a good idea. You should be able to figure it out from the
> device properties.

I don't think this will work out in the general case.  Some devices have 
properties which can be used for that, i.e. network cards have a mac 
address (no attribute yet but I expect it will come some day).  Not 
every device class has specific properties you can use to identify them 
though.  How would you identify a sound card for example?

> Plus there's no reason why a device can't implement
> multiple "classes" of device.

Hmm, good point.  How about using a bitmap then?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Paul Brook]

On Thursday 09 July 2009, Gerd Hoffmann wrote:
> On 07/09/09 15:19, Paul Brook wrote:
> > On Thursday 09 July 2009, Gerd Hoffmann wrote:
> >>    Hi,
> >>
> >> This patch series brings driver classes to qdev.
> >
> > I'm not sure this is a good idea. You should be able to figure it out
> > from the device properties.
>
> I don't think this will work out in the general case.  Some devices have
> properties which can be used for that, i.e. network cards have a mac
> address (no attribute yet but I expect it will come some day).  Not
> every device class has specific properties you can use to identify them
> though.  How would you identify a sound card for example?

Ok, put it annother way: Why do you need to identify them? Why would libvirt 
care whether a device is (say) a sound card or a VGA adapter? In principle 
there's no reason why you shouldn't have many or none of both.

IMHO the only reason we have the current grouping is because it's forced on us 
by the various commandline options which mix host and machine configuration.

Paul

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

On 07/09/09 15:48, Paul Brook wrote:
> On Thursday 09 July 2009, Gerd Hoffmann wrote:

>> I don't think this will work out in the general case.  Some devices have
>> properties which can be used for that, i.e. network cards have a mac
>> address (no attribute yet but I expect it will come some day).  Not
>> every device class has specific properties you can use to identify them
>> though.  How would you identify a sound card for example?
>
> Ok, put it annother way: Why do you need to identify them? Why would libvirt
> care whether a device is (say) a sound card or a VGA adapter?

Because libvirt-based applications want present that to the user?

The usual GUI workflow for adding devices is a two-step process:  First 
pick the device class, then pick the actual device from a (short) list.

> In principle
> there's no reason why you shouldn't have many or none of both.

Sure.  But when you ask virt-manager to add a sound card to your virtual 
machine you don't want to have a VGA adapter in the list of possible 
devices.

> IMHO the only reason we have the current grouping is because it's forced on us
> by the various commandline options which mix host and machine configuration.

That is actually another reason:  Allow easy support of legacy command 
line options.  Once all sound cards are converted to qdev '-soundhw ?' 
could list all sound cards by just walking the driver list and print the 
ones with class = SOUND.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Anthony Liguori]

Gerd Hoffmann wrote:
> On 07/09/09 15:48, Paul Brook wrote:
>> On Thursday 09 July 2009, Gerd Hoffmann wrote:
>
>>> I don't think this will work out in the general case.  Some devices 
>>> have
>>> properties which can be used for that, i.e. network cards have a mac
>>> address (no attribute yet but I expect it will come some day).  Not
>>> every device class has specific properties you can use to identify them
>>> though.  How would you identify a sound card for example?
>>
>> Ok, put it annother way: Why do you need to identify them? Why would 
>> libvirt
>> care whether a device is (say) a sound card or a VGA adapter?
>
> Because libvirt-based applications want present that to the user?
>
> The usual GUI workflow for adding devices is a two-step process:  
> First pick the device class, then pick the actual device from a 
> (short) list.

Wouldn't the definitions of classes be something that the GUI would want 
to define?

I think we do want a way to say that this device provides graphics 
capability and have that exposed to through the monitor.  But we also 
have to consider the case where a single device exposes multiple types 
of capabilities.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

On 07/09/09 17:46, Anthony Liguori wrote:

> I think we do want a way to say that this device provides graphics
> capability and have that exposed to through the monitor.

That is the case already.  There is both "info qdrv" and "-device ?". 
Right now they don't share code and print different formats which should 
probably be changed.

> But we also
> have to consider the case where a single device exposes multiple types
> of capabilities.

I think we already agreed to move to a bitmap model to handle that.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Paul Brook]

> > But we also
> > have to consider the case where a single device exposes multiple types
> > of capabilities.
>
> I think we already agreed to move to a bitmap model to handle that.

I still think this is best done via properties, rather than as a separate 
feature.

Paul

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

On 07/10/09 11:46, Paul Brook wrote:
>>> But we also
>>> have to consider the case where a single device exposes multiple types
>>> of capabilities.
>> I think we already agreed to move to a bitmap model to handle that.
>
> I still think this is best done via properties, rather than as a separate
> feature.

Properties really don't fit here IMO.  Property values belong to a 
device *instance*, not to a device *driver*.  If you stick two e1000 
network cards into your virtual machine both instances will have 
different pci bus and mac addresses.  They wouldn't have different 
device capabilities though.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Paul Brook]

On Friday 10 July 2009, Gerd Hoffmann wrote:
> On 07/10/09 11:46, Paul Brook wrote:
> >>> But we also
> >>> have to consider the case where a single device exposes multiple types
> >>> of capabilities.
> >>
> >> I think we already agreed to move to a bitmap model to handle that.
> >
> > I still think this is best done via properties, rather than as a separate
> > feature.
>
> Properties really don't fit here IMO.  Property values belong to a
> device *instance*, not to a device *driver*.  If you stick two e1000
> network cards into your virtual machine both instances will have
> different pci bus and mac addresses.  They wouldn't have different
> device capabilities though.

No, but the presence of a mac address (or network backend) property tells you 
that it's a network card.

Paul

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Gerd Hoffmann]

On 07/10/09 12:13, Paul Brook wrote:

> No, but the presence of a mac address (or network backend) property tells you
> that it's a network card.

I still think that will not work for all devices.

Network cards can be identified by mac property.  Ok.

Disks can be identified by a (planned) drive property (which links to 
the host config).  By looking at the bus you can figure this is ide, 
scsi, virtio or usb-storage disk.  Fine.

Serial ports, parallel ports and virtio console need a link to a 
CharDriver which can be used to identify this group.  But not to keep 
apart serial / parallel devices.

How do you identify vga cards?
How do you identify sound cards?
How do you identify storage (scsi/ide) controllers?
How do you identify watchdog devices?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/5] qdev: add driver class support.

[Filip Navara]

On Thu, Jul 9, 2009 at 3:19 PM, Paul Brook<paul@codesourcery.com> wrote:
> On Thursday 09 July 2009, Gerd Hoffmann wrote:
>>   Hi,
>>
>> This patch series brings driver classes to qdev.
>>
>> DeviceInfo gets a new field specifying what kind of device this is.
>> For starters there are Sound cards and Network cards.  The number
>> of device classes will probably grow over time.
>>
>> The device class will be shown in the listing printed by '-device ?', so
>> users and management apps can figure what kinds of -- say -- network
>> cards are supported by that particular qemu binary.
>
> I'm not sure this is a good idea. You should be able to figure it out from the
> device properties. Plus there's no reason why a device can't implement
> multiple "classes" of device.
>
> Paul

I was just thinking about the same. There are several multi-class devices and
I don't see a way to represent them with this mechanism.

Best regards,
Filip Navara