On Tue, Sep 14, 2010 at 05:46:55PM +0200, Bernhard Kohl wrote: > This patch was motivated by the following use case: In our system > the VMs usually have 4 NICs, any combination of virtio-net-pci and > pci-assign NIC devices. The VMs boot via gPXE preferably over the > pci-assign devices. > > There is no way to make this working with a combination of the > current options -net -pcidevice -device -optionrom -boot. > > With the parameter boot=off it is possible to avoid loading > and using gPXE option ROMs either for old style "-net nic" or > for "-device" NIC devices. So we can select which NIC is used > for booting. > > A side effect of the boot=off parameter is that unneeded ROMs > which might waste memory are not longer loaded. E.g. if you have > 2 virtio-net-pci and 2 pci-assign NICs in sum 4 option ROMs are > loaded and the virtio ROMs take precedence over the pci-assign > ROMs. The BIOS uses the first gPXE ROM which it finds and only > needs one of them even if there are more NICs of the same type. > > Without using the boot=on|off parameter the current behaviour > does not change. > > Signed-off-by: Thomas Ostler <thomas.ost...@nsn.com> > Signed-off-by: Bernhard Kohl <bernhard.k...@nsn.com>
I think this is useful, however: - We have bit properties which handle parsing on/off and other formats automatically. Please don't use string. - boot is not a great property name for PCI: what you actually do is disable option rom. So maybe call it 'rom' or something like that? - given you have added a property, it can now be changed with -device. and visible in -device ? This also has an advantage of only applying to pci devices (-net option would appear to apply to non-pci but have no effect). Please do not add more flag parsing in qdemu-options, net and vl.c To summarize, just add a qdev bit option and check the bit. > --- > hw/pci.c | 8 +++++++- > hw/pci.h | 1 + > hw/qdev.c | 6 ++++++ > hw/qdev.h | 1 + > net.c | 8 ++++++++ > net.h | 1 + > qemu-options.hx | 8 ++++++-- > vl.c | 27 +++++++++++++++++++++++++++ > 8 files changed, 57 insertions(+), 3 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index a98d6f3..055a2be 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -71,6 +71,7 @@ static struct BusInfo pci_bus_info = { > DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), > DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, > QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), > + DEFINE_PROP_STRING("boot", PCIDevice, boot), > DEFINE_PROP_END_OF_LIST() > } > }; > @@ -1513,6 +1514,10 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char > *default_model, > > pci_dev = pci_create(bus, devfn, pci_nic_names[i]); > dev = &pci_dev->qdev; > + if (nd->name) > + dev->id = qemu_strdup(nd->name); > + if (nd->no_boot) > + dev->no_boot = 1; > qdev_set_nic_properties(dev, nd); > if (qdev_init(dev) < 0) > return NULL; > @@ -1693,7 +1698,8 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo > *base) > /* rom loading */ > if (pci_dev->romfile == NULL && info->romfile != NULL) > pci_dev->romfile = qemu_strdup(info->romfile); > - pci_add_option_rom(pci_dev); > + if (!qdev->no_boot) > + pci_add_option_rom(pci_dev); > > if (qdev->hotplugged) { > rc = bus->hotplug(bus->hotplug_qdev, pci_dev, 1); > diff --git a/hw/pci.h b/hw/pci.h > index 1eab7e7..20aa038 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -172,6 +172,7 @@ struct PCIDevice { > char *romfile; > ram_addr_t rom_offset; > uint32_t rom_bar; > + char *boot; > }; > > PCIDevice *pci_register_device(PCIBus *bus, const char *name, > diff --git a/hw/qdev.c b/hw/qdev.c > index 35858cb..8445bc9 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -249,6 +249,10 @@ DeviceState *qdev_device_add(QemuOpts *opts) > qdev_free(qdev); > return NULL; > } > + if (qemu_opt_get(opts, "boot")) { > + if (!strcmp("off", qemu_strdup(qemu_opt_get(opts, "boot")))) > + qdev->no_boot = 1; > + } > if (qdev_init(qdev) < 0) { > qerror_report(QERR_DEVICE_INIT_FAILED, driver); > return NULL; > @@ -421,6 +425,8 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo > *nd) > qdev_prop_exists(dev, "vectors")) { > qdev_prop_set_uint32(dev, "vectors", nd->nvectors); > } > + if (nd->no_boot) > + qdev_prop_parse(dev, "boot", "off"); > } > > static int next_block_unit[IF_COUNT]; > diff --git a/hw/qdev.h b/hw/qdev.h > index 579328a..e7df371 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -45,6 +45,7 @@ struct DeviceState { > QLIST_ENTRY(DeviceState) sibling; > int instance_id_alias; > int alias_required_for_version; > + int no_boot; > }; > > typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent); > diff --git a/net.c b/net.c > index 3d0fde7..2370aca 100644 > --- a/net.c > +++ b/net.c > @@ -792,6 +792,10 @@ static int net_init_nic(QemuOpts *opts, > if (qemu_opt_get(opts, "addr")) { > nd->devaddr = qemu_strdup(qemu_opt_get(opts, "addr")); > } > + if (qemu_opt_get(opts, "boot")) { > + if (!strcmp("off", qemu_strdup(qemu_opt_get(opts, "boot")))) > + nd->no_boot = 1; > + } > > nd->macaddr[0] = 0x52; > nd->macaddr[1] = 0x54; > @@ -877,6 +881,10 @@ static const struct { > .type = QEMU_OPT_STRING, > .help = "PCI device address", > }, { > + .name = "boot", > + .type = QEMU_OPT_STRING, > + .help = "gPXE boot (on (default), off)", > + }, { > .name = "vectors", > .type = QEMU_OPT_NUMBER, > .help = "number of MSI-x vectors, 0 to disable MSI-X", > diff --git a/net.h b/net.h > index 518cf9c..288059b 100644 > --- a/net.h > +++ b/net.h > @@ -132,6 +132,7 @@ struct NICInfo { > VLANClientState *netdev; > int used; > int nvectors; > + int no_boot; > }; > > extern int nb_nics; > diff --git a/qemu-options.hx b/qemu-options.hx > index a0b5ae9..6084aa9 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -959,8 +959,10 @@ DEF("smb", HAS_ARG, QEMU_OPTION_smb, "", QEMU_ARCH_ALL) > #endif > > DEF("net", HAS_ARG, QEMU_OPTION_net, > - "-net > nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n" > + "-net > nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v][,boot=on|off]\n" > " create a new Network Interface Card and connect it to > VLAN 'n'\n" > + " use 'boot=on|off' to enable/disable loading of an > option rom;\n" > + " loading enabled is the default\n" > #ifdef CONFIG_SLIRP > "-net > user[,vlan=n][,name=str][,net=addr[/mask]][,host=addr][,restrict=y|n]\n" > " > [,hostname=host][,dhcpstart=addr][,dns=addr][,tftp=dir][,bootfile=f]\n" > @@ -1014,13 +1016,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, > #endif > "socket],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL) > STEXI > -...@item -net nic[,vl...@var{n}][,macad...@var{mac}][,mod...@var{type}] > [,na...@var{name}][,ad...@var{addr}][,vecto...@var{v}] > +...@item -net nic[,vl...@var{n}][,macad...@var{mac}][,mod...@var{type}] > [,na...@var{name}][,ad...@var{addr}][,vecto...@var{v}][,boot=on|off] > @findex -net > Create a new Network Interface Card and connect it to VLAN @var{n} (@var{n} > = 0 is the default). The NIC is an e1000 by default on the PC > target. Optionally, the MAC address can be changed to @var{mac}, the > device address set to @var{addr} (PCI cards only), > and a @var{name} can be assigned for use in monitor commands. > +Optionally, with @option{boot=on|off}, you can enable/disable the loading of > an option > +rom; by default, loading is enabled. > Optionally, for PCI cards, you can specify the number @var{v} of MSI-X > vectors > that the card should have; this option currently only affects virtio cards; > set > @var{v} = 0 to disable MSI-X. If no @option{-net} option is specified, a > single > diff --git a/vl.c b/vl.c > index 3f45aa9..2aad6b1 100644 > --- a/vl.c > +++ b/vl.c > @@ -2459,6 +2459,33 @@ int main(int argc, char **argv, char **envp) > if (!qemu_opts_parse(qemu_find_opts("device"), optarg, 1)) { > exit(1); > } > + > + /* check whether option "boot" is present in the cmd string > */ > + /* for this a modified string is created that does not */ > + /* contain the driver */ > + /* if "boot" is present and set to "on", the relevant */ > + /* variables are set in a way that net boot is possible and > */ > + /* that a present "romfile" is loaded for the given device */ > + /* note that "default_net" is set to zero in order to avoid > */ > + /* creation of a default device if option "-net" is not */ > + /* present in the complete command line */ > + { > + char mod_optarg[128]; > + char *mod_optarg_p; > + > + if ((mod_optarg_p = strchr(optarg, ','))) > + strcpy(mod_optarg, ++mod_optarg_p); > + else > + strcpy(mod_optarg, optarg); > + > + if (get_param_value(mod_optarg, 128, "boot", mod_optarg) > != 0) { > + if (!strcmp("on", mod_optarg)) { > + char buf[8]="n"; > + pstrcpy(boot_devices, sizeof(boot_devices), buf); > + default_net = 0; > + } > + } > + } > break; > case QEMU_OPTION_smp: > smp_parse(optarg); > -- > 1.7.2.2 >