On Mon, Jun 17, 2013 at 09:35:10PM +0800, Amos Kong wrote: > QEMU allocates macaddr to nic if user doesn't assigne macaddr. > But we didn't check if the allocated macaddr is used, it might > cause macaddr repeated. > > # qemu -device e1000,netdev=h1,mac=52:54:00:12:34:56 > (qemu) device_add e1000 > (qemu) info network > e1000.0: index=0,type=nic,model=e1000,macaddr=52:54:00:12:34:56 > \ h1: index=0,type=user,net=10.0.2.0,restrict=off > e1000.1: index=0,type=nic,model=e1000,macaddr=52:54:00:12:34:56 > > This patch adds a check in allocating macaddr, reallocate macaddr > if it's used by other nic. > > Signed-off-by: Amos Kong <ak...@redhat.com>
I'm not sure this is not exactly what was intended in this case. Also this ptotects against an unlikely case of mixing implicit and explicit addresses, but not against a likely case of multiple qemu on same LAN using same MAC. In short, implicit mac is a bad idea, don't use it. I'd ack a patch that marks mac non-optional in help text and documentation. > --- > net/net.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/net/net.c b/net/net.c > index 43a74e4..f019da4 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -143,15 +143,43 @@ void qemu_macaddr_default_if_unset(MACAddr *macaddr) > { > static int index = 0; > static const MACAddr zero = { .a = { 0,0,0,0,0,0 } }; > + char info_str[256]; > + NetClientState *nc, *peer; > + NetClientOptionsKind type; > > if (memcmp(macaddr, &zero, sizeof(zero)) != 0) > return; > + > +realloc_mac: > macaddr->a[0] = 0x52; > macaddr->a[1] = 0x54; > macaddr->a[2] = 0x00; > macaddr->a[3] = 0x12; > macaddr->a[4] = 0x34; > macaddr->a[5] = 0x56 + index++; > + > + QTAILQ_FOREACH(nc, &net_clients, next) { > + peer = nc->peer; > + type = nc->info->type; > + > + if (net_hub_id_for_client(nc, NULL) == 0) { > + continue; > + } > + > + if (!peer || type == NET_CLIENT_OPTIONS_KIND_NIC) { > + snprintf(info_str, sizeof(info_str), > + "model=%s,macaddr=%02x:%02x:%02x:%02x:%02x:%02x", > + nc->model, > + macaddr->a[0], macaddr->a[1], macaddr->a[2], > + macaddr->a[3], macaddr->a[4], macaddr->a[5]); > + > + /* reallocate macaddr if it's used by other nic */ > + if (!strcmp(nc->info_str, info_str)) { > + goto realloc_mac; Please don't code loops like that. > + } > + } > + } > + > } > > /** > -- > 1.8.1.4