From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Jc8XD-0000cD-4G for qemu-devel@nongnu.org; Wed, 19 Mar 2008 20:21:31 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Jc8XB-0000aq-Qa for qemu-devel@nongnu.org; Wed, 19 Mar 2008 20:21:30 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jc8XB-0000ab-Gm for qemu-devel@nongnu.org; Wed, 19 Mar 2008 20:21:29 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Jc8XA-00027F-WA for qemu-devel@nongnu.org; Wed, 19 Mar 2008 20:21:29 -0400 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e34.co.us.ibm.com (8.13.8/8.13.8) with ESMTP id m2K0Iei3010093 for ; Wed, 19 Mar 2008 20:18:40 -0400 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m2K0Jrlv221996 for ; Wed, 19 Mar 2008 18:19:53 -0600 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m2K0JqBI005582 for ; Wed, 19 Mar 2008 18:19:53 -0600 Date: Wed, 19 Mar 2008 19:19:51 -0500 From: Ryan Harper Message-ID: <20080320001951.GI14060@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] PATCH: dont call exit() from pci_nic_init(), let caller handle Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: kvm-devel@lists.sourceforge.net Cc: qemu-devel@nongnu.org While exploring the PCI hotplug code recently posted, I encountered a situation where I don't believe the current behavior is ideal. With hotplug, we can add additional pci-based nic devices like e1000 and rtl8139 from the qemu monitor. If one mistakenly specifies model=ne2000 (the ISA version), qemu just exits. If a command is run from the monitor and specifies bogus values, I don't believe the right behavior is to exit out of the guest entirely. The attached patch (which doesn't apply directly against qemu-cvs since hotplug hasn't been merged) changes pci_nic_init() to return NULL on error instead of exiting and then I've replaced all callers to check the return value and exit(), preserving the existing behavior, but allowing flexibility so hotplug can do the right thing and just report the error rather than exiting the guest. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 ryanh@us.ibm.com diffstat output: ipf.c | 3 ++- mips_malta.c | 6 ++++-- pc.c | 3 ++- pci.c | 4 ++-- ppc_chrp.c | 3 ++- ppc_prep.c | 3 ++- realview.c | 3 ++- sun4u.c | 3 ++- versatilepb.c | 3 ++- 9 files changed, 20 insertions(+), 11 deletions(-) --- Don't call exit() in the pci_nit_init() function, let the caller handle it. This prevents silly things like device-hotplug trying to add a bogus nic and exiting a running guest. That shouldn't happen. All callsites have been fix to keep the existing behavior of exiting on a bogus nic, but device-hotplug.c can handle bogus nic hot-adds without exiting the guest. Signed-off-by: Ryan Harper diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c index 8c5304d..a84e343 100644 --- a/qemu/hw/ipf.c +++ b/qemu/hw/ipf.c @@ -560,7 +560,8 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, } else if (pci_enabled) { if (strcmp(nd->model, "?") == 0) fprintf(stderr, "qemu: Supported ISA NICs: ne2k_isa\n"); - pci_nic_init(pci_bus, nd, -1); + if (!pci_nic_init(pci_bus, nd, -1)) + exit(1); } else if (strcmp(nd->model, "?") == 0) { fprintf(stderr, "qemu: Supported ISA NICs: ne2k_isa\n"); exit(1); diff --git a/qemu/hw/mips_malta.c b/qemu/hw/mips_malta.c index d39a02b..a50d2b9 100644 --- a/qemu/hw/mips_malta.c +++ b/qemu/hw/mips_malta.c @@ -495,9 +495,11 @@ static void network_init (PCIBus *pci_bus) } if (i == 0 && strcmp(nd->model, "pcnet") == 0) { /* The malta board has a PCNet card using PCI SLOT 11 */ - pci_nic_init(pci_bus, nd, 88); + if (!pci_nic_init(pci_bus, nd, 88)) + exit(1); } else { - pci_nic_init(pci_bus, nd, -1); + if (!pci_nic_init(pci_bus, nd, -1)) + exit(1); } } } diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c index 1122b87..0d2e6c3 100644 --- a/qemu/hw/pc.c +++ b/qemu/hw/pc.c @@ -1016,7 +1016,8 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size, } else if (pci_enabled) { if (strcmp(nd->model, "?") == 0) fprintf(stderr, "qemu: Supported ISA NICs: ne2k_isa\n"); - pci_nic_init(pci_bus, nd, -1); + if (!pci_nic_init(pci_bus, nd, -1)) + exit(1); } else if (strcmp(nd->model, "?") == 0) { fprintf(stderr, "qemu: Supported ISA NICs: ne2k_isa\n"); exit(1); diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c index e587410..b8f4fbb 100644 --- a/qemu/hw/pci.c +++ b/qemu/hw/pci.c @@ -691,10 +691,10 @@ PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn) } else if (strcmp(nd->model, "?") == 0) { fprintf(stderr, "qemu: Supported PCI NICs: i82551 i82557b i82559er" " ne2k_pci pcnet rtl8139 e1000 virtio\n"); - exit (1); + return NULL; } else { fprintf(stderr, "qemu: Unsupported NIC: %s\n", nd->model); - exit (1); + return NULL; } nd->devfn = pci_dev->devfn; return pci_dev; diff --git a/qemu/hw/ppc_chrp.c b/qemu/hw/ppc_chrp.c index 4437a10..ff09c6a 100644 --- a/qemu/hw/ppc_chrp.c +++ b/qemu/hw/ppc_chrp.c @@ -268,7 +268,8 @@ static void ppc_core99_init (int ram_size, int vga_ram_size, for(i = 0; i < nb_nics; i++) { if (!nd_table[i].model) nd_table[i].model = "ne2k_pci"; - pci_nic_init(pci_bus, &nd_table[i], -1); + if (!pci_nic_init(pci_bus, &nd_table[i], -1)) + exit(1); } if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) { fprintf(stderr, "qemu: too many IDE bus\n"); diff --git a/qemu/hw/ppc_prep.c b/qemu/hw/ppc_prep.c index c42688e..8b21ff2 100644 --- a/qemu/hw/ppc_prep.c +++ b/qemu/hw/ppc_prep.c @@ -676,7 +676,8 @@ static void ppc_prep_init (int ram_size, int vga_ram_size, || strcmp(nd_table[i].model, "ne2k_isa") == 0) { isa_ne2000_init(ne2000_io[i], i8259[ne2000_irq[i]], &nd_table[i]); } else { - pci_nic_init(pci_bus, &nd_table[i], -1); + if (!pci_nic_init(pci_bus, &nd_table[i], -1)) + exit(1); } } diff --git a/qemu/hw/realview.c b/qemu/hw/realview.c index 29579d8..edae2f0 100644 --- a/qemu/hw/realview.c +++ b/qemu/hw/realview.c @@ -121,7 +121,8 @@ static void realview_init(int ram_size, int vga_ram_size, if (strcmp(nd->model, "smc91c111") == 0) { smc91c111_init(nd, 0x4e000000, pic[28]); } else { - pci_nic_init(pci_bus, nd, -1); + if (!pci_nic_init(pci_bus, nd, -1)) + exit(1); } } diff --git a/qemu/hw/sun4u.c b/qemu/hw/sun4u.c index 183f64a..e1a5985 100644 --- a/qemu/hw/sun4u.c +++ b/qemu/hw/sun4u.c @@ -342,7 +342,8 @@ static void sun4u_init(int ram_size, int vga_ram_size, for(i = 0; i < nb_nics; i++) { if (!nd_table[i].model) nd_table[i].model = "ne2k_pci"; - pci_nic_init(pci_bus, &nd_table[i], -1); + if (!pci_nic_init(pci_bus, &nd_table[i], -1)) + exit(1); } irq = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, 32); diff --git a/qemu/hw/versatilepb.c b/qemu/hw/versatilepb.c index da6e4ec..a918d9e 100644 --- a/qemu/hw/versatilepb.c +++ b/qemu/hw/versatilepb.c @@ -201,7 +201,8 @@ static void versatile_init(int ram_size, int vga_ram_size, if (strcmp(nd->model, "smc91c111") == 0) { smc91c111_init(nd, 0x10010000, sic[25]); } else { - pci_nic_init(pci_bus, nd, -1); + if (!pci_nic_init(pci_bus, nd, -1)) + exit(1); } } if (usb_enabled) {