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(a)solarflare.com&gt; --- 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(a)redhat.com&gt; + */ + +#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(a)redhat.com&gt; + */ + +#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 12:12 PM, Laine Stump wrote: 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; }

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(a)solarflare.com&gt; --- 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

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 09:42 AM, Shradha Shah wrote: 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(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch updates the network driver to properly utilize the new attributes/elements that are now in virNetworkDef Signed-off-by: Shradha Shah <sshah(a)solarflare.com&gt; --- 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>&lt;virtualport&gt;</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>&lt; + hostdev&gt;</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>&lt; + hostdev&gt;</code> device, the difference being that this + method allows specifying a MAC address and <code>&lt;virtualport + &gt;</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>&lt;hostdev&gt;</code> definition to assign the device to + the guest instead of <code>&lt;forward mode='hostdev'/&gt;</code>. + </p> + </dd> </dl> As mentioned above, a <code>&lt;forward&gt;</code> element can have multiple <code>&lt;interface&gt;</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>&lt;address&gt;</code> element and <code>&lt; + type&gt;</code> <code>&lt;domain&gt;</code> <code>&lt;bus&gt;</code> + <code>&lt;slot&gt;</code> and <code>&lt;function&gt;</code> + sub-elements. + + <pre> +... + &lt;forward mode='hostdev' managed='yes'&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='1'/&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='2'/&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='3'/&gt; + &lt;/forward&gt; +... + </pre> + + Alternatively the interface pool can also be defined using a + single physical function <code>&lt;pf&gt;</code> subelement to + call out the corresponding physical interface associated with + multiple virtual interfaces (similar to passthrough mode): + + <pre> +... + &lt;forward mode='hostdev' managed='yes'&gt; + &lt;pf dev='eth0'/&gt; + &lt;/forward&gt; +... + </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

From: Shradha Shah <sshah(a)solarflare.com&gt; This patch updates the network driver to properly utilize the new attributes/elements that are now in virNetworkDef Signed-off-by: Shradha Shah <sshah(a)solarflare.com&gt; Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- 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>&lt;virtualport&gt;</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>&lt; hostdev&gt;</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>&lt; hostdev&gt;</code> device, the + difference being that this method allows specifying a MAC + address, vlan tag, and <code>&lt;virtualport &gt;</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>&lt;hostdev&gt;</code> device definition in the + domain's configuration to assign the device to the guest + instead of defining an <code>&lt;interface + type='network'&gt;</code> pointing to a network + with <code>&lt;forward mode='hostdev'/&gt;</code>. + </p> + </dd> </dl> As mentioned above, a <code>&lt;forward&gt;</code> element can have multiple <code>&lt;interface&gt;</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>&lt;address&gt;</code> elements, each of which has + <code>&lt; type&gt;</code> (must always be <code>'pci'</code>, + <code>&lt;domain&gt;</code>, <code>&lt;bus&gt;</code>, + <code>&lt;slot&gt;</code>, and <code>&lt;function&gt;</code> + attributes. + </p> + <pre> +... + &lt;forward mode='hostdev' managed='yes'&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='1'/&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='2'/&gt; + &lt;address type='pci' domain='0' bus='4' slot='0' function='3'/&gt; + &lt;/forward&gt; +... + </pre> + + Alternatively the interface pool can also be defined using a + single physical function <code>&lt;pf&gt;</code> subelement to + call out the corresponding physical interface associated with + multiple virtual interfaces (similar to passthrough mode): + + <pre> +... + &lt;forward mode='hostdev' managed='yes'&gt; + &lt;pf dev='eth0'/&gt; + &lt;/forward&gt; +... + </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

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: qemuAssignDeviceHostdevAlias logs its own errors, so I'm removing this log message. 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(). This function also logs its own errors, so I'm removing this message 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.