On 10/28/2011 04:04 PM, Roopa Prabhu wrote:
From: Roopa Prabhu<roprabhu@cisco.com>
- 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<roprabhu@cisco.com> Reported-by: Eric Blake<eblake@redhat.com> Reported-by: Laine Stump<laine@laine.org>
Looks mostly good. I'll squash in some fixes as I finish auditing your changes...
--- 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; + }
No TABs.
if ((features& IFF_VNET_HDR)) { new_flags = ifreq.ifr_flags | IFF_VNET_HDR; errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); @@ -335,7 +338,7 @@ create_name: macaddress, linkdev, virtPortProfile, - vmuuid, vmOp) != 0) { + vmuuid, vmOp)< 0) {
Needed some touchups before vpAssociatePortProfileId had a safe return value. I also adjusted global callers, such as in src/qemu.
@@ -552,6 +554,8 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf, } }
+ return rc; +
Spurious addition.
@@ -963,10 +964,13 @@ getPhysfnDev(const char *linkdev, *physfndev = strdup(linkdev); if (!*physfndev) { virReportOOMError(); - rc = -1; - } + goto err_exit; + } + rc = 0;
More TABs.
@@ -1011,7 +1015,7 @@ doPortProfileOp8021Qbh(const char *ifname, case PREASSOCIATE_RR: case ASSOCIATE: rc = virGetHostUUID(hostuuid); - if (rc) + if (rc< 0)
Won't work unless we also fix virGetHostUUID. Let's save that one for later, since it touches 7 or so files.
@@ -1971,7 +1971,7 @@ pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, }
for (i = 0; i< num_virt_fns; i++) { - if (pciConfigAddressEqual(vf_bdf, virt_fns[i])) { + if (pciConfigAddressEqual(vf_bdf, virt_fns[i]) == 1) {
Spurious change. Here's what I added before pushing: diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c index decb0f2..838a31f 100644 --- i/src/qemu/qemu_migration.c +++ w/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 i/src/util/interface.c w/src/util/interface.c index 4ab74b5..12bf7bc 100644 --- i/src/util/interface.c +++ w/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]) { diff --git i/src/util/macvtap.c w/src/util/macvtap.c index 329cbf1..54dc670 100644 --- i/src/util/macvtap.c +++ w/src/util/macvtap.c @@ -214,7 +214,7 @@ configMacvtapTap(int tapfd, int vnet_hdr) 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"); @@ -295,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; } } @@ -473,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, @@ -482,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, }; @@ -554,8 +554,6 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf, } } - return rc; - err_exit: if (msg) macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", msg); @@ -753,6 +751,7 @@ buffer_too_small: } +/* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int doPortProfileOpCommon(bool nltarget_kernel, const char *ifname, int ifindex, @@ -808,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; } @@ -863,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 @@ -879,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 */ @@ -896,7 +896,7 @@ doPortProfileOp8021Qbg(const char *ifname, if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname, &vlanid) < 0) { - rc = 1; + rc = -1; goto err_exit; } @@ -920,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; } @@ -969,8 +969,8 @@ getPhysfnDev(const char *linkdev, if (!*physfndev) { virReportOOMError(); goto err_exit; - } - rc = 0; + } + rc = 0; } err_exit: @@ -979,6 +979,7 @@ err_exit: } # 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, @@ -986,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 @@ -997,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 */ @@ -1019,9 +1020,11 @@ doPortProfileOp8021Qbh(const char *ifname, switch (virtPortOp) { case PREASSOCIATE_RR: case ASSOCIATE: - rc = virGetHostUUID(hostuuid); - if (rc < 0) + errno = virGetHostUUID(hostuuid); + if (errno) { + rc = -1; goto err_exit; + } rc = doPortProfileOpCommon(nltarget_kernel, NULL, ifindex, macaddr, @@ -1062,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: @@ -1087,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 i/src/util/pci.c w/src/util/pci.c index 077f3a2..9d44edf 100644 --- i/src/util/pci.c +++ w/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) { @@ -1971,7 +1971,7 @@ pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, } for (i = 0; i < num_virt_fns; i++) { - if (pciConfigAddressEqual(vf_bdf, virt_fns[i]) == 1) { + if (pciConfigAddressEqual(vf_bdf, virt_fns[i])) { *vf_index = i; ret = 0; break; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org