PATCH: dont call exit() from pci_nic_init(), let caller handle
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.
Signature · 27 lines
Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 [email protected] 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 <[email protected]>
qemu/hw/ipf.c +2 -1
@@ -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 );
qemu/hw/mips_malta.c +4 -2
@@ -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 ); } } }
qemu/hw/pc.c +2 -1
@@ -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 );
qemu/hw/pci.c +2 -2
@@ -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;
qemu/hw/ppc_chrp.c +2 -1
@@ -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" );
qemu/hw/ppc_prep.c +2 -1
@@ -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 ); } }
qemu/hw/realview.c +2 -1
@@ -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 ); } }
qemu/hw/sun4u.c +2 -1
@@ -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 );
qemu/hw/versatilepb.c +2 -1
@@ -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) {
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Quoted 5 lines · Ryan Harper
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
Quoted 8 lines
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 andexit(), preserving the existing behavior, but allowing flexibility so hotplug can do the right thing and just report the error rather than exiting the guest.
Hi Ryan,
Looks good, thanks.
There might still be some exit()'s lurking around due to device/cpu hot/add failure.
Avi Kivity
AppliedQuoted 5 lines · Ryan Harper
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
Quoted 10 lines
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 andexit(), preserving the existing behavior, but allowing flexibility so hotplug can do the right thing and just report the error rather than exiting the guest.
Applied, thanks.
[this didn't make it to kvm-devel for some reason?]
Signature · 7 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Quoted 17 lines · Avi Kivity
Quoted 5 lines · Ryan Harper
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
Quoted 10 lines
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 andexit(), preserving the existing behavior, but allowing flexibility so hotplug can do the right thing and just report the error rather than exiting the guest.
Quoted 3 lines
Applied, thanks.
[this didn't make it to kvm-devel for some reason?]
Yeah, not sure about that, sometimes it gets clogged in our outgoing system; they tend to not get along with some servers for unknown reasons to me. It has worked in the past for me. *shrugs*
Signature · 5 lines
Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 [email protected]