Re: [Qemu-devel] [PATCH 02/16] net: Use hubs for the vlan feature

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 2:55 PM, Laszlo Ersek <address@hidden> wrote:
> On 07/20/12 14:01, Stefan Hajnoczi wrote:
>
>> diff --git a/net.c b/net.c
>> index e7a8d81..f2752c7 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -25,6 +25,7 @@
>>
>>  #include "config-host.h"
>>
>> +#include "net/hub.h"
>>  #include "net/tap.h"
>>  #include "net/socket.h"
>>  #include "net/dump.h"
>
> This includes "net/hub.h" for the second time in "net.c" (still true at
> the end of the series). Causes no problems, of course.

Thanks, will remove.

>> @@ -876,17 +879,17 @@ static int net_client_init1(const void *object, int 
>> is_netdev, Error **errp)
>>      }
>>
>>      if (net_client_init_fun[opts->kind]) {
>> -        VLANState *vlan = NULL;
>> +        VLANClientState *peer = NULL;
>>
>>          /* Do not add to a vlan if it's a -netdev or a nic with a netdev=
>>           * parameter. */
>>          if (!is_netdev &&
>>              (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
>>               !opts->nic->has_netdev)) {
>> -            vlan = qemu_find_vlan(u.net->has_vlan ? u.net->vlan : 0, true);
>> +            peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, 
>> NULL);
>>          }
>
> "u.net->vlan" is int32_t ('int32' in the schema
> <http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg01675.html>); 
> net_hub_add_port()
> expects unsigned.

Will have to decide how to handle the vlan id input.  Ideally the
option itself would be type-checked in enough detail (uint32_t,
int32_t, etc) so that we can just use the value here.  I'll figure out
which way to resolve this best.

>> diff --git a/net/dump.h b/net/dump.h
>> index 0fa2dd7..a1beb0d 100644
>> --- a/net/dump.h
>> +++ b/net/dump.h
>> @@ -28,6 +28,6 @@
>>  #include "qapi-types.h"
>>
>>  int net_init_dump(const NetClientOptions *opts, const char *name,
>> -                  VLANState *vlan);
>> +                  VLANClientState *vlan);
>>
>>  #endif /* QEMU_NET_DUMP_H */
>
> Should be called "peer", but it will be renamed later on. Same for
> "net/slirp.h".

I'll push the renames as early as possible in the series.  Thanks.

Stefan