[libvirt] [PATCH 2/3] virNetDevMacVLanCreateWithVPortProfile: Drop @rc

Ján Tomko jtomko at redhat.com
Wed Aug 10 14:28:01 UTC 2016


On Tue, Aug 09, 2016 at 07:32:44PM +0200, Michal Privoznik wrote:
>This variable is very misleading. We use VIR_FORCE_CLOSE to set

The function is also hard to follow.

>it to -1 and returning it even though it does not refer to a FD
>at all. It merely holds 0 or -1. Drop it completely.
>

Yes, introduced by commit 136fe2f7:
    virNetDevMacVLanTapSetup: Rework to support multiple FDs

>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/util/virnetdevmacvlan.c | 35 ++++++++++++-----------------------
> 1 file changed, 12 insertions(+), 23 deletions(-)
>

>@@ -1079,9 +1079,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
>             return -1;
>         }
>         snprintf(ifname, sizeof(ifname), pattern, reservedID);
>-        rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
>-                                    macvtapMode, &do_retry);
>-        if (rc < 0) {
>+        if (virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
>+                                   macvtapMode, &do_retry) < 0) {
>             virNetDevMacVLanReleaseID(reservedID, flags);
>             virMutexUnlock(&virNetDevMacVLanCreateMutex);
>             if (!do_retry)
>@@ -1107,36 +1106,26 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
>                                        macaddress,
>                                        linkdev,
>                                        vf,
>-                                       vmuuid, vmOp, false) < 0) {
>-        rc = -1;
>+                                       vmuuid, vmOp, false) < 0)
>         goto link_del_exit;
>-    }
>
>     if (flags & VIR_NETDEV_MACVLAN_CREATE_IFUP) {
>-        if (virNetDevSetOnline(ifnameCreated, true) < 0) {
>-            rc = -1;
>+        if (virNetDevSetOnline(ifnameCreated, true) < 0)
>             goto disassociate_exit;
>-        }
>     }
>
>     if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
>-        if (virNetDevMacVLanTapOpen(ifnameCreated, tapfd, tapfdSize, 10) < 0) {
>-            VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>+        if (virNetDevMacVLanTapOpen(ifnameCreated, tapfd, tapfdSize, 10) < 0)
>             goto disassociate_exit;
>-        }
>
>-        if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0) {
>-            VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>+        if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0)
>             goto disassociate_exit;
>-        }
>-        if (VIR_STRDUP(*ifnameResult, ifnameCreated) < 0) {
>-            VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>+
>+        if (VIR_STRDUP(*ifnameResult, ifnameCreated) < 0)
>             goto disassociate_exit;
>-        }

All the gotos above are preceded by rc = -1, no functional change there.

>     } else {
>         if (VIR_STRDUP(*ifnameResult, ifnameCreated) < 0)
>             goto disassociate_exit;

Before, if the if (ifnameRequested) block gets skipped because the
requested name matches the automatic prefix, rc is 0 here,
otherwise it's uninitialized. And we would return it on the unlikely OOM
error..

>-        rc = 0;
>     }
>
>     if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_CREATE ||
>@@ -1149,10 +1138,10 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
>                                                          linkdev, vmuuid,
>                                                          virtPortProfile,
>                                                          vmOp) < 0)
>-        goto disassociate_exit;
>+            goto disassociate_exit;

..and here too, even though we delete the device. This patch fixes it.

ACK, it might be nice to mention in the commit message that this also
fixes the return value in some corner cases.

Jan

>     }
>
>-    return rc;
>+    return 0;
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160810/14f2e8f9/attachment-0001.sig>


More information about the libvir-list mailing list