While using "definitely-not-virtio" as a model name is very cute, it will also cause the relevant test to fail once we introduce stricter validation. Use "e1000", which is definitely not virtio but also a valid model name, instead. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml index 1d6c0ff..a1532cb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml @@ -24,7 +24,7 @@ <controller type='pci' index='0' model='pci-root'/> <interface type='user'> <mac address='52:54:00:e5:48:58'/> - <model type='definitely-not-virtio'/> + <model type='e1000'/> <driver name='vhost' queues='5'/> <backend tap='/dev/null' vhost='/dev/zero'/> </interface> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml index a39b8cb..3a95b60 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml @@ -28,7 +28,7 @@ <controller type='pci' index='0' model='pci-root'/> <interface type='user'> <mac address='52:54:00:e5:48:58'/> - <model type='definitely-not-virtio'/> + <model type='e1000'/> <backend tap='/dev/null'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> -- 2.7.5

Up until now, we have stored the model name for network interfaces as raw strings in virDomainNetDef: this is suboptimal for a number of reasons, such as having to perform relatively expensive string allocations and comparisons all the time and not giving the compiler the opportunity to have our back in certain situations. Replace the strings with an enumeration similar to the ones we already use for pretty much everything else. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- Most drivers already performed pretty strict validation of the model name, so there should be no problems there; the QEMU driver, however, though it would be a good idea to just accept any string that possibly kinda resembled a valid model name. The models I've included in the enumeration are those that were already referenced somewhere else in libvirt, but there's no guarantee that other model names are not used in the wild. One possibility would be to also add (a subset of) all models QEMU ever supported, even though some of them might have never been used, just to be safe; on the other side of the spectrum, we could go with the minimal set and possibly add more if breakages are reported. src/bhyve/bhyve_command.c | 6 ++-- src/bhyve/bhyve_parse_command.c | 6 +++- src/conf/domain_conf.c | 63 ++++++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 28 +++++++++++++++++- src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 7 +++-- src/qemu/qemu_command.c | 14 +++++---- src/qemu/qemu_domain.c | 24 +++++++--------- src/qemu/qemu_domain_address.c | 47 +++++++++++++++++++----------- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 11 ++++--- src/qemu/qemu_interface.c | 8 +++--- src/qemu/qemu_parse_command.c | 10 +++++-- src/security/virt-aa-helper.c | 2 +- src/vbox/vbox_common.c | 27 +++++++++--------- src/vmx/vmx.c | 53 ++++++++++++++++------------------ src/xenconfig/xen_common.c | 21 ++++++++------ src/xenconfig/xen_sxpr.c | 23 +++++++++------ 18 files changed, 225 insertions(+), 130 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index b3ae315..ac9ea56 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -57,10 +57,10 @@ bhyveBuildNetArgStr(virConnectPtr conn, int ret = -1; virDomainNetType actualType = virDomainNetGetActualType(net); - if (STREQ(net->model, "virtio")) { + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { if (VIR_STRDUP(nic_model, "virtio-net") < 0) return -1; - } else if (STREQ(net->model, "e1000")) { + } else if (net->model == VIR_DOMAIN_NET_MODEL_E1000) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) { if (VIR_STRDUP(nic_model, "e1000") < 0) return -1; @@ -73,7 +73,7 @@ bhyveBuildNetArgStr(virConnectPtr conn, } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("NIC model '%s' is not supported"), - net->model); + NULLSTR(virDomainNetModelTypeToString(net->model))); return -1; } diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index fcaaed2..f77ed27 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -515,8 +515,12 @@ bhyveParsePCINet(virDomainDefPtr def, if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0) goto error; - if (VIR_STRDUP(net->model, model) < 0) + if ((net->model = virDomainNetModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("NIC model '%s' is not supported"), + model); goto error; + } net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; net->info.addr.pci.slot = pcislot; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 34c8f45..7a2ff8d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -418,6 +418,28 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "hostdev", "udp") +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, + "none", + "usb-net", + "netfront", + "vlance", + "vmxnet", + "vmxnet2", + "vmxnet3", + "am79c970a", + "am79c973", + "82540em", + "82543gc", + "82545em", + "spapr-vlan", + "smc91c111", + "lan9118", + "rtl8139", + "e1000", + "e1000e", + "virtio", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -1986,7 +2008,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) if (!def) return; - VIR_FREE(def->model); + def->model = VIR_DOMAIN_NET_MODEL_NONE; switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -4484,7 +4506,7 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET) { virDomainNetDefPtr net = dev->data.net; - if (STRNEQ_NULLABLE(net->model, "virtio") && + if (net->model != VIR_DOMAIN_NET_MODEL_VIRTIO && virDomainCheckVirtioOptions(net->virtio) < 0) return -1; } @@ -9769,9 +9791,6 @@ virDomainActualNetDefParseXML(xmlNodePtr node, return ret; } -#define NET_MODEL_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-" - int virDomainNetAppendIPAddress(virDomainNetDefPtr def, @@ -10330,18 +10349,20 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio * QEMU PPC64 supports spapr-vlan */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); + if (model) { + if ((val = virDomainNetModelTypeFromString(model)) < 0 || + val == VIR_DOMAIN_NET_MODEL_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown interface <model type='%s'> " + "has been specified"), + model); goto error; } - def->model = model; - model = NULL; + def->model = val; } if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - STREQ_NULLABLE(def->model, "virtio")) { + def->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { if (backend != NULL) { if ((val = virDomainNetBackendTypeFromString(backend)) < 0 || val == VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT) { @@ -19479,10 +19500,11 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } - if (STRNEQ_NULLABLE(src->model, dst->model)) { + if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card model %s does not match source %s"), - NULLSTR(dst->model), NULLSTR(src->model)); + NULLSTR(virDomainNetModelTypeToString(dst->model)), + NULLSTR(virDomainNetModelTypeToString(src->model))); return false; } @@ -22734,9 +22756,18 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } if (def->model) { + const char *modelName = virDomainNetModelTypeToString(def->model); + + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net model %d"), def->model); + return -1; + } + virBufferEscapeString(buf, "<model type='%s'/>\n", - def->model); - if (STREQ(def->model, "virtio")) { + modelName); + + if (def->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { char *str = NULL, *gueststr = NULL, *hoststr = NULL; int rc = 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 239b218..3d8dfdb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -900,6 +900,32 @@ typedef enum { VIR_DOMAIN_NET_TYPE_LAST } virDomainNetType; +typedef enum { + VIR_DOMAIN_NET_MODEL_NONE = 0, + VIR_DOMAIN_NET_MODEL_USB_NET, + VIR_DOMAIN_NET_MODEL_NETFRONT, + VIR_DOMAIN_NET_MODEL_VLANCE, + VIR_DOMAIN_NET_MODEL_VMXNET, + VIR_DOMAIN_NET_MODEL_VMXNET2, + VIR_DOMAIN_NET_MODEL_VMXNET3, + VIR_DOMAIN_NET_MODEL_AM79C970A, + VIR_DOMAIN_NET_MODEL_AM79C973, + VIR_DOMAIN_NET_MODEL_82540EM, + VIR_DOMAIN_NET_MODEL_82543GC, + VIR_DOMAIN_NET_MODEL_82545EM, + VIR_DOMAIN_NET_MODEL_SPAPR_VLAN, + VIR_DOMAIN_NET_MODEL_SMC91C111, + VIR_DOMAIN_NET_MODEL_LAN9118, + VIR_DOMAIN_NET_MODEL_RTL8139, + VIR_DOMAIN_NET_MODEL_E1000, + VIR_DOMAIN_NET_MODEL_E1000E, + VIR_DOMAIN_NET_MODEL_VIRTIO, + + VIR_DOMAIN_NET_MODEL_LAST +} virDomainNetModel; + +VIR_ENUM_DECL(virDomainNetModel); + /* the backend driver used for virtio interfaces */ typedef enum { VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */ @@ -960,7 +986,7 @@ struct _virDomainActualNetDef { struct _virDomainNetDef { virDomainNetType type; virMacAddr mac; - char *model; + int model; /* enum virDomainNetModel */ union { struct { virDomainNetBackendType name; /* which driver backend to use */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fa2cd08..579ce98 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -433,6 +433,8 @@ virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; virDomainNetInsert; +virDomainNetModelTypeFromString; +virDomainNetModelTypeToString; virDomainNetRemove; virDomainNetRemoveHostdev; virDomainNetTypeFromString; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4416a09..b7950b4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1022,15 +1022,16 @@ libxlMakeNic(virDomainDefPtr def, */ if (l_nic->model) { if (def->os.type == VIR_DOMAIN_OSTYPE_XEN && - STRNEQ(l_nic->model, "netfront")) { + l_nic->model != VIR_DOMAIN_NET_MODEL_NETFRONT) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only model 'netfront' is supported for " "Xen PV domains")); return -1; } - if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) + if (VIR_STRDUP(x_nic->model, + virDomainNetModelTypeToString(l_nic->model)) < 0) goto cleanup; - if (STREQ(l_nic->model, "netfront")) + if (l_nic->model == VIR_DOMAIN_NET_MODEL_NETFRONT) x_nic->nictype = LIBXL_NIC_TYPE_VIF; else x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 59ad93a..8302eb4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3582,16 +3582,18 @@ qemuBuildNicStr(virDomainNetDefPtr net, { char *str; char macaddr[VIR_MAC_STRING_BUFLEN]; + const char *modelName = virDomainNetModelTypeToString(net->model); ignore_value(virAsprintf(&str, "%smacaddr=%s,vlan=%d%s%s%s%s", prefix ? prefix : "", virMacAddrFormat(&net->mac, macaddr), vlan, - (net->model ? ",model=" : ""), - (net->model ? net->model : ""), + (modelName ? ",model=" : ""), + (modelName ? modelName : ""), (net->info.alias ? ",name=" : ""), (net->info.alias ? net->info.alias : ""))); + return str; } @@ -3605,11 +3607,11 @@ qemuBuildNicDevStr(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - const char *nic = net->model; + const char *nic = NULL; bool usingVirtio = false; char macaddr[VIR_MAC_STRING_BUFLEN]; - if (STREQ(net->model, "virtio")) { + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) nic = "virtio-net-ccw"; else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) @@ -3620,9 +3622,11 @@ qemuBuildNicDevStr(virDomainDefPtr def, nic = "virtio-net-pci"; usingVirtio = true; + } else { + nic = virDomainNetModelTypeToString(net->model); } - virBufferAdd(&buf, nic, -1); + virBufferAdd(&buf, NULLSTR(nic), -1); if (usingVirtio && net->driver.virtio.txmode) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) { virBufferAddLit(&buf, ",tx="); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5c07302..b265ed4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3228,7 +3228,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, goto cleanup; } - if (STREQ_NULLABLE(net->model, "virtio") && + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO && net->driver.virtio.rx_queue_size & (net->driver.virtio.rx_queue_size - 1)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("rx_queue_size has to be a power of two")); @@ -3258,39 +3258,39 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, } -static const char * +static virDomainNetModel qemuDomainDefaultNetModel(const virDomainDef *def, virQEMUCapsPtr qemuCaps) { if (ARCH_IS_S390(def->os.arch)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) { if (STREQ(def->os.machine, "versatilepb")) - return "smc91c111"; + return VIR_DOMAIN_NET_MODEL_SMC91C111; if (qemuDomainIsVirt(def)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; /* Incomplete. vexpress (and a few others) use this, but not all * arm boards */ - return "lan9118"; + return VIR_DOMAIN_NET_MODEL_LAN9118; } /* Try several network devices in turn; each of these devices is * less likely be supported out-of-the-box by the guest operating * system than the previous one */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RTL8139)) - return "rtl8139"; + return VIR_DOMAIN_NET_MODEL_RTL8139; else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000)) - return "e1000"; + return VIR_DOMAIN_NET_MODEL_E1000; else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; /* We've had no luck detecting support for any network device, * but we have to return something: might as well be rtl8139 */ - return "rtl8139"; + return VIR_DOMAIN_NET_MODEL_RTL8139; } @@ -3569,9 +3569,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && !dev->data.net->model) { - if (VIR_STRDUP(dev->data.net->model, - qemuDomainDefaultNetModel(def, qemuCaps)) < 0) - goto cleanup; + dev->data.net->model = qemuDomainDefaultNetModel(def, qemuCaps); } /* set default disk types and drivers */ diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 8066ed1..7c5e87e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -216,10 +216,8 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (net->model && - STREQ(net->model, "spapr-vlan")) { + if (net->model == VIR_DOMAIN_NET_MODEL_SPAPR_VLAN) net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - } if (qemuDomainAssignSpaprVIOAddress(def, &net->info, VIO_ADDR_NET) < 0) goto cleanup; @@ -291,7 +289,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (STREQ(net->model, "virtio") && + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO && net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { net->info.type = type; } @@ -561,26 +559,43 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, /* the only type of filesystem so far is virtio-9p-pci */ return virtioFlags; - case VIR_DOMAIN_DEVICE_NET: { - virDomainNetDefPtr net = dev->data.net; - + case VIR_DOMAIN_DEVICE_NET: /* NB: a type='hostdev' will use PCI, but its * address is assigned when we're assigning the * addresses for other hostdev devices. */ - if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || - STREQ(net->model, "usb-net")) { + if (dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) return 0; - } - if (STREQ(net->model, "virtio")) - return virtioFlags; - - if (STREQ(net->model, "e1000e")) + switch ((virDomainNetModel) dev->data.net->model) { + case VIR_DOMAIN_NET_MODEL_E1000E: return pcieFlags; - return pciFlags; - } + case VIR_DOMAIN_NET_MODEL_VIRTIO: + return virtioFlags; + + case VIR_DOMAIN_NET_MODEL_RTL8139: + case VIR_DOMAIN_NET_MODEL_E1000: + return pciFlags; + + case VIR_DOMAIN_NET_MODEL_USB_NET: + case VIR_DOMAIN_NET_MODEL_NETFRONT: + case VIR_DOMAIN_NET_MODEL_VLANCE: + case VIR_DOMAIN_NET_MODEL_VMXNET: + case VIR_DOMAIN_NET_MODEL_VMXNET2: + case VIR_DOMAIN_NET_MODEL_VMXNET3: + case VIR_DOMAIN_NET_MODEL_AM79C970A: + case VIR_DOMAIN_NET_MODEL_AM79C973: + case VIR_DOMAIN_NET_MODEL_82540EM: + case VIR_DOMAIN_NET_MODEL_82545EM: + case VIR_DOMAIN_NET_MODEL_82543GC: + case VIR_DOMAIN_NET_MODEL_SPAPR_VLAN: + case VIR_DOMAIN_NET_MODEL_SMC91C111: + case VIR_DOMAIN_NET_MODEL_LAN9118: + case VIR_DOMAIN_NET_MODEL_NONE: + case VIR_DOMAIN_NET_MODEL_LAST: + return 0; + } case VIR_DOMAIN_DEVICE_SOUND: switch ((virDomainSoundModel) dev->data.sound->model) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9c54571..ab71e2c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6991,11 +6991,10 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, for (i = 0; i < vm->def->nnets; i++) { virDomainNetDefPtr net = vm->def->nets[i]; unsigned int bootIndex = net->info.bootIndex; - char *model = net->model; + int model = net->model; virMacAddr mac = net->mac; char *script = net->script; - net->model = NULL; net->script = NULL; virDomainNetDefClear(net); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4be0f54..9ba18b4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3053,15 +3053,18 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, goto cleanup; } - if (STRNEQ_NULLABLE(olddev->model, newdev->model)) { + if (olddev->model != newdev->model) { + const char *oldModel = virDomainNetModelTypeToString(olddev->model); + const char *newModel = virDomainNetModelTypeToString(newdev->model); + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("cannot modify network device model from %s to %s"), - olddev->model ? olddev->model : "(default)", - newdev->model ? newdev->model : "(default)"); + oldModel ? oldModel : "(default)", + newModel ? newModel : "(default)"); goto cleanup; } - if (olddev->model && STREQ(olddev->model, "virtio") && + if (olddev->model == VIR_DOMAIN_NET_MODEL_VIRTIO && (olddev->driver.virtio.name != newdev->driver.virtio.name || olddev->driver.virtio.txmode != newdev->driver.virtio.txmode || olddev->driver.virtio.ioeventfd != newdev->driver.virtio.ioeventfd || diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index cebb490..ec1a92f 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -264,7 +264,7 @@ qemuInterfaceDirectConnect(virDomainDefPtr def, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP; - if (net->model && STREQ(net->model, "virtio")) + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR; if (virNetDevMacVLanCreateWithVPortProfile(net->ifname, @@ -437,7 +437,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, template_ifname = true; } - if (net->model && STREQ(net->model, "virtio")) + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize, @@ -536,7 +536,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, template_ifname = true; } - if (net->model && STREQ(net->model, "virtio")) + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; if (virQEMUDriverIsPrivileged(driver)) { @@ -658,7 +658,7 @@ qemuInterfaceOpenVhostNet(virDomainDefPtr def, } /* If the nic model isn't virtio, don't try to open. */ - if (!(net->model && STREQ(net->model, "virtio"))) { + if (net->model != VIR_DOMAIN_NET_MODEL_VIRTIO) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is only supported for " diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index ee71127..b23de18 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1118,8 +1118,14 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt, goto cleanup; } } else if (STREQ(keywords[i], "model")) { - def->model = values[i]; - values[i] = NULL; + if ((def->model = virDomainNetModelTypeFromString(values[i])) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown network interface model '%s'"), + values[i]); + virDomainNetDefFree(def); + def = NULL; + goto cleanup; + } } else if (STREQ(keywords[i], "vhost")) { if ((values[i] == NULL) || STREQ(values[i], "on")) { def->driver.virtio.name = VIR_DOMAIN_NET_BACKEND_TYPE_VHOST; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a751d6d..97a4e63 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1137,7 +1137,7 @@ get_files(vahControl * ctl) if (net && net->model) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) continue; - if (STRNEQ(net->model, "virtio")) + if (net->model != VIR_DOMAIN_NET_MODEL_VIRTIO) continue; } needsvhost = true; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 92ee371..72a9ea8 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1294,7 +1294,8 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) macaddrvbox[VIR_MAC_STRING_BUFLEN - 6] = '\0'; VIR_DEBUG("NIC(%zu): Type: %d", i, def->nets[i]->type); - VIR_DEBUG("NIC(%zu): Model: %s", i, def->nets[i]->model); + VIR_DEBUG("NIC(%zu): Model: %s", i, + NULLSTR(virDomainNetModelTypeToString(def->nets[i]->model))); VIR_DEBUG("NIC(%zu): Mac: %s", i, macaddr); VIR_DEBUG("NIC(%zu): ifname: %s", i, def->nets[i]->ifname); if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -1324,18 +1325,18 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) gVBoxAPI.UINetworkAdapter.SetEnabled(adapter, 1); if (def->nets[i]->model) { - if (STRCASEEQ(def->nets[i]->model, "Am79C970A")) { + if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_AM79C970A) { adapterType = NetworkAdapterType_Am79C970A; - } else if (STRCASEEQ(def->nets[i]->model, "Am79C973")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_AM79C973) { adapterType = NetworkAdapterType_Am79C973; - } else if (STRCASEEQ(def->nets[i]->model, "82540EM")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82540EM) { adapterType = NetworkAdapterType_I82540EM; - } else if (STRCASEEQ(def->nets[i]->model, "82545EM")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82545EM) { adapterType = NetworkAdapterType_I82545EM; - } else if (STRCASEEQ(def->nets[i]->model, "82543GC")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82543GC) { adapterType = NetworkAdapterType_I82543GC; } else if (gVBoxAPI.APIVersion >= 3000051 && - STRCASEEQ(def->nets[i]->model, "virtio")) { + def->nets[i]->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ adapterType = NetworkAdapterType_Virtio; } @@ -3526,19 +3527,19 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); if (adapterType == NetworkAdapterType_Am79C970A) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "Am79C970A")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_AM79C970A; } else if (adapterType == NetworkAdapterType_Am79C973) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "Am79C973")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_AM79C973; } else if (adapterType == NetworkAdapterType_I82540EM) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82540EM")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_82540EM; } else if (adapterType == NetworkAdapterType_I82545EM) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82545EM")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_82545EM; } else if (adapterType == NetworkAdapterType_I82543GC) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82543GC")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_82543GC; } else if (gVBoxAPI.APIVersion >= 3000051 && adapterType == NetworkAdapterType_Virtio) { /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "virtio")); + def->nets[netAdpIncCnt]->model = VIR_DOMAIN_NET_MODEL_VIRTIO; } gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 96507f1..9af25c2 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2520,6 +2520,7 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) char virtualDev_name[48] = ""; char *virtualDev = NULL; + int model = VIR_DOMAIN_NET_MODEL_NONE; char features_name[48] = ""; long long features = 0; @@ -2621,11 +2622,13 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) } if (virtualDev != NULL) { - if (STRCASENEQ(virtualDev, "vlance") && - STRCASENEQ(virtualDev, "vmxnet") && - STRCASENEQ(virtualDev, "vmxnet3") && - STRCASENEQ(virtualDev, "e1000") && - STRCASENEQ(virtualDev, "e1000e")) { + model = virDomainNetModelTypeFromString(virtualDev); + + if (model != VIR_DOMAIN_NET_MODEL_VLANCE && + model != VIR_DOMAIN_NET_MODEL_VMXNET && + model != VIR_DOMAIN_NET_MODEL_VMXNET3 && + model != VIR_DOMAIN_NET_MODEL_E1000 && + model != VIR_DOMAIN_NET_MODEL_E1000E) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting VMX entry '%s' to be 'vlance' or 'vmxnet' or " "'vmxnet3' or 'e1000' or 'e1000e' but found '%s'"), @@ -2633,12 +2636,8 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } - if (STRCASEEQ(virtualDev, "vmxnet") && features == 15) { - VIR_FREE(virtualDev); - - if (VIR_STRDUP(virtualDev, "vmxnet2") < 0) - goto cleanup; - } + if (model == VIR_DOMAIN_NET_MODEL_VMXNET && features == 15) + model = VIR_DOMAIN_NET_MODEL_VMXNET2; } /* vmx:networkName -> def:data.bridge.brname */ @@ -2662,10 +2661,9 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) /* Setup virDomainNetDef */ if (connectionType == NULL || STRCASEEQ(connectionType, "bridged")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - (*def)->model = virtualDev; + (*def)->model = model; (*def)->data.bridge.brname = networkName; - virtualDev = NULL; networkName = NULL; } else if (STRCASEEQ(connectionType, "hostonly")) { /* FIXME */ @@ -2675,16 +2673,13 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } else if (STRCASEEQ(connectionType, "nat")) { (*def)->type = VIR_DOMAIN_NET_TYPE_USER; - (*def)->model = virtualDev; - - virtualDev = NULL; + (*def)->model = model; } else if (STRCASEEQ(connectionType, "custom")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - (*def)->model = virtualDev; + (*def)->model = model; (*def)->data.bridge.brname = networkName; (*def)->ifname = vnet; - virtualDev = NULL; networkName = NULL; vnet = NULL; } else { @@ -3718,28 +3713,30 @@ virVMXFormatEthernet(virDomainNetDefPtr def, int controller, virBufferAsprintf(buffer, "ethernet%d.present = \"true\"\n", controller); /* def:model -> vmx:virtualDev, vmx:features */ - if (def->model != NULL) { - if (STRCASENEQ(def->model, "vlance") && - STRCASENEQ(def->model, "vmxnet") && - STRCASENEQ(def->model, "vmxnet2") && - STRCASENEQ(def->model, "vmxnet3") && - STRCASENEQ(def->model, "e1000") && - STRCASENEQ(def->model, "e1000e")) { + if (def->model) { + if (def->model != VIR_DOMAIN_NET_MODEL_VLANCE && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET2 && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET3 && + def->model != VIR_DOMAIN_NET_MODEL_E1000 && + def->model != VIR_DOMAIN_NET_MODEL_E1000E) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML entry 'devices/interface/model' " "to be 'vlance' or 'vmxnet' or 'vmxnet2' or 'vmxnet3' " - "or 'e1000' or 'e1000e' but found '%s'"), def->model); + "or 'e1000' or 'e1000e' but found '%s'"), + NULLSTR(virDomainNetModelTypeToString(def->model))); return -1; } - if (STRCASEEQ(def->model, "vmxnet2")) { + if (def->model == VIR_DOMAIN_NET_MODEL_VMXNET2) { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"vmxnet\"\n", controller); virBufferAsprintf(buffer, "ethernet%d.features = \"15\"\n", controller); } else { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"%s\"\n", - controller, def->model); + controller, + NULLSTR(virDomainNetModelTypeToString(def->model))); } } diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 6d7dc2c..0cad78a 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -972,12 +972,11 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) goto cleanup; if (model[0] && - VIR_STRDUP(net->model, model) < 0) + (net->model = virDomainNetModelTypeFromString(model)) < 0) goto cleanup; - if (!model[0] && type[0] && STREQ(type, vif_typename) && - VIR_STRDUP(net->model, "netfront") < 0) - goto cleanup; + if (!model[0] && type[0] && STREQ(type, vif_typename)) + net->model = VIR_DOMAIN_NET_MODEL_NETFRONT; if (vifname[0] && VIR_STRDUP(net->ifname, vifname) < 0) @@ -1240,14 +1239,18 @@ xenFormatNet(virConnectPtr conn, } if (!hvm) { - if (net->model != NULL) - virBufferAsprintf(&buf, ",model=%s", net->model); + if (net->model) { + virBufferAsprintf(&buf, ",model=%s", + NULLSTR(virDomainNetModelTypeToString(net->model))); + } } else { - if (net->model != NULL && STREQ(net->model, "netfront")) { + if (net->model == VIR_DOMAIN_NET_MODEL_NETFRONT) { virBufferAsprintf(&buf, ",type=%s", vif_typename); } else { - if (net->model != NULL) - virBufferAsprintf(&buf, ",model=%s", net->model); + if (net->model) { + virBufferAsprintf(&buf, ",model=%s", + NULLSTR(virDomainNetModelTypeToString(net->model))); + } } } diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index fefa61a..7351319 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -651,12 +651,13 @@ xenParseSxprNets(virDomainDefPtr def, } } - if (VIR_STRDUP(net->model, model) < 0) + if (model && + (net->model = virDomainNetModelTypeFromString(model)) < 0) { goto cleanup; + } - if (!model && type && STREQ(type, "netfront") && - VIR_STRDUP(net->model, "netfront") < 0) - goto cleanup; + if (!model && type && STREQ(type, "netfront")) + net->model = VIR_DOMAIN_NET_MODEL_NETFRONT; tmp = sexpr_node(node, "device/vif/rate"); if (tmp) { @@ -1945,14 +1946,18 @@ xenFormatSxprNet(virConnectPtr conn, virBufferEscapeSexpr(buf, "(vifname '%s')", def->ifname); if (!hvm) { - if (def->model != NULL) - virBufferEscapeSexpr(buf, "(model '%s')", def->model); + if (def->model) { + virBufferEscapeSexpr(buf, "(model '%s')", + NULLSTR(virDomainNetModelTypeToString(def->model))); + } } else { - if (def->model != NULL && STREQ(def->model, "netfront")) { + if (def->model == VIR_DOMAIN_NET_MODEL_NETFRONT) { virBufferAddLit(buf, "(type netfront)"); } else { - if (def->model != NULL) - virBufferEscapeSexpr(buf, "(model '%s')", def->model); + if (def->model) { + virBufferEscapeSexpr(buf, "(model '%s')", + NULLSTR(virDomainNetModelTypeToString(def->model))); + } } } -- 2.7.5

On Wed, 2017-07-26 at 18:21 +0200, Peter Krempa wrote: Would you still be against it if the enumeration was extended to include every NIC model ever supported by QEMU and Xen? As mentioned, other drivers already perform their own validation and only accept a very limited number of models, so there's no risk of regression there. If not, then I'll probably not attempt to push this forward. Your suggestion to keep storing the raw string and use it during validation would be workable, but throwing together a quick PoC confirmed my initial impression that it would not result in a clear win, if anything because of the additional logic making everything less straightforward. --  Andrea Bolognani / Red Hat / Virtualization