From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38472 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OMGCa-0006E1-Je for qemu-devel@nongnu.org; Wed, 09 Jun 2010 03:59:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OMGCZ-0007cB-Fn for qemu-devel@nongnu.org; Wed, 09 Jun 2010 03:59:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36924) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OMGCZ-0007c0-5U for qemu-devel@nongnu.org; Wed, 09 Jun 2010 03:59:55 -0400 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o597xq5l020718 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 9 Jun 2010 03:59:53 -0400 From: Juan Quintela In-Reply-To: (Markus Armbruster's message of "Wed, 09 Jun 2010 08:37:26 +0200") References: <356ef9bdde008d695e7c75dd1566222d6160d4b6.1276011638.git.amit.shah@redhat.com> <20100608172525.GB10305@amit-laptop.redhat.com> Date: Wed, 09 Jun 2010 09:59:50 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: [PATCH v2] net: Fix hotplug with pci_add List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Amit Shah , Gerd Hoffmann , qemu list Markus Armbruster wrote: > Amit Shah writes: > >> On (Tue) Jun 08 2010 [18:33:00], Markus Armbruster wrote: >>> Amit Shah writes: >>> >>> > The correct model type wasn't getting added when hotplugging nics with >>> > pci_add. >>> > >>> > Testcase: start VM with default nic type. In the qemu_monitor: >>> > >>> > (qemu) pci_add auto nic model=virtio >>> > >>> > This results in a nic hot-plug of the same nic type as the default. >>> >>> Works fine for me on master, fd1dc858370d9a9ac7ea2512812c3a152ee6484b. >>> What am I doing wrong? >> >> Did you start with a virtio nic added? The 'default' here is the nic >> type that's added as the first nic. Try this: start a VM with model >> e1000 and use pci_add to add a nic type of virtio. > > I do. Nevertheless, "pci_add auto nic model=e1000" adds an e1000. > > Also works if I start without a NIC. > > Ah, if I start with a -net nic, then pci_add breaks as described! > > Now my next question is *how* your patch fixes this. It's not at all > obvious to me. Far away form the patch file. Look at: hw/pci-hotplug.c static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, const char *devaddr, const char *opts_str) { ..... ret = net_client_init(mon, opts, 0); ..... return pci_nic_init(&nd_table[ret], "rtl8139", devaddr); } You can see that accessing nd_table with value 1 as before was wrong. BTW, once here, didn't default nic should be e1000? not rtl8139? >>> > This was broken in 5294e2c774f120e10b44652ac143abda356f44eb >>> > >>> > Also changes the behaviour where no .init is defined for a >>> > net_client_type. Previously, 0 was returned, which indicated the init >>> > was successful and that 0 was the index into the nd_tables[] array. >>> > Return -1, indicating unsuccessful init, in such a case. >>> >>> The only element of net_client_types[] without an init() method is type >>> "none", index 0. So, doesn't this break -net none? And what does it >>> fix? >> >> The net_client_types[] index isn't relevant here. -net none works fine, >> no problem. > > Let me rephrase: Behavior changes for -net types without an init() > method. The only one without an init() method is "none". Before, > net_client_init() succeeded for it. Now it fails. What's the impact of > that change? And why does it make sense? Here, I don't know. Later, Juan.