[Qemu-devel] Re: [PATCH 3/7] net: create the VLANClientState for NICs early

[Gerd Hoffmann]

  Hi,

> diff --git a/hw/e1000.c b/hw/e1000.c
> index 00f6a57..d9c9f79 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -1107,11 +1107,11 @@ static int pci_e1000_init(PCIDevice *pci_dev)
>       checksum = (uint16_t) EEPROM_SUM - checksum;
>       d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum;
>
> -    d->vc = qemu_new_vlan_client(NET_CLIENT_TYPE_NIC,
> -                                 d->conf.vlan, d->conf.peer,
> -                                 d->dev.qdev.info->name, d->dev.qdev.id,
> -                                 e1000_can_receive, e1000_receive, NULL,
> -                                 NULL, e1000_cleanup, d);
> +    d->vc = d->conf.client;
> +    d->vc->opaque = d;
> +    d->vc->can_receive = e1000_can_receive;
> +    d->vc->receive = e1000_receive;
> +    d->vc->cleanup = e1000_cleanup;
>       d->vc->link_status_changed = e1000_set_link_status;
>
>       qemu_format_nic_info_str(d->vc, macaddr);

... and now -device $nic,args is completely broken.

There are netdev, vlan and net-client properties but none of them will work.


If you want to have net.c prepare vlanclientstate (there might be good 
reasons for it), then it must be properly:

  * Create a named vlanclientstate.
  * Have drivers lookup the vlanclientstate by name.  Pretty much like
    the netdev peer lookup works today, only with the difference that
    it is used directly instead of being passed as peer on vlanclient
    creation.

i.e. something like

  -netdev client,id=foo,<moreargs>
  -device e1000,client=foo,mac=11:22:33:44:55:66

It is probably a good idea to zap the vlan and netdev properties then, 
so we don't have tons of different ways to setup a nic.  Fortunaly we 
had no release with the vlan+netdev properties yet, so this shouldn't be 
a backward compatibility issue IMHO.

Also take care that the creation and destruction is symmetric.  If net.c 
creates the vlanclient it should also net.c's job to clean it up, i.e. 
all the qemu_del_vlan_client() calls in the nic drivers should go away, 
otherwise hotplug will have some unpleasant surprises for you.

cheers,
  Gerd