[libvirt] [PATCH] macvtap: Fix error return value convention/inconsistencies
Eric Blake
eblake at redhat.com
Fri Oct 28 22:50:07 UTC 2011
On 10/28/2011 04:04 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu<roprabhu at 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 at cisco.com>
> Reported-by: Eric Blake<eblake at redhat.com>
> Reported-by: Laine Stump<laine at 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 at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list