From: Brian Warner <warner@allmydata.com>
Date: Thu, 17 Jul 2008 23:47:09 +0000 (-0700)
Subject: deep-check: add webapi, add 'DEEP-CHECK' button to wui, add tests, rearrange checker... 
X-Git-Url: https://git.rkrishnan.org/listings/...?a=commitdiff_plain;h=67db0a4967f5d2037d22b58fcac121cbfba2da68;p=tahoe-lafs%2Ftahoe-lafs.git

deep-check: add webapi, add 'DEEP-CHECK' button to wui, add tests, rearrange checker API a bit
---

diff --git a/docs/webapi.txt b/docs/webapi.txt
index 029460d6..05621292 100644
--- a/docs/webapi.txt
+++ b/docs/webapi.txt
@@ -658,6 +658,22 @@ POST $URL?t=check
   If a verify=true argument is provided, the node will perform a more
   intensive check, downloading and verifying every single bit of every share.
 
+POST $URL?t=deep-check
+
+  This triggers a recursive walk of all files and directories reachable from
+  the target, performing a check on each one just like t=check. The result
+  page will contain a summary of the results, including details on any
+  file/directory that was not fully healthy.
+
+  t=deep-check is most useful to invoke on a directory. If invoked on a file,
+  it will just check that single object. The recursive walker will deal with
+  loops safely.
+
+  This accepts the same verify=, when_done=, and return_to= arguments as
+  t=check.
+
+  Be aware that this can take a long time: perhaps a second per object.
+
 GET $DIRURL?t=manifest
 
   Return an HTML-formatted manifest of the given directory, for debugging.
diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py
index f0c2e5e3..984bc1cb 100644
--- a/src/allmydata/dirnode.py
+++ b/src/allmydata/dirnode.py
@@ -536,7 +536,8 @@ class NewDirectoryNode:
 
     def deep_check(self, verify=False, repair=False):
         # shallow-check each object first, then traverse children
-        results = DeepCheckResults()
+        root_si = self._node.get_storage_index()
+        results = DeepCheckResults(root_si)
         found = set()
         limiter = ConcurrencyLimiter(10)
 
diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py
index 33a12931..8b734326 100644
--- a/src/allmydata/immutable/checker.py
+++ b/src/allmydata/immutable/checker.py
@@ -48,7 +48,13 @@ class Results:
 class DeepCheckResults:
     implements(IDeepCheckResults)
 
-    def __init__(self):
+    def __init__(self, root_storage_index):
+        self.root_storage_index = root_storage_index
+        if root_storage_index is None:
+            self.root_storage_index_s = "<none>"
+        else:
+            self.root_storage_index_s = base32.b2a(root_storage_index)[:6]
+
         self.objects_checked = 0
         self.objects_healthy = 0
         self.repairs_attempted = 0
@@ -56,6 +62,9 @@ class DeepCheckResults:
         self.problems = []
         self.server_problems = {}
 
+    def get_root_storage_index_string(self):
+        return self.root_storage_index_s
+
     def add_check(self, r):
         self.objects_checked += 1
         if r.is_healthy:
@@ -86,10 +95,12 @@ class SimpleCHKFileChecker:
     """Return a list of (needed, total, found, sharemap), where sharemap maps
     share number to a list of (binary) nodeids of the shareholders."""
 
-    def __init__(self, peer_getter, uri_to_check):
-        self.peer_getter = peer_getter
+    def __init__(self, client, storage_index, needed_shares, total_shares):
+        self.peer_getter = client.get_permuted_peers
+        self.needed_shares = needed_shares
+        self.total_shares = total_shares
         self.found_shares = set()
-        self.uri_to_check = IVerifierURI(uri_to_check)
+        self.storage_index = storage_index
         self.sharemap = {}
 
     '''
@@ -103,8 +114,8 @@ class SimpleCHKFileChecker:
         return len(found)
     '''
 
-    def check(self):
-        d = self._get_all_shareholders(self.uri_to_check.storage_index)
+    def start(self):
+        d = self._get_all_shareholders(self.storage_index)
         d.addCallback(self._done)
         return d
 
@@ -132,11 +143,10 @@ class SimpleCHKFileChecker:
         pass
 
     def _done(self, res):
-        u = self.uri_to_check
-        r = Results(self.uri_to_check.storage_index)
-        r.healthy = bool(len(self.found_shares) >= u.needed_shares)
-        r.stuff = (u.needed_shares, u.total_shares, len(self.found_shares),
-                   self.sharemap)
+        r = Results(self.storage_index)
+        r.healthy = bool(len(self.found_shares) >= self.total_shares)
+        r.stuff = (self.needed_shares, self.total_shares,
+                   len(self.found_shares), self.sharemap)
         return r
 
 class VerifyingOutput:
@@ -179,15 +189,14 @@ class SimpleCHKFileVerifier(download.FileDownloader):
     # remaining shareholders, and it cannot verify the plaintext.
     check_plaintext_hash = False
 
-    def __init__(self, client, u):
+    def __init__(self, client, storage_index, k, N, size, ueb_hash):
         self._client = client
 
-        u = IVerifierURI(u)
-        self._storage_index = u.storage_index
-        self._uri_extension_hash = u.uri_extension_hash
-        self._total_shares = u.total_shares
-        self._size = u.size
-        self._num_needed_shares = u.needed_shares
+        self._storage_index = storage_index
+        self._uri_extension_hash = ueb_hash
+        self._total_shares = N
+        self._size = size
+        self._num_needed_shares = k
 
         self._si_s = storage.si_b2a(self._storage_index)
         self.init_logging()
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index 1ed88982..b6874265 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -8,9 +8,12 @@ from allmydata.immutable.checker import Results, DeepCheckResults, \
 
 class FileNode:
     implements(IFileNode, ICheckable)
+    checker_class = SimpleCHKFileChecker
+    verifier_class = SimpleCHKFileVerifier
 
     def __init__(self, uri, client):
         u = IFileURI(uri)
+        self.u = u
         self.uri = u.to_string()
         self._client = client
 
@@ -27,7 +30,7 @@ class FileNode:
         return self.uri
 
     def get_size(self):
-        return IFileURI(self.uri).get_size()
+        return self.u.get_size()
 
     def __hash__(self):
         return hash((self.__class__, self.uri))
@@ -39,23 +42,26 @@ class FileNode:
         return cmp(self.uri, them.uri)
 
     def get_verifier(self):
-        return IFileURI(self.uri).get_verifier()
+        return self.u.get_verifier()
 
     def check(self, verify=False, repair=False):
         assert repair is False  # not implemented yet
-        vcap = self.get_verifier()
+        storage_index = self.u.storage_index
+        k = self.u.needed_shares
+        N = self.u.total_shares
+        size = self.u.size
+        ueb_hash = self.u.uri_extension_hash
         if verify:
-            v = SimpleCHKFileVerifier(self._client, vcap)
-            return v.start()
+            v = self.verifier_class(self._client,
+                                    storage_index, k, N, size, ueb_hash)
         else:
-            peer_getter = self._client.get_permuted_peers
-            v = SimpleCHKFileChecker(peer_getter, vcap)
-            return v.check()
+            v = self.checker_class(self._client, storage_index, k, N)
+        return v.start()
 
     def deep_check(self, verify=False, repair=False):
         d = self.check(verify, repair)
         def _done(r):
-            dr = DeepCheckResults()
+            dr = DeepCheckResults(self.get_verifier().storage_index)
             dr.add_check(r)
             return dr
         d.addCallback(_done)
@@ -114,6 +120,15 @@ class LiteralFileNode:
         r.problems = []
         return defer.succeed(r)
 
+    def deep_check(self, verify=False, repair=False):
+        d = self.check(verify, repair)
+        def _done(r):
+            dr = DeepCheckResults(None)
+            dr.add_check(r)
+            return dr
+        d.addCallback(_done)
+        return d
+
     def download(self, target):
         # note that this does not update the stats_provider
         data = IURI(self.uri).data
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index d63dd9b2..c567fe12 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -1521,6 +1521,9 @@ class IDeepCheckResults(Interface):
     This is returned by a call to ICheckable.deep_check().
     """
 
+    def get_root_storage_index_string():
+        """Return the storage index (abbreviated human-readable string) of
+        the first object checked."""
     def count_objects_checked():
         """Return the number of objects that were checked."""
     def count_objects_healthy():
diff --git a/src/allmydata/mutable/node.py b/src/allmydata/mutable/node.py
index 38c132a1..d3c91a3b 100644
--- a/src/allmydata/mutable/node.py
+++ b/src/allmydata/mutable/node.py
@@ -51,6 +51,7 @@ class MutableFileNode:
     implements(IMutableFileNode, ICheckable)
     SIGNATURE_KEY_SIZE = 2048
     DEFAULT_ENCODING = (3, 10)
+    checker_class = MutableChecker
 
     def __init__(self, client):
         self._client = client
@@ -217,6 +218,9 @@ class MutableFileNode:
     def get_verifier(self):
         return IMutableFileURI(self._uri).get_verifier()
 
+    def get_storage_index(self):
+        return self._uri.storage_index
+
     def _do_serialized(self, cb, *args, **kwargs):
         # note: to avoid deadlock, this callable is *not* allowed to invoke
         # other serialized methods within this (or any other)
@@ -238,13 +242,13 @@ class MutableFileNode:
     #################################
 
     def check(self, verify=False, repair=False):
-        checker = MutableChecker(self)
+        checker = self.checker_class(self)
         return checker.check(verify, repair)
 
     def deep_check(self, verify=False, repair=False):
         d = self.check(verify, repair)
         def _done(r):
-            dr = DeepCheckResults()
+            dr = DeepCheckResults(self.get_storage_index())
             dr.add_check(r)
             return dr
         d.addCallback(_done)
diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py
index a867c927..5567451c 100644
--- a/src/allmydata/test/common.py
+++ b/src/allmydata/test/common.py
@@ -6,7 +6,7 @@ from twisted.python import failure
 from twisted.application import service
 from allmydata import uri, dirnode
 from allmydata.interfaces import IURI, IMutableFileNode, IFileNode, \
-     FileTooLargeError
+     FileTooLargeError, ICheckable
 from allmydata.immutable import checker
 from allmydata.immutable.encode import NotEnoughSharesError
 from allmydata.util import log
@@ -74,7 +74,7 @@ class FakeMutableFileNode:
     """I provide IMutableFileNode, but all of my data is stored in a
     class-level dictionary."""
 
-    implements(IMutableFileNode)
+    implements(IMutableFileNode, ICheckable)
     MUTABLE_SIZELIMIT = 10000
     all_contents = {}
 
@@ -108,12 +108,24 @@ class FakeMutableFileNode:
     def get_size(self):
         return "?" # TODO: see mutable.MutableFileNode.get_size
 
+    def get_storage_index(self):
+        return self.storage_index
+
     def check(self, verify=False, repair=False):
         r = checker.Results(None)
         r.healthy = True
         r.problems = []
         return defer.succeed(r)
 
+    def deep_check(self, verify=False, repair=False):
+        d = self.check(verify, repair)
+        def _done(r):
+            dr = DeepCheckResults(self.storage_index)
+            dr.add_check(r)
+            return dr
+        d.addCallback(_done)
+        return d
+
     def download_best_version(self):
         return defer.succeed(self.all_contents[self.storage_index])
     def overwrite(self, new_contents):
diff --git a/src/allmydata/test/test_filenode.py b/src/allmydata/test/test_filenode.py
index a08e97bf..e1a4b17b 100644
--- a/src/allmydata/test/test_filenode.py
+++ b/src/allmydata/test/test_filenode.py
@@ -1,7 +1,8 @@
 
 from twisted.trial import unittest
+from twisted.internet import defer
 from allmydata import uri
-from allmydata.immutable import filenode, download
+from allmydata.immutable import filenode, download, checker
 from allmydata.mutable.node import MutableFileNode
 from allmydata.util import hashutil
 
@@ -31,6 +32,7 @@ class Node(unittest.TestCase):
         v = fn1.get_verifier()
         self.failUnless(isinstance(v, uri.CHKFileVerifierURI))
 
+
     def test_literal_filenode(self):
         DATA = "I am a short file."
         u = uri.LiteralFileURI(data=DATA)
@@ -51,17 +53,14 @@ class Node(unittest.TestCase):
         v = fn1.get_verifier()
         self.failUnlessEqual(v, None)
 
-        d = fn1.check()
-        def _check_checker_results(cr):
-            self.failUnless(cr.is_healthy())
-        d.addCallback(_check_checker_results)
-        d.addCallback(lambda res: fn1.download(download.Data()))
+        d = fn1.download(download.Data())
         def _check(res):
             self.failUnlessEqual(res, DATA)
         d.addCallback(_check)
 
         d.addCallback(lambda res: fn1.download_to_data())
         d.addCallback(_check)
+
         return d
 
     def test_mutable_filenode(self):
@@ -109,3 +108,100 @@ class Node(unittest.TestCase):
         v = n.get_verifier()
         self.failUnless(isinstance(v, uri.SSKVerifierURI))
 
+class Checker(unittest.TestCase):
+    def test_chk_filenode(self):
+        u = uri.CHKFileURI(key="\x00"*16,
+                           uri_extension_hash="\x00"*32,
+                           needed_shares=3,
+                           total_shares=10,
+                           size=1000)
+        c = None
+        fn1 = filenode.FileNode(u, c)
+
+        fn1.checker_class = FakeImmutableChecker
+        fn1.verifier_class = FakeImmutableVerifier
+
+        d = fn1.check()
+        def _check_checker_results(cr):
+            self.failUnless(cr.is_healthy())
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: fn1.check(verify=True))
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: fn1.deep_check())
+        def _check_deepcheck_results(dcr):
+            self.failIf(dcr.get_problems())
+        d.addCallback(_check_deepcheck_results)
+        return d
+
+    def test_literal_filenode(self):
+        DATA = "I am a short file."
+        u = uri.LiteralFileURI(data=DATA)
+        c = None
+        fn1 = filenode.LiteralFileNode(u, c)
+
+        d = fn1.check()
+        def _check_checker_results(cr):
+            self.failUnless(cr.is_healthy())
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: fn1.check(verify=True))
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: fn1.deep_check())
+        def _check_deepcheck_results(dcr):
+            self.failIf(dcr.get_problems())
+        d.addCallback(_check_deepcheck_results)
+
+        return d
+
+    def test_mutable_filenode(self):
+        client = None
+        wk = "\x00"*16
+        fp = "\x00"*32
+        rk = hashutil.ssk_readkey_hash(wk)
+        si = hashutil.ssk_storage_index_hash(rk)
+
+        u = uri.WriteableSSKFileURI("\x00"*16, "\x00"*32)
+        n = MutableFileNode(client).init_from_uri(u)
+
+        n.checker_class = FakeMutableChecker
+
+        d = n.check()
+        def _check_checker_results(cr):
+            self.failUnless(cr.is_healthy())
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: n.check(verify=True))
+        d.addCallback(_check_checker_results)
+
+        d.addCallback(lambda res: n.deep_check())
+        def _check_deepcheck_results(dcr):
+            self.failIf(dcr.get_problems())
+        d.addCallback(_check_deepcheck_results)
+        return d
+
+class FakeMutableChecker:
+    def __init__(self, node):
+        self.r = checker.Results(node.get_storage_index())
+        self.r.healthy = True
+        self.r.problems = []
+
+    def check(self, verify, repair):
+        return defer.succeed(self.r)
+
+class FakeImmutableChecker:
+    def __init__(self, client, storage_index, needed_shares, total_shares):
+        self.r = checker.Results(storage_index)
+        self.r.healthy = True
+        self.r.problems = []
+
+    def start(self):
+        return defer.succeed(self.r)
+
+def FakeImmutableVerifier(client,
+                          storage_index, needed_shares, total_shares, size,
+                          ueb_hash):
+    return FakeImmutableChecker(client,
+                                storage_index, needed_shares, total_shares)
diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py
index 10dcddfb..99095a86 100644
--- a/src/allmydata/test/test_web.py
+++ b/src/allmydata/test/test_web.py
@@ -1446,6 +1446,33 @@ class Web(WebMixin, unittest.TestCase):
         d.addCallback(_check3)
         return d
 
+    def test_POST_DIRURL_deepcheck(self):
+        d = self.POST(self.public_url, t="deep-check")
+        def _check(res):
+            self.failUnless("Objects Checked: <span>8</span>" in res)
+            self.failUnless("Objects Healthy: <span>8</span>" in res)
+            self.failUnless("Repairs Attempted: <span>0</span>" in res)
+            self.failUnless("Repairs Successful: <span>0</span>" in res)
+        d.addCallback(_check)
+        redir_url = "http://allmydata.org/TARGET"
+        def _check2(statuscode, target):
+            self.failUnlessEqual(statuscode, str(http.FOUND))
+            self.failUnlessEqual(target, redir_url)
+        d.addCallback(lambda res:
+                      self.shouldRedirect2("test_POST_DIRURL_check",
+                                           _check2,
+                                           self.POST, self.public_url,
+                                           t="deep-check",
+                                           when_done=redir_url))
+        d.addCallback(lambda res:
+                      self.POST(self.public_url, t="deep-check",
+                                return_to=redir_url))
+        def _check3(res):
+            self.failUnless("Return to parent directory" in res)
+            self.failUnless(redir_url in res)
+        d.addCallback(_check3)
+        return d
+
     def test_POST_FILEURL_bad_t(self):
         d = self.shouldFail2(error.Error, "POST_bad_t", "400 Bad Request",
                              "POST to file: bad t=bogus",
diff --git a/src/allmydata/web/checker_results.py b/src/allmydata/web/checker_results.py
index 351a7c00..5a4fff5f 100644
--- a/src/allmydata/web/checker_results.py
+++ b/src/allmydata/web/checker_results.py
@@ -1,11 +1,13 @@
 
 from nevow import rend, inevow, tags as T
 from allmydata.web.common import getxmlfile, get_arg
+from allmydata.interfaces import ICheckerResults, IDeepCheckResults
 
 class CheckerResults(rend.Page):
     docFactory = getxmlfile("checker-results.xhtml")
 
     def __init__(self, results):
+        assert ICheckerResults(results)
         self.r = results
 
     def render_storage_index(self, ctx, data):
@@ -23,3 +25,33 @@ class CheckerResults(rend.Page):
         if return_to:
             return T.div[T.a(href=return_to)["Return to parent directory"]]
         return ""
+
+class DeepCheckResults(rend.Page):
+    docFactory = getxmlfile("deep-check-results.xhtml")
+
+    def __init__(self, results):
+        assert IDeepCheckResults(results)
+        self.r = results
+
+    def render_root_storage_index(self, ctx, data):
+        return self.r.get_root_storage_index_string()
+
+    def data_objects_checked(self, ctx, data):
+        return self.r.count_objects_checked()
+    def data_objects_healthy(self, ctx, data):
+        return self.r.count_objects_healthy()
+    def data_repairs_attempted(self, ctx, data):
+        return self.r.count_repairs_attempted()
+    def data_repairs_successful(self, ctx, data):
+        return self.r.count_repairs_successful()
+
+    def data_problems(self, ctx, data):
+        for cr in self.r.get_problems():
+            yield cr
+
+    def render_return(self, ctx, data):
+        req = inevow.IRequest(ctx)
+        return_to = get_arg(req, "return_to", None)
+        if return_to:
+            return T.div[T.a(href=return_to)["Return to parent directory"]]
+        return ""
diff --git a/src/allmydata/web/deep-check-results.xhtml b/src/allmydata/web/deep-check-results.xhtml
new file mode 100644
index 00000000..d89da6c6
--- /dev/null
+++ b/src/allmydata/web/deep-check-results.xhtml
@@ -0,0 +1,34 @@
+<html xmlns:n="http://nevow.com/ns/nevow/0.1">
+  <head>
+    <title>AllMyData - Tahoe - Deep Check Results</title>
+    <!-- <link href="http://www.allmydata.com/common/css/styles.css"
+          rel="stylesheet" type="text/css"/> -->
+    <link href="/webform_css" rel="stylesheet" type="text/css"/>
+    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+  </head>
+  <body>
+
+<h1>Deep-Check Results for root SI=<span n:render="root_storage_index" /></h1>
+
+<ul>
+  <li>Objects Checked: <span n:render="data" n:data="objects_checked" /></li>
+  <li>Objects Healthy: <span n:render="data" n:data="objects_healthy" /></li>
+</ul>
+
+<h2>Problems:</h2>
+
+<ul n:render="sequence" n:data="problems">
+  <li n:pattern="item" />
+  <li n:pattern="empty">None</li>
+</ul>
+
+<h2>Repair Results:</h2>
+<ul>
+  <li>Repairs Attempted: <span n:render="data" n:data="repairs_attempted" /></li>
+  <li>Repairs Successful: <span n:render="data" n:data="repairs_successful" /></li>
+</ul>
+
+<div n:render="return" />
+
+  </body>
+</html>
diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py
index 4c6d0351..7ab7080d 100644
--- a/src/allmydata/web/directory.py
+++ b/src/allmydata/web/directory.py
@@ -21,7 +21,7 @@ from allmydata.web.common import text_plain, WebError, IClient, \
      getxmlfile, RenderMixin
 from allmydata.web.filenode import ReplaceMeMixin, \
      FileNodeHandler, PlaceHolderNodeHandler
-from allmydata.web.checker_results import CheckerResults
+from allmydata.web.checker_results import CheckerResults, DeepCheckResults
 
 class BlockingFileError(Exception):
     # TODO: catch and transform
@@ -182,6 +182,8 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
             d = self._POST_rename(req)
         elif t == "check":
             d = self._POST_check(req)
+        elif t == "deep-check":
+            d = self._POST_deep_check(req)
         elif t == "set_children":
             # TODO: docs
             d = self._POST_set_children(req)
@@ -334,6 +336,12 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
         d.addCallback(lambda res: CheckerResults(res))
         return d
 
+    def _POST_deep_check(self, req):
+        # check this directory and everything reachable from it
+        d = self.node.deep_check()
+        d.addCallback(lambda res: DeepCheckResults(res))
+        return d
+
     def _POST_set_children(self, req):
         replace = boolean_of_arg(get_arg(req, "replace", "true"))
         req.content.seek(0)
@@ -539,8 +547,24 @@ class DirectoryAsHTML(rend.Page):
         return ctx.tag
 
     def render_forms(self, ctx, data):
+        forms = []
+        deep_check = T.form(action=".", method="post",
+                            enctype="multipart/form-data")[
+            T.fieldset[
+            T.input(type="hidden", name="t", value="deep-check"),
+            T.input(type="hidden", name="return_to", value="."),
+            T.legend(class_="freeform-form-label")["Run a deep-check operation (EXPENSIVE)"],
+            T.input(type="submit", value="Deep-Check"),
+            " ",
+            "Verify every bit? (EVEN MORE EXPENSIVE):",
+            T.input(type="checkbox", name="verify"),
+            ]]
+        forms.append(T.div(class_="freeform-form")[deep_check])
+
         if self.node.is_readonly():
-            return T.div["No upload forms: directory is read-only"]
+            forms.append(T.div["No upload forms: directory is read-only"])
+            return forms
+
         mkdir = T.form(action=".", method="post",
                        enctype="multipart/form-data")[
             T.fieldset[
@@ -551,6 +575,7 @@ class DirectoryAsHTML(rend.Page):
             T.input(type="text", name="name"), " ",
             T.input(type="submit", value="Create"),
             ]]
+        forms.append(T.div(class_="freeform-form")[mkdir])
 
         upload = T.form(action=".", method="post",
                         enctype="multipart/form-data")[
@@ -565,6 +590,7 @@ class DirectoryAsHTML(rend.Page):
             " Mutable?:",
             T.input(type="checkbox", name="mutable"),
             ]]
+        forms.append(T.div(class_="freeform-form")[upload])
 
         mount = T.form(action=".", method="post",
                         enctype="multipart/form-data")[
@@ -580,10 +606,8 @@ class DirectoryAsHTML(rend.Page):
             T.input(type="text", name="uri"), " ",
             T.input(type="submit", value="Attach"),
             ]]
-        return [T.div(class_="freeform-form")[mkdir],
-                T.div(class_="freeform-form")[upload],
-                T.div(class_="freeform-form")[mount],
-                ]
+        forms.append(T.div(class_="freeform-form")[mount])
+        return forms
 
     def build_overwrite_form(self, ctx, name, target):
         if IMutableFileNode.providedBy(target) and not target.is_readonly():