From 8da6f37f76606a811357b138b4e1533404b4bb11 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Fri, 28 Apr 2017 18:07:36 +0200
Subject: Make tap deletion more robust

---
 bbot/machine.cxx | 103 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 37 deletions(-)

diff --git a/bbot/machine.cxx b/bbot/machine.cxx
index 0bd2e86..ce07c94 100644
--- a/bbot/machine.cxx
+++ b/bbot/machine.cxx
@@ -35,34 +35,42 @@ namespace bbot
   {
     string addr (iface_addr (br));
 
-    run (t,
-         "sudo",             "iptables",
-         "-t",               "nat",
-         a,                  "PREROUTING",
-         "-m",               "udp",
-         "-p",               "udp",
-         "-m",               "physdev",
-         "-i",               br,
-         "--physdev-in",     tap,
-         "--dport",          69,
-         "-j",               "DNAT",
-         "--to-destination", addr + ':' + to_string (port));
+    process_exit::code_type e;
+
+    e = run_exit (t,
+                  "sudo",             "iptables",
+                  "-t",               "nat",
+                  a,                  "PREROUTING",
+                  "-m",               "udp",
+                  "-p",               "udp",
+                  "-m",               "physdev",
+                  "-i",               br,
+                  "--physdev-in",     tap,
+                  "--dport",          69,
+                  "-j",               "DNAT",
+                  "--to-destination", addr + ':' + to_string (port));
+
+    if (e != 0 && port != 0)
+      fail << "process iptables terminated with non-zero exit code";
 
     // Nobody really knows whether this is really needed (really)...
     //
-    run (t,
-         "sudo",             "iptables",
-         a,                  "FORWARD",
-         "-m",               "udp",
-         "-p",               "udp",
-         "-m",               "physdev",
-         "-o",               br,
-         "--physdev-out",    tap,
-         "-d",               addr,
-         "--dport",          port,
-         "-m",               "state",
-         "--state",          "NEW,ESTABLISHED,RELATED",
-         "-j",               "ACCEPT");
+    e = run_exit (t,
+                  "sudo",             "iptables",
+                  a,                  "FORWARD",
+                  "-m",               "udp",
+                  "-p",               "udp",
+                  "-m",               "physdev",
+                  "-o",               br,
+                  "--physdev-out",    tap,
+                  "-d",               addr,
+                  "--dport",          port,
+                  "-m",               "state",
+                  "--state",          "NEW,ESTABLISHED,RELATED",
+                  "-j",               "ACCEPT");
+
+    if (e != 0 && port != 0)
+      fail << "process iptables terminated with non-zero exit code";
   }
 
   static string
@@ -74,6 +82,7 @@ namespace bbot
 
     // First try to delete it in case there is one from a previous run.
     //
+    //iptables (trace, "-D", t, br, 0); // Any port.
     run_exit (trace, "sudo", "ip", "tuntap", "delete", t, "mode", "tap");
 
     run (trace, "sudo", "ip", "tuntap", "add", t, "mode", "tap", "user", uid);
@@ -93,6 +102,33 @@ namespace bbot
     run (trace, "sudo", "ip", "tuntap", "delete", t, "mode", "tap");
   }
 
+  class tap
+  {
+  public:
+    string iface;
+
+    string bridge; // Bridge interface to which this tap belongs
+    uint16_t port; // UDP port to forward TFTP traffic to.
+
+    tap (string b, uint16_t p)
+        : iface (create_tap (b, p)), bridge (move (b)), port (p) {}
+
+    ~tap ()
+    {
+      if (!iface.empty ())
+      {
+        try {destroy ();} catch (...) {}
+      }
+    }
+
+    void
+    destroy ()
+    {
+      destroy_tap (iface, bridge, port);
+      iface.clear ();
+    }
+  };
+
   static string
   generate_mac ()
   {
@@ -144,11 +180,7 @@ namespace bbot
 
   private:
     path kvm;     // Hypervisor binary.
-
-    string br;     // Bridge network interface.
-    string tap;    // Tap network interface.
-    uint16_t port; // TFTP port.
-
+    tap net;      // Tap network interface.
     string vnc;   // QEMU VNC TCP addr:port.
     path monitor; // QEMU monitor UNIX socket.
     process proc;
@@ -164,9 +196,7 @@ namespace bbot
                  omac   ? *omac   : // Generated mac from previous bootstrap.
                  generate_mac ()),
         kvm ("kvm"),
-        br (br),
-        tap (create_tap (br, port)),
-        port (port),
+        net (br, port),
         vnc ("127.0.0.1:" + to_string (5900 + tc_num)),
         monitor ("/tmp/" + tc_name + "-monitor")
   {
@@ -224,7 +254,7 @@ namespace bbot
 
       for (string& o: os)
       {
-        sub (o, "ifname=?", tap);
+        sub (o, "ifname=?", net.iface);
         sub (o, "mac=?", mac);
       }
     }
@@ -238,7 +268,7 @@ namespace bbot
 
       // Network.
       //
-      add ("-netdev", "tap,id=net0,script=no,ifname=" + tap);
+      add ("-netdev", "tap,id=net0,script=no,ifname=" + net.iface);
       add ("-device", "virtio-net-pci,netdev=net0,mac=" + mac);
 
       // Disk.
@@ -340,8 +370,7 @@ namespace bbot
     if (t)
     {
       run_io_finish (trace, proc, kvm);
-
-      destroy_tap (tap, br, port);
+      net.destroy ();
       try_rmfile (monitor, true); // QEMU doesn't seem to remove it.
     }
 
-- 
cgit v1.1