From 228a9ec312808a07c074e8d1b3f0f3d6776433d3 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Fri, 28 Oct 2011 15:04:07 -0700 Subject: [PATCH] macvtap: Fix error return value convention/inconsistencies - changed some return 1's to return -1 - changed if (rc) error checks to if (rc < 0) - fixed some other minor convention violations I might have missed some. Can fix in another patch or can respin Signed-off-by: Roopa Prabhu Reported-by: Eric Blake Reported-by: Laine Stump Signed-off-by: Eric Blake --- src/qemu/qemu_migration.c | 2 +- src/util/interface.c | 12 ++--- src/util/macvtap.c | 96 +++++++++++++++++++++------------------ src/util/pci.c | 8 ++-- 4 files changed, 62 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index decb0f2623..838a31fdfd 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2527,7 +2527,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { virDomainNetGetActualDirectDev(net), virDomainNetGetActualDirectVirtPortProfile(net), def->uuid, - VIR_VM_OP_MIGRATE_IN_FINISH) != 0) + VIR_VM_OP_MIGRATE_IN_FINISH) < 0) goto err_exit; } last_good_net = i; diff --git a/src/util/interface.c b/src/util/interface.c index 5d473b7510..12bf7bc873 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -1016,7 +1016,7 @@ ifaceMacvtapLinkDump(bool nltarget_kernel ATTRIBUTE_UNUSED, * Get the nth parent interface of the given interface. 0 is the interface * itself. * - * Return 0 on success, != 0 otherwise + * Return 0 on success, < 0 otherwise */ #if defined(__linux__) && WITH_MACVTAP int @@ -1037,7 +1037,7 @@ ifaceGetNthParent(int ifindex, const char *ifname, unsigned int nthParent, while (!end && i <= nthParent) { rc = ifaceMacvtapLinkDump(true, ifname, ifindex, tb, &recvbuf, NULL); - if (rc) + if (rc < 0) break; if (tb[IFLA_IFNAME]) { @@ -1244,7 +1244,7 @@ ifaceIsVirtualFunction(const char *ifname) char *if_sysfs_device_link = NULL; int ret = -1; - if (ifaceSysfsFile(&if_sysfs_device_link, ifname, "device")) + if (ifaceSysfsFile(&if_sysfs_device_link, ifname, "device") < 0) return ret; ret = pciDeviceIsVirtualFunction(if_sysfs_device_link); @@ -1272,10 +1272,10 @@ ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; int ret = -1; - if (ifaceSysfsFile(&pf_sysfs_device_link, pfname, "device")) + if (ifaceSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) return ret; - if (ifaceSysfsFile(&vf_sysfs_device_link, vfname, "device")) { + if (ifaceSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0) { VIR_FREE(pf_sysfs_device_link); return ret; } @@ -1306,7 +1306,7 @@ ifaceGetPhysicalFunction(const char *ifname, char **pfname) char *physfn_sysfs_path = NULL; int ret = -1; - if (ifaceSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn")) + if (ifaceSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0) return ret; ret = pciDeviceNetName(physfn_sysfs_path, pfname); diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 7fd6eb54b9..54dc6709c3 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -210,8 +210,11 @@ configMacvtapTap(int tapfd, int vnet_hdr) rc_on_fail = -1; errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap"); } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) { - if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) - return errno; + if (ioctl(tapfd, TUNGETFEATURES, &features) < 0) { + virReportSystemError(errno, "%s", + _("cannot get feature flags on macvtap tap")); + return -1; + } if ((features & IFF_VNET_HDR)) { new_flags = ifreq.ifr_flags | IFF_VNET_HDR; errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); @@ -292,7 +295,7 @@ openMacvtapTap(const char *tgifname, * emulate their switch in firmware. */ if (mode == VIR_MACVTAP_MODE_PASSTHRU) { - if (ifaceReplaceMacAddress(macaddress, linkdev, stateDir) != 0) { + if (ifaceReplaceMacAddress(macaddress, linkdev, stateDir) < 0) { return -1; } } @@ -335,7 +338,7 @@ create_name: macaddress, linkdev, virtPortProfile, - vmuuid, vmOp) != 0) { + vmuuid, vmOp) < 0) { rc = -1; goto link_del_exit; } @@ -352,7 +355,6 @@ create_name: } rc = openTap(cr_ifname, 10); - if (rc >= 0) { if (configMacvtapTap(rc, vnet_hdr) < 0) { VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ @@ -471,7 +473,7 @@ getLldpadPid(void) { * status: pointer to a uint16 where the status will be written into * * Get the status from the IFLA_PORT_RESPONSE field; Returns 0 in - * case of success, != 0 otherwise with error having been reported + * case of success, < 0 otherwise with error having been reported */ static int getPortProfileStatus(struct nlattr **tb, int32_t vf, @@ -480,7 +482,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf, bool is8021Qbg, uint16_t *status) { - int rc = 1; + int rc = -1; const char *msg = NULL; struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, }; @@ -572,7 +574,7 @@ doPortProfileOpSetLink(bool nltarget_kernel, int32_t vf, uint8_t op) { - int rc = 0; + int rc = -1; struct nlmsghdr *resp; struct nlmsgerr *err; struct ifinfomsg ifinfo = { @@ -588,7 +590,7 @@ doPortProfileOpSetLink(bool nltarget_kernel, nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST); if (!nl_msg) { virReportOOMError(); - return -1; + return rc; } if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) @@ -690,16 +692,12 @@ doPortProfileOpSetLink(bool nltarget_kernel, if (!nltarget_kernel) { pid = getLldpadPid(); - if (pid == 0) { - rc = -1; + if (pid == 0) goto err_exit; - } } - if (nlComm(nl_msg, &recvbuf, &recvbuflen, pid) < 0) { - rc = -1; + if (nlComm(nl_msg, &recvbuf, &recvbuflen, pid) < 0) goto err_exit; - } if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) goto malformed_resp; @@ -716,7 +714,7 @@ doPortProfileOpSetLink(bool nltarget_kernel, virReportSystemError(-err->error, _("error during virtual port configuration of ifindex %d"), ifindex); - rc = -1; + goto err_exit; } break; @@ -727,6 +725,8 @@ doPortProfileOpSetLink(bool nltarget_kernel, goto malformed_resp; } + rc = 0; + err_exit: nlmsg_free(nl_msg); @@ -740,17 +740,18 @@ malformed_resp: macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); VIR_FREE(recvbuf); - return -1; + return rc; buffer_too_small: nlmsg_free(nl_msg); macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - return -1; + return rc; } +/* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int doPortProfileOpCommon(bool nltarget_kernel, const char *ifname, int ifindex, @@ -780,8 +781,7 @@ doPortProfileOpCommon(bool nltarget_kernel, hostUUID, vf, op); - - if (rc) { + if (rc < 0) { macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("sending of PortProfileRequest failed.")); return rc; @@ -790,11 +790,12 @@ doPortProfileOpCommon(bool nltarget_kernel, while (--repeats >= 0) { rc = ifaceMacvtapLinkDump(nltarget_kernel, NULL, ifindex, tb, &recvbuf, getLldpadPid); - if (rc) + if (rc < 0) goto err_exit; + rc = getPortProfileStatus(tb, vf, instanceId, nltarget_kernel, is8021Qbg, &status); - if (rc) + if (rc < 0) goto err_exit; if (status == PORT_PROFILE_RESPONSE_SUCCESS || status == PORT_VDP_RESPONSE_SUCCESS) { @@ -806,7 +807,7 @@ doPortProfileOpCommon(bool nltarget_kernel, _("error %d during port-profile setlink on " "interface %s (%d)"), status, ifname, ifindex); - rc = 1; + rc = -1; break; } @@ -818,7 +819,7 @@ doPortProfileOpCommon(bool nltarget_kernel, if (status == PORT_PROFILE_RESPONSE_INPROGRESS) { macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("port-profile setlink timed out")); - rc = -ETIMEDOUT; + rc = -2; } err_exit: @@ -843,7 +844,7 @@ getPhysdevAndVlan(const char *ifname, int *root_ifindex, char *root_ifname, *vlanid = -1; while (1) { if ((ret = ifaceGetNthParent(ifindex, ifname, 1, - root_ifindex, root_ifname, &nth))) + root_ifindex, root_ifname, &nth)) < 0) return ret; if (nth == 0) break; @@ -861,13 +862,14 @@ getPhysdevAndVlan(const char *ifname, int *root_ifindex, char *root_ifname, # endif +/* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int doPortProfileOp8021Qbg(const char *ifname, const unsigned char *macaddr, const virVirtualPortProfileParamsPtr virtPort, enum virVirtualPortOp virtPortOp) { - int rc; + int rc = 0; # ifndef IFLA_VF_PORT_MAX @@ -877,7 +879,7 @@ doPortProfileOp8021Qbg(const char *ifname, (void)virtPortOp; macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("Kernel VF Port support was missing at compile time.")); - rc = 1; + rc = -1; # else /* IFLA_VF_PORT_MAX */ @@ -893,8 +895,8 @@ doPortProfileOp8021Qbg(const char *ifname, int vf = PORT_SELF_VF; if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname, - &vlanid) != 0) { - rc = 1; + &vlanid) < 0) { + rc = -1; goto err_exit; } @@ -918,7 +920,7 @@ doPortProfileOp8021Qbg(const char *ifname, default: macvtapError(VIR_ERR_INTERNAL_ERROR, _("operation type %d not supported"), virtPortOp); - rc = 1; + rc = -1; goto err_exit; } @@ -947,10 +949,9 @@ getPhysfnDev(const char *linkdev, int32_t *vf, char **physfndev) { - int rc = 0; - - if (ifaceIsVirtualFunction(linkdev)) { + int rc = -1; + if (ifaceIsVirtualFunction(linkdev) == 1) { /* if linkdev is SR-IOV VF, then set vf = VF index */ /* and set linkdev = PF device */ @@ -967,14 +968,18 @@ getPhysfnDev(const char *linkdev, *physfndev = strdup(linkdev); if (!*physfndev) { virReportOOMError(); - rc = -1; + goto err_exit; } + rc = 0; } +err_exit: + return rc; } # endif /* IFLA_VF_PORT_MAX */ +/* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int doPortProfileOp8021Qbh(const char *ifname, const unsigned char *macaddr, @@ -982,7 +987,7 @@ doPortProfileOp8021Qbh(const char *ifname, const unsigned char *vm_uuid, enum virVirtualPortOp virtPortOp) { - int rc; + int rc = 0; # ifndef IFLA_VF_PORT_MAX @@ -993,7 +998,7 @@ doPortProfileOp8021Qbh(const char *ifname, (void)virtPortOp; macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", _("Kernel VF Port support was missing at compile time.")); - rc = 1; + rc = -1; # else /* IFLA_VF_PORT_MAX */ @@ -1005,20 +1010,21 @@ doPortProfileOp8021Qbh(const char *ifname, int vlanid = -1; rc = getPhysfnDev(ifname, &vf, &physfndev); - if (rc) + if (rc < 0) goto err_exit; - if (ifaceGetIndex(true, physfndev, &ifindex) < 0) { - rc = 1; + rc = ifaceGetIndex(true, physfndev, &ifindex); + if (rc < 0) goto err_exit; - } switch (virtPortOp) { case PREASSOCIATE_RR: case ASSOCIATE: - rc = virGetHostUUID(hostuuid); - if (rc) + errno = virGetHostUUID(hostuuid); + if (errno) { + rc = -1; goto err_exit; + } rc = doPortProfileOpCommon(nltarget_kernel, NULL, ifindex, macaddr, @@ -1031,7 +1037,7 @@ doPortProfileOp8021Qbh(const char *ifname, (virtPortOp == PREASSOCIATE_RR) ? PORT_REQUEST_PREASSOCIATE_RR : PORT_REQUEST_ASSOCIATE); - if (rc == -ETIMEDOUT) + if (rc == -2) /* Association timed out, disassociate */ doPortProfileOpCommon(nltarget_kernel, NULL, ifindex, NULL, @@ -1059,7 +1065,7 @@ doPortProfileOp8021Qbh(const char *ifname, default: macvtapError(VIR_ERR_INTERNAL_ERROR, _("operation type %d not supported"), virtPortOp); - rc = 1; + rc = -1; } err_exit: @@ -1084,7 +1090,7 @@ err_exit: * by the user, then this function returns without doing * anything. * - * Returns 0 in case of success, != 0 otherwise with error + * Returns 0 in case of success, < 0 otherwise with error * having been reported. */ int diff --git a/src/util/pci.c b/src/util/pci.c index 33b4b0ee12..9d44edfa08 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1707,9 +1707,9 @@ int pciDeviceIsAssignable(pciDevice *dev, #ifdef __linux__ /* - * returns 1 if equal and 0 if not + * returns true if equal */ -static int +static bool pciConfigAddressEqual(struct pci_config_address *bdf1, struct pci_config_address *bdf2) { @@ -1959,11 +1959,11 @@ pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, struct pci_config_address **virt_fns = NULL; if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, - &vf_bdf)) + &vf_bdf) < 0) return ret; if (pciGetVirtualFunctions(pf_sysfs_device_link, &virt_fns, - &num_virt_fns)) { + &num_virt_fns) < 0) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("Error getting physical function's '%s' " "virtual_functions"), pf_sysfs_device_link); -- GitLab