On 03/05/2012 08:12 PM, Roopa Prabhu wrote:

From: Roopa Prabhu <roprabhu@cisco.com>

pciDeviceGetVirtualFunctionInfo returns pf netdevice name and virtual function index for a given vf. This is just a wrapper around existing functions to return vf's pf and vf_index with one api call

pciConfigAddressToSysfsfile returns the sysfile pci device link from a 'struct pci_config_address'

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com> --- src/util/pci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/pci.h | 7 +++++++ 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c index c660e8d..c8a5287 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -2081,6 +2081,20 @@ pciSysfsFile(char *pciDeviceName, char **pci_sysfs_device_link) return 0; }

+int +pciConfigAddressToSysfsFile(struct pci_config_address *dev, + char **pci_sysfs_device_link) +{ + if (virAsprintf(pci_sysfs_device_link, + PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain, + dev->bus, dev->slot, dev->function) < 0) { + virReportOOMError(); + return -1; + } + + return 0; +} + /* * Returns the network device name of a pci device */ @@ -2123,6 +2137,38 @@ out:

return ret; } + +int +pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path, + char **pfname, int *vf_index) +{ + struct pci_config_address *pf_config_address = NULL; + char *pf_sysfs_device_path = NULL; + int ret = -1; + + if (pciGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address) < 0) + return ret; + + if (pciConfigAddressToSysfsFile(pf_config_address, + &pf_sysfs_device_path) < 0) { + + VIR_FREE(pf_config_address); + return ret; + } + + if (pciGetVirtualFunctionIndex(pf_sysfs_device_path, vf_sysfs_device_path, + vf_index) < 0) + goto cleanup; + + ret = pciDeviceNetName(pf_sysfs_device_path, pfname); + +cleanup: + VIR_FREE(pf_config_address); + VIR_FREE(pf_sysfs_device_path); + + return ret; +} + #else int pciGetPhysicalFunction(const char *vf_sysfs_path ATTRIBUTE_UNUSED, @@ -2170,4 +2216,13 @@ pciDeviceNetName(char *device_link_sysfs_path ATTRIBUTE_UNUSED, "supported on non-linux platforms")); return -1; } + +int +pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path ATTRIBUTE_UNUSED, + char **pfname, int *vf_index ATTRIBUTE_UNUSED) +{ + pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceGetVirtualFunctionInfo " + "is not supported on non-linux platforms")); + return -1; +} #endif /* __linux__ */ diff --git a/src/util/pci.h b/src/util/pci.h index 25b5b66..b71bb12 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -111,6 +111,9 @@ int pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, const char *vf_sysfs_device_link, int *vf_index);

+int pciConfigAddressToSysfsFile(struct pci_config_address *dev, + char **pci_sysfs_device_link); + int pciDeviceNetName(char *device_link_sysfs_path, char **netname);

int pciSysfsFile(char *pciDeviceName, char **pci_sysfs_device_link) @@ -122,4 +125,8 @@ int pciGetDeviceAddrString(unsigned domain, unsigned function, char **pciConfigAddr) ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; + +int pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path, + char **pfname, int *vf_index); + #endif /* __VIR_PCI_H__ */

After already ACKing this, I realized that the new functions weren't added to libvirt_private.syms. I'm squashing the following in before I push: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2639b48..5d37618 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -864,6 +864,7 @@ virNWFilterVarValueGetSimple; # pci.h +pciConfigAddressToSysfsFile; pciDettachDevice; pciDeviceFileIterate; pciDeviceGetManaged; @@ -872,6 +873,7 @@ pciDeviceGetRemoveSlot; pciDeviceGetReprobe; pciDeviceGetUnbindFromStub; pciDeviceGetUsedBy; +pciDeviceGetVirtualFunctionInfo; pciDeviceIsAssignable; pciDeviceIsVirtualFunction; pciDeviceListAdd;

On 03/05/2012 08:12 PM, Roopa Prabhu wrote:

From: Roopa Prabhu <roprabhu@cisco.com>

This patch adds the following: - functions to set and get vf configs - Functions to replace and store vf configs (Only mac address is handled today. But the functions can be easily extended for vlans and other vf configs) - function to dump link dev info (This is moved from virnetdevvportprofile.c)

Looks like you addressed all my issues. (I need to add "cleanup return paths of virnetdev" to my todo list to address the bits of ugliness the code movement pointed out).

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com> --- src/util/virnetdev.c | 535 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdev.h | 19 ++ 2 files changed, 553 insertions(+), 1 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 4aa7639..9f93fda 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1127,8 +1127,47 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)

return ret; } -#else /* !__linux__ */

+/** + * virNetDevGetVirtualFunctionInfo: + * @vfname: name of the virtual function interface + * @pfname: name of the physical function + * @vf: vf index + * + * Returns 0 on success, -errno on failure. + * + */ +int +virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname, + int *vf) +{ + char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL; + int ret = -1; + + *pfname = NULL; + + if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) + return ret; + + if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0) + goto cleanup; + + if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0) + goto cleanup; + + ret = pciGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf); + +cleanup: + if (ret < 0) + VIR_FREE(*pfname); + + VIR_FREE(vf_sysfs_path); + VIR_FREE(pf_sysfs_path); + + return ret; +}

I didn't notice before that this new function wasn't being defined in the case that HAVE_LIBNL isn't defined. I'm squashing in an empty function just like all the others.

+ +#else /* !__linux__ */ int virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED, char ***vfname ATTRIBUTE_UNUSED, @@ -1165,4 +1204,498 @@ virNetDevGetPhysicalFunction(const char *ifname ATTRIBUTE_UNUSED, _("Unable to get physical function status on this platform")); return -1; } + #endif /* !__linux__ */ +#if defined(__linux__) && defined(HAVE_LIBNL) + +static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { + [IFLA_VF_MAC] = { .type = NLA_UNSPEC, + .maxlen = sizeof(struct ifla_vf_mac) }, + [IFLA_VF_VLAN] = { .type = NLA_UNSPEC, + .maxlen = sizeof(struct ifla_vf_vlan) }, +}; + +/** + * virNetDevLinkDump: + * + * @ifname: The name of the interface; only use if ifindex < 0 + * @ifindex: The interface index; may be < 0 if ifname is given + * @nltarget_kernel: whether to send the message to the kernel or another + * process + * @nlattr: pointer to a pointer of netlink attributes that will contain + * the results + * @recvbuf: Pointer to the buffer holding the returned netlink response + * message; free it, once not needed anymore + * @getPidFunc: Pointer to a function that will be invoked if the kernel + * is not the target of the netlink message but it is to be + * sent to another process. + * + * Get information about an interface given its name or index. + * + * Returns 0 on success, -1 on fatal error. + */ +int +virNetDevLinkDump(const char *ifname, int ifindex, + bool nltarget_kernel, struct nlattr **tb, + unsigned char **recvbuf, + uint32_t (*getPidFunc)(void)) +{ + int rc = 0; + struct nlmsghdr *resp; + struct nlmsgerr *err; + struct ifinfomsg ifinfo = { + .ifi_family = AF_UNSPEC, + .ifi_index = ifindex + }; + unsigned int recvbuflen; + uint32_t pid = 0; + struct nl_msg *nl_msg; + + *recvbuf = NULL; + + if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0) + return -1; + + ifinfo.ifi_index = ifindex; + + nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST); + if (!nl_msg) { + virReportOOMError(); + return -1; + } + + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) + goto buffer_too_small; + + if (ifname) { + if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) + goto buffer_too_small; + } + + if (!nltarget_kernel) { + pid = getPidFunc(); + if (pid == 0) { + rc = -1; + goto cleanup; + } + } + + if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) { + rc = -1; + goto cleanup; + } + + if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) + goto malformed_resp; + + resp = (struct nlmsghdr *)*recvbuf; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + goto malformed_resp; + + if (err->error) { + virReportSystemError(-err->error, + _("error dumping %s (%d) interface"), + ifname, ifindex); + rc = -1; + } + break; + + case GENL_ID_CTRL: + case NLMSG_DONE: + rc = nlmsg_parse(resp, sizeof(struct ifinfomsg), + tb, IFLA_MAX, NULL); + if (rc < 0) + goto malformed_resp; + break; + + default: + goto malformed_resp; + } + + if (rc != 0) + VIR_FREE(*recvbuf); + +cleanup: + nlmsg_free(nl_msg); + + return rc; + +malformed_resp: + nlmsg_free(nl_msg); + + virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + VIR_FREE(*recvbuf); + return -1; + +buffer_too_small: + nlmsg_free(nl_msg); + + virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", + _("allocated netlink buffer is too small")); + return -1; +} + +static int +virNetDevSetVfConfig(const char *ifname, int ifindex, int vf, + bool nltarget_kernel, const unsigned char *macaddr, + int vlanid, uint32_t (*getPidFunc)(void)) +{ + int rc = -1; + struct nlmsghdr *resp; + struct nlmsgerr *err; + unsigned char *recvbuf = NULL; + unsigned int recvbuflen = 0; + uint32_t pid = 0; + struct nl_msg *nl_msg; + struct nlattr *vfinfolist, *vfinfo; + struct ifinfomsg ifinfo = { + .ifi_family = AF_UNSPEC, + .ifi_index = ifindex + }; + + if (!macaddr && vlanid < 0) + return -1; + + nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST); + if (!nl_msg) { + virReportOOMError(); + return rc; + } + + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) + goto buffer_too_small; + + if (ifname && + nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) + goto buffer_too_small; + + + if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST))) + goto buffer_too_small; + + if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO))) + goto buffer_too_small; + + if (macaddr) { + struct ifla_vf_mac ifla_vf_mac = { + .vf = vf, + .mac = { 0, }, + }; + + memcpy(ifla_vf_mac.mac, macaddr, VIR_MAC_BUFLEN); + + if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), + &ifla_vf_mac) < 0) + goto buffer_too_small; + } + + if (vlanid >= 0) { + struct ifla_vf_vlan ifla_vf_vlan = { + .vf = vf, + .vlan = vlanid, + .qos = 0, + }; + + if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), + &ifla_vf_vlan) < 0) + goto buffer_too_small; + } + + nla_nest_end(nl_msg, vfinfo); + nla_nest_end(nl_msg, vfinfolist); + + if (!nltarget_kernel) { + pid = getPidFunc(); + if (pid == 0) { + rc = -1; + goto cleanup; + } + } + + if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, pid) < 0) + goto cleanup; + + if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + goto malformed_resp; + + resp = (struct nlmsghdr *)recvbuf; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + goto malformed_resp; + + if (err->error) { + virReportSystemError(-err->error, + _("error during set %s of ifindex %d"), + (macaddr ? (vlanid >= 0 ? "mac/vlan" : "mac") : "vlanid"), + ifindex); + goto cleanup; + } + break; + + case NLMSG_DONE: + break; + + default: + goto malformed_resp; + } + + rc = 0; + +cleanup: + nlmsg_free(nl_msg); + + VIR_FREE(recvbuf); + + return rc; + +malformed_resp: + nlmsg_free(nl_msg); + + virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + VIR_FREE(recvbuf); + return rc; + +buffer_too_small: + nlmsg_free(nl_msg); + + virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", + _("allocated netlink buffer is too small")); + return rc; +} + +static int +virNetDevParseVfConfig(struct nlattr **tb, int32_t vf, unsigned char *mac, + int *vlanid) +{ + const char *msg = NULL; + int rc = -1; + + if (tb[IFLA_VFINFO_LIST]) { + struct ifla_vf_mac *vf_mac; + struct ifla_vf_vlan *vf_vlan; + struct nlattr *tb_vf_info = {NULL, }; + struct nlattr *tb_vf[IFLA_VF_MAX+1]; + int found = 0; + int rem; + + nla_for_each_nested(tb_vf_info, tb[IFLA_VFINFO_LIST], rem) { + if (nla_type(tb_vf_info) != IFLA_VF_INFO) + continue; + + if (nla_parse_nested(tb_vf, IFLA_VF_MAX, tb_vf_info, + ifla_vf_policy)) { + msg = _("error parsing IFLA_VF_INFO"); + goto cleanup; + } + + if (tb[IFLA_VF_MAC]) { + vf_mac = RTA_DATA(tb_vf[IFLA_VF_MAC]); + if (vf_mac && vf_mac->vf == vf) { + memcpy(mac, vf_mac->mac, VIR_MAC_BUFLEN); + found = 1; + } + } + + if (tb[IFLA_VF_VLAN]) { + vf_vlan = RTA_DATA(tb_vf[IFLA_VF_VLAN]); + if (vf_vlan && vf_vlan->vf == vf) { + *vlanid = vf_vlan->vlan; + found = 1; + } + } + if (found) { + rc = 0; + break; + } + } + } + +cleanup: + if (msg) + virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", msg); + + return rc; +} + +static int +virNetDevGetVfConfig(const char *ifname, int vf, unsigned char *mac, + int *vlanid) +{ + int rc = -1; + unsigned char *recvbuf = NULL; + struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; + int ifindex = -1; + + rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); + if (rc < 0) + return rc; + + rc = virNetDevParseVfConfig(tb, vf, mac, vlanid); + + VIR_FREE(recvbuf); + + return rc; +} + +static int +virNetDevReplaceVfConfig(const char *pflinkdev, int vf, + const unsigned char *macaddress, + int vlanid, + const char *stateDir) +{ + unsigned char oldmac[6]; + int oldvlanid = -1; + char *path = NULL; + char macstr[VIR_MAC_STRING_BUFLEN]; + int ifindex = -1; + + if (virNetDevGetVfConfig(pflinkdev, vf, oldmac, &oldvlanid) < 0) + return -1; + + if (virAsprintf(&path, "%s/%s_vf%d", + stateDir, pflinkdev, vf) < 0) { + virReportOOMError(); + return -1; + } + + virMacAddrFormat(oldmac, macstr); + if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) { + virReportSystemError(errno, _("Unable to preserve mac for pf = %s," + " vf = %d"), pflinkdev, vf); + VIR_FREE(path); + return -1; + } + + VIR_FREE(path); + + return virNetDevSetVfConfig(pflinkdev, ifindex, vf, true, + macaddress, vlanid, NULL); +} + +static int +virNetDevRestoreVfConfig(const char *pflinkdev, int vf, + const char *stateDir) +{ + int rc = -1; + char *macstr = NULL; + char *path = NULL; + unsigned char oldmac[6]; + int vlanid = -1; + int ifindex = -1; + + if (virAsprintf(&path, "%s/%s_vf%d", + stateDir, pflinkdev, vf) < 0) { + virReportOOMError(); + return rc; + } + + if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0) { + goto cleanup; + } + + if (virMacAddrParse(macstr, &oldmac[0]) != 0) { + virNetDevError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse MAC address from '%s'"), + macstr); + goto cleanup; + } + + /*reset mac and remove file-ignore results*/ + rc = virNetDevSetVfConfig(pflinkdev, ifindex, vf, true, + oldmac, vlanid, NULL); + ignore_value(unlink(path)); + +cleanup: + VIR_FREE(path); + VIR_FREE(macstr); + + return rc; +} + +/** + * virNetDevReplaceNetConfig: + * @linkdev: name of the interface + * @vf: vf index if linkdev is a pf + * @macaddress: new MAC address for interface + * @vlanid: new vlanid + * @stateDir: directory to store old net config + * + * Returns 0 on success, -1 on failure + * + */ +int +virNetDevReplaceNetConfig(char *linkdev, int vf, + const unsigned char *macaddress, int vlanid, + char *stateDir) +{ + if (vf == -1) + return virNetDevReplaceMacAddress(linkdev, macaddress, stateDir); + else + return virNetDevReplaceVfConfig(linkdev, vf, macaddress, vlanid, + stateDir); +} + +/** + * virNetDevRestoreNetConfig: + * @linkdev: name of the interface + * @vf: vf index if linkdev is a pf + * @stateDir: directory containing old net config + * + * Returns 0 on success, -errno on failure. + * + */ +int +virNetDevRestoreNetConfig(char *linkdev, int vf, char *stateDir) +{ + if (vf == -1) + return virNetDevRestoreMacAddress(linkdev, stateDir); + else + return virNetDevRestoreVfConfig(linkdev, vf, stateDir); +} + +#else /* defined(__linux__) && defined(HAVE_LIBNL) */ +virNetDevLinkDump(const char *ifname ATTRIBUTE_UNUSED, + int ifindex ATTRIBUTE_UNUSED, + bool nltarget_kernel ATTRIBUTE_UNUSED, + struct nlattr **tb ATTRIBUTE_UNUSED, + unsigned char **recvbuf ATTRIBUTE_UNUSED, + uint32_t (*getPidFunc)(void) ATTRIBUTE_UNUSED)

This function definition was missing an "int" return type at the beginning. I'm squashing one in. Also, this function uses struct nlattr, while is defined in linux/netlink.h, but that isn't #included when HAVE_LIBNL isn't defined. I added a "forward" declaration of struct nlattr in util/virnetlink.h (there are already examples of declaring netlink/libnl structs there). We may later decide we need to use something less platform-specific there, but for now the function is only useful/used on systems with libnl anyway, and this fix at least allows it to compile properly on platforms without libnl.

+{ + virReportSystemError(ENOSYS, "%s", + _("Unable to dump link info on this platform")); + return -1; +} + +int +virNetDevReplaceNetConfig(char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, + const unsigned char *macaddress ATTRIBUTE_UNUSED, + int vlanid ATTRIBUTE_UNUSED, + char *stateDir ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to replace net config on this platform")); + return -1; + +} + +int +virNetDevRestoreNetConfig(char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, + char *stateDir ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to restore net config on this platform")); + return -1; +} + +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 3456113..73f4c64 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -24,6 +24,7 @@ # define __VIR_NETDEV_H__

# include "virsocketaddr.h" +# include "virnetlink.h"

int virNetDevExists(const char *brname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; @@ -105,4 +106,22 @@ int virNetDevGetVirtualFunctions(const char *pfname, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;

+int virNetDevLinkDump(const char *ifname, int ifindex, + bool nltarget_kernel, struct nlattr **tb, + unsigned char **recvbuf, + uint32_t (*getPidFunc)(void)) + ATTRIBUTE_RETURN_CHECK; + +int virNetDevReplaceNetConfig(char *linkdev, int vf, + const unsigned char *macaddress, int vlanid, + char *stateDir) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); + +int virNetDevRestoreNetConfig(char *linkdev, int vf, char *stateDir) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + +int virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname, + int *vf) + ATTRIBUTE_NONNULL(1); + #endif /* __VIR_NETDEV_H__ */

Again, we forgot to add the new global functions to src/libvirt_private.syms. I'm attaching what I'll squash in before I push. ACK with the extra bits squashed in.

On 03/05/2012 08:12 PM, Roopa Prabhu wrote:

From: Roopa Prabhu <roprabhu@cisco.com>

This patch includes the following changes - removes some netlink functions which are now available in virnetdev.c - Adds a vf argument to all port profile functions

For 802.1Qbh devices, the port profile calls can use a vf argument if passed by the caller. If the vf argument is -1 it will try to derive the vf if the device passed is a virtual function.

For 802.1Qbg devices, This patch introduces a null check for the device argument because during port profile assignment on a hostdev, this argument can be null. Stefan CC'ed for comments

ACK. (I'm shortening the summary line so that it's < 72 columns :-) I'll push this along with the others after I review 4/4.

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com> --- src/qemu/qemu_migration.c | 2 src/util/virnetdevmacvlan.c | 8 + src/util/virnetdevvportprofile.c | 221 ++++++++------------------------------ src/util/virnetdevvportprofile.h | 8 + 4 files changed, 59 insertions(+), 180 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5c4297c..77d40c0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2650,6 +2650,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { virDomainNetGetActualVirtPortProfile(net), net->mac, virDomainNetGetActualDirectDev(net), + -1, def->uuid, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0) goto err_exit; @@ -2667,6 +2668,7 @@ err_exit: virDomainNetGetActualVirtPortProfile(net), net->mac, virDomainNetGetActualDirectDev(net), + -1, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)); } } diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index f38a98c..647679f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -452,6 +452,7 @@ struct virNetlinkCallbackData { virNetDevVPortProfilePtr virtPortProfile; unsigned char *macaddress; char *linkdev; + int vf; unsigned char *vmuuid; enum virNetDevVPortProfileOp vmOp; unsigned int linkState; @@ -719,6 +720,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, calld->virtPortProfile, calld->macaddress, calld->linkdev, + calld->vf, calld->vmuuid, calld->vmOp, true)); *handled = true; @@ -810,6 +812,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const char *cr_ifname; virNetlinkCallbackDataPtr calld = NULL; int ret; + int vf = -1;

macvtapMode = modeMap[mode];

@@ -871,6 +874,7 @@ create_name: virtPortProfile, macaddress, linkdev, + vf, vmuuid, vmOp, false) < 0) { rc = -1; goto link_del_exit; @@ -948,6 +952,7 @@ disassociate_exit: virtPortProfile, macaddress, linkdev, + vf, vmOp));

link_del_exit: @@ -975,6 +980,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, char *stateDir) { int ret = 0; + int vf = -1; + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir)); } @@ -984,6 +991,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, virtPortProfile, macaddr, linkdev, + vf, VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0) ret = -1; if (virNetDevMacVLanDelete(ifname) < 0) diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index f6db292..bd356d8 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -126,11 +126,6 @@ static struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = { [IFLA_PORT_RESPONSE] = { .type = NLA_U16 }, }; -static struct nla_policy ifla_policy[IFLA_MAX + 1] = -{ - [IFLA_VF_PORTS] = { .type = NLA_NESTED }, -}; -

static uint32_t virNetDevVPortProfileGetLldpadPid(void) { @@ -164,126 +159,6 @@ virNetDevVPortProfileGetLldpadPid(void) { return pid; }

- -/** - * virNetDevVPortProfileLinkDump: - * - * @ifname: The name of the interface; only use if ifindex < 0 - * @ifindex: The interface index; may be < 0 if ifname is given - * @nltarget_kernel: whether to send the message to the kernel or another - * process - * @nlattr: pointer to a pointer of netlink attributes that will contain - * the results - * @recvbuf: Pointer to the buffer holding the returned netlink response - * message; free it, once not needed anymore - * @getPidFunc: Pointer to a function that will be invoked if the kernel - * is not the target of the netlink message but it is to be - * sent to another process. - * - * Get information about an interface given its name or index. - * - * Returns 0 on success, -1 on fatal error. - */ -static int -virNetDevVPortProfileLinkDump(const char *ifname, int ifindex, bool nltarget_kernel, - struct nlattr **tb, unsigned char **recvbuf, - uint32_t (*getPidFunc)(void)) -{ - int rc = 0; - struct nlmsghdr *resp; - struct nlmsgerr *err; - struct ifinfomsg ifinfo = { - .ifi_family = AF_UNSPEC, - .ifi_index = ifindex - }; - unsigned int recvbuflen; - uint32_t pid = 0; - struct nl_msg *nl_msg; - - *recvbuf = NULL; - - nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST); - if (!nl_msg) { - virReportOOMError(); - return -1; - } - - if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) - goto buffer_too_small; - - if (ifindex < 0 && ifname) { - if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) - goto buffer_too_small; - } - - if (!nltarget_kernel) { - pid = getPidFunc(); - if (pid == 0) { - rc = -1; - goto cleanup; - } - } - - if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) { - rc = -1; - goto cleanup; - } - - if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) - goto malformed_resp; - - resp = (struct nlmsghdr *)*recvbuf; - - switch (resp->nlmsg_type) { - case NLMSG_ERROR: - err = (struct nlmsgerr *)NLMSG_DATA(resp); - if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) - goto malformed_resp; - - if (err->error) { - virReportSystemError(-err->error, - _("error dumping %s (%d) interface"), - ifname, ifindex); - rc = -1; - } - break; - - case GENL_ID_CTRL: - case NLMSG_DONE: - if (nlmsg_parse(resp, sizeof(struct ifinfomsg), - tb, IFLA_MAX, ifla_policy)) { - goto malformed_resp; - } - break; - - default: - goto malformed_resp; - } - - if (rc != 0) - VIR_FREE(*recvbuf); - -cleanup: - nlmsg_free(nl_msg); - - return rc; - -malformed_resp: - nlmsg_free(nl_msg); - - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed netlink response message")); - VIR_FREE(*recvbuf); - return -1; - -buffer_too_small: - nlmsg_free(nl_msg); - - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", - _("allocated netlink buffer is too small")); - return -1; -} - /** * virNetDevVPortProfileGetStatus: * @@ -607,7 +482,7 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int return -1;

while (!end && i <= nthParent) { - rc = virNetDevVPortProfileLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); + rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); if (rc < 0) break;

@@ -680,8 +555,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, return 0;

while (--repeats >= 0) { - rc = virNetDevVPortProfileLinkDump(NULL, ifindex, nltarget_kernel, tb, - &recvbuf, virNetDevVPortProfileGetLldpadPid); + rc = virNetDevLinkDump(NULL, ifindex, nltarget_kernel, tb, + &recvbuf, virNetDevVPortProfileGetLldpadPid); if (rc < 0) goto err_exit;

@@ -754,6 +629,7 @@ virNetDevVPortProfileGetPhysdevAndVlan(const char *ifname, int *root_ifindex, ch static int virNetDevVPortProfileOp8021Qbg(const char *ifname, const unsigned char *macaddr, + int vf, const virNetDevVPortProfilePtr virtPort, enum virNetDevVPortProfileLinkOp virtPortOp, bool setlink_only) @@ -768,7 +644,11 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, int vlanid; int physdev_ifindex = 0; char physdev_ifname[IFNAMSIZ] = { 0, }; - int vf = PORT_SELF_VF; + + if (!ifname) + return -1; + + vf = PORT_SELF_VF;

if (virNetDevVPortProfileGetPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname, &vlanid) < 0) { @@ -817,46 +697,11 @@ err_exit: return rc; }

- -static int -virNetDevVPortProfileGetPhysfnDev(const char *linkdev, - int32_t *vf, - char **physfndev) -{ - int rc = -1; - - if (virNetDevIsVirtualFunction(linkdev) == 1) { - /* if linkdev is SR-IOV VF, then set vf = VF index */ - /* and set linkdev = PF device */ - - rc = virNetDevGetPhysicalFunction(linkdev, physfndev); - if (!rc) - rc = virNetDevGetVirtualFunctionIndex(*physfndev, linkdev, vf); - } else { - - /* Not SR-IOV VF: physfndev is linkdev and VF index - * refers to linkdev self - */ - - *vf = PORT_SELF_VF; - *physfndev = strdup(linkdev); - if (!*physfndev) { - virReportOOMError(); - goto err_exit; - } - rc = 0; - } - -err_exit: - - return rc; -} - - /* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int virNetDevVPortProfileOp8021Qbh(const char *ifname, const unsigned char *macaddr, + int32_t vf, const virNetDevVPortProfilePtr virtPort, const unsigned char *vm_uuid, enum virNetDevVPortProfileLinkOp virtPortOp) @@ -864,18 +709,36 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, int rc = 0; char *physfndev = NULL; unsigned char hostuuid[VIR_UUID_BUFLEN]; - int32_t vf; bool nltarget_kernel = true; int ifindex; int vlanid = -1; + bool is_vf = false;

- rc = virNetDevVPortProfileGetPhysfnDev(ifname, &vf, &physfndev); - if (rc < 0) - goto err_exit; + if (vf == -1) { + int isvf_ret = virNetDevIsVirtualFunction(ifname); + + if (isvf_ret == -1) + goto cleanup; + is_vf = !!isvf_ret; + } + + if (is_vf) { + if (virNetDevGetVirtualFunctionInfo(ifname, &physfndev, &vf) < 0) { + rc = -1; + goto cleanup; + } + } else { + physfndev = strdup(ifname); + if (!physfndev) { + virReportOOMError(); + rc = -1; + goto cleanup; + } + }

rc = virNetDevGetIndex(physfndev, &ifindex); if (rc < 0) - goto err_exit; + goto cleanup;

switch (virtPortOp) { case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR: @@ -883,7 +746,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, errno = virGetHostUUID(hostuuid); if (errno) { rc = -1; - goto err_exit; + goto cleanup; }

rc = virNetDevVPortProfileOpCommon(NULL, ifindex, @@ -935,7 +798,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, rc = -1; }

-err_exit: +cleanup: VIR_FREE(physfndev); return rc; } @@ -963,6 +826,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macvtap_macaddr, const char *linkdev, + int vf, const unsigned char *vmuuid, enum virNetDevVPortProfileOp vmOp, bool setlink_only) @@ -970,7 +834,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, int rc = 0;

VIR_DEBUG("Associating port profile '%p' on link device '%s'", - virtPort, macvtap_ifname); + virtPort, (macvtap_ifname ? macvtap_ifname : linkdev));

VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__, virNetDevVPortProfileOpTypeToString(vmOp));

@@ -985,7 +849,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname,

case VIR_NETDEV_VPORT_PROFILE_8021QBG: rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, - virtPort, + vf, virtPort, (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE : VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE, @@ -993,7 +857,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, break;

case VIR_NETDEV_VPORT_PROFILE_8021QBH: - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf, virtPort, vmuuid, (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR @@ -1026,6 +890,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macvtap_macaddr, const char *linkdev, + int vf, enum virNetDevVPortProfileOp vmOp) { int rc = 0; @@ -1045,7 +910,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, break;

case VIR_NETDEV_VPORT_PROFILE_8021QBG: - rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, vf, virtPort, VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE, false); break; @@ -1055,7 +920,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) break; ignore_value(virNetDevSetOnline(linkdev, false)); - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf, virtPort, NULL, VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE); break; @@ -1069,6 +934,7 @@ int virNetDevVPortProfileAssociate(const char *macvtap_ifname ATTRIBUTE_UNUSED, const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED, const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, const unsigned char *vmuuid ATTRIBUTE_UNUSED, enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED, bool setlink_only ATTRIBUTE_UNUSED) @@ -1082,6 +948,7 @@ int virNetDevVPortProfileDisassociate(const char *macvtap_ifname ATTRIBUTE_UNUSE const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED, const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index eb9f2b1..dff5943 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -85,18 +85,20 @@ int virNetDevVPortProfileAssociate(const char *ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macaddr, const char *linkdev, + int vf, const unsigned char *vmuuid, enum virNetDevVPortProfileOp vmOp, bool setlink_only) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) - ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6) + ATTRIBUTE_RETURN_CHECK;

int virNetDevVPortProfileDisassociate(const char *ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macaddr, const char *linkdev, + int vf, enum virNetDevVPortProfileOp vmOp) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;

On 03/05/2012 08:12 PM, Roopa Prabhu wrote:

From: Roopa Prabhu <roprabhu@cisco.com>

These changes are applied only if the hostdev has a parent net device. If the parent netdevice has virtual port information, the original virtualport associate functions are called (these set and restore both mac and port profile on an interface). Else, only mac address is set on the device using other methods depending on if its a sriov device or not.

Changes also include hotplug pci devices

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com> --- src/qemu/qemu_hostdev.c | 241 +++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_hostdev.h | 8 +- src/qemu/qemu_hotplug.c | 10 ++ 3 files changed, 246 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index b3cad8e..ebcdc52 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -29,6 +29,13 @@ #include "memory.h" #include "pci.h" #include "hostusb.h" +#include "virnetdev.h" + +VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, + "none", + "802.1Qbg", + "802.1Qbh", + "openvswitch")

static pciDeviceList * qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) @@ -156,19 +163,192 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, return 0; }

+static int +qemuDomainHostdevPciSysfsPath(virDomainHostdevDefPtr hostdev, char **sysfs_path) +{ + struct pci_config_address config_address; + + config_address.domain = hostdev->source.subsys.u.pci.domain; + config_address.bus = hostdev->source.subsys.u.pci.bus; + config_address.slot = hostdev->source.subsys.u.pci.slot; + config_address.function = hostdev->source.subsys.u.pci.function; + + return pciConfigAddressToSysfsFile(&config_address, sysfs_path); +} + +int +qemuDomainHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev) +{ + char *sysfs_path = NULL; + int ret = -1; + + if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0) + return ret; + + ret = pciDeviceIsVirtualFunction(sysfs_path); + + VIR_FREE(sysfs_path); + + return ret; +} + +static int +qemuDomainHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, + int *vf) +{ + int ret = -1; + char *sysfs_path = NULL; + + if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0) + return ret; + + if (pciDeviceIsVirtualFunction(sysfs_path) == 1) { + if (pciDeviceGetVirtualFunctionInfo(sysfs_path, linkdev, + vf) < 0) + goto cleanup; + } else { + if (pciDeviceNetName(sysfs_path, linkdev) < 0) + goto cleanup; + *vf = -1; + } + + ret = 0; + +cleanup: + VIR_FREE(sysfs_path); + + return ret; +} + +static int +qemuDomainHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, + virNetDevVPortProfilePtr virtPort, + const unsigned char *macaddr, + const unsigned char *uuid, + int associate) +{ + int ret = -1; + + if (!virtPort) + return ret; + + switch(virtPort->virtPortType) { + case VIR_NETDEV_VPORT_PROFILE_NONE: + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + case VIR_NETDEV_VPORT_PROFILE_8021QBG: + case VIR_NETDEV_VPORT_PROFILE_LAST: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("virtualport type %s is " + "currently not supported on interfaces of type " + "hostdev"), + virNetDevVPortTypeToString(virtPort->virtPortType)); + break; + + case VIR_NETDEV_VPORT_PROFILE_8021QBH: + if (associate) + ret = virNetDevVPortProfileAssociate(NULL, virtPort, macaddr, + linkdev, vf, uuid, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, false); + else + ret = virNetDevVPortProfileDisassociate(NULL, virtPort, + macaddr, linkdev, vf, + VIR_NETDEV_VPORT_PROFILE_OP_DESTROY); + break; + } + + return ret; +} + +int +qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev, + const unsigned char *uuid, + char *stateDir) +{ + char *linkdev = NULL; + virNetDevVPortProfilePtr virtPort; + int ret = -1; + int vf = -1; + int vlanid = -1; + int port_profile_associate = 1; + int isvf; + + isvf = qemuDomainHostdevIsVirtualFunction(hostdev); + if (isvf <= 0) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Interface type hostdev is currently supported on" + " sriov Virtual functions only"));

I changed the capitalization a bit here: "SR-IOV Virtual Functions".

+ return ret; + } + + if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0) + return ret; + + virtPort = virDomainNetGetActualVirtPortProfile( + hostdev->parent.data.net); + if (virtPort) + ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf, + virtPort, hostdev->parent.data.net->mac, uuid, + port_profile_associate); + else + /* Set only mac */ + ret = virNetDevReplaceNetConfig(linkdev, vf, + hostdev->parent.data.net->mac, vlanid, + stateDir); + VIR_FREE(linkdev); + + return ret; +} + +int +qemuDomainHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, + char *stateDir) +{ + char *linkdev = NULL; + virNetDevVPortProfilePtr virtPort; + int ret = -1; + int vf = -1; + int port_profile_associate = 0; + int isvf; + + isvf = qemuDomainHostdevIsVirtualFunction(hostdev); + if (isvf <= 0) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Interface type hostdev is currently supported on" + " sriov Virtual functions only"));

+ return ret; + } + + if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0) + return ret; + + virtPort = virDomainNetGetActualVirtPortProfile( + hostdev->parent.data.net); + if (virtPort) + ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, + hostdev->parent.data.net->mac, NULL, + port_profile_associate); + else + ret = virNetDevRestoreNetConfig(linkdev, vf, stateDir); + + VIR_FREE(linkdev); + + return ret; +} + int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, const char *name, + const unsigned char *uuid, virDomainHostdevDefPtr *hostdevs, int nhostdevs) { pciDeviceList *pcidevs; + int last_processed_hostdev_vf = -1; int i; int ret = -1;

if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs))) return -1;

- /* We have to use 7 loops here. *All* devices must + /* We have to use 9 loops here. *All* devices must * be detached before we reset any of them, because * in some cases you have to reset the whole PCI, * which impacts all devices on it. Also, all devices @@ -225,7 +405,22 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, goto reattachdevs; }

- /* Loop 4: Now mark all the devices as active */ + /* Loop 4: For SRIOV network devices, Now that we have detached the + * the network device, set the netdev config */ + for (i = 0; i < nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + continue; + if (hostdev->parent.data.net) While this currently will work, I had figured that (to allow for the

ditto. possibility of other smart hostdev types) we should check this way: if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && hostdev->parent.data.net) I'm squashing that in to the three places you check for this.

+ if (qemuDomainHostdevNetConfigReplace(hostdev, uuid, + driver->stateDir) < 0) + goto resetvfnetconfig; + last_processed_hostdev_vf = i; + } + + /* Loop 5: Now mark all the devices as active */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); if (pciDeviceListAdd(driver->activePciHostdevs, dev) < 0) { @@ -234,13 +429,13 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, } }

- /* Loop 5: Now remove the devices from inactive list. */ + /* Loop 6: Now remove the devices from inactive list. */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDeviceListDel(driver->inactivePciHostdevs, dev); }

- /* Loop 6: Now set the used_by_domain of the device in + /* Loop 7: Now set the used_by_domain of the device in * driver->activePciHostdevs as domain name. */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { @@ -252,7 +447,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciDeviceSetUsedBy(activeDev, name); }

- /* Loop 7: Now set the original states for hostdev def */ + /* Loop 8: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { pciDevice *dev; pciDevice *pcidev; @@ -284,7 +479,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciFreeDevice(dev); }

- /* Loop 8: Now steal all the devices from pcidevs */ + /* Loop 9: Now steal all the devices from pcidevs */ while (pciDeviceListCount(pcidevs) > 0) { pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDeviceListSteal(pcidevs, dev); @@ -302,6 +497,13 @@ inactivedevs: pciDeviceListSteal(driver->activePciHostdevs, dev); }

+resetvfnetconfig: + for (i = 0; i < last_processed_hostdev_vf; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; + if (hostdev->parent.data.net) + qemuDomainHostdevNetConfigRestore(hostdev, driver->stateDir); + } + reattachdevs: for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); @@ -317,10 +519,10 @@ static int qemuPrepareHostPCIDevices(struct qemud_driver *driver, virDomainDefPtr def) { - return qemuPrepareHostdevPCIDevices(driver, def->name, def->hostdevs, def->nhostdevs); + return qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, + def->hostdevs, def->nhostdevs); }

- int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver, const char *name, @@ -494,8 +696,10 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, return; }

- /* Again 3 loops; mark all devices as inactive before reset - * them and reset all the devices before re-attach */ + /* Again 4 loops; mark all devices as inactive before reset + * them and reset all the devices before re-attach. + * Attach mac and port profile parameters to devices + */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDevice *activeDev = NULL; @@ -514,6 +718,20 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, pciDeviceListSteal(driver->activePciHostdevs, dev); }

+ /* + * For SRIOV net host devices, unset mac and port profile before + * reset and reattach device + */ + for (i = 0; i < nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + continue; + if (hostdev->parent.data.net) + qemuDomainHostdevNetConfigRestore(hostdev, driver->stateDir); + } + for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); if (pciResetDevice(dev, driver->activePciHostdevs, @@ -540,5 +758,6 @@ void qemuDomainReAttachHostDevices(struct qemud_driver *driver, if (!def->nhostdevs) return;

- qemuDomainReAttachHostdevDevices(driver, def->name, def->hostdevs, def->nhostdevs); + qemuDomainReAttachHostdevDevices(driver, def->name, def->hostdevs, + def->nhostdevs); } diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h index d852f5b..173e4f4 100644 --- a/src/qemu/qemu_hostdev.h +++ b/src/qemu/qemu_hostdev.h @@ -31,6 +31,7 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, virDomainDefPtr def); int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, const char *name, + const unsigned char *uuid, virDomainHostdevDefPtr *hostdevs, int nhostdevs); int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver, @@ -46,6 +47,11 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, int nhostdevs); void qemuDomainReAttachHostDevices(struct qemud_driver *driver, virDomainDefPtr def); - +int qemuDomainHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev); +int qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev, + const unsigned char *uuid, + char *stateDir); +int qemuDomainHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, + char *stateDir);

#endif /* __QEMU_HOSTDEV_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 50563c5..c5ed9f7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -927,7 +927,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, return -1; }

- if (qemuPrepareHostdevPCIDevices(driver, vm->def->name, &hostdev, 1) < 0) + if (qemuPrepareHostdevPCIDevices(driver, vm->def->name, vm->def->uuid, + &hostdev, 1) < 0) return -1;

if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { @@ -1886,6 +1887,13 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, if (ret < 0) return -1;

+ /* + * For SRIOV net host devices, unset mac and port profile before + * reset and reattach device + */ + if (detach->parent.data.net) + qemuDomainHostdevNetConfigRestore(detach, driver->stateDir); + pci = pciGetDevice(subsys->u.pci.domain, subsys->u.pci.bus, subsys->u.pci.slot, subsys->u.pci.function); if (pci) {

It turned out just like I'd hoped it would! ACK with those small nits (which I'm applying before I push).