[PATCH] virnetdevveth: Do report error if creating veth fails

Michal Privoznik posted 1 patch 2 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/fd0790a3a4b44d76c14b3f53cb81d4106f1d2eb6.1638447020.git.mprivozn@redhat.com
There is a newer version of this series
src/util/virnetdevveth.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[PATCH] virnetdevveth: Do report error if creating veth fails
Posted by Michal Privoznik 2 years, 4 months ago
For some weird reason we are ignoring errors when creating veth
pair that netlink reports. This affects the LXC driver which
creates interfaces for container in
virLXCProcessSetupInterfaces(). If creating a veth pair fails, no
error is reported and the control jumps onto cleanup label where
some cryptic error message is reported instead (something about
inability to remove veth pair).

Let's report error that netlink returned - it's probably the most
accurate reason anyways.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/225
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virnetdevveth.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index 7133af44a2..ddf304036a 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -38,10 +38,17 @@ VIR_LOG_INIT("util.netdevveth");
 static int
 virNetDevVethCreateInternal(const char *veth1, const char *veth2)
 {
-    int status;     /* Just ignore it */
+    int error;
     virNetlinkNewLinkData data = { .veth_peer = veth2 };
 
-    return virNetlinkNewLink(veth1, "veth", &data, &status);
+    if (virNetlinkNewLink(veth1, "veth", &data, &error) < 0) {
+        virReportSystemError(-error,
+                             _("unable to create %s <-> %s veth pair"),
+                             veth1, veth2);
+        return -1;
+    }
+
+    return 0;
 }
 
 static int
-- 
2.32.0

Re: [PATCH] virnetdevveth: Do report error if creating veth fails
Posted by Martin Kletzander 2 years, 4 months ago
On Thu, Dec 02, 2021 at 01:10:20PM +0100, Michal Privoznik wrote:
>For some weird reason we are ignoring errors when creating veth
>pair that netlink reports. This affects the LXC driver which
>creates interfaces for container in
>virLXCProcessSetupInterfaces(). If creating a veth pair fails, no
>error is reported and the control jumps onto cleanup label where
>some cryptic error message is reported instead (something about
>inability to remove veth pair).
>
>Let's report error that netlink returned - it's probably the most
>accurate reason anyways.
>
>Resolves: https://gitlab.com/libvirt/libvirt/-/issues/225
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/util/virnetdevveth.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
>index 7133af44a2..ddf304036a 100644
>--- a/src/util/virnetdevveth.c
>+++ b/src/util/virnetdevveth.c
>@@ -38,10 +38,17 @@ VIR_LOG_INIT("util.netdevveth");
> static int
> virNetDevVethCreateInternal(const char *veth1, const char *veth2)
> {
>-    int status;     /* Just ignore it */
>+    int error;
>     virNetlinkNewLinkData data = { .veth_peer = veth2 };
>
>-    return virNetlinkNewLink(veth1, "veth", &data, &status);
>+    if (virNetlinkNewLink(veth1, "veth", &data, &error) < 0) {
>+        virReportSystemError(-error,
>+                             _("unable to create %s <-> %s veth pair"),
>+                             veth1, veth2);

This might be overriding a previous error and @error might be just 0.

The function does not report an error in this one case only:

         if (err->error < 0) {
             if (error)
                 *error = err->error;
             else
                 virReportSystemError(-err->error, "%s", _("netlink error"));

which suggests you should either pass NULL to the function or report an
error only if @error != 0.

>+        return -1;
>+    }
>+
>+    return 0;
> }
>
> static int
>-- 
>2.32.0
>