[libvirt] [PATCH 0/7] Forward mode='hostdev' patch series
This patch series supports the forward mode='hostdev'. The functionality of this mode is the same as interface type='hostdev' but with the added benefit of using interface pools. The patch series also contains a patch to support use of interface names and PCI device addresses interchangeably in a network xml, and return the appropriate one in actualDevice when networkAllocateActualDevice is called. At the top level managed attribute can be specified with identical results as when it's specified for a hostdev. Currently forward mode='hostdev' does not support USB devices. Shradha Shah (7): conf: move DevicePCIAddress functions to separate file network: helper function to create interface pool from PF RNG updates, new xml parser/formatter code to support forward mode=hostdev Code to return interface name or pci_addr of the VF in actualDevice Add function virDevicePCIAddressEqual Forward Mode Hostdev network driver Implementation Forward Mode 'Hostdev' qemu driver implementation docs/formatnetwork.html.in | 62 +++++ docs/schemas/basictypes.rng | 46 ++++ docs/schemas/domaincommon.rng | 44 ---- docs/schemas/network.rng | 53 ++++- include/libvirt/virterror.h | 1 + src/Makefile.am | 7 +- src/conf/device_conf.c | 147 +++++++++++ src/conf/device_conf.h | 68 ++++++ src/conf/domain_conf.c | 114 +-------- src/conf/domain_conf.h | 25 +-- src/conf/network_conf.c | 130 +++++++++-- src/conf/network_conf.h | 26 ++- src/libvirt_private.syms | 11 +- src/network/bridge_driver.c | 414 +++++++++++++++++++++++--------- src/qemu/qemu_command.c | 52 +++- src/qemu/qemu_hotplug.c | 7 +- src/qemu/qemu_monitor.c | 14 +- src/qemu/qemu_monitor.h | 17 +- src/qemu/qemu_monitor_json.c | 14 +- src/qemu/qemu_monitor_json.h | 14 +- src/qemu/qemu_monitor_text.c | 16 +- src/qemu/qemu_monitor_text.h | 14 +- src/util/virnetdev.c | 25 +- src/util/virnetdev.h | 4 +- src/util/virterror.c | 3 +- src/xen/xend_internal.c | 3 +- tests/networkxml2xmlin/hostdev-pf.xml | 7 + tests/networkxml2xmlin/hostdev.xml | 10 + tests/networkxml2xmlout/hostdev-pf.xml | 7 + tests/networkxml2xmlout/hostdev.xml | 10 + tests/networkxml2xmltest.c | 2 + 31 files changed, 976 insertions(+), 391 deletions(-) create mode 100644 src/conf/device_conf.c create mode 100644 src/conf/device_conf.h create mode 100644 tests/networkxml2xmlin/hostdev-pf.xml create mode 100644 tests/networkxml2xmlin/hostdev.xml create mode 100644 tests/networkxml2xmlout/hostdev-pf.xml create mode 100644 tests/networkxml2xmlout/hostdev.xml -- 1.7.4.4
Move the functions the parse/format, and validate PCI addresses to their own file so they can be conveniently used in other places besides device_conf.c Refactoring existing code without causing any functional changes to prepare for new code. This patch makes the code reusable. Signed-off-by: Shradha Shah <sshah@solarflare.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 7 ++- src/conf/device_conf.c | 131 ++++++++++++++++++++++++++++++++++++++++++ src/conf/device_conf.h | 65 +++++++++++++++++++++ src/conf/domain_conf.c | 114 +++++-------------------------------- src/conf/domain_conf.h | 25 +------- src/libvirt_private.syms | 10 ++- src/qemu/qemu_command.c | 13 ++-- src/qemu/qemu_hotplug.c | 7 +- src/qemu/qemu_monitor.c | 14 ++-- src/qemu/qemu_monitor.h | 17 +++--- src/qemu/qemu_monitor_json.c | 14 ++-- src/qemu/qemu_monitor_json.h | 14 ++-- src/qemu/qemu_monitor_text.c | 16 +++--- src/qemu/qemu_monitor_text.h | 14 ++-- src/util/virterror.c | 3 +- src/xen/xend_internal.c | 3 +- 17 files changed, 287 insertions(+), 181 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 913fc5d..d0af43d 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -110,6 +110,7 @@ typedef enum { VIR_FROM_AUTH = 46, /* Error from auth handling */ VIR_FROM_DBUS = 47, /* Error from DBus */ VIR_FROM_PARALLELS = 48, /* Error from Parallels */ + VIR_FROM_DEVICE = 49, /* Error from Device */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/src/Makefile.am b/src/Makefile.am index b5f8056..ad24534 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -199,6 +199,10 @@ CPU_CONF_SOURCES = \ CONSOLE_CONF_SOURCES = \ conf/virconsole.c conf/virconsole.h +# Device Helper APIs +DEVICE_CONF_SOURCES = \ + conf/device_conf.c conf/device_conf.h + CONF_SOURCES = \ $(NETDEV_CONF_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ @@ -211,7 +215,8 @@ CONF_SOURCES = \ $(INTERFACE_CONF_SOURCES) \ $(SECRET_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ - $(CONSOLE_CONF_SOURCES) + $(CONSOLE_CONF_SOURCES) \ + $(DEVICE_CONF_SOURCES) # The remote RPC driver, covering domains, storage, networks, etc REMOTE_DRIVER_GENERATED = \ diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c new file mode 100644 index 0000000..ca600c5 --- /dev/null +++ b/src/conf/device_conf.c @@ -0,0 +1,131 @@ +/* + * device_conf.c: device XML handling + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> +#include "virterror_internal.h" +#include "datatypes.h" +#include "memory.h" +#include "xml.h" +#include "uuid.h" +#include "util.h" +#include "buf.h" +#include "device_conf.h" + +#define VIR_FROM_THIS VIR_FROM_DEVICE + +VIR_ENUM_IMPL(virDeviceAddressPciMulti, + VIR_DEVICE_ADDRESS_PCI_MULTI_LAST, + "default", + "on", + "off") + +int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr) +{ + /* PCI bus has 32 slots and 8 functions per slot */ + if (addr->slot >= 32 || addr->function >= 8) + return 0; + return addr->domain || addr->bus || addr->slot; +} + + +int +virDevicePCIAddressParseXML(xmlNodePtr node, + virDevicePCIAddressPtr addr) +{ + char *domain, *slot, *bus, *function, *multi; + int ret = -1; + + memset(addr, 0, sizeof(*addr)); + + domain = virXMLPropString(node, "domain"); + bus = virXMLPropString(node, "bus"); + slot = virXMLPropString(node, "slot"); + function = virXMLPropString(node, "function"); + multi = virXMLPropString(node, "multifunction"); + + if (domain && + virStrToLong_ui(domain, NULL, 0, &addr->domain) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'domain' attribute")); + goto cleanup; + } + + if (bus && + virStrToLong_ui(bus, NULL, 0, &addr->bus) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'bus' attribute")); + goto cleanup; + } + + if (slot && + virStrToLong_ui(slot, NULL, 0, &addr->slot) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'slot' attribute")); + goto cleanup; + } + + if (function && + virStrToLong_ui(function, NULL, 0, &addr->function) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'function' attribute")); + goto cleanup; + } + + if (multi && + ((addr->multi = virDeviceAddressPciMultiTypeFromString(multi)) <= 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown value '%s' for <address> 'multifunction' attribute"), + multi); + goto cleanup; + + } + if (!virDevicePCIAddressIsValid(addr)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Insufficient specification for PCI address")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(domain); + VIR_FREE(bus); + VIR_FREE(slot); + VIR_FREE(function); + VIR_FREE(multi); + return ret; +} + +int +virDevicePCIAddressFormat(virBufferPtr buf, + virDevicePCIAddress addr, + bool includeTypeInAddr) +{ + virBufferAsprintf(buf, " <address %sdomain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + includeTypeInAddr ? "type='pci' " : "", + addr.domain, + addr.bus, + addr.slot, + addr.function); + return 0; +} diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h new file mode 100644 index 0000000..c679bce --- /dev/null +++ b/src/conf/device_conf.h @@ -0,0 +1,65 @@ +/* + * device_conf.h: device XML handling entry points + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __DEVICE_CONF_H__ +# define __DEVICE_CONF_H__ + +# include <libxml/parser.h> +# include <libxml/tree.h> +# include <libxml/xpath.h> + +# include "internal.h" +# include "util.h" +# include "threads.h" +# include "buf.h" + +enum virDeviceAddressPciMulti { + VIR_DEVICE_ADDRESS_PCI_MULTI_DEFAULT = 0, + VIR_DEVICE_ADDRESS_PCI_MULTI_ON, + VIR_DEVICE_ADDRESS_PCI_MULTI_OFF, + + VIR_DEVICE_ADDRESS_PCI_MULTI_LAST +}; + +typedef struct _virDevicePCIAddress virDevicePCIAddress; +typedef virDevicePCIAddress *virDevicePCIAddressPtr; +struct _virDevicePCIAddress { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; + int multi; /* enum virDomainDeviceAddressPciMulti */ +}; + +int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr); + +int virDevicePCIAddressParseXML(xmlNodePtr node, + virDevicePCIAddressPtr addr); + +int virDevicePCIAddressFormat(virBufferPtr buf, + virDevicePCIAddress addr, + bool includeTypeInAddr); + + +VIR_ENUM_DECL(virDeviceAddressPciMulti) + +#endif /* __DEVICE_CONF_H__ */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2e73c6..78d5685 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -52,6 +52,7 @@ #include "netdev_vport_profile_conf.h" #include "netdev_bandwidth_conf.h" #include "netdev_vlan_conf.h" +#include "device_conf.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -153,12 +154,6 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "spapr-vio", "virtio-s390") -VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti, - VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST, - "default", - "on", - "off") - VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", @@ -1893,7 +1888,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, switch (info->type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - return virDomainDevicePCIAddressIsValid(&info->addr.pci); + return virDevicePCIAddressIsValid(&info->addr.pci); case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: return 1; @@ -1905,16 +1900,6 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 0; } - -int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr) -{ - /* PCI bus has 32 slots and 8 functions per slot */ - if (addr->slot >= 32 || addr->function >= 8) - return 0; - return addr->domain || addr->bus || addr->slot; -} - - static bool virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) { @@ -2139,7 +2124,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.pci.function); if (info->addr.pci.multi) { virBufferAsprintf(buf, " multifunction='%s'", - virDomainDeviceAddressPciMultiTypeToString(info->addr.pci.multi)); + virDeviceAddressPciMultiTypeToString(info->addr.pci.multi)); } break; @@ -2187,75 +2172,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf, } static int -virDomainDevicePCIAddressParseXML(xmlNodePtr node, - virDomainDevicePCIAddressPtr addr) -{ - char *domain, *slot, *bus, *function, *multi; - int ret = -1; - - memset(addr, 0, sizeof(*addr)); - - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); - - if (domain && - virStrToLong_ui(domain, NULL, 0, &addr->domain) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'domain' attribute")); - goto cleanup; - } - - if (bus && - virStrToLong_ui(bus, NULL, 0, &addr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'bus' attribute")); - goto cleanup; - } - - if (slot && - virStrToLong_ui(slot, NULL, 0, &addr->slot) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'slot' attribute")); - goto cleanup; - } - - if (function && - virStrToLong_ui(function, NULL, 0, &addr->function) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'function' attribute")); - goto cleanup; - } - - if (multi && - ((addr->multi = virDomainDeviceAddressPciMultiTypeFromString(multi)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown value '%s' for <address> 'multifunction' attribute"), - multi); - goto cleanup; - - } - if (!virDomainDevicePCIAddressIsValid(addr)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Insufficient specification for PCI address")); - goto cleanup; - } - - ret = 0; - -cleanup: - VIR_FREE(domain); - VIR_FREE(bus); - VIR_FREE(slot); - VIR_FREE(function); - VIR_FREE(multi); - return ret; -} - - -static int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddressPtr addr) { @@ -2616,7 +2532,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, switch (info->type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - if (virDomainDevicePCIAddressParseXML(address, &info->addr.pci) < 0) + if (virDevicePCIAddressParseXML(address, &info->addr.pci) < 0) goto cleanup; break; @@ -2664,7 +2580,7 @@ cleanup: static int virDomainParseLegacyDeviceAddress(char *devaddr, - virDomainDevicePCIAddressPtr pci) + virDevicePCIAddressPtr pci) { char *tmp; @@ -2849,10 +2765,10 @@ virDomainHostdevSubsysPciDefParseXML(const xmlNodePtr node, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { if (xmlStrEqual(cur->name, BAD_CAST "address")) { - virDomainDevicePCIAddressPtr addr = + virDevicePCIAddressPtr addr = &def->source.subsys.u.pci; - if (virDomainDevicePCIAddressParseXML(cur, addr) < 0) + if (virDevicePCIAddressParseXML(cur, addr) < 0) goto out; } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { @@ -11537,14 +11453,12 @@ virDomainHostdevSourceFormat(virBufferPtr buf, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - virBufferAsprintf(buf, " <address %sdomain='0x%.4x' bus='0x%.2x' " - "slot='0x%.2x' function='0x%.1x'/>\n", - includeTypeInAddr ? "type='pci' " : "", - def->source.subsys.u.pci.domain, - def->source.subsys.u.pci.bus, - def->source.subsys.u.pci.slot, - def->source.subsys.u.pci.function); - + if (virDevicePCIAddressFormat(buf, + def->source.subsys.u.pci, + includeTypeInAddr) != 0) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("PCI address Formatting failed")); + if ((flags & VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES) && (def->origstates.states.pci.unbind_from_stub || def->origstates.states.pci.remove_slot || @@ -11557,7 +11471,7 @@ virDomainHostdevSourceFormat(virBufferPtr buf, if (def->origstates.states.pci.reprobe) virBufferAddLit(buf, " <reprobe/>\n"); virBufferAddLit(buf, " </origstates>\n"); - } + } break; default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fae7792..fd0e89e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -45,6 +45,7 @@ # include "virnetdevbandwidth.h" # include "virnetdevvlan.h" # include "virobject.h" +# include "device_conf.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -180,14 +181,6 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; -enum virDomainDeviceAddressPciMulti { - VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_DEFAULT = 0, - VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON, - VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF, - - VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST -}; - enum virDomainPciRombarMode { VIR_DOMAIN_PCI_ROMBAR_DEFAULT = 0, VIR_DOMAIN_PCI_ROMBAR_ON, @@ -196,16 +189,6 @@ enum virDomainPciRombarMode { VIR_DOMAIN_PCI_ROMBAR_LAST }; -typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress; -typedef virDomainDevicePCIAddress *virDomainDevicePCIAddressPtr; -struct _virDomainDevicePCIAddress { - unsigned int domain; - unsigned int bus; - unsigned int slot; - unsigned int function; - int multi; /* enum virDomainDeviceAddressPciMulti */ -}; - typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress; typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr; struct _virDomainDeviceDriveAddress { @@ -267,7 +250,7 @@ struct _virDomainDeviceInfo { char *alias; int type; union { - virDomainDevicePCIAddress pci; + virDevicePCIAddress pci; virDomainDeviceDriveAddress drive; virDomainDeviceVirtioSerialAddress vioserial; virDomainDeviceCcidAddress ccid; @@ -378,7 +361,7 @@ struct _virDomainHostdevSubsys { unsigned vendor; unsigned product; } usb; - virDomainDevicePCIAddress pci; /* host address */ + virDevicePCIAddress pci; /* host address */ } u; }; @@ -1896,7 +1879,6 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virCapsPtr caps, virDomainDeviceDefPtr src); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); -int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); @@ -2168,7 +2150,6 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) -VIR_ENUM_DECL(virDomainDeviceAddressPciMulti) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e91540d..1f32f8e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -218,6 +218,13 @@ virStorageVolClass; virStreamClass; +# device_conf.h +virDeviceAddressPciMultiTypeFromString; +virDeviceAddressPciMultiTypeToString; +virDevicePCIAddressIsValid; +virDevicePCIAddressParseXML; +virDevicePCIAddressFormat; + # dnsmasq.h dnsmasqAddDhcpHost; dnsmasqAddHost; @@ -297,14 +304,11 @@ virDomainDefParseNode; virDomainDefParseString; virDomainDeleteConfig; virDomainDeviceAddressIsValid; -virDomainDeviceAddressPciMultiTypeFromString; -virDomainDeviceAddressPciMultiTypeToString; virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceInfoIterate; -virDomainDevicePCIAddressIsValid; virDomainDeviceTypeToString; virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..6f6c6cd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -40,6 +40,7 @@ #include "network/bridge_driver.h" #include "virnetdevtap.h" #include "base64.h" +#include "device_conf.h" #include <sys/utsname.h> #include <sys/stat.h> @@ -1027,7 +1028,7 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, addr = NULL; if ((info->addr.pci.function == 0) && - (info->addr.pci.multi != VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON)) { + (info->addr.pci.multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON)) { /* a function 0 w/o multifunction=on must reserve the entire slot */ int function; virDomainDeviceInfo temp_info = *info; @@ -1610,7 +1611,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* USB2 needs special handling to put all companions in the same slot */ if (IS_USB2_CONTROLLER(def->controllers[i])) { - virDomainDevicePCIAddress addr = { 0, 0, 0, 0, false }; + virDevicePCIAddress addr = { 0, 0, 0, 0, false }; for (j = 0 ; j < i ; j++) { if (IS_USB2_CONTROLLER(def->controllers[j]) && def->controllers[j]->idx == def->controllers[i]->idx) { @@ -1625,7 +1626,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) break; case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: addr.function = 0; - addr.multi = VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON; + addr.multi = VIR_DEVICE_ADDRESS_PCI_MULTI_ON; break; case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: addr.function = 1; @@ -1779,7 +1780,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, "are supported with this QEMU binary")); return -1; } - if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) { + if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'multifunction=on' is not supported with " "this QEMU binary")); @@ -1797,9 +1798,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, virBufferAsprintf(buf, ",bus=pci.0"); else virBufferAsprintf(buf, ",bus=pci"); - if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) + if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) virBufferAddLit(buf, ",multifunction=on"); - else if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF) + else if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF) virBufferAddLit(buf, ",multifunction=off"); virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); if (info->addr.pci.function != 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 339906e..1251d6b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -43,6 +43,7 @@ #include "virnetdev.h" #include "virnetdevbridge.h" #include "virnetdevtap.h" +#include "device_conf.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -258,7 +259,7 @@ int qemuDomainAttachPciDiskDevice(virConnectPtr conn, } } } else { - virDomainDevicePCIAddress guestAddr = disk->info.addr.pci; + virDevicePCIAddress guestAddr = disk->info.addr.pci; ret = qemuMonitorAddPCIDisk(priv->mon, disk->src, type, @@ -655,7 +656,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, char *netstr = NULL; virNetDevVPortProfilePtr vport = NULL; int ret = -1; - virDomainDevicePCIAddress guestAddr; + virDevicePCIAddress guestAddr; int vlan; bool releaseaddr = false; bool iface_connected = false; @@ -974,7 +975,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, configfd, configfd_name); qemuDomainObjExitMonitorWithDriver(driver, vm); } else { - virDomainDevicePCIAddress guestAddr = hostdev->info->addr.pci; + virDevicePCIAddress guestAddr = hostdev->info->addr.pci; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorAddPCIHostDevice(priv->mon, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b0f3bb6..6ce1839 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2151,8 +2151,8 @@ int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon, int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *hostAddr, + virDevicePCIAddress *guestAddr) { int ret; VIR_DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", @@ -2176,7 +2176,7 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, int qemuMonitorAddPCIDisk(qemuMonitorPtr mon, const char *path, const char *bus, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { int ret; VIR_DEBUG("mon=%p path=%s bus=%s", @@ -2198,7 +2198,7 @@ int qemuMonitorAddPCIDisk(qemuMonitorPtr mon, int qemuMonitorAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { int ret; VIR_DEBUG("mon=%p nicstr=%s", mon, nicstr); @@ -2218,7 +2218,7 @@ int qemuMonitorAddPCINetwork(qemuMonitorPtr mon, int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { int ret; VIR_DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", @@ -2454,7 +2454,7 @@ int qemuMonitorGetPtyPaths(qemuMonitorPtr mon, int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { VIR_DEBUG("mon=%p type=%s", mon, bus); int ret; @@ -2476,7 +2476,7 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, int qemuMonitorAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress *controllerAddr, + virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr) { VIR_DEBUG("mon=%p drivestr=%s domain=%d bus=%d slot=%d function=%d", diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 42f33d1..ad8d2f1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -32,6 +32,7 @@ # include "bitmap.h" # include "virhash.h" # include "json.h" +# include "device_conf.h" typedef struct _qemuMonitor qemuMonitor; typedef qemuMonitor *qemuMonitorPtr; @@ -415,8 +416,8 @@ int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon, int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *hostAddr, + virDevicePCIAddress *guestAddr); /* XXX disk driver type eg, qcow/etc. * XXX cache mode @@ -424,17 +425,17 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, int qemuMonitorAddPCIDisk(qemuMonitorPtr mon, const char *path, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); /* XXX do we really want to hardcode 'nicstr' as the * sendable item here */ int qemuMonitorAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorSendFileHandle(qemuMonitorPtr mon, @@ -473,11 +474,11 @@ int qemuMonitorGetPtyPaths(qemuMonitorPtr mon, int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress *controllerAddr, + virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr); @@ -485,7 +486,7 @@ typedef struct _qemuMonitorPCIAddress qemuMonitorPCIAddress; struct _qemuMonitorPCIAddress { unsigned int vendor; unsigned int product; - virDomainDevicePCIAddress addr; + virDevicePCIAddress addr; }; int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cba97a7..643431c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2628,8 +2628,8 @@ int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *hostAddr ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) + virDevicePCIAddress *hostAddr ATTRIBUTE_UNUSED, + virDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pci_add not supported in JSON mode")); @@ -2640,7 +2640,7 @@ int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, const char *bus ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) + virDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pci_add not supported in JSON mode")); @@ -2650,7 +2650,7 @@ int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *nicstr ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) + virDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pci_add not supported in JSON mode")); @@ -2659,7 +2659,7 @@ int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) + virDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pci_del not supported in JSON mode")); @@ -2916,7 +2916,7 @@ int qemuMonitorJSONGetPtyPaths(qemuMonitorPtr mon, int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *bus ATTRIBUTE_UNUSED, - virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) + virDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pci_add not supported in JSON mode")); @@ -2955,7 +2955,7 @@ qemuMonitorJSONGetGuestDriveAddress(virJSONValuePtr reply, int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress* controllerAddr, + virDevicePCIAddress* controllerAddr, virDomainDeviceDriveAddress* driveAddr) { int ret; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e732178..3255007 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -163,20 +163,20 @@ int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon, int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *hostAddr, + virDevicePCIAddress *guestAddr); int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon, const char *path, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, const char *fdname, @@ -203,11 +203,11 @@ int qemuMonitorJSONGetPtyPaths(qemuMonitorPtr mon, int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress *controllerAddr, + virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr); int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index fa17927..a575e30 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1898,7 +1898,7 @@ int qemuMonitorTextAddUSBDeviceMatch(qemuMonitorPtr mon, static int qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *reply, - virDomainDevicePCIAddress *addr) + virDevicePCIAddress *addr) { char *s, *e; @@ -1960,8 +1960,8 @@ qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *hostAddr, + virDevicePCIAddress *guestAddr) { char *cmd; char *reply = NULL; @@ -2006,7 +2006,7 @@ cleanup: int qemuMonitorTextAddPCIDisk(qemuMonitorPtr mon, const char *path, const char *bus, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { char *cmd = NULL; char *reply = NULL; @@ -2058,7 +2058,7 @@ cleanup: int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { char *cmd; char *reply = NULL; @@ -2091,7 +2091,7 @@ cleanup: int qemuMonitorTextRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { char *cmd = NULL; char *reply = NULL; @@ -2439,7 +2439,7 @@ cleanup: int qemuMonitorTextAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, - virDomainDevicePCIAddress *guestAddr) + virDevicePCIAddress *guestAddr) { char *cmd = NULL; char *reply = NULL; @@ -2528,7 +2528,7 @@ qemudParseDriveAddReply(const char *reply, int qemuMonitorTextAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress *controllerAddr, + virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr) { char *cmd = NULL; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c6fd464..aca32a0 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -147,20 +147,20 @@ int qemuMonitorTextAddUSBDeviceMatch(qemuMonitorPtr mon, int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *hostAddr, + virDevicePCIAddress *guestAddr); int qemuMonitorTextAddPCIDisk(qemuMonitorPtr mon, const char *path, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorTextRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorTextSendFileHandle(qemuMonitorPtr mon, const char *fdname, @@ -187,11 +187,11 @@ int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon, int qemuMonitorTextAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, - virDomainDevicePCIAddress *guestAddr); + virDevicePCIAddress *guestAddr); int qemuMonitorTextAttachDrive(qemuMonitorPtr mon, const char *drivestr, - virDomainDevicePCIAddress *controllerAddr, + virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr); int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, diff --git a/src/util/virterror.c b/src/util/virterror.c index c438de8..e3d5de8 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -112,7 +112,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "URI Utils", /* 45 */ "Authentication Utils", "DBus Utils", - "Parallels Cloud Server" + "Parallels Cloud Server", + "Device Config" ) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 892d0e5..f93b249 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -46,6 +46,7 @@ #include "count-one-bits.h" #include "virfile.h" #include "viruri.h" +#include "device_conf.h" /* required for cpumap_t */ #include <xen/dom0_ops.h> @@ -2725,7 +2726,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, if (xenFormatSxprOnePCI(dev->data.hostdev, &buf, 0) < 0) goto cleanup; - virDomainDevicePCIAddress PCIAddr; + virDevicePCIAddress PCIAddr; PCIAddr = dev->data.hostdev->source.subsys.u.pci; virAsprintf(&target, "PCI device: %.4x:%.2x:%.2x", PCIAddr.domain, -- 1.7.4.4
On 08/16/2012 11:41 AM, Shradha Shah wrote:
Move the functions the parse/format, and validate PCI addresses to their own file so they can be conveniently used in other places besides device_conf.c
Refactoring existing code without causing any functional changes to prepare for new code.
This patch makes the code reusable.
Signed-off-by: Shradha Shah <sshah@solarflare.com>
ACK. You've taken care of all my nits from the previous version.
On 08/16/2012 12:12 PM, Laine Stump wrote:
On 08/16/2012 11:41 AM, Shradha Shah wrote:
Move the functions the parse/format, and validate PCI addresses to their own file so they can be conveniently used in other places besides device_conf.c
Refactoring existing code without causing any functional changes to prepare for new code.
This patch makes the code reusable.
Signed-off-by: Shradha Shah <sshah@solarflare.com> ACK. You've taken care of all my nits from the previous version.
Actually, make check was failing on a bunch of cases. But it turned out to just be that the <address> line was indented 4 extra spaces instead of two. I'll squash in this change when I push: diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index ca600c5..aefffec 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -120,7 +120,7 @@ virDevicePCIAddressFormat(virBufferPtr buf, virDevicePCIAddress addr, bool includeTypeInAddr) { - virBufferAsprintf(buf, " <address %sdomain='0x%.4x' bus='0x%.2x' " + virBufferAsprintf(buf, "<address %sdomain='0x%.4x' bus='0x%.2x' " "slot='0x%.2x' function='0x%.1x'/>\n", includeTypeInAddr ? "type='pci' " : "", addr.domain, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 78d5685..ff225e6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11435,18 +11435,19 @@ virDomainHostdevSourceFormat(virBufferPtr buf, bool includeTypeInAddr) { virBufferAddLit(buf, "<source>\n"); + virBufferAdjustIndent(buf, 2); switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (def->source.subsys.u.usb.vendor) { - virBufferAsprintf(buf, " <vendor id='0x%.4x'/>\n", + virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", def->source.subsys.u.usb.vendor); - virBufferAsprintf(buf, " <product id='0x%.4x'/>\n", + virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", def->source.subsys.u.usb.product); } if (def->source.subsys.u.usb.bus || def->source.subsys.u.usb.device) { - virBufferAsprintf(buf, " <address %sbus='%d' device='%d'/>\n", + virBufferAsprintf(buf, "<address %sbus='%d' device='%d'/>\n", includeTypeInAddr ? "type='usb' " : "", def->source.subsys.u.usb.bus, def->source.subsys.u.usb.device); @@ -11463,14 +11464,14 @@ virDomainHostdevSourceFormat(virBufferPtr buf, (def->origstates.states.pci.unbind_from_stub || def->origstates.states.pci.remove_slot || def->origstates.states.pci.reprobe)) { - virBufferAddLit(buf, " <origstates>\n"); + virBufferAddLit(buf, "<origstates>\n"); if (def->origstates.states.pci.unbind_from_stub) - virBufferAddLit(buf, " <unbind/>\n"); + virBufferAddLit(buf, " <unbind/>\n"); if (def->origstates.states.pci.remove_slot) - virBufferAddLit(buf, " <removeslot/>\n"); + virBufferAddLit(buf, " <removeslot/>\n"); if (def->origstates.states.pci.reprobe) - virBufferAddLit(buf, " <reprobe/>\n"); - virBufferAddLit(buf, " </origstates>\n"); + virBufferAddLit(buf, " <reprobe/>\n"); + virBufferAddLit(buf, "</origstates>\n"); } break; default: @@ -11480,6 +11481,7 @@ virDomainHostdevSourceFormat(virBufferPtr buf, return -1; } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</source>\n"); return 0; }
Existing code that creates a list of forwardIfs from a single PF was moved to the new utility function networkCreateInterfacePool. No functional change. Signed-off-by: Shradha Shah <sshah@solarflare.com> --- src/network/bridge_driver.c | 82 ++++++++++++++++++++++++++----------------- 1 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 474bbfa..ff17c7f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2771,6 +2771,55 @@ int networkRegister(void) { * "backend" function table. */ +/* networkCreateInterfacePool: + * @netdef: the original NetDef from the network + * + * Creates an implicit interface pool of VF's when a PF dev is given + */ +static int +networkCreateInterfacePool(virNetworkDefPtr netdef) { + unsigned int num_virt_fns = 0; + char **vfname = NULL; + int ret = -1, ii = 0; + + if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev, + &vfname, &num_virt_fns)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get Virtual functions on %s"), + netdef->forwardPfs->dev); + goto finish; + } + + if (num_virt_fns == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No Vf's present on SRIOV PF %s"), + netdef->forwardPfs->dev); + goto finish; + } + + if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) { + virReportOOMError(); + goto finish; + } + + netdef->nForwardIfs = num_virt_fns; + + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + netdef->forwardIfs[ii].dev = strdup(vfname[ii]); + if (!netdef->forwardIfs[ii].dev) { + virReportOOMError(); + goto finish; + } + } + + ret = 0; +finish: + for (ii = 0; ii < num_virt_fns; ii++) + VIR_FREE(vfname[ii]); + VIR_FREE(vfname); + return ret; +} + /* networkAllocateActualDevice: * @iface: the original NetDef from the domain * @@ -2793,8 +2842,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virNetDevVPortProfilePtr virtport = iface->virtPortProfile; virNetDevVlanPtr vlan = NULL; virNetworkForwardIfDefPtr dev = NULL; - unsigned int num_virt_fns = 0; - char **vfname = NULL; int ii; int ret = -1; @@ -2969,35 +3016,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) */ if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) { if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { - if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev, - &vfname, &num_virt_fns)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not get Virtual functions on %s"), - netdef->forwardPfs->dev); + if ((networkCreateInterfacePool(netdef)) < 0) { goto error; } - - if (num_virt_fns == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No Vf's present on SRIOV PF %s"), - netdef->forwardPfs->dev); - goto error; - } - - if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) { - virReportOOMError(); - goto error; - } - - netdef->nForwardIfs = num_virt_fns; - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - netdef->forwardIfs[ii].dev = strdup(vfname[ii]); - if (!netdef->forwardIfs[ii].dev) { - virReportOOMError(); - goto error; - } - } } /* pick first dev with 0 connections */ @@ -3105,9 +3126,6 @@ validate: ret = 0; cleanup: - for (ii = 0; ii < num_virt_fns; ii++) - VIR_FREE(vfname[ii]); - VIR_FREE(vfname); if (network) virNetworkObjUnlock(network); return ret; -- 1.7.4.4
On 08/16/2012 11:41 AM, Shradha Shah wrote:
Existing code that creates a list of forwardIfs from a single PF was moved to the new utility function networkCreateInterfacePool. No functional change.
Signed-off-by: Shradha Shah <sshah@solarflare.com>
ACK. Just one small change to the code (and a shorter title :-) had been requested, and both have been resolved.
This patch introduces the new forward mode='hostdev' along with attribute managed Includes updates to the network RNG and new xml parser/formatter code. Signed-off-by: Shradha Shah <sshah@solarflare.com> --- docs/schemas/basictypes.rng | 46 +++++++++++ docs/schemas/domaincommon.rng | 44 ----------- docs/schemas/network.rng | 53 ++++++++++--- src/conf/network_conf.c | 130 +++++++++++++++++++++++++++----- src/conf/network_conf.h | 26 ++++++- src/network/bridge_driver.c | 18 ++-- tests/networkxml2xmlin/hostdev-pf.xml | 7 ++ tests/networkxml2xmlin/hostdev.xml | 10 +++ tests/networkxml2xmlout/hostdev-pf.xml | 7 ++ tests/networkxml2xmlout/hostdev.xml | 10 +++ tests/networkxml2xmltest.c | 2 + 11 files changed, 266 insertions(+), 87 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 9dbda4a..766f9a0 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -54,6 +54,31 @@ </choice> </define> + <define name="pciaddress"> + <optional> + <attribute name="domain"> + <ref name="pciDomain"/> + </attribute> + </optional> + <attribute name="bus"> + <ref name="pciBus"/> + </attribute> + <attribute name="slot"> + <ref name="pciSlot"/> + </attribute> + <attribute name="function"> + <ref name="pciFunc"/> + </attribute> + <optional> + <attribute name="multifunction"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> + </define> + <!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" --> <!-- The lowest bit of the 1st byte is the "multicast" bit. a --> <!-- uniMacAddr requires that bit to be 0, and a multiMacAddr --> @@ -167,4 +192,25 @@ <ref name='unsignedLong'/> </define> + <define name="pciDomain"> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + </define> + <define name="pciBus"> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + </define> + <define name="pciSlot"> + <data type="string"> + <param name="pattern">(0x)?[0-1]?[0-9a-fA-F]</param> + </data> + </define> + <define name="pciFunc"> + <data type="string"> + <param name="pattern">(0x)?[0-7]</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4903ca6..35e9f82 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2652,30 +2652,6 @@ </attribute> </optional> </define> - <define name="pciaddress"> - <optional> - <attribute name="domain"> - <ref name="pciDomain"/> - </attribute> - </optional> - <attribute name="bus"> - <ref name="pciBus"/> - </attribute> - <attribute name="slot"> - <ref name="pciSlot"/> - </attribute> - <attribute name="function"> - <ref name="pciFunc"/> - </attribute> - <optional> - <attribute name="multifunction"> - <choice> - <value>on</value> - <value>off</value> - </choice> - </attribute> - </optional> - </define> <define name="driveaddress"> <optional> <attribute name="controller"> @@ -3376,26 +3352,6 @@ <param name="pattern">((0x)?[0-9a-fA-F]{1,3}\.){0,3}(0x)?[0-9a-fA-F]{1,3}</param> </data> </define> - <define name="pciDomain"> - <data type="string"> - <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> - </data> - </define> - <define name="pciBus"> - <data type="string"> - <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> - </data> - </define> - <define name="pciSlot"> - <data type="string"> - <param name="pattern">(0x)?[0-1]?[0-9a-fA-F]</param> - </data> - </define> - <define name="pciFunc"> - <data type="string"> - <param name="pattern">(0x)?[0-7]</param> - </data> - </define> <define name="driveController"> <data type="string"> <param name="pattern">[0-9]{1,2}</param> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index e55105a..4abfd91 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -87,22 +87,51 @@ <value>passthrough</value> <value>private</value> <value>vepa</value> + <value>hostdev</value> + </choice> + </attribute> + </optional> + + <optional> + <attribute name="managed"> + <choice> + <value>yes</value> + <value>no</value> </choice> </attribute> </optional> <interleave> - <zeroOrMore> - <element name='interface'> - <attribute name='dev'> - <ref name='deviceName'/> - </attribute> - <optional> - <attribute name="connections"> - <data type="unsignedInt"/> - </attribute> - </optional> - </element> - </zeroOrMore> + <choice> + <group> + <zeroOrMore> + <element name='interface'> + <attribute name='dev'> + <ref name='deviceName'/> + </attribute> + <optional> + <attribute name="connections"> + <data type="unsignedInt"/> + </attribute> + </optional> + </element> + </zeroOrMore> + </group> + <group> + <zeroOrMore> + <element name='address'> + <attribute name='type'> + <value>pci</value> + </attribute> + <ref name="pciaddress"/> + <optional> + <attribute name="connections"> + <data type="unsignedInt"/> + </attribute> + </optional> + </element> + </zeroOrMore> + </group> + </choice> <optional> <element name='pf'> <attribute name='dev'> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index db8c62f..2be7346 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -50,7 +50,12 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, - "none", "nat", "route", "bridge", "private", "vepa", "passthrough" ) + "none", "nat", "route", "bridge", "private", "vepa", "passthrough", "hostdev") + +VIR_ENUM_DECL(virNetworkForwardHostdevDevice) +VIR_ENUM_IMPL(virNetworkForwardHostdevDevice, + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST, + "none", "pci", "netdev") virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const unsigned char *uuid) @@ -96,7 +101,8 @@ virPortGroupDefClear(virPortGroupDefPtr def) static void virNetworkForwardIfDefClear(virNetworkForwardIfDefPtr def) { - VIR_FREE(def->dev); + if (def->type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) + VIR_FREE(def->device.dev); } static void @@ -943,11 +949,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) xmlNodePtr *portGroupNodes = NULL; xmlNodePtr *forwardIfNodes = NULL; xmlNodePtr *forwardPfNodes = NULL; + xmlNodePtr *forwardAddrNodes = NULL; xmlNodePtr dnsNode = NULL; xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; - int nIps, nPortGroups, nForwardIfs, nForwardPfs; + int nIps, nPortGroups, nForwardIfs, nForwardPfs, nForwardAddrs; char *forwardDev = NULL; + char *forwardManaged = NULL; + char *type = NULL; xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; xmlNodePtr vlanNode; @@ -1100,17 +1109,35 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } forwardDev = virXPathString("string(./@dev)", ctxt); + forwardManaged = virXPathString("string(./@managed)", ctxt); + if(forwardManaged != NULL) { + if (STRCASEEQ(forwardManaged, "yes")) + def->managed = 1; + } /* all of these modes can use a pool of physical interfaces */ nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes); nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes); + nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes); - if (nForwardIfs < 0 || nForwardPfs < 0) { + if (nForwardIfs < 0 || nForwardPfs < 0 || nForwardAddrs < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("No interface pool or SRIOV physical device given")); goto error; } + if ((nForwardIfs > 0) && (nForwardAddrs > 0)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Address and interface attributes are mutually exclusive")); + goto error; + } + + if ((nForwardPfs > 0) && ((nForwardIfs > 0) || (nForwardAddrs > 0))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Address/interface attributes and Physical function are mutually exclusive ")); + goto error; + } + if (nForwardPfs == 1) { if (VIR_ALLOC_N(def->forwardPfs, nForwardPfs) < 0) { virReportOOMError(); @@ -1139,7 +1166,53 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) _("Use of more than one physical interface is not allowed")); goto error; } - if (nForwardIfs > 0 || forwardDev) { + if (nForwardAddrs > 0) { + int ii; + + if (VIR_ALLOC_N(def->forwardIfs, nForwardAddrs) < 0) { + virReportOOMError(); + goto error; + } + + if (forwardDev) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("A forward Dev should not be used when using address attribute")); + goto error; + } + + for (ii = 0; ii < nForwardAddrs; ii++) { + type = virXMLPropString(forwardAddrNodes[ii], "type"); + + if (type) { + if ((def->forwardIfs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown address type '%s'"), type); + goto error; + } + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("No type specified for device address")); + goto error; + } + + switch (def->forwardIfs[ii].type) { + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: + if (virDevicePCIAddressParseXML(forwardAddrNodes[ii], &(def->forwardIfs[ii].device.pci)) < 0) + goto error; + break; + + /* Add USB case here */ + + default: + virReportError(VIR_ERR_XML_ERROR, + _("unknown address type '%s'"), type); + goto error; + } + VIR_FREE(type); + def->nForwardIfs++; + } + } + else if (nForwardIfs > 0 || forwardDev) { int ii; /* allocate array to hold all the portgroups */ @@ -1149,7 +1222,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } if (forwardDev) { - def->forwardIfs[0].dev = forwardDev; + def->forwardIfs[0].device.dev = forwardDev; + def->forwardIfs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; forwardDev = NULL; def->nForwardIfs++; } @@ -1167,10 +1241,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if ((ii == 0) && (def->nForwardIfs == 1)) { /* both forwardDev and an interface element are present. * If they don't match, it's an error. */ - if (STRNEQ(forwardDev, def->forwardIfs[0].dev)) { - virReportError(VIR_ERR_XML_ERROR, + if (STRNEQ(forwardDev, def->forwardIfs[0].device.dev)) { + virReportError(VIR_ERR_XML_ERROR, _("forward dev '%s' must match first interface element dev '%s' in network '%s'"), - def->forwardIfs[0].dev, + def->forwardIfs[0].device.dev, forwardDev, def->name); goto error; } @@ -1178,15 +1252,18 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) continue; } - def->forwardIfs[ii].dev = forwardDev; + def->forwardIfs[ii].device.dev = forwardDev; forwardDev = NULL; + def->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; def->nForwardIfs++; } } + VIR_FREE(type); VIR_FREE(forwardDev); + VIR_FREE(forwardManaged); VIR_FREE(forwardPfNodes); VIR_FREE(forwardIfNodes); - + VIR_FREE(forwardAddrNodes); switch (def->forwardType) { case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_NAT: @@ -1211,6 +1288,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: if (def->bridge) { virReportError(VIR_ERR_XML_ERROR, _("bridge name not allowed in %s mode (network '%s')"), @@ -1502,6 +1580,12 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) } virBufferAddLit(&buf, " <forward"); virBufferEscapeString(&buf, " dev='%s'", dev); + if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + if (def->managed == 1) + virBufferAddLit(&buf, " managed='yes'"); + else + virBufferAddLit(&buf, " managed='no'"); + } virBufferAsprintf(&buf, " mode='%s'%s>\n", mode, (def->nForwardIfs || def->nForwardPfs) ? "" : "/"); @@ -1513,14 +1597,24 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) if (def->nForwardIfs && (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) { for (ii = 0; ii < def->nForwardIfs; ii++) { - virBufferEscapeString(&buf, " <interface dev='%s'", - def->forwardIfs[ii].dev); - if (!(flags & VIR_NETWORK_XML_INACTIVE) && - (def->forwardIfs[ii].connections > 0)) { - virBufferAsprintf(&buf, " connections='%d'", - def->forwardIfs[ii].connections); + if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV) { + virBufferEscapeString(&buf, " <interface dev='%s'", + def->forwardIfs[ii].device.dev); + if (!(flags & VIR_NETWORK_XML_INACTIVE) && + (def->forwardIfs[ii].connections > 0)) { + virBufferAsprintf(&buf, " connections='%d'", + def->forwardIfs[ii].connections); + } + virBufferAddLit(&buf, "/>\n"); + } + else { + if (def->forwardIfs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI) { + if (virDevicePCIAddressFormat(&buf, + def->forwardIfs[ii].device.pci, + true) < 0) + goto error; + } } - virBufferAddLit(&buf, "/>\n"); } } if (def->nForwardPfs || def->nForwardIfs) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index a029f70..6b080f5 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -37,6 +37,7 @@ # include "virnetdevvportprofile.h" # include "virnetdevvlan.h" # include "virmacaddr.h" +# include "device_conf.h" enum virNetworkForwardType { VIR_NETWORK_FORWARD_NONE = 0, @@ -46,10 +47,20 @@ enum virNetworkForwardType { VIR_NETWORK_FORWARD_PRIVATE, VIR_NETWORK_FORWARD_VEPA, VIR_NETWORK_FORWARD_PASSTHROUGH, + VIR_NETWORK_FORWARD_HOSTDEV, VIR_NETWORK_FORWARD_LAST, }; +enum virNetworkForwardHostdevDeviceType { + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NONE = 0, + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI, + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV, + /* USB Device to be added here when supported */ + + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST, +}; + typedef struct _virNetworkDHCPRangeDef virNetworkDHCPRangeDef; typedef virNetworkDHCPRangeDef *virNetworkDHCPRangeDefPtr; struct _virNetworkDHCPRangeDef { @@ -132,14 +143,20 @@ struct _virNetworkIpDef { typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef; typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr; struct _virNetworkForwardIfDef { - char *dev; /* name of device */ - int connections; /* how many guest interfaces are connected to this device? */ + int type; + union { + virDevicePCIAddress pci; /*PCI Address of device */ + /* when USB devices are supported a new variable to be added here */ + char *dev; /* name of device */ + }device; + int connections; /* how many guest interfaces are connected to this device? */ }; typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef; typedef virNetworkForwardPfDef *virNetworkForwardPfDefPtr; struct _virNetworkForwardPfDef { - char *dev; /* name of device */ + char *dev; /* name of device */ + int connections; /* how many guest interfaces are connected to this device? */ }; typedef struct _virPortGroupDef virPortGroupDef; @@ -168,6 +185,7 @@ struct _virNetworkDef { bool mac_specified; int forwardType; /* One of virNetworkForwardType constants */ + int managed; /* managed attribute for hostdev mode */ /* If there are multiple forward devices (i.e. a pool of * interfaces), they will be listed here. @@ -244,7 +262,7 @@ static inline const char * virNetworkDefForwardIf(const virNetworkDefPtr def, size_t n) { return ((def->forwardIfs && (def->nForwardIfs > n)) - ? def->forwardIfs[n].dev : NULL); + ? def->forwardIfs[n].device.dev : NULL); } virPortGroupDefPtr virPortGroupFindByName(virNetworkDefPtr net, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ff17c7f..ddd66e5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2805,8 +2805,8 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { netdef->nForwardIfs = num_virt_fns; for (ii = 0; ii < netdef->nForwardIfs; ii++) { - netdef->forwardIfs[ii].dev = strdup(vfname[ii]); - if (!netdef->forwardIfs[ii].dev) { + netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]); + if (!netdef->forwardIfs[ii].device.dev) { virReportOOMError(); goto finish; } @@ -3057,7 +3057,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->name); goto error; } - iface->data.network.actual->data.direct.linkdev = strdup(dev->dev); + iface->data.network.actual->data.direct.linkdev = strdup(dev->device.dev); if (!iface->data.network.actual->data.direct.linkdev) { virReportOOMError(); goto error; @@ -3115,7 +3115,7 @@ validate: /* we are now assured of success, so mark the allocation */ dev->connections++; VIR_DEBUG("Using physical device %s, %d connections", - dev->dev, dev->connections); + dev->device.dev, dev->connections); } if (netdef) { @@ -3198,7 +3198,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) /* find the matching interface and increment its connections */ for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { dev = &netdef->forwardIfs[ii]; break; } @@ -3229,7 +3229,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) /* we are now assured of success, so mark the allocation */ dev->connections++; VIR_DEBUG("Using physical device %s, %d connections", - dev->dev, dev->connections); + dev->device.dev, dev->connections); } success: @@ -3305,7 +3305,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virNetworkForwardIfDefPtr dev = NULL; for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { dev = &netdef->forwardIfs[ii]; break; } @@ -3320,7 +3320,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) dev->connections--; VIR_DEBUG("Releasing physical device %s, %d connections", - dev->dev, dev->connections); + dev->device.dev, dev->connections); } success: @@ -3410,7 +3410,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: if ((netdef->nForwardIfs > 0) && netdef->forwardIfs) - dev_name = netdef->forwardIfs[0].dev; + dev_name = netdef->forwardIfs[0].device.dev; if (!dev_name) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/tests/networkxml2xmlin/hostdev-pf.xml b/tests/networkxml2xmlin/hostdev-pf.xml new file mode 100644 index 0000000..fc82c55 --- /dev/null +++ b/tests/networkxml2xmlin/hostdev-pf.xml @@ -0,0 +1,7 @@ +<network> + <name>hostdev</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode="hostdev" managed="yes"> + <pf dev='eth2'/> + </forward> +</network> diff --git a/tests/networkxml2xmlin/hostdev.xml b/tests/networkxml2xmlin/hostdev.xml new file mode 100644 index 0000000..0ec52d2 --- /dev/null +++ b/tests/networkxml2xmlin/hostdev.xml @@ -0,0 +1,10 @@ +<network> + <name>hostdev</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode="hostdev" managed="yes"> + <address type='pci' domain='0' bus='3' slot='0' function='1'/> + <address type='pci' domain='0' bus='3' slot='0' function='2'/> + <address type='pci' domain='0' bus='3' slot='0' function='3'/> + <address type='pci' domain='0' bus='3' slot='0' function='4'/> + </forward> +</network> diff --git a/tests/networkxml2xmlout/hostdev-pf.xml b/tests/networkxml2xmlout/hostdev-pf.xml new file mode 100644 index 0000000..e955312 --- /dev/null +++ b/tests/networkxml2xmlout/hostdev-pf.xml @@ -0,0 +1,7 @@ +<network> + <name>hostdev</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode="hostdev" managed="yes"> + <pf dev='eth2'/> + </forward> +</network> diff --git a/tests/networkxml2xmlout/hostdev.xml b/tests/networkxml2xmlout/hostdev.xml new file mode 100644 index 0000000..0ec52d2 --- /dev/null +++ b/tests/networkxml2xmlout/hostdev.xml @@ -0,0 +1,10 @@ +<network> + <name>hostdev</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode="hostdev" managed="yes"> + <address type='pci' domain='0' bus='3' slot='0' function='1'/> + <address type='pci' domain='0' bus='3' slot='0' function='2'/> + <address type='pci' domain='0' bus='3' slot='0' function='3'/> + <address type='pci' domain='0' bus='3' slot='0' function='4'/> + </forward> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 5a5531a..e57d190 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -106,6 +106,8 @@ mymain(void) DO_TEST("bandwidth-network"); DO_TEST("openvswitch-net"); DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE); + DO_TEST("hostdev"); + DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.4.4
On 08/16/2012 11:41 AM, Shradha Shah wrote:
This patch introduces the new forward mode='hostdev' along with attribute managed Includes updates to the network RNG and new xml parser/formatter code.
Signed-off-by: Shradha Shah <sshah@solarflare.com>
I'm in tears! ACK. You've taken care of every one of my points in the last review, *AND* rebased on top of my conflicting changes! (Thanks for putting up with that, btw :-)
Patch 3/7 fails make check, so I need to sqaush in the following to fix it. It seemed a bit too much to squash in without getting somebody to review it first: 1) The indentation was off for the new use of virDevicePCIAddressFormat in network_conf.c. Rather than just hacking in even more tweaks of virBufferAdjustIndent(), I corrected virNetworkDefFormat and the other format functions it calls (i.e. the rest of the file) to properly use the buffer indent everywhere. 2) the accessor function virNetworkDefForwardIf() needed to be updated to not access the newly unionized member device.dev unless the type was NETDEV. This caused a segfault when non-pointer data was interpreted as a pointer. 3) Some of the network XML test cases didn't match the output format (PCI addresses are always printed in hex with a 0x at the beginning). 4) Previously it was legal to put both a <pf> and a list of <interface> elements inside a <forward> element. That has now been made illegal, so it can't show up even in the *input* XML. --- src/conf/network_conf.c | 83 +++++++++++++++++-------------- src/conf/network_conf.h | 3 +- tests/networkxml2xmlin/hostdev-pf.xml | 2 +- tests/networkxml2xmlin/hostdev.xml | 10 ++-- tests/networkxml2xmlin/passthrough-pf.xml | 2 - tests/networkxml2xmlout/hostdev-pf.xml | 4 +- tests/networkxml2xmlout/hostdev.xml | 10 ++-- 7 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index ef519f6..9d53d8e 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1388,17 +1388,18 @@ virNetworkDNSDefFormat(virBufferPtr buf, if (def == NULL) goto out; - virBufferAddLit(buf, " <dns>\n"); + virBufferAddLit(buf, "<dns>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0 ; i < def->ntxtrecords ; i++) { - virBufferAsprintf(buf, " <txt name='%s' value='%s' />\n", + virBufferAsprintf(buf, "<txt name='%s' value='%s' />\n", def->txtrecords[i].name, def->txtrecords[i].value); } for (i = 0 ; i < def->nsrvrecords ; i++) { if (def->srvrecords[i].service && def->srvrecords[i].protocol) { - virBufferAsprintf(buf, " <srv service='%s' protocol='%s'", + virBufferAsprintf(buf, "<srv service='%s' protocol='%s'", def->srvrecords[i].service, def->srvrecords[i].protocol); @@ -1423,18 +1424,19 @@ virNetworkDNSDefFormat(virBufferPtr buf, for (ii = 0 ; ii < def->nhosts; ii++) { char *ip = virSocketAddrFormat(&def->hosts[ii].ip); - virBufferAsprintf(buf, " <host ip='%s'>\n", ip); - + virBufferAsprintf(buf, "<host ip='%s'>\n", ip); + virBufferAdjustIndent(buf, 2); for (j = 0; j < def->hosts[ii].nnames; j++) - virBufferAsprintf(buf, " <hostname>%s</hostname>\n", - def->hosts[ii].names[j]); + virBufferAsprintf(buf, "<hostname>%s</hostname>\n", + def->hosts[ii].names[j]); - virBufferAsprintf(buf, " </host>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</host>\n"); VIR_FREE(ip); } } - - virBufferAddLit(buf, " </dns>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</dns>\n"); out: return result; } @@ -1445,7 +1447,7 @@ virNetworkIpDefFormat(virBufferPtr buf, { int result = -1; - virBufferAddLit(buf, " <ip"); + virBufferAddLit(buf, "<ip"); if (def->family) { virBufferAsprintf(buf, " family='%s'", def->family); @@ -1468,14 +1470,17 @@ virNetworkIpDefFormat(virBufferPtr buf, virBufferAsprintf(buf," prefix='%u'", def->prefix); } virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (def->tftproot) { - virBufferEscapeString(buf, " <tftp root='%s' />\n", + virBufferEscapeString(buf, "<tftp root='%s' />\n", def->tftproot); } if ((def->nranges || def->nhosts)) { int ii; - virBufferAddLit(buf, " <dhcp>\n"); + virBufferAddLit(buf, "<dhcp>\n"); + virBufferAdjustIndent(buf, 2); + for (ii = 0 ; ii < def->nranges ; ii++) { char *saddr = virSocketAddrFormat(&def->ranges[ii].start); if (!saddr) @@ -1485,13 +1490,13 @@ virNetworkIpDefFormat(virBufferPtr buf, VIR_FREE(saddr); goto error; } - virBufferAsprintf(buf, " <range start='%s' end='%s' />\n", + virBufferAsprintf(buf, "<range start='%s' end='%s' />\n", saddr, eaddr); VIR_FREE(saddr); VIR_FREE(eaddr); } for (ii = 0 ; ii < def->nhosts ; ii++) { - virBufferAddLit(buf, " <host "); + virBufferAddLit(buf, "<host "); if (def->hosts[ii].mac) virBufferAsprintf(buf, "mac='%s' ", def->hosts[ii].mac); if (def->hosts[ii].name) @@ -1506,7 +1511,7 @@ virNetworkIpDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } if (def->bootfile) { - virBufferEscapeString(buf, " <bootp file='%s' ", + virBufferEscapeString(buf, "<bootp file='%s' ", def->bootfile); if (VIR_SOCKET_ADDR_VALID(&def->bootserver)) { char *ipaddr = virSocketAddrFormat(&def->bootserver); @@ -1516,12 +1521,15 @@ virNetworkIpDefFormat(virBufferPtr buf, VIR_FREE(ipaddr); } virBufferAddLit(buf, "/>\n"); + } - virBufferAddLit(buf, " </dhcp>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</dhcp>\n"); } - virBufferAddLit(buf, " </ip>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</ip>\n"); result = 0; error: @@ -1532,19 +1540,19 @@ static int virPortGroupDefFormat(virBufferPtr buf, const virPortGroupDefPtr def) { - virBufferAsprintf(buf, " <portgroup name='%s'", def->name); + virBufferAsprintf(buf, "<portgroup name='%s'", def->name); if (def->isDefault) { virBufferAddLit(buf, " default='yes'"); } virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 4); + virBufferAdjustIndent(buf, 2); if (virNetDevVlanFormat(&def->vlan, buf) < 0) return -1; if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) return -1; virNetDevBandwidthFormat(def->bandwidth, buf); - virBufferAdjustIndent(buf, -4); - virBufferAddLit(buf, " </portgroup>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</portgroup>\n"); return 0; } @@ -1560,11 +1568,12 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) virBufferAsprintf(&buf, " connections='%d'", def->connections); } virBufferAddLit(&buf, ">\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuidstr); if (def->forwardType != VIR_NETWORK_FORWARD_NONE) { const char *dev = NULL; @@ -1578,27 +1587,29 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) def->forwardType, def->name); goto error; } - virBufferAddLit(&buf, " <forward"); + virBufferAddLit(&buf, "<forward"); virBufferEscapeString(&buf, " dev='%s'", dev); + virBufferAsprintf(&buf, " mode='%s'", mode); if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { if (def->managed == 1) virBufferAddLit(&buf, " managed='yes'"); else virBufferAddLit(&buf, " managed='no'"); } - virBufferAsprintf(&buf, " mode='%s'%s>\n", mode, + virBufferAsprintf(&buf, "%s>\n", (def->nForwardIfs || def->nForwardPfs) ? "" : "/"); + virBufferAdjustIndent(&buf, 2); /* For now, hard-coded to at most 1 forwardPfs */ if (def->nForwardPfs) - virBufferEscapeString(&buf, " <pf dev='%s'/>\n", + virBufferEscapeString(&buf, "<pf dev='%s'/>\n", def->forwardPfs[0].dev); if (def->nForwardIfs && (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) { for (ii = 0; ii < def->nForwardIfs; ii++) { if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV) { - virBufferEscapeString(&buf, " <interface dev='%s'", + virBufferEscapeString(&buf, "<interface dev='%s'", def->forwardIfs[ii].device.dev); if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->forwardIfs[ii].connections > 0)) { @@ -1617,15 +1628,16 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) } } } + virBufferAdjustIndent(&buf, -2); if (def->nForwardPfs || def->nForwardIfs) - virBufferAddLit(&buf, " </forward>\n"); + virBufferAddLit(&buf, "</forward>\n"); } if (def->forwardType == VIR_NETWORK_FORWARD_NONE || def->forwardType == VIR_NETWORK_FORWARD_NAT || def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { - virBufferAddLit(&buf, " <bridge"); + virBufferAddLit(&buf, "<bridge"); if (def->bridge) virBufferEscapeString(&buf, " name='%s'", def->bridge); virBufferAsprintf(&buf, " stp='%s' delay='%ld' />\n", @@ -1633,43 +1645,40 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) def->delay); } else if (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && def->bridge) { - virBufferEscapeString(&buf, " <bridge name='%s' />\n", def->bridge); + virBufferEscapeString(&buf, "<bridge name='%s' />\n", def->bridge); } if (def->mac_specified) { char macaddr[VIR_MAC_STRING_BUFLEN]; virMacAddrFormat(&def->mac, macaddr); - virBufferAsprintf(&buf, " <mac address='%s'/>\n", macaddr); + virBufferAsprintf(&buf, "<mac address='%s'/>\n", macaddr); } if (def->domain) - virBufferAsprintf(&buf, " <domain name='%s'/>\n", def->domain); + virBufferAsprintf(&buf, "<domain name='%s'/>\n", def->domain); if (virNetworkDNSDefFormat(&buf, def->dns) < 0) goto error; - virBufferAdjustIndent(&buf, 2); if (virNetDevVlanFormat(&def->vlan, &buf) < 0) goto error; if (virNetDevBandwidthFormat(def->bandwidth, &buf) < 0) goto error; - virBufferAdjustIndent(&buf, -2); for (ii = 0; ii < def->nips; ii++) { if (virNetworkIpDefFormat(&buf, &def->ips[ii]) < 0) goto error; } - virBufferAdjustIndent(&buf, 2); if (virNetDevVPortProfileFormat(def->virtPortProfile, &buf) < 0) goto error; - virBufferAdjustIndent(&buf, -2); for (ii = 0; ii < def->nPortGroups; ii++) if (virPortGroupDefFormat(&buf, &def->portGroups[ii]) < 0) goto error; + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</network>\n"); if (virBufferError(&buf)) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 2a2a0ba..f49c367 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -261,7 +261,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags); static inline const char * virNetworkDefForwardIf(const virNetworkDefPtr def, size_t n) { - return ((def->forwardIfs && (def->nForwardIfs > n)) + return ((def->forwardIfs && (def->nForwardIfs > n) && + def->forwardIfs[n].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) ? def->forwardIfs[n].device.dev : NULL); } diff --git a/tests/networkxml2xmlin/hostdev-pf.xml b/tests/networkxml2xmlin/hostdev-pf.xml index fc82c55..7bf857d 100644 --- a/tests/networkxml2xmlin/hostdev-pf.xml +++ b/tests/networkxml2xmlin/hostdev-pf.xml @@ -1,7 +1,7 @@ <network> <name>hostdev</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> - <forward mode="hostdev" managed="yes"> + <forward mode='hostdev' managed='yes'> <pf dev='eth2'/> </forward> </network> diff --git a/tests/networkxml2xmlin/hostdev.xml b/tests/networkxml2xmlin/hostdev.xml index 0ec52d2..03f1411 100644 --- a/tests/networkxml2xmlin/hostdev.xml +++ b/tests/networkxml2xmlin/hostdev.xml @@ -1,10 +1,10 @@ <network> <name>hostdev</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> - <forward mode="hostdev" managed="yes"> - <address type='pci' domain='0' bus='3' slot='0' function='1'/> - <address type='pci' domain='0' bus='3' slot='0' function='2'/> - <address type='pci' domain='0' bus='3' slot='0' function='3'/> - <address type='pci' domain='0' bus='3' slot='0' function='4'/> + <forward mode='hostdev' managed='yes'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x2'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x4'/> </forward> </network> diff --git a/tests/networkxml2xmlin/passthrough-pf.xml b/tests/networkxml2xmlin/passthrough-pf.xml index e63aae0..ecdb953 100644 --- a/tests/networkxml2xmlin/passthrough-pf.xml +++ b/tests/networkxml2xmlin/passthrough-pf.xml @@ -3,8 +3,6 @@ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> <forward mode="passthrough"> <pf dev='eth0'/> - <interface dev='eth10'/> - <interface dev='eth11'/> </forward> <ip address="192.168.122.1" netmask="255.255.255.0"/> </network> diff --git a/tests/networkxml2xmlout/hostdev-pf.xml b/tests/networkxml2xmlout/hostdev-pf.xml index e955312..7bf857d 100644 --- a/tests/networkxml2xmlout/hostdev-pf.xml +++ b/tests/networkxml2xmlout/hostdev-pf.xml @@ -1,7 +1,7 @@ <network> <name>hostdev</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> - <forward mode="hostdev" managed="yes"> + <forward mode='hostdev' managed='yes'> <pf dev='eth2'/> - </forward> + </forward> </network> diff --git a/tests/networkxml2xmlout/hostdev.xml b/tests/networkxml2xmlout/hostdev.xml index 0ec52d2..03f1411 100644 --- a/tests/networkxml2xmlout/hostdev.xml +++ b/tests/networkxml2xmlout/hostdev.xml @@ -1,10 +1,10 @@ <network> <name>hostdev</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> - <forward mode="hostdev" managed="yes"> - <address type='pci' domain='0' bus='3' slot='0' function='1'/> - <address type='pci' domain='0' bus='3' slot='0' function='2'/> - <address type='pci' domain='0' bus='3' slot='0' function='3'/> - <address type='pci' domain='0' bus='3' slot='0' function='4'/> + <forward mode='hostdev' managed='yes'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x2'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x4'/> </forward> </network> -- 1.7.11.4
The network pool should be able to keep track of both, network device names nad PCI addresses, and return the appropriate one in the actualDevice when networkAllocateActualDevice is called. Signed-off-by: Shradha Shah <sshah@solarflare.com> --- src/network/bridge_driver.c | 60 +++++++++++++++++++++++++------------------ src/util/virnetdev.c | 25 ++++++++--------- src/util/virnetdev.h | 4 ++- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ddd66e5..38f6d12 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -59,6 +59,7 @@ #include "dnsmasq.h" #include "configmake.h" #include "virnetdev.h" +#include "pci.h" #include "virnetdevbridge.h" #include "virnetdevtap.h" #include "virnetdevvportprofile.h" @@ -2780,10 +2781,11 @@ static int networkCreateInterfacePool(virNetworkDefPtr netdef) { unsigned int num_virt_fns = 0; char **vfname = NULL; + struct pci_config_address **virt_fns; int ret = -1, ii = 0; if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev, - &vfname, &num_virt_fns)) < 0) { + &vfname, &virt_fns, &num_virt_fns)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), netdef->forwardPfs->dev); @@ -2805,18 +2807,34 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { netdef->nForwardIfs = num_virt_fns; for (ii = 0; ii < netdef->nForwardIfs; ii++) { - netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]); - if (!netdef->forwardIfs[ii].device.dev) { - virReportOOMError(); - goto finish; + if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || + (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || + (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || + (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) { + netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + if(vfname[ii]) { + netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]); + if (!netdef->forwardIfs[ii].device.dev) { + virReportOOMError(); + goto finish; + } + } + else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Direct mode types requires interface names")); + goto finish; + } } } ret = 0; finish: - for (ii = 0; ii < num_virt_fns; ii++) + for (ii = 0; ii < num_virt_fns; ii++) { VIR_FREE(vfname[ii]); + VIR_FREE(virt_fns[ii]); + } VIR_FREE(vfname); + VIR_FREE(virt_fns); return ret; } @@ -3008,31 +3026,23 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } else { /* pick an interface from the pool */ + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { + if ((networkCreateInterfacePool(netdef)) < 0) { + goto error; + } + } + /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both * require exclusive access to a device, so current * connections count must be 0. Other modes can share, so * just search for the one with the lowest number of * connections. */ - if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) { - if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { - if ((networkCreateInterfacePool(netdef)) < 0) { - goto error; - } - } - - /* pick first dev with 0 connections */ - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].connections == 0) { - dev = &netdef->forwardIfs[ii]; - break; - } - } - } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && - iface->data.network.actual->virtPortProfile && - (iface->data.network.actual->virtPortProfile->virtPortType - == VIR_NETDEV_VPORT_PROFILE_8021QBH)) { + if ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || + ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && + iface->data.network.actual->virtPortProfile && + (iface->data.network.actual->virtPortProfile->virtPortType + == VIR_NETDEV_VPORT_PROFILE_8021QBH))) { /* pick first dev with 0 connections */ for (ii = 0; ii < netdef->nForwardIfs; ii++) { diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 25bdf01..f9eba1a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -29,6 +29,7 @@ #include "command.h" #include "memory.h" #include "pci.h" +#include "logging.h" #include <sys/ioctl.h> #ifdef HAVE_NET_IF_H @@ -981,18 +982,18 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, int virNetDevGetVirtualFunctions(const char *pfname, char ***vfname, + struct pci_config_address ***virt_fns, unsigned int *n_vfname) { int ret = -1, i; char *pf_sysfs_device_link = NULL; char *pci_sysfs_device_link = NULL; - struct pci_config_address **virt_fns; char *pciConfigAddr; if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) return ret; - if (pciGetVirtualFunctions(pf_sysfs_device_link, &virt_fns, + if (pciGetVirtualFunctions(pf_sysfs_device_link, virt_fns, n_vfname) < 0) goto cleanup; @@ -1003,10 +1004,10 @@ virNetDevGetVirtualFunctions(const char *pfname, for (i = 0; i < *n_vfname; i++) { - if (pciGetDeviceAddrString(virt_fns[i]->domain, - virt_fns[i]->bus, - virt_fns[i]->slot, - virt_fns[i]->function, + if (pciGetDeviceAddrString((*virt_fns)[i]->domain, + (*virt_fns)[i]->bus, + (*virt_fns)[i]->slot, + (*virt_fns)[i]->function, &pciConfigAddr) < 0) { virReportSystemError(ENOSYS, "%s", _("Failed to get PCI Config Address String")); @@ -1019,20 +1020,17 @@ virNetDevGetVirtualFunctions(const char *pfname, } if (pciDeviceNetName(pci_sysfs_device_link, &((*vfname)[i])) < 0) { - virReportSystemError(ENOSYS, "%s", - _("Failed to get interface name of the VF")); - goto cleanup; + VIR_INFO("VF does not have an interface name"); } } ret = 0; cleanup: - if (ret < 0) + if (ret < 0) { VIR_FREE(*vfname); - for (i = 0; i < *n_vfname; i++) - VIR_FREE(virt_fns[i]); - VIR_FREE(virt_fns); + VIR_FREE(*virt_fns); + } VIR_FREE(pf_sysfs_device_link); VIR_FREE(pci_sysfs_device_link); VIR_FREE(pciConfigAddr); @@ -1169,6 +1167,7 @@ cleanup: int virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED, char ***vfname ATTRIBUTE_UNUSED, + struct pci_config_address ***virt_fns ATTRIBUTE_UNUSED, unsigned int *n_vfname ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index c663e49..705ad9c 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -26,6 +26,7 @@ # include "virsocketaddr.h" # include "virnetlink.h" # include "virmacaddr.h" +# include "pci.h" int virNetDevExists(const char *brname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; @@ -103,9 +104,10 @@ int virNetDevGetPhysicalFunction(const char *ifname, char **pfname) int virNetDevGetVirtualFunctions(const char *pfname, char ***vfname, + struct pci_config_address ***virt_fns, unsigned int *n_vfname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; int virNetDevLinkDump(const char *ifname, int ifindex, struct nlattr **tb, -- 1.7.4.4
On 08/16/2012 11:41 AM, Shradha Shah wrote:
The network pool should be able to keep track of both, network device names nad PCI addresses, and return the appropriate one in the actualDevice when networkAllocateActualDevice is called.
Signed-off-by: Shradha Shah <sshah@solarflare.com>
ACK again. It's good that the autogeneration of the device pool from pf now works for PRIVATE and BRIDGE modes too.
This function is needed by the network driver in a later commit. This function is useful in functions like networkNotifyActualDevice and networkReleaseActualDevice --- src/conf/device_conf.c | 16 ++++++++++++++++ src/conf/device_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 20 insertions(+), 0 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index ca600c5..8edcc0a 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -129,3 +129,19 @@ virDevicePCIAddressFormat(virBufferPtr buf, addr.function); return 0; } + +int +virDevicePCIAddressEqual(virDevicePCIAddress addr1, + virDevicePCIAddress addr2) +{ + int ret = -1; + + if (addr1.domain == addr2.domain && + addr1.bus == addr2.bus && + addr1.slot == addr2.slot && + addr1.function == addr2.function) { + ret = 0; + } + + return ret; +} diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index c679bce..7c4d356 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -59,6 +59,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf, virDevicePCIAddress addr, bool includeTypeInAddr); +int virDevicePCIAddressEqual(virDevicePCIAddress addr1, + virDevicePCIAddress addr2); + VIR_ENUM_DECL(virDeviceAddressPciMulti) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f32f8e..063d0bc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; virDevicePCIAddressFormat; +virDevicePCIAddressEqual; # dnsmasq.h dnsmasqAddDhcpHost; -- 1.7.4.4
On 08/16/2012 11:42 AM, Shradha Shah wrote:
This function is needed by the network driver in a later commit. This function is useful in functions like networkNotifyActualDevice and networkReleaseActualDevice --- src/conf/device_conf.c | 16 ++++++++++++++++ src/conf/device_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index ca600c5..8edcc0a 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -129,3 +129,19 @@ virDevicePCIAddressFormat(virBufferPtr buf, addr.function); return 0; } + +int +virDevicePCIAddressEqual(virDevicePCIAddress addr1, + virDevicePCIAddress addr2) +{ + int ret = -1;
The other xxxEqual() functions in libvirt return a bool true/false rather than 0 / -1. ACK with that fix (don't bother re-sending - I'll just fix it up (along with the places you call it in later patches) before I push.
+ + if (addr1.domain == addr2.domain && + addr1.bus == addr2.bus && + addr1.slot == addr2.slot && + addr1.function == addr2.function) { + ret = 0; + } + + return ret; +} diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index c679bce..7c4d356 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -59,6 +59,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf, virDevicePCIAddress addr, bool includeTypeInAddr);
+int virDevicePCIAddressEqual(virDevicePCIAddress addr1, + virDevicePCIAddress addr2); +
VIR_ENUM_DECL(virDeviceAddressPciMulti)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f32f8e..063d0bc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; virDevicePCIAddressFormat; +virDevicePCIAddressEqual;
# dnsmasq.h dnsmasqAddDhcpHost;
On 08/16/2012 09:42 AM, Shradha Shah wrote:
This function is needed by the network driver in a later commit. This function is useful in functions like networkNotifyActualDevice and networkReleaseActualDevice --- src/conf/device_conf.c | 16 ++++++++++++++++ src/conf/device_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 20 insertions(+), 0 deletions(-)
+++ b/src/libvirt_private.syms @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; virDevicePCIAddressFormat; +virDevicePCIAddressEqual;
Several patches have now failed to sort this section of the file. I'm okay with either rebasing to touch it up as you go, or one big cleanup at the end of the series. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/16/2012 04:07 PM, Eric Blake wrote:
On 08/16/2012 09:42 AM, Shradha Shah wrote:
This function is needed by the network driver in a later commit. This function is useful in functions like networkNotifyActualDevice and networkReleaseActualDevice --- src/conf/device_conf.c | 16 ++++++++++++++++ src/conf/device_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 20 insertions(+), 0 deletions(-) +++ b/src/libvirt_private.syms @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; virDevicePCIAddressFormat; +virDevicePCIAddressEqual; Several patches have now failed to sort this section of the file. I'm okay with either rebasing to touch it up as you go, or one big cleanup at the end of the series.
Now that you've pointed it out, I'm just squashing in the simple fixes to keep the order alphabetic as they go (along with a few other minor changes like extra spaces at the end of lines, etc). (Doing a "make check" and "make syntax-check" after applying each patch, I also found some problems in 3/7 that cause networkxml2xmltest to fail - that required enough changes that I'll be posting a patch for review (and squashing into 3/7 before pushing) rather than just silently squashing in the changes.
This patch updates the network driver to properly utilize the new attributes/elements that are now in virNetworkDef Signed-off-by: Shradha Shah <sshah@solarflare.com> --- docs/formatnetwork.html.in | 62 ++++++++++ src/network/bridge_driver.c | 282 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 284 insertions(+), 60 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index ed9f7a9..5ab5f3c 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -223,6 +223,37 @@ (usually either a domain start, or a hotplug interface attach to a domain).<span class="since">Since 0.9.4</span> </dd> + <dt><code>hostdev</code></dt> + <dd> + This network facilitates PCI Passthrough of a network device. + A network device is chosen from the interface pool and + directly assigned to the guest using generic device + passthrough, after first optionally setting the device's MAC + address to the configured value, and optionally associating + the device with an 802.1Qbh capable switch using an optionally + specified <code><virtualport></code> element. + Note that - due to limitations in standard single-port PCI + ethernet card driver design - only SR-IOV (Single Root I/O + Virtualization) virtual function (VF) devices can be assigned + in this manner; to assign a standard single-port PCI or PCIe + ethernet card to a guest, use the traditional <code>< + hostdev></code> device definition. <span class="since"> + Since 0.10.0</span> + + <p>Note that this "intelligent passthrough" of network devices is + very similar to the functionality of a standard <code>< + hostdev></code> device, the difference being that this + method allows specifying a MAC address and <code><virtualport + ></code> for the passed-through device. If these capabilities + are not required, if you have a standard single-port PCI, PCIe, + or USB network card that doesn't support SR-IOV (and hence would + anyway lose the configured MAC address during reset after being + assigned to the guest domain), or if you are using a version of + libvirt older than 0.10.0, you should use a standard + <code><hostdev></code> definition to assign the device to + the guest instead of <code><forward mode='hostdev'/></code>. + </p> + </dd> </dl> As mentioned above, a <code><forward></code> element can have multiple <code><interface></code> subelements, each @@ -272,6 +303,37 @@ particular, 'passthrough' mode, and 'private' mode when using 802.1Qbh), libvirt will choose an unused physical interface or, if it can't find an unused interface, fail the operation.</p> + + <span class="since">since 0.9.12</span> and when using forward mode + 'hostdev' we specify the interface pool by using the + <code><address></code> element and <code>< + type></code> <code><domain></code> <code><bus></code> + <code><slot></code> and <code><function></code> + sub-elements. + + <pre> +... + <forward mode='hostdev' managed='yes'> + <address type='pci' domain='0' bus='4' slot='0' function='1'/> + <address type='pci' domain='0' bus='4' slot='0' function='2'/> + <address type='pci' domain='0' bus='4' slot='0' function='3'/> + </forward> +... + </pre> + + Alternatively the interface pool can also be defined using a + single physical function <code><pf></code> subelement to + call out the corresponding physical interface associated with + multiple virtual interfaces (similar to passthrough mode): + + <pre> +... + <forward mode='hostdev' managed='yes'> + <pf dev='eth0'/> + </forward> +... + </pre> + </dd> </dl> <h5><a name="elementQoS">Quality of service</a></h5> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 38f6d12..065af3e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -47,6 +47,7 @@ #include "datatypes.h" #include "bridge_driver.h" #include "network_conf.h" +#include "device_conf.h" #include "driver.h" #include "buf.h" #include "virpidfile.h" @@ -1935,7 +1936,7 @@ networkStartNetworkExternal(struct network_driver *driver ATTRIBUTE_UNUSED, virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1946,7 +1947,7 @@ static int networkShutdownNetworkExternal(struct network_driver *driver ATTRIBUT virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is shutdown. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1977,6 +1978,7 @@ networkStartNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkStartNetworkExternal(driver, network); break; } @@ -2036,6 +2038,7 @@ static int networkShutdownNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkShutdownNetworkExternal(driver, network); break; } @@ -2825,6 +2828,13 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { goto finish; } } + else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; /*Assuming PCI as VF's are PCI devices */ + netdef->forwardIfs[ii].device.pci.domain = virt_fns[ii]->domain; + netdef->forwardIfs[ii].device.pci.bus = virt_fns[ii]->bus; + netdef->forwardIfs[ii].device.pci.slot = virt_fns[ii]->slot; + netdef->forwardIfs[ii].device.pci.function = virt_fns[ii]->function; + } } ret = 0; @@ -2958,6 +2968,65 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } } + } else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { + virReportOOMError(); + goto cleanup; + } + + iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV; + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { + if(networkCreateInterfacePool(netdef) < 0) { + goto cleanup; + } + } + /* pick first dev with 0 connections */ + + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (netdef->forwardIfs[ii].connections == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' requires exclusive access to interfaces, but none are available"), + netdef->name); + goto cleanup; + } + iface->data.network.actual->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET; + iface->data.network.actual->data.hostdev.def.parent.data.net = iface; + iface->data.network.actual->data.hostdev.def.info = &iface->info; + iface->data.network.actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + iface->data.network.actual->data.hostdev.def.managed = netdef->managed; + iface->data.network.actual->data.hostdev.def.source.subsys.type = dev->type; + iface->data.network.actual->data.hostdev.def.source.subsys.u.pci = dev->device.pci; + + /* merge virtualports from interface, network, and portgroup to + * arrive at actual virtualport to use + */ + if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, + iface->virtPortProfile, + netdef->virtPortProfile, + portgroup + ? portgroup->virtPortProfile : NULL) < 0) { + goto error; + } + virtport = iface->data.network.actual->virtPortProfile; + if (virtport) { + /* make sure type is supported for macvtap connections */ + if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && + virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<virtualport type='%s'> not supported for network " + "'%s' which uses a macvtap device"), + virNetDevVPortTypeToString(virtport->virtPortType), + netdef->name); + goto error; + } + } + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || @@ -3124,8 +3193,14 @@ validate: if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) + VIR_DEBUG("Using physical device %s, %d connections", + dev->device.dev, dev->connections); + else + VIR_DEBUG("Using physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); } if (netdef) { @@ -3164,9 +3239,12 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) struct network_driver *driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; + const char *actualDev = NULL; + virDomainHostdevDefPtr def = NULL; int ret = -1; + enum virDomainNetType actualType = virDomainNetGetActualType(iface); + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; @@ -3181,52 +3259,86 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } netdef = network->def; - if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + if ((!iface->data.network.actual) || + ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && + (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); goto success; } - actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); - goto error; + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a direct mode, but has no source dev")); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + def = virDomainNetGetActualHostdev(iface); + if (!def) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a hostdev mode, but has no hostdev")); + goto error; + } } - if (netdef->nForwardIfs == 0) { + if ((netdef->nForwardIfs == 0) && (netdef->nForwardPfs == 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " - "has no forward dev and no interface pool"), + _("network '%s' uses a direct/hostdev mode, but has no forward dev and no interface pool"), netdef->name); goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; - /* find the matching interface and increment its connections */ - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; - break; + /* find the matching interface in the pool and increment its connections */ + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + dev = &netdef->forwardIfs[ii]; + break; + } } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%s' in use by domain"), + netdef->name, actualDev); + goto error; + } } - /* dev points at the physical device we want to use */ - if (!dev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), - netdef->name, actualDev); - goto error; + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { + if(networkCreateInterfacePool(netdef) < 0) { + goto error; + } + } + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (virDevicePCIAddressEqual(def->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%04x:%02x:%02x.%x' in use by domain"), + netdef->name, + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + goto error; + } } - /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require - * exclusive access to a device, so current connections count - * must be 0 in those cases. - */ + /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require + * exclusive access to a device, so current connections count + * must be 0 in those cases. + */ if ((dev->connections > 0) && ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || + (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) || ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && iface->data.network.actual->virtPortProfile && (iface->data.network.actual->virtPortProfile->virtPortType @@ -3238,8 +3350,16 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + VIR_DEBUG("Using physical device %s, connections %d", + dev->device.dev, dev->connections); + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_DEBUG("Using physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); + } } success: @@ -3273,9 +3393,12 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) struct network_driver *driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; + const char *actualDev = NULL; + virDomainHostdevDefPtr def = NULL; int ret = -1; + enum virDomainNetType actualType = virDomainNetGetActualType(iface); + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; @@ -3290,47 +3413,86 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) } netdef = network->def; - if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + if ((!iface->data.network.actual) || + ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && + (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); goto success; } - actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); - goto error; + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a direct mode, but has no source dev")); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + def = virDomainNetGetActualHostdev(iface); + if (!def) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a hostdev mode, but has no hostdev")); + goto error; + } } - if (netdef->nForwardIfs == 0) { + if ((netdef->nForwardIfs == 0) && (netdef->nForwardPfs == 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " + _("network '%s' uses a direct/hostdev mode, but " "has no forward dev and no interface pool"), netdef->name); goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; - break; + + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%s' in use by domain"), + netdef->name, actualDev); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (virDevicePCIAddressEqual(def->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%04x:%02x:%02x.%x' in use by domain"), + netdef->name, + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + goto error; } } - /* dev points at the physical device we've been using */ - if (!dev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), - netdef->name, actualDev); - goto error; - } - dev->connections--; - VIR_DEBUG("Releasing physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + VIR_DEBUG("Releasing physical device %s, connections %d", + dev->device.dev, dev->connections); + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_DEBUG("Releasing physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, + dev->device.pci.bus, + dev->device.pci.slot, + dev->device.pci.function, + dev->connections); + } } success: -- 1.7.4.4
On 08/16/2012 11:42 AM, Shradha Shah wrote:
This patch updates the network driver to properly utilize the new attributes/elements that are now in virNetworkDef
Signed-off-by: Shradha Shah <sshah@solarflare.com> --- docs/formatnetwork.html.in | 62 ++++++++++ src/network/bridge_driver.c | 282 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 284 insertions(+), 60 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 38f6d12..065af3e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -47,6 +47,7 @@ #include "datatypes.h" #include "bridge_driver.h" #include "network_conf.h" +#include "device_conf.h" #include "driver.h" #include "buf.h" #include "virpidfile.h" @@ -1935,7 +1936,7 @@ networkStartNetworkExternal(struct network_driver *driver ATTRIBUTE_UNUSED, virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1946,7 +1947,7 @@ static int networkShutdownNetworkExternal(struct network_driver *driver ATTRIBUT virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is shutdown. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1977,6 +1978,7 @@ networkStartNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkStartNetworkExternal(driver, network); break; } @@ -2036,6 +2038,7 @@ static int networkShutdownNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkShutdownNetworkExternal(driver, network); break; } @@ -2825,6 +2828,13 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { goto finish; } } + else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; /*Assuming PCI as VF's are PCI devices */
Long lines are generally discouraged.
+ netdef->forwardIfs[ii].device.pci.domain = virt_fns[ii]->domain; + netdef->forwardIfs[ii].device.pci.bus = virt_fns[ii]->bus; + netdef->forwardIfs[ii].device.pci.slot = virt_fns[ii]->slot; + netdef->forwardIfs[ii].device.pci.function = virt_fns[ii]->function; + } }
ret = 0; @@ -2958,6 +2968,65 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } }
+ } else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { + virReportOOMError(); + goto cleanup;
My patches that created the merge conflicts for you changed the meaning of cleanup, and added an "error" goto that should now be used for error exits, so this (and the others below) should be "goto error;"
+ } + + iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV; + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { + if(networkCreateInterfacePool(netdef) < 0) { + goto cleanup; + } + }
Unnecessary double nesting - you could put it all in one expression.
+ /* pick first dev with 0 connections */ + + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (netdef->forwardIfs[ii].connections == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' requires exclusive access to interfaces, but none are available"), + netdef->name); + goto cleanup; + } + iface->data.network.actual->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET; + iface->data.network.actual->data.hostdev.def.parent.data.net = iface; + iface->data.network.actual->data.hostdev.def.info = &iface->info; + iface->data.network.actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + iface->data.network.actual->data.hostdev.def.managed = netdef->managed; + iface->data.network.actual->data.hostdev.def.source.subsys.type = dev->type; + iface->data.network.actual->data.hostdev.def.source.subsys.u.pci = dev->device.pci; + + /* merge virtualports from interface, network, and portgroup to + * arrive at actual virtualport to use + */ + if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, + iface->virtPortProfile, + netdef->virtPortProfile, + portgroup + ? portgroup->virtPortProfile : NULL) < 0) { + goto error; + } + virtport = iface->data.network.actual->virtPortProfile; + if (virtport) { + /* make sure type is supported for macvtap connections */ + if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && + virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<virtualport type='%s'> not supported for network " + "'%s' which uses a macvtap device"),
You didn't change the string when you did the cut-paste (same for the comment a few lines up)
+ virNetDevVPortTypeToString(virtport->virtPortType), + netdef->name); + goto error; + } + } + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || @@ -3124,8 +3193,14 @@ validate: if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) + VIR_DEBUG("Using physical device %s, %d connections", + dev->device.dev, dev->connections); + else + VIR_DEBUG("Using physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); }
if (netdef) { @@ -3164,9 +3239,12 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
This function (and also networkReleaseActualDevice) were very badly affected by all the churn and the merge conflict resolution. It ended up to be something like this: if (typeA) Astuff else if (typeB) Bstuff if (typeA) Astuff else if (typeB) Bstuff dev->connections++; if (typeA) Astuff else if (typeB) Bstuff This is what made me decide to refactor the those two functions based on your latest code. I turned it into: if (typeA) Astuff Astuff dev->connections++; Astuff else /* if (typeB) */ Bstuff Bstuff dev->connections++; Bstuff (the 2nd if can go into comments because we already determined at the top of the function that it is either typeA or typeB (i.e. HOSTDEV or DIRECT) I'll be posting a proposed replacement patch for this one in a few minutes. Please try it out as soon as possible. I haven't gone through 7/7 yet, but with the small changes I've squashed in elsewhere, plus the 3.5/7 I posted and the 6/7 refactor I'm about to post, definitely the first 6 are ready to push. (By the way, a general note - I've been running "make check" and "make syntax-check" after applying each of your patches in succession, and came up with a *lot* of errors (see my add-on patch that I want to squash into 3/7). In particular, you seem to end up with a lot of lines that have trailing blanks - if you run "make syntax-check" on each patch, it will catch those. There are occasional slipups, but in general every patch is supposed to pass both make check and make syntax-check (and for a patch series, both of those should complete after applying each patch, not just after the end of the series). ===================
struct network_driver *driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; + const char *actualDev = NULL; + virDomainHostdevDefPtr def = NULL; int ret = -1;
+ enum virDomainNetType actualType = virDomainNetGetActualType(iface); + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0;
@@ -3181,52 +3259,86 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } netdef = network->def;
- if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + if ((!iface->data.network.actual) || + ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && + (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); goto success; }
- actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); - goto error; + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a direct mode, but has no source dev")); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + def = virDomainNetGetActualHostdev(iface); + if (!def) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a hostdev mode, but has no hostdev")); + goto error; + } }
- if (netdef->nForwardIfs == 0) { + if ((netdef->nForwardIfs == 0) && (netdef->nForwardPfs == 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " - "has no forward dev and no interface pool"), + _("network '%s' uses a direct/hostdev mode, but has no forward dev and no interface pool"), netdef->name); goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL;
- /* find the matching interface and increment its connections */ - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; - break; + /* find the matching interface in the pool and increment its connections */ + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + dev = &netdef->forwardIfs[ii]; + break; + } } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%s' in use by domain"), + netdef->name, actualDev); + goto error; + } } - /* dev points at the physical device we want to use */ - if (!dev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), - netdef->name, actualDev); - goto error; + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) { + if(networkCreateInterfacePool(netdef) < 0) { + goto error; + } + } + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (virDevicePCIAddressEqual(def->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%04x:%02x:%02x.%x' in use by domain"), + netdef->name, + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + goto error; + } }
- /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require - * exclusive access to a device, so current connections count - * must be 0 in those cases. - */ + /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require + * exclusive access to a device, so current connections count + * must be 0 in those cases. + */ if ((dev->connections > 0) && ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || + (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) || ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && iface->data.network.actual->virtPortProfile && (iface->data.network.actual->virtPortProfile->virtPortType @@ -3238,8 +3350,16 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + VIR_DEBUG("Using physical device %s, connections %d", + dev->device.dev, dev->connections); + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_DEBUG("Using physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); + } }
success: @@ -3273,9 +3393,12 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) struct network_driver *driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; + const char *actualDev = NULL; + virDomainHostdevDefPtr def = NULL; int ret = -1;
+ enum virDomainNetType actualType = virDomainNetGetActualType(iface); + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0;
@@ -3290,47 +3413,86 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) } netdef = network->def;
- if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + if ((!iface->data.network.actual) || + ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && + (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); goto success; }
- actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); - goto error; + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a direct mode, but has no source dev")); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + def = virDomainNetGetActualHostdev(iface); + if (!def) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a hostdev mode, but has no hostdev")); + goto error; + } }
- if (netdef->nForwardIfs == 0) { + if ((netdef->nForwardIfs == 0) && (netdef->nForwardPfs == 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " + _("network '%s' uses a direct/hostdev mode, but " "has no forward dev and no interface pool"), netdef->name); goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; - - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; - break; + + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%s' in use by domain"), + netdef->name, actualDev); + goto error; + } + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (virDevicePCIAddressEqual(def->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have dev='%04x:%02x:%02x.%x' in use by domain"), + netdef->name, + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + goto error; } } - /* dev points at the physical device we've been using */ - if (!dev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), - netdef->name, actualDev); - goto error; - } - dev->connections--; - VIR_DEBUG("Releasing physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + VIR_DEBUG("Releasing physical device %s, connections %d", + dev->device.dev, dev->connections); + } + else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_DEBUG("Releasing physical device with domain=%d bus=%d slot=%d function=%d, connections %d", + dev->device.pci.domain, + dev->device.pci.bus, + dev->device.pci.slot, + dev->device.pci.function, + dev->connections); + } }
success:
I'll be posting a proposed replacement patch for this one in a few minutes. Please try it out as soon as possible. I haven't gone through 7/7 yet, but with the small changes I've squashed in elsewhere, plus the 3.5/7 I posted and the 6/7 refactor I'm about to post, definitely the first 6 are ready to push.
Laine, Sorry for the errors that have been caused in this patch and thank you for refactoring the code. I have tested the patches 3.5 and 6 that you have sent and everything works fine. Do you want me to post another iteration of the patch series?
(By the way, a general note - I've been running "make check" and "make syntax-check" after applying each of your patches in succession, and came up with a *lot* of errors (see my add-on patch that I want to squash into 3/7). In particular, you seem to end up with a lot of lines that have trailing blanks - if you run "make syntax-check" on each patch, it will catch those. There are occasional slipups, but in general every patch is supposed to pass both make check and make syntax-check (and for a patch series, both of those should complete after applying each patch, not just after the end of the series).
Sorry I did not realize I had to perform these checks as well. I will make sure I perform these checks henceforth.
===================
On 08/17/2012 10:11 AM, Shradha Shah wrote:
I'll be posting a proposed replacement patch for this one in a few minutes. Please try it out as soon as possible. I haven't gone through 7/7 yet, but with the small changes I've squashed in elsewhere, plus the 3.5/7 I posted and the 6/7 refactor I'm about to post, definitely the first 6 are ready to push. Laine,
Sorry for the errors that have been caused in this patch and thank you for refactoring the code. I have tested the patches 3.5 and 6 that you have sent and everything works fine. Do you want me to post another iteration of the patch series?
No need, if you ACK them, then I can push them.
(By the way, a general note - I've been running "make check" and "make syntax-check" after applying each of your patches in succession, and came up with a *lot* of errors (see my add-on patch that I want to squash into 3/7). In particular, you seem to end up with a lot of lines that have trailing blanks - if you run "make syntax-check" on each patch, it will catch those. There are occasional slipups, but in general every patch is supposed to pass both make check and make syntax-check (and for a patch series, both of those should complete after applying each patch, not just after the end of the series). Sorry I did not realize I had to perform these checks as well. I will make sure I perform these checks henceforth.
Don't worry about it. Even those of us who know it forget now and then. I'll squash in the changes, rebase and rerun make check one last time, and push the series within the hour. Thanks again for the contribution - this is a very useful feature!
From: Shradha Shah <sshah@solarflare.com> This patch updates the network driver to properly utilize the new attributes/elements that are now in virNetworkDef Signed-off-by: Shradha Shah <sshah@solarflare.com> Signed-off-by: Laine Stump <laine@laine.org> --- This patch should *replace* patch 6/7 in Shradha's "Forward mode='hostdev' patch series". I decided that networkNotifyActualDevice and networkReleaseActualDevice needed to be refactored (see my review of that patch for more detail). This patch should provide exactly the same functionality as that patch, but the resulting code is easier to follow. docs/formatnetwork.html.in | 69 +++++++++++ src/network/bridge_driver.c | 281 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 305 insertions(+), 45 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index ed9f7a9..49206dd 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -223,6 +223,42 @@ (usually either a domain start, or a hotplug interface attach to a domain).<span class="since">Since 0.9.4</span> </dd> + <dt><code>hostdev</code></dt> + <dd> + This network facilitates PCI Passthrough of a network + device. A network device is chosen from the interface + pool and directly assigned to the guest using generic + device passthrough, after first optionally setting the + device's MAC address and vlan tag to the configured value, + and optionally associating the device with an 802.1Qbh + capable switch using a <code><virtualport></code> + element. Note that - due to limitations in standard + single-port PCI ethernet card driver design - only SR-IOV + (Single Root I/O Virtualization) virtual function (VF) + devices can be assigned in this manner; to assign a + standard single-port PCI or PCIe ethernet card to a guest, + use the traditional <code>< hostdev></code> device + definition. <span class="since"> Since 0.10.0</span> + + <p>Note that this "intelligent passthrough" of network + devices is very similar to the functionality of a + standard <code>< hostdev></code> device, the + difference being that this method allows specifying a MAC + address, vlan tag, and <code><virtualport ></code> + for the passed-through device. If these capabilities are + not required, if you have a standard single-port PCI, + PCIe, or USB network card that doesn't support SR-IOV (and + hence would anyway lose the configured MAC address during + reset after being assigned to the guest domain), or if you + are using a version of libvirt older than 0.10.0, you + should use a standard + <code><hostdev></code> device definition in the + domain's configuration to assign the device to the guest + instead of defining an <code><interface + type='network'></code> pointing to a network + with <code><forward mode='hostdev'/></code>. + </p> + </dd> </dl> As mentioned above, a <code><forward></code> element can have multiple <code><interface></code> subelements, each @@ -272,6 +308,39 @@ particular, 'passthrough' mode, and 'private' mode when using 802.1Qbh), libvirt will choose an unused physical interface or, if it can't find an unused interface, fail the operation.</p> + + <p> + <span class="since">since 0.10.0</span> When using forward + mode 'hostdev', the interface pool is specified with a list + of <code><address></code> elements, each of which has + <code>< type></code> (must always be <code>'pci'</code>, + <code><domain></code>, <code><bus></code>, + <code><slot></code>, and <code><function></code> + attributes. + </p> + <pre> +... + <forward mode='hostdev' managed='yes'> + <address type='pci' domain='0' bus='4' slot='0' function='1'/> + <address type='pci' domain='0' bus='4' slot='0' function='2'/> + <address type='pci' domain='0' bus='4' slot='0' function='3'/> + </forward> +... + </pre> + + Alternatively the interface pool can also be defined using a + single physical function <code><pf></code> subelement to + call out the corresponding physical interface associated with + multiple virtual interfaces (similar to passthrough mode): + + <pre> +... + <forward mode='hostdev' managed='yes'> + <pf dev='eth0'/> + </forward> +... + </pre> + </dd> </dl> <h5><a name="elementQoS">Quality of service</a></h5> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5cb79af..ea05c38 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -47,6 +47,7 @@ #include "datatypes.h" #include "bridge_driver.h" #include "network_conf.h" +#include "device_conf.h" #include "driver.h" #include "buf.h" #include "virpidfile.h" @@ -1935,7 +1936,7 @@ networkStartNetworkExternal(struct network_driver *driver ATTRIBUTE_UNUSED, virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1946,7 +1947,7 @@ static int networkShutdownNetworkExternal(struct network_driver *driver ATTRIBUT virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of - * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is shutdown. On + * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On * failure, undo anything you've done, and return -1. On success * return 0. */ @@ -1977,6 +1978,7 @@ networkStartNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkStartNetworkExternal(driver, network); break; } @@ -2036,6 +2038,7 @@ static int networkShutdownNetwork(struct network_driver *driver, case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: ret = networkShutdownNetworkExternal(driver, network); break; } @@ -2825,6 +2828,14 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { goto finish; } } + else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + /* VF's are always PCI devices */ + netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; + netdef->forwardIfs[ii].device.pci.domain = virt_fns[ii]->domain; + netdef->forwardIfs[ii].device.pci.bus = virt_fns[ii]->bus; + netdef->forwardIfs[ii].device.pci.slot = virt_fns[ii]->slot; + netdef->forwardIfs[ii].device.pci.function = virt_fns[ii]->function; + } } ret = 0; @@ -2958,6 +2969,67 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } } + } else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { + virReportOOMError(); + goto error; + } + + iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV; + if (netdef->nForwardPfs > 0 && netdef->nForwardIfs <= 0 && + networkCreateInterfacePool(netdef) < 0) { + goto error; + } + + /* pick first dev with 0 connections */ + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (netdef->forwardIfs[ii].connections == 0) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' requires exclusive access " + "to interfaces, but none are available"), + netdef->name); + goto error; + } + iface->data.network.actual->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET; + iface->data.network.actual->data.hostdev.def.parent.data.net = iface; + iface->data.network.actual->data.hostdev.def.info = &iface->info; + iface->data.network.actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + iface->data.network.actual->data.hostdev.def.managed = netdef->managed; + iface->data.network.actual->data.hostdev.def.source.subsys.type = dev->type; + iface->data.network.actual->data.hostdev.def.source.subsys.u.pci = dev->device.pci; + + /* merge virtualports from interface, network, and portgroup to + * arrive at actual virtualport to use + */ + if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, + iface->virtPortProfile, + netdef->virtPortProfile, + portgroup + ? portgroup->virtPortProfile : NULL) < 0) { + goto error; + } + virtport = iface->data.network.actual->virtPortProfile; + if (virtport) { + /* make sure type is supported for hostdev connections */ + if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && + virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<virtualport type='%s'> not supported for network " + "'%s' which uses an SR-IOV Virtual Function " + "via PCI passthrough"), + virNetDevVPortTypeToString(virtport->virtPortType), + netdef->name); + goto error; + } + } + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || @@ -3123,8 +3195,15 @@ validate: if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); + if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_DEBUG("Using physical device %s, %d connections", + dev->device.dev, dev->connections); + } else { + VIR_DEBUG("Using physical device %04x:%02x:%02x.%x, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); + } } if (netdef) { @@ -3161,10 +3240,11 @@ int networkNotifyActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; + enum virDomainNetType actualType = virDomainNetGetActualType(iface); virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; - int ret = -1; + virNetworkForwardIfDefPtr dev = NULL; + int ii, ret = -1; if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; @@ -3181,33 +3261,40 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) netdef = network->def; if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + (actualType != VIR_DOMAIN_NET_TYPE_DIRECT && + actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV)) { VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); goto success; } - actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); + if (netdef->nForwardPfs > 0 && netdef->nForwardIfs == 0 && + networkCreateInterfacePool(netdef) < 0) { goto error; } - if (netdef->nForwardIfs == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " - "has no forward dev and no interface pool"), + _("network '%s' uses a direct or hostdev mode, " + "but has no forward dev and no interface pool"), netdef->name); goto error; - } else { - int ii; - virNetworkForwardIfDefPtr dev = NULL; + } - /* find the matching interface and increment its connections */ + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + const char *actualDev; + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("the interface uses a direct mode, " + "but has no source dev")); + goto error; + } + + /* find the matching interface and increment its connections */ for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + if (netdef->forwardIfs[ii].type + == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV && + STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { dev = &netdef->forwardIfs[ii]; break; } @@ -3215,12 +3302,13 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) /* dev points at the physical device we want to use */ if (!dev) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), + _("network '%s' doesn't have dev='%s' " + "in use by domain"), netdef->name, actualDev); goto error; } - /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require + /* PASSTHROUGH mode and PRIVATE Mode + 802.1Qbh both require * exclusive access to a device, so current connections count * must be 0 in those cases. */ @@ -3231,14 +3319,73 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) (iface->data.network.actual->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH)))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' claims dev='%s' is already in use by a different domain"), + _("network '%s' claims dev='%s' is already in " + "use by a different domain"), netdef->name, actualDev); goto error; } + /* we are now assured of success, so mark the allocation */ dev->connections++; - VIR_DEBUG("Using physical device %s, %d connections", + VIR_DEBUG("Using physical device %s, connections %d", dev->device.dev, dev->connections); + + } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ { + virDomainHostdevDefPtr hostdev; + + hostdev = virDomainNetGetActualHostdev(iface); + if (!hostdev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("the interface uses a hostdev mode, " + "but has no hostdev")); + goto error; + } + + /* find the matching interface and increment its connections */ + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (netdef->forwardIfs[ii].type + == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && + (virDevicePCIAddressEqual(hostdev->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0)) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + /* dev points at the physical device we want to use */ + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have " + "PCI device %04x:%02x:%02x.%x in use by domain"), + netdef->name, + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + goto error; + } + + /* PASSTHROUGH mode, PRIVATE Mode + 802.1Qbh, and hostdev (PCI + * passthrough) all require exclusive access to a device, so + * current connections count must be 0 in those cases. + */ + if ((dev->connections > 0) && + netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' claims the PCI device at " + "domain=%d bus=%d slot=%d function=%d " + "is already in use by a different domain"), + netdef->name, + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function); + goto error; + } + + /* we are now assured of success, so mark the allocation */ + dev->connections++; + VIR_DEBUG("Using physical device %04x:%02x:%02x.%x, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); } success: @@ -3270,10 +3417,11 @@ int networkReleaseActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; + enum virDomainNetType actualType = virDomainNetGetActualType(iface); virNetworkObjPtr network; virNetworkDefPtr netdef; - const char *actualDev; - int ret = -1; + virNetworkForwardIfDefPtr dev = NULL; + int ii, ret = -1; if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; @@ -3289,48 +3437,91 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) } netdef = network->def; - if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + if ((!iface->data.network.actual) || + ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && + (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); goto success; } - actualDev = virDomainNetGetActualDirectDev(iface); - if (!actualDev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("the interface uses a direct " - "mode, but has no source dev")); - goto error; - } - if (netdef->nForwardIfs == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' uses a direct mode, but " + _("network '%s' uses a direct/hostdev mode, but " "has no forward dev and no interface pool"), netdef->name); goto error; - } else { - int ii; - virNetworkForwardIfDefPtr dev = NULL; + } + + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + const char *actualDev; + + actualDev = virDomainNetGetActualDirectDev(iface); + if (!actualDev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("the interface uses a direct mode, " + "but has no source dev")); + goto error; + } for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { + if (netdef->forwardIfs[ii].type + == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV && + STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { dev = &netdef->forwardIfs[ii]; break; } } - /* dev points at the physical device we've been using */ + if (!dev) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have dev='%s' in use by domain"), + _("network '%s' doesn't have dev='%s' " + "in use by domain"), netdef->name, actualDev); goto error; } dev->connections--; - VIR_DEBUG("Releasing physical device %s, %d connections", + VIR_DEBUG("Releasing physical device %s, connections %d", dev->device.dev, dev->connections); - } + + } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ { + virDomainHostdevDefPtr hostdev; + + hostdev = virDomainNetGetActualHostdev(iface); + if (!hostdev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("the interface uses a hostdev mode, but has no hostdev")); + goto error; + } + + for (ii = 0; ii < netdef->nForwardIfs; ii++) { + if (netdef->forwardIfs[ii].type + == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && + (virDevicePCIAddressEqual(hostdev->source.subsys.u.pci, + netdef->forwardIfs[ii].device.pci) == 0)) { + dev = &netdef->forwardIfs[ii]; + break; + } + } + + if (!dev) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have " + "PCI device %04x:%02x:%02x.%x in use by domain"), + netdef->name, + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + goto error; + } + + dev->connections--; + VIR_DEBUG("Releasing physical device %04x:%02x:%02x.%x, connections %d", + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); + } success: netdef->connections--; -- 1.7.11.4
For a network with <forward mode='hostdev', there is a need to add the newly minted hostdev to the hostdevs array. In this case we also call qemuPrepareHostDevicesas it has already been called by the time we get to here and are building the qemu commandline Signed-off-by: Shradha Shah <sshah@solarflare.com> --- src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f6c6cd..1470edd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -24,6 +24,7 @@ #include <config.h> #include "qemu_command.h" +#include "qemu_hostdev.h" #include "qemu_capabilities.h" #include "qemu_bridge_filter.h" #include "cpu/cpu.h" @@ -5221,12 +5222,38 @@ qemuBuildCommandLine(virConnectPtr conn, actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - /* type='hostdev' interfaces are handled in codepath - * for standard hostdev (NB: when there is a network - * with <forward mode='hostdev', there will need to be - * code here that adds the newly minted hostdev to the - * hostdevs array). - */ + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevDefPtr found; + /* For a network with <forward mode='hostdev', there is a need to + * add the newly minted hostdev to the hostdevs array. + */ + if (qemuAssignDeviceHostdevAlias(def, hostdev, + (def->nhostdevs-1)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not assign alias to Net Hostdev")); + goto error; + } + + if (virDomainHostdevFind(def, hostdev, &found) < 0) { + if (virDomainHostdevInsert(def, hostdev) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Hostdev not inserted into the array")); + goto error; + } + if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, + &hostdev, 1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Prepare Hostdev PCI Devices failed")); + goto error; + } + } + else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The Hostdev is being used by some other device")); + goto error; + } + } continue; } -- 1.7.4.4
I just found this review that I had typed up and not actually sent. I've made the small changes and pushed the series in the meantime, but figured I should send this just for archival purposes. On 08/16/2012 11:42 AM, Shradha Shah wrote:
For a network with <forward mode='hostdev', there is a need to add the newly minted hostdev to the hostdevs array.
In this case we also call qemuPrepareHostDevicesas it has already been called by the time we get to here and are building the qemu commandline
Signed-off-by: Shradha Shah <sshah@solarflare.com> --- src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f6c6cd..1470edd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -24,6 +24,7 @@ #include <config.h>
#include "qemu_command.h" +#include "qemu_hostdev.h" #include "qemu_capabilities.h" #include "qemu_bridge_filter.h" #include "cpu/cpu.h" @@ -5221,12 +5222,38 @@ qemuBuildCommandLine(virConnectPtr conn,
actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - /* type='hostdev' interfaces are handled in codepath - * for standard hostdev (NB: when there is a network - * with <forward mode='hostdev', there will need to be - * code here that adds the newly minted hostdev to the - * hostdevs array). - */ + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevDefPtr found; + /* For a network with <forward mode='hostdev', there is a need to + * add the newly minted hostdev to the hostdevs array. + */ + if (qemuAssignDeviceHostdevAlias(def, hostdev, + (def->nhostdevs-1)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not assign alias to Net Hostdev"));
qemuAssignDeviceHostdevAlias logs its own errors, so I'm removing this log message.
+ goto error; + } + + if (virDomainHostdevFind(def, hostdev, &found) < 0) { + if (virDomainHostdevInsert(def, hostdev) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Hostdev not inserted into the array"));
virDomainHostdevInsert *doesn't* log its own errors. However, the only reason it can fail is if we're out of memory, so I'm replacing this log with virReportOOMError().
+ goto error; + } + if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, + &hostdev, 1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Prepare Hostdev PCI Devices failed"));
This function also logs its own errors, so I'm removing this message
+ goto error; + } + } + else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The Hostdev is being used by some other device"));
How about this instead? (by this time we're guaranteed that all of those bits of info are valid) virReportError(VIR_ERR_INTERNAL_ERROR, _("PCI device %04x:%02x:%02x.%x " "allocated from network %s is already " "in use by domain %s"), hostdev->source.subsys.u.pci.domain, hostdev->source.subsys.u.pci.bus, hostdev->source.subsys.u.pci.slot, hostdev->source.subsys.u.pci.function, net->data.network.name, def->name); ACK with the log message changes. I'll squash them in before I push.
participants (3)
-
Eric Blake -
Laine Stump -
Shradha Shah