[Qemu-devel] [PATCH] qdev: Revert the hack to let -net nic and pci_add set qdev ID

[Qemu-devel] [PATCH] qdev: Revert the hack to let -net nic and pci_add set qdev ID

[Markus Armbruster]

Setting the ID in pci_nic_init() is a blatant violation of the
DeviceState abstraction.  Which even carries a comment advising
against this:

/* This structure should not be accessed directly.  We declare it here
   so that it can be embedded in individual device state structures.  */

What's worse, it bypasses the code ensuring unique qdev IDs: "-device
virtio-net-pci,id=foo -net nic,id=foo -net nic,name=foo" happily
creates three qdevs with ID "foo".  That's because qdev relies on
qemu_opts_create() to ensure unique IDs, but -net nic uses a different
QemuOptsList, which means id is in a different namespace.  And its
name is not checked for uniqueness at all.

-net nic and pci_add are legacy.  Use -device and device_add if you
want a NIC with a qdev ID.

This reverts what's still left of commit eb54b6dc "qdev: add id=
support for pci nics."

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index cbbd1dd..fab8c09 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1446,8 +1446,6 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
 
     pci_dev = pci_create(bus, devfn, pci_nic_names[i]);
     dev = &pci_dev->qdev;
-    if (nd->name)
-        dev->id = qemu_strdup(nd->name);
     qdev_set_nic_properties(dev, nd);
     if (qdev_init(dev) < 0)
         return NULL;
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH] qdev: Revert the hack to let -net nic and pci_add set qdev ID

[Anthony Liguori]

On 06/08/2010 06:54 AM, Markus Armbruster wrote:
> Setting the ID in pci_nic_init() is a blatant violation of the
> DeviceState abstraction.  Which even carries a comment advising
> against this:
>
> /* This structure should not be accessed directly.  We declare it here
>     so that it can be embedded in individual device state structures.  */
>
> What's worse, it bypasses the code ensuring unique qdev IDs: "-device
> virtio-net-pci,id=foo -net nic,id=foo -net nic,name=foo" happily
> creates three qdevs with ID "foo".  That's because qdev relies on
> qemu_opts_create() to ensure unique IDs, but -net nic uses a different
> QemuOptsList, which means id is in a different namespace.  And its
> name is not checked for uniqueness at all.
>
> -net nic and pci_add are legacy.  Use -device and device_add if you
> want a NIC with a qdev ID.
>
> This reverts what's still left of commit eb54b6dc "qdev: add id=
> support for pci nics."
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   hw/pci.c |    2 --
>   1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index cbbd1dd..fab8c09 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1446,8 +1446,6 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
>
>       pci_dev = pci_create(bus, devfn, pci_nic_names[i]);
>       dev =&pci_dev->qdev;
> -    if (nd->name)
> -        dev->id = qemu_strdup(nd->name);
>       qdev_set_nic_properties(dev, nd);
>       if (qdev_init(dev)<  0)
>           return NULL;
>