From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NkcVc-0002N2-T1 for qemu-devel@nongnu.org; Thu, 25 Feb 2010 07:08:00 -0500 Received: from [199.232.76.173] (port=39249 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NkcVc-0002Me-Ci for qemu-devel@nongnu.org; Thu, 25 Feb 2010 07:08:00 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NkcVa-0001gM-Jv for qemu-devel@nongnu.org; Thu, 25 Feb 2010 07:08:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45579) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NkcVZ-0001gA-Qe for qemu-devel@nongnu.org; Thu, 25 Feb 2010 07:07:58 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o1PC7uRs018110 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 25 Feb 2010 07:07:56 -0500 From: Markus Armbruster References: <20100225114256.GC9116@redhat.com> Date: Thu, 25 Feb 2010 13:07:54 +0100 In-Reply-To: <20100225114256.GC9116@redhat.com> (Michael S. Tsirkin's message of "Thu, 25 Feb 2010 13:42:56 +0200") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: [PATCH] qdev: Catch attempt to attach more than one device to a netdev List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Mark McLoughlin , qemu-devel@nongnu.org, Gerd Hoffmann "Michael S. Tsirkin" writes: > On Thu, Feb 25, 2010 at 11:10:15AM +0100, Markus Armbruster wrote: >> Guest device and host netdev are peers, i.e. it's a 1:1 relation. >> However, we fail to enforce that: >> >> $ qemu -nodefaults --nographic -netdev user,id=net0 -device e1000,netdev=net0 -device virtio-net-pci,netdev=net0 -monitor stdio >> QEMU 0.12.50 monitor - type 'help' for more information >> (qemu) info network >> Devices not on any VLAN: >> net0: net=10.0.2.0, restricted=n peer=virtio-net-pci.0 >> e1000.0: model=e1000,macaddr=52:54:00:12:34:56 peer=net0 >> virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:57 peer=net0 >> >> It's all downhill from there. >> >> Signed-off-by: Markus Armbruster >> --- >> hw/qdev-properties.c | 3 +++ >> net.c | 1 + >> 2 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c >> index 277ff9e..89efd91 100644 >> --- a/hw/qdev-properties.c >> +++ b/hw/qdev-properties.c >> @@ -341,6 +341,9 @@ static int parse_netdev(DeviceState *dev, Property *prop, const char *str) >> *ptr = qemu_find_netdev(str); >> if (*ptr == NULL) >> return -1; >> + if ((*ptr)->peer) { >> + return -1; >> + } > > Not even -EBUSY? > Can this produce a helpful diagnostic message? Callers take care of that. It's how property parse methods work. In this case: property "virtio-net-pci.netdev": failed to parse "net0" can't set property "netdev" to "net0" for "virtio-net-pci" >> return 0; >> } >> >> diff --git a/net.c b/net.c >> index a1bf49f..e6c96d3 100644 >> --- a/net.c >> +++ b/net.c >> @@ -245,6 +245,7 @@ VLANClientState *qemu_new_net_client(NetClientInfo *info, >> QTAILQ_INSERT_TAIL(&vc->vlan->clients, vc, next); >> } else { >> if (peer) { >> + assert(!peer->peer); > > Do we ever get herE? If "get here" means "conditional entered": yes. For instance, with the command line from the commit message, we get here for each of the -device, like this: #0 qemu_new_net_client (info=0x8b2ce0, vlan=0x0, peer=0xcd83f0, model=0x5ea826 "e1000", name=0x0) at /work/armbru/qemu/net.c:227 #1 0x000000000047682f in qemu_new_nic (info=0x8b2ce0, conf=0x7fffece4a270, model=0x5ea826 "e1000", name=0x0, opaque=0x7fffece4a010) at /work/armbru/qemu/net.c:273 #2 0x0000000000447c74 in pci_e1000_init (pci_dev=0x7fffece4a010) at /work/armbru/qemu/hw/e1000.c:1125 #3 0x0000000000422335 in pci_qdev_init (qdev=0x7fffece4a010, base=0x8b2dc0) at /work/armbru/qemu/hw/pci.c:1647 #4 0x00000000004bbb29 in qdev_init (dev=0x7fffece4a010) at /work/armbru/qemu/hw/qdev.c:265 If "get here" means "assertion fails": only if somebody breaks something elsewhere. And then assertion will slap his wrist. >> vc->peer = peer; >> peer->peer = vc; >> } >> -- >> 1.6.6 >> >>