All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
@ 2018-11-14  7:46 Omar Sandoval
  2018-11-14  7:46 ` [PATCH 01/10] libbtrfsutil: use top=0 as default for SubvolumeIterator() Omar Sandoval
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

Hi,

This series contains my backlog of libbtrfsutil changes which I've been
collecting over the past few weeks.

Patches 1-4 are fixes. Patches 5-6 add functionality to the unit tests
which is needed for patches 7-8. Patches 7-8 add support for the
unprivileged ioctls added in Linux 4.18; more on those below. Patch 9
bumps the library version. Patch 10 adds documentation for the available
API along with examples.

Patches 7-8 are based on Misono Tomohiro's previous patch series [1],
with a few important changes.

- Both subvolume_info() and create_subvolume_iterator() now have unit
  tests for the unprivileged case.
- Both no longer explicitly check that top == 0 in the unprivileged
  case, since that will already fail with a clear permission error.
- Unprivileged iteration is much simpler: it uses openat() instead of
  fchdir() and is based more closely on the original tree search
  variant. This fixes a bug in post-order iteration in Misono's version.
- Unprivileged iteration does _not_ support passing in a non-subvolume
  path; if this behavior is desired, I'd like it to be a separate change
  with an explicit flag.

Please take a look.

Thanks!

1: https://www.spinics.net/lists/linux-btrfs/msg79285.html

Omar Sandoval (10):
  libbtrfsutil: use top=0 as default for SubvolumeIterator()
  libbtrfsutil: change async parameters to async_ in Python bindings
  libbtrfsutil: document qgroup_inherit parameter in Python bindings
  libbtrfsutil: use SubvolumeIterator as context manager in tests
  libbtrfsutil: add test helpers for dropping privileges
  libbtrfsutil: allow tests to create multiple Btrfs instances
  libbtrfsutil: relax the privileges of subvolume_info()
  libbtrfsutil: relax the privileges of subvolume iterator
  libbtrfsutil: bump version to 1.1.0
  libbtrfsutil: document API in README

 libbtrfsutil/README.md                      | 422 +++++++++++++++++++-
 libbtrfsutil/btrfsutil.h                    |  21 +-
 libbtrfsutil/errors.c                       |   8 +
 libbtrfsutil/python/module.c                |  17 +-
 libbtrfsutil/python/subvolume.c             |   6 +-
 libbtrfsutil/python/tests/__init__.py       |  66 ++-
 libbtrfsutil/python/tests/test_subvolume.py | 215 +++++++---
 libbtrfsutil/subvolume.c                    | 407 ++++++++++++++++---
 8 files changed, 1029 insertions(+), 133 deletions(-)

-- 
2.19.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/10] libbtrfsutil: use top=0 as default for SubvolumeIterator()
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
@ 2018-11-14  7:46 ` Omar Sandoval
  2018-11-14  7:46 ` [PATCH 02/10] libbtrfsutil: change async parameters to async_ in Python bindings Omar Sandoval
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

Right now, we're defaulting to top=5 (i.e, all subvolumes). The
documented default is top=0 (i.e, only beneath the given path). This is
the expected behavior. Fix it and make the test cases cover it.

Reported-by: Jonathan Lemon <bsd@fb.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/subvolume.c             | 2 +-
 libbtrfsutil/python/tests/test_subvolume.py | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/libbtrfsutil/python/subvolume.c b/libbtrfsutil/python/subvolume.c
index 069e606b..6ecde1f6 100644
--- a/libbtrfsutil/python/subvolume.c
+++ b/libbtrfsutil/python/subvolume.c
@@ -525,7 +525,7 @@ static int SubvolumeIterator_init(SubvolumeIterator *self, PyObject *args,
 	static char *keywords[] = {"path", "top", "info", "post_order", NULL};
 	struct path_arg path = {.allow_fd = true};
 	enum btrfs_util_error err;
-	unsigned long long top = 5;
+	unsigned long long top = 0;
 	int info = 0;
 	int post_order = 0;
 	int flags = 0;
diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index 93396cba..0788a564 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -353,6 +353,7 @@ class TestSubvolume(BtrfsTestCase):
                 with self.subTest(type=type(arg)):
                     self.assertEqual(list(btrfsutil.SubvolumeIterator(arg)), subvols)
             self.assertEqual(list(btrfsutil.SubvolumeIterator('.', top=0)), subvols)
+            self.assertEqual(list(btrfsutil.SubvolumeIterator('foo', top=5)), subvols)
 
             self.assertEqual(list(btrfsutil.SubvolumeIterator('.', post_order=True)),
                              [('foo/bar/baz', 258),
@@ -365,6 +366,7 @@ class TestSubvolume(BtrfsTestCase):
             ]
 
             self.assertEqual(list(btrfsutil.SubvolumeIterator('.', top=256)), subvols)
+            self.assertEqual(list(btrfsutil.SubvolumeIterator('foo')), subvols)
             self.assertEqual(list(btrfsutil.SubvolumeIterator('foo', top=0)), subvols)
 
             os.rename('foo/bar/baz', 'baz')
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/10] libbtrfsutil: change async parameters to async_ in Python bindings
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
  2018-11-14  7:46 ` [PATCH 01/10] libbtrfsutil: use top=0 as default for SubvolumeIterator() Omar Sandoval
@ 2018-11-14  7:46 ` Omar Sandoval
  2018-11-14  7:46 ` [PATCH 03/10] libbtrfsutil: document qgroup_inherit parameter " Omar Sandoval
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

async became a keyword in Python 3.7, so, e.g., create_subvolume('foo',
async=True) is now a syntax error. Fix it with the Python convention of
adding a trailing underscore to the keyword (async -> async_). This is
what several other Python libraries did to handle this.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/module.c                | 8 ++++----
 libbtrfsutil/python/subvolume.c             | 4 ++--
 libbtrfsutil/python/tests/test_subvolume.py | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/libbtrfsutil/python/module.c b/libbtrfsutil/python/module.c
index 2dbdc7be..625cc9c6 100644
--- a/libbtrfsutil/python/module.c
+++ b/libbtrfsutil/python/module.c
@@ -233,22 +233,22 @@ static PyMethodDef btrfsutil_methods[] = {
 	 "this ID instead of the given path"},
 	{"create_subvolume", (PyCFunction)create_subvolume,
 	 METH_VARARGS | METH_KEYWORDS,
-	 "create_subvolume(path, async=False)\n\n"
+	 "create_subvolume(path, async_=False)\n\n"
 	 "Create a new subvolume.\n\n"
 	 "Arguments:\n"
 	 "path -- string, bytes, or path-like object\n"
-	 "async -- create the subvolume without waiting for it to commit to\n"
+	 "async_ -- create the subvolume without waiting for it to commit to\n"
 	 "disk and return the transaction ID"},
 	{"create_snapshot", (PyCFunction)create_snapshot,
 	 METH_VARARGS | METH_KEYWORDS,
-	 "create_snapshot(source, path, recursive=False, read_only=False, async=False)\n\n"
+	 "create_snapshot(source, path, recursive=False, read_only=False, async_=False)\n\n"
 	 "Create a new snapshot.\n\n"
 	 "Arguments:\n"
 	 "source -- string, bytes, path-like object, or open file descriptor\n"
 	 "path -- string, bytes, or path-like object\n"
 	 "recursive -- also snapshot child subvolumes\n"
 	 "read_only -- create a read-only snapshot\n"
-	 "async -- create the subvolume without waiting for it to commit to\n"
+	 "async_ -- create the subvolume without waiting for it to commit to\n"
 	 "disk and return the transaction ID"},
 	{"delete_subvolume", (PyCFunction)delete_subvolume,
 	 METH_VARARGS | METH_KEYWORDS,
diff --git a/libbtrfsutil/python/subvolume.c b/libbtrfsutil/python/subvolume.c
index 6ecde1f6..0f893b91 100644
--- a/libbtrfsutil/python/subvolume.c
+++ b/libbtrfsutil/python/subvolume.c
@@ -322,7 +322,7 @@ PyObject *set_default_subvolume(PyObject *self, PyObject *args, PyObject *kwds)
 
 PyObject *create_subvolume(PyObject *self, PyObject *args, PyObject *kwds)
 {
-	static char *keywords[] = {"path", "async", "qgroup_inherit", NULL};
+	static char *keywords[] = {"path", "async_", "qgroup_inherit", NULL};
 	struct path_arg path = {.allow_fd = false};
 	enum btrfs_util_error err;
 	int async = 0;
@@ -352,7 +352,7 @@ PyObject *create_subvolume(PyObject *self, PyObject *args, PyObject *kwds)
 PyObject *create_snapshot(PyObject *self, PyObject *args, PyObject *kwds)
 {
 	static char *keywords[] = {
-		"source", "path", "recursive", "read_only", "async",
+		"source", "path", "recursive", "read_only", "async_",
 		"qgroup_inherit", NULL,
 	};
 	struct path_arg src = {.allow_fd = true}, dst = {.allow_fd = false};
diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index 0788a564..f2a4cdb8 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -202,7 +202,7 @@ class TestSubvolume(BtrfsTestCase):
         btrfsutil.create_subvolume(subvol + '6//')
         self.assertTrue(btrfsutil.is_subvolume(subvol + '6'))
 
-        transid = btrfsutil.create_subvolume(subvol + '7', async=True)
+        transid = btrfsutil.create_subvolume(subvol + '7', async_=True)
         self.assertTrue(btrfsutil.is_subvolume(subvol + '7'))
         self.assertGreater(transid, 0)
 
@@ -265,7 +265,7 @@ class TestSubvolume(BtrfsTestCase):
         btrfsutil.create_snapshot(subvol, snapshot + '2', recursive=True)
         self.assertTrue(os.path.exists(os.path.join(snapshot + '2', 'nested/more_nested/nested_dir')))
 
-        transid = btrfsutil.create_snapshot(subvol, snapshot + '3', recursive=True, async=True)
+        transid = btrfsutil.create_snapshot(subvol, snapshot + '3', recursive=True, async_=True)
         self.assertTrue(os.path.exists(os.path.join(snapshot + '3', 'nested/more_nested/nested_dir')))
         self.assertGreater(transid, 0)
 
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/10] libbtrfsutil: document qgroup_inherit parameter in Python bindings
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
  2018-11-14  7:46 ` [PATCH 01/10] libbtrfsutil: use top=0 as default for SubvolumeIterator() Omar Sandoval
  2018-11-14  7:46 ` [PATCH 02/10] libbtrfsutil: change async parameters to async_ in Python bindings Omar Sandoval
@ 2018-11-14  7:46 ` Omar Sandoval
  2018-11-14  7:46 ` [PATCH 04/10] libbtrfsutil: use SubvolumeIterator as context manager in tests Omar Sandoval
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

This has been supported since day one, but it wasn't documented.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/module.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/libbtrfsutil/python/module.c b/libbtrfsutil/python/module.c
index 625cc9c6..f8260c84 100644
--- a/libbtrfsutil/python/module.c
+++ b/libbtrfsutil/python/module.c
@@ -233,15 +233,18 @@ static PyMethodDef btrfsutil_methods[] = {
 	 "this ID instead of the given path"},
 	{"create_subvolume", (PyCFunction)create_subvolume,
 	 METH_VARARGS | METH_KEYWORDS,
-	 "create_subvolume(path, async_=False)\n\n"
+	 "create_subvolume(path, async_=False, qgroup_inherit=None)\n\n"
 	 "Create a new subvolume.\n\n"
 	 "Arguments:\n"
 	 "path -- string, bytes, or path-like object\n"
 	 "async_ -- create the subvolume without waiting for it to commit to\n"
-	 "disk and return the transaction ID"},
+	 "disk and return the transaction ID\n"
+	 "qgroup_inherit -- optional QgroupInherit object of qgroups to\n"
+	 "inherit from"},
 	{"create_snapshot", (PyCFunction)create_snapshot,
 	 METH_VARARGS | METH_KEYWORDS,
-	 "create_snapshot(source, path, recursive=False, read_only=False, async_=False)\n\n"
+	 "create_snapshot(source, path, recursive=False, read_only=False,\n"
+	 "                async_=False, qgroup_inherit=None)\n\n"
 	 "Create a new snapshot.\n\n"
 	 "Arguments:\n"
 	 "source -- string, bytes, path-like object, or open file descriptor\n"
@@ -249,7 +252,9 @@ static PyMethodDef btrfsutil_methods[] = {
 	 "recursive -- also snapshot child subvolumes\n"
 	 "read_only -- create a read-only snapshot\n"
 	 "async_ -- create the subvolume without waiting for it to commit to\n"
-	 "disk and return the transaction ID"},
+	 "disk and return the transaction ID\n"
+	 "qgroup_inherit -- optional QgroupInherit object of qgroups to\n"
+	 "inherit from"},
 	{"delete_subvolume", (PyCFunction)delete_subvolume,
 	 METH_VARARGS | METH_KEYWORDS,
 	 "delete_subvolume(path, recursive=False)\n\n"
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/10] libbtrfsutil: use SubvolumeIterator as context manager in tests
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (2 preceding siblings ...)
  2018-11-14  7:46 ` [PATCH 03/10] libbtrfsutil: document qgroup_inherit parameter " Omar Sandoval
@ 2018-11-14  7:46 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 05/10] libbtrfsutil: add test helpers for dropping privileges Omar Sandoval
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

We're leaking file descriptors, which makes it impossible to clean up
the temporary mount point created by the test.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/tests/test_subvolume.py | 51 ++++++++++++---------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index f2a4cdb8..4049b08e 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -334,11 +334,13 @@ class TestSubvolume(BtrfsTestCase):
             os.chdir(self.mountpoint)
             btrfsutil.create_subvolume('foo')
 
-            path, subvol = next(btrfsutil.SubvolumeIterator('.', info=True))
-            self.assertEqual(path, 'foo')
-            self.assertIsInstance(subvol, btrfsutil.SubvolumeInfo)
-            self.assertEqual(subvol.id, 256)
-            self.assertEqual(subvol.parent_id, 5)
+            with btrfsutil.SubvolumeIterator('.', info=True) as it:
+                path, subvol = next(it)
+                self.assertEqual(path, 'foo')
+                self.assertIsInstance(subvol, btrfsutil.SubvolumeInfo)
+                self.assertEqual(subvol.id, 256)
+                self.assertEqual(subvol.parent_id, 5)
+                self.assertRaises(StopIteration, next, it)
 
             btrfsutil.create_subvolume('foo/bar')
             btrfsutil.create_subvolume('foo/bar/baz')
@@ -350,30 +352,37 @@ class TestSubvolume(BtrfsTestCase):
             ]
 
             for arg in self.path_or_fd('.'):
-                with self.subTest(type=type(arg)):
-                    self.assertEqual(list(btrfsutil.SubvolumeIterator(arg)), subvols)
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('.', top=0)), subvols)
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('foo', top=5)), subvols)
-
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('.', post_order=True)),
-                             [('foo/bar/baz', 258),
-                              ('foo/bar', 257),
-                              ('foo', 256)])
+                with self.subTest(type=type(arg)), btrfsutil.SubvolumeIterator(arg) as it:
+                    self.assertEqual(list(it), subvols)
+            with btrfsutil.SubvolumeIterator('.', top=0) as it:
+                self.assertEqual(list(it), subvols)
+            with btrfsutil.SubvolumeIterator('foo', top=5) as it:
+                self.assertEqual(list(it), subvols)
+
+            with btrfsutil.SubvolumeIterator('.', post_order=True) as it:
+                self.assertEqual(list(it),
+                                 [('foo/bar/baz', 258),
+                                  ('foo/bar', 257),
+                                  ('foo', 256)])
 
             subvols = [
                 ('bar', 257),
                 ('bar/baz', 258),
             ]
 
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('.', top=256)), subvols)
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('foo')), subvols)
-            self.assertEqual(list(btrfsutil.SubvolumeIterator('foo', top=0)), subvols)
+            with btrfsutil.SubvolumeIterator('.', top=256) as it:
+                self.assertEqual(list(it), subvols)
+            with btrfsutil.SubvolumeIterator('foo') as it:
+                self.assertEqual(list(it), subvols)
+            with btrfsutil.SubvolumeIterator('foo', top=0) as it:
+                self.assertEqual(list(it), subvols)
 
             os.rename('foo/bar/baz', 'baz')
-            self.assertEqual(sorted(btrfsutil.SubvolumeIterator('.')),
-                             [('baz', 258),
-                              ('foo', 256),
-                              ('foo/bar', 257)])
+            with btrfsutil.SubvolumeIterator('.') as it:
+                self.assertEqual(sorted(it),
+                                 [('baz', 258),
+                                  ('foo', 256),
+                                  ('foo/bar', 257)])
 
             with btrfsutil.SubvolumeIterator('.') as it:
                 self.assertGreaterEqual(it.fileno(), 0)
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/10] libbtrfsutil: add test helpers for dropping privileges
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (3 preceding siblings ...)
  2018-11-14  7:46 ` [PATCH 04/10] libbtrfsutil: use SubvolumeIterator as context manager in tests Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 06/10] libbtrfsutil: allow tests to create multiple Btrfs instances Omar Sandoval
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

These will be used for testing some upcoming changes which allow
unprivileged operations.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/tests/__init__.py | 31 ++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/libbtrfsutil/python/tests/__init__.py b/libbtrfsutil/python/tests/__init__.py
index 35550e0a..4bc11990 100644
--- a/libbtrfsutil/python/tests/__init__.py
+++ b/libbtrfsutil/python/tests/__init__.py
@@ -15,14 +15,44 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with libbtrfsutil.  If not, see <http://www.gnu.org/licenses/>.
 
+import contextlib
 import os
 from pathlib import PurePath
+import pwd
 import subprocess
 import tempfile
 import unittest
 
 
 HAVE_PATH_LIKE = hasattr(PurePath, '__fspath__')
+try:
+    NOBODY_UID = pwd.getpwnam('nobody').pw_uid
+    skipUnlessHaveNobody = lambda func: func
+except KeyError:
+    NOBODY_UID = None
+    skipUnlessHaveNobody = unittest.skip('must have nobody user')
+
+
+@contextlib.contextmanager
+def drop_privs():
+    try:
+        os.seteuid(NOBODY_UID)
+        yield
+    finally:
+        os.seteuid(0)
+
+
+@contextlib.contextmanager
+def regain_privs():
+    uid = os.geteuid()
+    if uid:
+        try:
+            os.seteuid(0)
+            yield
+        finally:
+            os.seteuid(uid)
+    else:
+        yield
 
 
 @unittest.skipIf(os.geteuid() != 0, 'must be run as root')
@@ -67,4 +97,3 @@ class BtrfsTestCase(unittest.TestCase):
             yield fd
         finally:
             os.close(fd)
-
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/10] libbtrfsutil: allow tests to create multiple Btrfs instances
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (4 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 05/10] libbtrfsutil: add test helpers for dropping privileges Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 07/10] libbtrfsutil: relax the privileges of subvolume_info() Omar Sandoval
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

Some upcoming tests will need to create a second Btrfs filesystem, so
add support for this to the test helpers.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/tests/__init__.py | 35 +++++++++++++++++----------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/libbtrfsutil/python/tests/__init__.py b/libbtrfsutil/python/tests/__init__.py
index 4bc11990..9fd6f6de 100644
--- a/libbtrfsutil/python/tests/__init__.py
+++ b/libbtrfsutil/python/tests/__init__.py
@@ -57,14 +57,18 @@ def regain_privs():
 
 @unittest.skipIf(os.geteuid() != 0, 'must be run as root')
 class BtrfsTestCase(unittest.TestCase):
-    def setUp(self):
-        self.mountpoint = tempfile.mkdtemp()
+    def __init__(self, *args, **kwds):
+        super().__init__(*args, **kwds)
+        self._mountpoints = []
+
+    def mount_btrfs(self):
+        mountpoint = tempfile.mkdtemp()
         try:
             with tempfile.NamedTemporaryFile(delete=False) as f:
                 os.truncate(f.fileno(), 1024 * 1024 * 1024)
-                self.image = f.name
+                image = f.name
         except Exception as e:
-            os.rmdir(self.mountpoint)
+            os.rmdir(mountpoint)
             raise e
 
         if os.path.exists('../../mkfs.btrfs'):
@@ -72,19 +76,24 @@ class BtrfsTestCase(unittest.TestCase):
         else:
             mkfs = 'mkfs.btrfs'
         try:
-            subprocess.check_call([mkfs, '-q', self.image])
-            subprocess.check_call(['mount', '-o', 'loop', '--', self.image, self.mountpoint])
+            subprocess.check_call([mkfs, '-q', image])
+            subprocess.check_call(['mount', '-o', 'loop', '--', image, mountpoint])
         except Exception as e:
-            os.remove(self.image)
-            os.rmdir(self.mountpoint)
+            os.rmdir(mountpoint)
+            os.remove(image)
             raise e
 
+        self._mountpoints.append((mountpoint, image))
+        return mountpoint, image
+
+    def setUp(self):
+        self.mountpoint, self.image = self.mount_btrfs()
+
     def tearDown(self):
-        try:
-            subprocess.check_call(['umount', self.mountpoint])
-        finally:
-            os.remove(self.image)
-            os.rmdir(self.mountpoint)
+        for mountpoint, image in self._mountpoints:
+            subprocess.call(['umount', '-R', mountpoint])
+            os.rmdir(mountpoint)
+            os.remove(image)
 
     @staticmethod
     def path_or_fd(path, open_flags=os.O_RDONLY):
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/10] libbtrfsutil: relax the privileges of subvolume_info()
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (5 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 06/10] libbtrfsutil: allow tests to create multiple Btrfs instances Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 08/10] libbtrfsutil: relax the privileges of subvolume iterator Omar Sandoval
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

Attempt to use the BTRFS_IOC_GET_SUBVOL_INFO ioctl (added in kernel
4.18) for subvolume_info() if not root. Also, rename
get_subvolume_info_root() -> get_subvolume_info_privileged() for
consistency with further changes.

This is based on a patch from Misono Tomohiro.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/btrfsutil.h                    |  4 +-
 libbtrfsutil/errors.c                       |  2 +
 libbtrfsutil/python/tests/test_subvolume.py | 42 ++++++++++++----
 libbtrfsutil/subvolume.c                    | 53 +++++++++++++++++++--
 4 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index 6d655f49..c1925007 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -63,6 +63,7 @@ enum btrfs_util_error {
 	BTRFS_UTIL_ERROR_SYNC_FAILED,
 	BTRFS_UTIL_ERROR_START_SYNC_FAILED,
 	BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED,
+	BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED,
 };
 
 /**
@@ -266,7 +267,8 @@ struct btrfs_util_subvolume_info {
  * to check whether the subvolume exists; %BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND
  * will be returned if it does not.
  *
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) unless @id is zero and
+ * the kernel supports BTRFS_IOC_GET_SUBVOL_INFO (kernel >= 4.18).
  *
  * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
  */
diff --git a/libbtrfsutil/errors.c b/libbtrfsutil/errors.c
index 634edc65..cf968b03 100644
--- a/libbtrfsutil/errors.c
+++ b/libbtrfsutil/errors.c
@@ -45,6 +45,8 @@ static const char * const error_messages[] = {
 	[BTRFS_UTIL_ERROR_SYNC_FAILED] = "Could not sync filesystem",
 	[BTRFS_UTIL_ERROR_START_SYNC_FAILED] = "Could not start filesystem sync",
 	[BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED] = "Could not wait for filesystem sync",
+	[BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED] =
+		"Could not get subvolume information with BTRFS_IOC_GET_SUBVOL_INFO",
 };
 
 PUBLIC const char *btrfs_util_strerror(enum btrfs_util_error err)
diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index 4049b08e..55ebf34d 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -23,7 +23,12 @@ from pathlib import PurePath
 import traceback
 
 import btrfsutil
-from tests import BtrfsTestCase, HAVE_PATH_LIKE
+from tests import (
+    BtrfsTestCase,
+    drop_privs,
+    HAVE_PATH_LIKE,
+    skipUnlessHaveNobody,
+)
 
 
 class TestSubvolume(BtrfsTestCase):
@@ -87,7 +92,7 @@ class TestSubvolume(BtrfsTestCase):
         finally:
             os.chdir(pwd)
 
-    def test_subvolume_info(self):
+    def _test_subvolume_info(self, subvol, snapshot):
         for arg in self.path_or_fd(self.mountpoint):
             with self.subTest(type=type(arg)):
                 info = btrfsutil.subvolume_info(arg)
@@ -100,7 +105,7 @@ class TestSubvolume(BtrfsTestCase):
                 self.assertEqual(info.parent_uuid, bytes(16))
                 self.assertEqual(info.received_uuid, bytes(16))
                 self.assertNotEqual(info.generation, 0)
-                self.assertEqual(info.ctransid, 0)
+                self.assertGreaterEqual(info.ctransid, 0)
                 self.assertEqual(info.otransid, 0)
                 self.assertEqual(info.stransid, 0)
                 self.assertEqual(info.rtransid, 0)
@@ -109,9 +114,6 @@ class TestSubvolume(BtrfsTestCase):
                 self.assertEqual(info.stime, 0)
                 self.assertEqual(info.rtime, 0)
 
-        subvol = os.path.join(self.mountpoint, 'subvol')
-        btrfsutil.create_subvolume(subvol)
-
         info = btrfsutil.subvolume_info(subvol)
         self.assertEqual(info.id, 256)
         self.assertEqual(info.parent_id, 5)
@@ -132,19 +134,43 @@ class TestSubvolume(BtrfsTestCase):
         self.assertEqual(info.rtime, 0)
 
         subvol_uuid = info.uuid
-        snapshot = os.path.join(self.mountpoint, 'snapshot')
-        btrfsutil.create_snapshot(subvol, snapshot)
 
         info = btrfsutil.subvolume_info(snapshot)
         self.assertEqual(info.parent_uuid, subvol_uuid)
 
         # TODO: test received_uuid, stransid, rtransid, stime, and rtime
 
+    def test_subvolume_info(self):
+        subvol = os.path.join(self.mountpoint, 'subvol')
+        btrfsutil.create_subvolume(subvol)
+        snapshot = os.path.join(self.mountpoint, 'snapshot')
+        btrfsutil.create_snapshot(subvol, snapshot)
+
+        self._test_subvolume_info(subvol, snapshot)
+
         for arg in self.path_or_fd(self.mountpoint):
             with self.subTest(type=type(arg)):
                 with self.assertRaises(btrfsutil.BtrfsUtilError) as e:
                     # BTRFS_EXTENT_TREE_OBJECTID
                     btrfsutil.subvolume_info(arg, 2)
+                self.assertEqual(e.exception.btrfsutilerror,
+                                 btrfsutil.ERROR_SUBVOLUME_NOT_FOUND)
+
+    @skipUnlessHaveNobody
+    def test_subvolume_info_unprivileged(self):
+        subvol = os.path.join(self.mountpoint, 'subvol')
+        btrfsutil.create_subvolume(subvol)
+        snapshot = os.path.join(self.mountpoint, 'snapshot')
+        btrfsutil.create_snapshot(subvol, snapshot)
+
+        with drop_privs():
+            try:
+                self._test_subvolume_info(subvol, snapshot)
+            except OSError as e:
+                if e.errno == errno.ENOTTY:
+                    self.skipTest('BTRFS_IOC_GET_SUBVOL_INFO is not available')
+                else:
+                    raise
 
     def test_read_only(self):
         for arg in self.path_or_fd(self.mountpoint):
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 0d7ef5bf..69654db4 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -31,6 +31,11 @@
 
 #include "btrfsutil_internal.h"
 
+static bool is_root(void)
+{
+	return geteuid() == 0;
+}
+
 /*
  * This intentionally duplicates btrfs_util_is_subvolume_fd() instead of opening
  * a file descriptor and calling it, because fstat() and fstatfs() don't accept
@@ -295,8 +300,8 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_info(const char *path,
 	return err;
 }
 
-static enum btrfs_util_error get_subvolume_info_root(int fd, uint64_t id,
-						     struct btrfs_util_subvolume_info *subvol)
+static enum btrfs_util_error get_subvolume_info_privileged(int fd, uint64_t id,
+							   struct btrfs_util_subvolume_info *subvol)
 {
 	struct btrfs_ioctl_search_args search = {
 		.key = {
@@ -383,6 +388,45 @@ static enum btrfs_util_error get_subvolume_info_root(int fd, uint64_t id,
 	return BTRFS_UTIL_OK;
 }
 
+static enum btrfs_util_error get_subvolume_info_unprivileged(int fd,
+							     struct btrfs_util_subvolume_info *subvol)
+{
+	struct btrfs_ioctl_get_subvol_info_args info;
+	int ret;
+
+	ret = ioctl(fd, BTRFS_IOC_GET_SUBVOL_INFO, &info);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED;
+
+	subvol->id = info.treeid;
+	subvol->parent_id = info.parent_id;
+	subvol->dir_id = info.dirid;
+	subvol->flags = info.flags;
+	subvol->generation = info.generation;
+
+	memcpy(subvol->uuid, info.uuid, sizeof(subvol->uuid));
+	memcpy(subvol->parent_uuid, info.parent_uuid,
+	       sizeof(subvol->parent_uuid));
+	memcpy(subvol->received_uuid, info.received_uuid,
+	       sizeof(subvol->received_uuid));
+
+	subvol->ctransid = info.ctransid;
+	subvol->otransid = info.otransid;
+	subvol->stransid = info.stransid;
+	subvol->rtransid = info.rtransid;
+
+	subvol->ctime.tv_sec = info.ctime.sec;
+	subvol->ctime.tv_nsec = info.ctime.nsec;
+	subvol->otime.tv_sec = info.otime.sec;
+	subvol->otime.tv_nsec = info.otime.nsec;
+	subvol->stime.tv_sec = info.stime.sec;
+	subvol->stime.tv_nsec = info.stime.nsec;
+	subvol->rtime.tv_sec = info.rtime.sec;
+	subvol->rtime.tv_nsec = info.rtime.nsec;
+
+	return BTRFS_UTIL_OK;
+}
+
 PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
 							  struct btrfs_util_subvolume_info *subvol)
 {
@@ -393,6 +437,9 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
 		if (err)
 			return err;
 
+		if (!is_root())
+			return get_subvolume_info_unprivileged(fd, subvol);
+
 		err = btrfs_util_subvolume_id_fd(fd, &id);
 		if (err)
 			return err;
@@ -404,7 +451,7 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
 		return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
 	}
 
-	return get_subvolume_info_root(fd, id, subvol);
+	return get_subvolume_info_privileged(fd, id, subvol);
 }
 
 PUBLIC enum btrfs_util_error btrfs_util_get_subvolume_read_only_fd(int fd,
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/10] libbtrfsutil: relax the privileges of subvolume iterator
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (6 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 07/10] libbtrfsutil: relax the privileges of subvolume_info() Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 09/10] libbtrfsutil: bump version to 1.1.0 Omar Sandoval
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

We can use the new BTRFS_IOC_GET_SUBVOL_ROOTREF and
BTRFS_IOC_INO_LOOKUP_USER ioctls to allow non-root users to list
subvolumes.

This is based on a patch from Misono Tomohiro but takes a different
approach (mainly, this approach is more similar to the existing tree
search approach).

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/btrfsutil.h                    |  15 +-
 libbtrfsutil/errors.c                       |   6 +
 libbtrfsutil/python/tests/test_subvolume.py | 180 +++++++---
 libbtrfsutil/subvolume.c                    | 354 +++++++++++++++++---
 4 files changed, 450 insertions(+), 105 deletions(-)

diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index c1925007..d88c39e5 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -64,6 +64,9 @@ enum btrfs_util_error {
 	BTRFS_UTIL_ERROR_START_SYNC_FAILED,
 	BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED,
 	BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED,
+	BTRFS_UTIL_ERROR_GET_SUBVOL_ROOTREF_FAILED,
+	BTRFS_UTIL_ERROR_INO_LOOKUP_USER_FAILED,
+	BTRFS_UTIL_ERROR_FS_INFO_FAILED,
 };
 
 /**
@@ -507,6 +510,12 @@ struct btrfs_util_subvolume_iterator;
  * @flags: Bitmask of BTRFS_UTIL_SUBVOLUME_ITERATOR_* flags.
  * @ret: Returned iterator.
  *
+ * Subvolume iterators require appropriate privilege (CAP_SYS_ADMIN) unless @top
+ * is zero and the kernel supports BTRFS_IOC_GET_SUBVOL_ROOTREF and
+ * BTRFS_IOC_INO_LOOKUP_USER (kernel >= 4.18). In this case, subvolumes which
+ * cannot be accessed (e.g., due to permissions or other mounts) will be
+ * skipped.
+ *
  * The returned iterator must be freed with
  * btrfs_util_destroy_subvolume_iterator().
  *
@@ -555,7 +564,8 @@ int btrfs_util_subvolume_iterator_fd(const struct btrfs_util_subvolume_iterator
  * Must be freed with free().
  * @id_ret: Returned subvolume ID. May be %NULL.
  *
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) for kernels < 4.18. See
+ * btrfs_util_create_subvolume_iterator().
  *
  * Return: %BTRFS_UTIL_OK on success, %BTRFS_UTIL_ERROR_STOP_ITERATION if there
  * are no more subvolumes, non-zero error code on failure.
@@ -574,7 +584,8 @@ enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct btrfs_util_subvo
  * This convenience function basically combines
  * btrfs_util_subvolume_iterator_next() and btrfs_util_subvolume_info().
  *
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) for kernels < 4.18. See
+ * btrfs_util_create_subvolume_iterator().
  *
  * Return: See btrfs_util_subvolume_iterator_next().
  */
diff --git a/libbtrfsutil/errors.c b/libbtrfsutil/errors.c
index cf968b03..d39b38d0 100644
--- a/libbtrfsutil/errors.c
+++ b/libbtrfsutil/errors.c
@@ -47,6 +47,12 @@ static const char * const error_messages[] = {
 	[BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED] = "Could not wait for filesystem sync",
 	[BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED] =
 		"Could not get subvolume information with BTRFS_IOC_GET_SUBVOL_INFO",
+	[BTRFS_UTIL_ERROR_GET_SUBVOL_ROOTREF_FAILED] =
+		"Could not get rootref information with BTRFS_IOC_GET_SUBVOL_ROOTREF",
+	[BTRFS_UTIL_ERROR_INO_LOOKUP_USER_FAILED] =
+		"Could not resolve subvolume path with BTRFS_IOC_INO_LOOKUP_USER",
+	[BTRFS_UTIL_ERROR_FS_INFO_FAILED] =
+		"Could not get filesystem information",
 };
 
 PUBLIC const char *btrfs_util_strerror(enum btrfs_util_error err)
diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index 55ebf34d..99ec97bc 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -20,6 +20,7 @@ import errno
 import os
 import os.path
 from pathlib import PurePath
+import subprocess
 import traceback
 
 import btrfsutil
@@ -27,6 +28,8 @@ from tests import (
     BtrfsTestCase,
     drop_privs,
     HAVE_PATH_LIKE,
+    NOBODY_UID,
+    regain_privs,
     skipUnlessHaveNobody,
 )
 
@@ -354,69 +357,136 @@ class TestSubvolume(BtrfsTestCase):
             with self.subTest(type=type(arg)):
                 self.assertEqual(btrfsutil.deleted_subvolumes(arg), [256])
 
-    def test_subvolume_iterator(self):
-        pwd = os.getcwd()
-        try:
-            os.chdir(self.mountpoint)
-            btrfsutil.create_subvolume('foo')
-
-            with btrfsutil.SubvolumeIterator('.', info=True) as it:
-                path, subvol = next(it)
-                self.assertEqual(path, 'foo')
-                self.assertIsInstance(subvol, btrfsutil.SubvolumeInfo)
-                self.assertEqual(subvol.id, 256)
-                self.assertEqual(subvol.parent_id, 5)
-                self.assertRaises(StopIteration, next, it)
-
-            btrfsutil.create_subvolume('foo/bar')
-            btrfsutil.create_subvolume('foo/bar/baz')
-
-            subvols = [
-                ('foo', 256),
-                ('foo/bar', 257),
-                ('foo/bar/baz', 258),
-            ]
-
-            for arg in self.path_or_fd('.'):
-                with self.subTest(type=type(arg)), btrfsutil.SubvolumeIterator(arg) as it:
-                    self.assertEqual(list(it), subvols)
-            with btrfsutil.SubvolumeIterator('.', top=0) as it:
+    def _test_subvolume_iterator(self):
+        btrfsutil.create_subvolume('foo')
+
+        with btrfsutil.SubvolumeIterator('.', info=True) as it:
+            path, subvol = next(it)
+            self.assertEqual(path, 'foo')
+            self.assertIsInstance(subvol, btrfsutil.SubvolumeInfo)
+            self.assertEqual(subvol.id, 256)
+            self.assertEqual(subvol.parent_id, 5)
+            self.assertRaises(StopIteration, next, it)
+
+        btrfsutil.create_subvolume('foo/bar')
+        btrfsutil.create_subvolume('foo/bar/baz')
+
+        subvols = [
+            ('foo', 256),
+            ('foo/bar', 257),
+            ('foo/bar/baz', 258),
+        ]
+
+        for arg in self.path_or_fd('.'):
+            with self.subTest(type=type(arg)), btrfsutil.SubvolumeIterator(arg) as it:
                 self.assertEqual(list(it), subvols)
+        with btrfsutil.SubvolumeIterator('.', top=0) as it:
+            self.assertEqual(list(it), subvols)
+        if os.geteuid() == 0:
             with btrfsutil.SubvolumeIterator('foo', top=5) as it:
                 self.assertEqual(list(it), subvols)
 
-            with btrfsutil.SubvolumeIterator('.', post_order=True) as it:
-                self.assertEqual(list(it),
-                                 [('foo/bar/baz', 258),
-                                  ('foo/bar', 257),
-                                  ('foo', 256)])
+        with btrfsutil.SubvolumeIterator('.', post_order=True) as it:
+            self.assertEqual(list(it),
+                             [('foo/bar/baz', 258),
+                              ('foo/bar', 257),
+                              ('foo', 256)])
 
-            subvols = [
-                ('bar', 257),
-                ('bar/baz', 258),
-            ]
+        subvols = [
+            ('bar', 257),
+            ('bar/baz', 258),
+        ]
 
+        if os.geteuid() == 0:
             with btrfsutil.SubvolumeIterator('.', top=256) as it:
                 self.assertEqual(list(it), subvols)
-            with btrfsutil.SubvolumeIterator('foo') as it:
-                self.assertEqual(list(it), subvols)
-            with btrfsutil.SubvolumeIterator('foo', top=0) as it:
-                self.assertEqual(list(it), subvols)
+        with btrfsutil.SubvolumeIterator('foo') as it:
+            self.assertEqual(list(it), subvols)
+        with btrfsutil.SubvolumeIterator('foo', top=0) as it:
+            self.assertEqual(list(it), subvols)
+
+        os.rename('foo/bar/baz', 'baz')
+        os.mkdir('dir')
+        btrfsutil.create_subvolume('dir/qux')
+        os.mkdir('dir/qux/dir2')
+        btrfsutil.create_subvolume('dir/qux/dir2/quux')
+
+        subvols = [
+            ('baz', 258),
+            ('dir/qux', 259),
+            ('dir/qux/dir2/quux', 260),
+            ('foo', 256),
+            ('foo/bar', 257),
+        ]
+
+        # Test various corner cases of the unprivileged implementation
+        # where we can't access the subvolume.
+        if os.geteuid() != 0:
+            with regain_privs():
+                # We don't have permission to traverse the path.
+                os.mkdir('directory_perms', 0o700)
+                btrfsutil.create_subvolume('directory_perms/subvol')
+
+                # We don't have permission to resolve the subvolume path.
+                os.mkdir('subvol_perms', 0o755)
+                btrfsutil.create_subvolume('subvol_perms/subvol')
+                os.chmod('subvol_perms/subvol', 0o700)
+
+                # The path doesn't exist.
+                os.mkdir('enoent', 0o755)
+                btrfsutil.create_subvolume('enoent/subvol')
+                subprocess.check_call(['mount', '-t', 'tmpfs', 'tmpfs', 'enoent'])
+
+                # The path exists but it's not a subvolume.
+                os.mkdir('notsubvol', 0o755)
+                btrfsutil.create_subvolume('notsubvol/subvol')
+                subprocess.check_call(['mount', '-t', 'tmpfs', 'tmpfs', 'notsubvol'])
+                os.mkdir('notsubvol/subvol')
+
+                # The path exists and is a subvolume, but on a different
+                # filesystem.
+                os.mkdir('wrongfs', 0o755)
+                btrfsutil.create_subvolume('wrongfs/subvol')
+                other_mountpoint, _ = self.mount_btrfs()
+                subprocess.check_call(['mount', '--bind', '--',
+                                       other_mountpoint, 'wrongfs/subvol'])
+
+                # The path exists and is a subvolume on the same
+                # filesystem, but not the right one.
+                os.mkdir('wrongsubvol', 0o755)
+                btrfsutil.create_subvolume('wrongsubvol/subvol')
+                subprocess.check_call(['mount', '--bind', 'baz', 'wrongsubvol/subvol'])
+
+
+        with btrfsutil.SubvolumeIterator('.') as it:
+            self.assertEqual(sorted(it), subvols)
+        with btrfsutil.SubvolumeIterator('.', post_order=True) as it:
+            self.assertEqual(sorted(it), subvols)
+
+        with btrfsutil.SubvolumeIterator('.') as it:
+            self.assertGreaterEqual(it.fileno(), 0)
+            it.close()
+            with self.assertRaises(ValueError):
+                next(iter(it))
+            with self.assertRaises(ValueError):
+                it.fileno()
+            it.close()
 
-            os.rename('foo/bar/baz', 'baz')
-            with btrfsutil.SubvolumeIterator('.') as it:
-                self.assertEqual(sorted(it),
-                                 [('baz', 258),
-                                  ('foo', 256),
-                                  ('foo/bar', 257)])
-
-            with btrfsutil.SubvolumeIterator('.') as it:
-                self.assertGreaterEqual(it.fileno(), 0)
-                it.close()
-                with self.assertRaises(ValueError):
-                    next(iter(it))
-                with self.assertRaises(ValueError):
-                    it.fileno()
-                it.close()
+    def test_subvolume_iterator(self):
+        pwd = os.getcwd()
+        try:
+            os.chdir(self.mountpoint)
+            self._test_subvolume_iterator()
+        finally:
+            os.chdir(pwd)
+
+    @skipUnlessHaveNobody
+    def test_subvolume_iterator_unprivileged(self):
+        os.chown(self.mountpoint, NOBODY_UID, -1)
+        pwd = os.getcwd()
+        try:
+            os.chdir(self.mountpoint)
+            with drop_privs():
+                self._test_subvolume_iterator()
         finally:
             os.chdir(pwd)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 69654db4..60ab9f9d 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -749,13 +749,28 @@ PUBLIC enum btrfs_util_error btrfs_util_create_subvolume_fd(int parent_fd,
 #define BTRFS_UTIL_SUBVOLUME_ITERATOR_CLOSE_FD (1 << 30)
 
 struct search_stack_entry {
-	struct btrfs_ioctl_search_args search;
-	size_t items_pos, buf_off;
+	union {
+		/* Used for subvolume_iterator_next_tree_search(). */
+		struct {
+			struct btrfs_ioctl_search_args search;
+			size_t buf_off;
+		};
+		/* Used for subvolume_iterator_next_unprivileged(). */
+		struct {
+			uint64_t id;
+			struct btrfs_ioctl_get_subvol_rootref_args rootref_args;
+		};
+	};
+	/* Used for both. */
+	size_t items_pos;
 	size_t path_len;
 };
 
 struct btrfs_util_subvolume_iterator {
+	bool use_tree_search;
 	int fd;
+	/* cur_fd is only used for subvolume_iterator_next_unprivileged(). */
+	int cur_fd;
 	int flags;
 
 	struct search_stack_entry *search_stack;
@@ -766,6 +781,58 @@ struct btrfs_util_subvolume_iterator {
 	size_t cur_path_capacity;
 };
 
+static struct search_stack_entry *top_search_stack_entry(struct btrfs_util_subvolume_iterator *iter)
+{
+	return &iter->search_stack[iter->search_stack_len - 1];
+}
+
+/*
+ * Check that a path that we opened is the subvolume which we expect. It may not
+ * be if there is another filesystem mounted over a parent directory or the
+ * subvolume itself.
+ */
+static enum btrfs_util_error check_expected_subvolume(int fd, int parent_fd,
+						      uint64_t tree_id)
+{
+	struct btrfs_ioctl_fs_info_args parent_fs_info, fs_info;
+	enum btrfs_util_error err;
+	uint64_t id;
+	int ret;
+
+	/* Make sure it's a subvolume. */
+	err = btrfs_util_is_subvolume_fd(fd);
+	if (err == BTRFS_UTIL_ERROR_NOT_BTRFS ||
+	    err == BTRFS_UTIL_ERROR_NOT_SUBVOLUME) {
+		errno = ENOENT;
+		return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
+	} else if (err) {
+		return err;
+	}
+
+	/* Make sure it's on the same filesystem. */
+	ret = ioctl(parent_fd, BTRFS_IOC_FS_INFO, &parent_fs_info);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_FS_INFO_FAILED;
+	ret = ioctl(fd, BTRFS_IOC_FS_INFO, &fs_info);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_FS_INFO_FAILED;
+	if (memcmp(parent_fs_info.fsid, fs_info.fsid, sizeof(fs_info.fsid)) != 0) {
+		errno = ENOENT;
+		return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
+	}
+
+	/* Make sure it's the subvolume that we expected. */
+	err = btrfs_util_subvolume_id_fd(fd, &id);
+	if (err)
+		return err;
+	if (id != tree_id) {
+		errno = ENOENT;
+		return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
+	}
+
+	return BTRFS_UTIL_OK;
+}
+
 static enum btrfs_util_error append_to_search_stack(struct btrfs_util_subvolume_iterator *iter,
 						    uint64_t tree_id,
 						    size_t path_len)
@@ -786,24 +853,84 @@ static enum btrfs_util_error append_to_search_stack(struct btrfs_util_subvolume_
 		iter->search_stack = new_search_stack;
 	}
 
-	entry = &iter->search_stack[iter->search_stack_len++];
+	entry = &iter->search_stack[iter->search_stack_len];
 
-	memset(&entry->search, 0, sizeof(entry->search));
-	entry->search.key.tree_id = BTRFS_ROOT_TREE_OBJECTID;
-	entry->search.key.min_objectid = tree_id;
-	entry->search.key.max_objectid = tree_id;
-	entry->search.key.min_type = BTRFS_ROOT_REF_KEY;
-	entry->search.key.max_type = BTRFS_ROOT_REF_KEY;
-	entry->search.key.min_offset = 0;
-	entry->search.key.max_offset = UINT64_MAX;
-	entry->search.key.min_transid = 0;
-	entry->search.key.max_transid = UINT64_MAX;
-	entry->search.key.nr_items = 0;
+	memset(entry, 0, sizeof(*entry));
+	entry->path_len = path_len;
+	if (iter->use_tree_search) {
+		entry->search.key.tree_id = BTRFS_ROOT_TREE_OBJECTID;
+		entry->search.key.min_objectid = tree_id;
+		entry->search.key.max_objectid = tree_id;
+		entry->search.key.min_type = BTRFS_ROOT_REF_KEY;
+		entry->search.key.max_type = BTRFS_ROOT_REF_KEY;
+		entry->search.key.min_offset = 0;
+		entry->search.key.max_offset = UINT64_MAX;
+		entry->search.key.min_transid = 0;
+		entry->search.key.max_transid = UINT64_MAX;
+		entry->search.key.nr_items = 0;
+	} else {
+		entry->id = tree_id;
 
-	entry->items_pos = 0;
-	entry->buf_off = 0;
+		if (iter->search_stack_len) {
+			struct search_stack_entry *top;
+			enum btrfs_util_error err;
+			char *path;
+			int fd;
 
-	entry->path_len = path_len;
+			top = top_search_stack_entry(iter);
+			path = &iter->cur_path[top->path_len];
+			if (*path == '/')
+				path++;
+			fd = openat(iter->cur_fd, path, O_RDONLY);
+			if (fd == -1)
+				return BTRFS_UTIL_ERROR_OPEN_FAILED;
+
+			err = check_expected_subvolume(fd, iter->cur_fd,
+						       tree_id);
+			if (err) {
+				close(fd);
+				return err;
+			}
+
+			close(iter->cur_fd);
+			iter->cur_fd = fd;
+		}
+	}
+
+	iter->search_stack_len++;
+
+	return BTRFS_UTIL_OK;
+}
+
+static enum btrfs_util_error pop_search_stack(struct btrfs_util_subvolume_iterator *iter)
+{
+	struct search_stack_entry *top, *parent;
+	int fd, parent_fd;
+	size_t i;
+
+	if (iter->use_tree_search || iter->search_stack_len == 1) {
+		iter->search_stack_len--;
+		return BTRFS_UTIL_OK;
+	}
+
+	top = top_search_stack_entry(iter);
+	iter->search_stack_len--;
+	parent = top_search_stack_entry(iter);
+
+	fd = iter->cur_fd;
+	for (i = parent->path_len; i < top->path_len; i++) {
+		if (i == 0 || iter->cur_path[i] == '/') {
+			parent_fd = openat(fd, "..", O_RDONLY);
+			if (fd != iter->cur_fd)
+				SAVE_ERRNO_AND_CLOSE(fd);
+			if (parent_fd == -1)
+				return BTRFS_UTIL_ERROR_OPEN_FAILED;
+			fd = parent_fd;
+		}
+	}
+	if (iter->cur_fd != iter->fd)
+		close(iter->cur_fd);
+	iter->cur_fd = fd;
 
 	return BTRFS_UTIL_OK;
 }
@@ -836,12 +963,14 @@ PUBLIC enum btrfs_util_error btrfs_util_create_subvolume_iterator_fd(int fd,
 {
 	struct btrfs_util_subvolume_iterator *iter;
 	enum btrfs_util_error err;
+	bool use_tree_search;
 
 	if (flags & ~BTRFS_UTIL_SUBVOLUME_ITERATOR_MASK) {
 		errno = EINVAL;
 		return BTRFS_UTIL_ERROR_INVALID_ARGUMENT;
 	}
 
+	use_tree_search = top != 0 || is_root();
 	if (top == 0) {
 		err = btrfs_util_is_subvolume_fd(fd);
 		if (err)
@@ -857,7 +986,9 @@ PUBLIC enum btrfs_util_error btrfs_util_create_subvolume_iterator_fd(int fd,
 		return BTRFS_UTIL_ERROR_NO_MEMORY;
 
 	iter->fd = fd;
+	iter->cur_fd = fd;
 	iter->flags = flags;
+	iter->use_tree_search = use_tree_search;
 
 	iter->search_stack_len = 0;
 	iter->search_stack_capacity = 4;
@@ -1166,6 +1297,8 @@ PUBLIC void btrfs_util_destroy_subvolume_iterator(struct btrfs_util_subvolume_it
 	if (iter) {
 		free(iter->cur_path);
 		free(iter->search_stack);
+		if (iter->cur_fd != iter->fd)
+			SAVE_ERRNO_AND_CLOSE(iter->cur_fd);
 		if (iter->flags & BTRFS_UTIL_SUBVOLUME_ITERATOR_CLOSE_FD)
 			SAVE_ERRNO_AND_CLOSE(iter->fd);
 		free(iter);
@@ -1177,32 +1310,14 @@ PUBLIC int btrfs_util_subvolume_iterator_fd(const struct btrfs_util_subvolume_it
 	return iter->fd;
 }
 
-static struct search_stack_entry *top_search_stack_entry(struct btrfs_util_subvolume_iterator *iter)
-{
-	return &iter->search_stack[iter->search_stack_len - 1];
-}
-
 static enum btrfs_util_error build_subvol_path(struct btrfs_util_subvolume_iterator *iter,
-					       const struct btrfs_ioctl_search_header *header,
-					       const struct btrfs_root_ref *ref,
-					       const char *name,
+					       const char *name, size_t name_len,
+					       const char *dir, size_t dir_len,
 					       size_t *path_len_ret)
 {
-	struct btrfs_ioctl_ino_lookup_args lookup = {
-		.treeid = header->objectid,
-		.objectid = le64_to_cpu(ref->dirid),
-	};
 	struct search_stack_entry *top = top_search_stack_entry(iter);
-	size_t dir_len, name_len, path_len;
+	size_t path_len;
 	char *p;
-	int ret;
-
-	ret = ioctl(iter->fd, BTRFS_IOC_INO_LOOKUP, &lookup);
-	if (ret == -1)
-		return BTRFS_UTIL_ERROR_INO_LOOKUP_FAILED;
-
-	dir_len = strlen(lookup.name);
-	name_len = le16_to_cpu(ref->name_len);
 
 	path_len = top->path_len;
 	/*
@@ -1220,33 +1335,75 @@ static enum btrfs_util_error build_subvol_path(struct btrfs_util_subvolume_itera
 		path_len++;
 	path_len += name_len;
 
-	if (path_len > iter->cur_path_capacity) {
-		char *tmp = realloc(iter->cur_path, path_len);
+	/* We need one extra character for the NUL terminator. */
+	if (path_len + 1 > iter->cur_path_capacity) {
+		char *tmp = realloc(iter->cur_path, path_len + 1);
 
 		if (!tmp)
 			return BTRFS_UTIL_ERROR_NO_MEMORY;
 		iter->cur_path = tmp;
-		iter->cur_path_capacity = path_len;
+		iter->cur_path_capacity = path_len + 1;
 	}
 
 	p = iter->cur_path + top->path_len;
 	if (top->path_len && dir_len)
 		*p++ = '/';
-	memcpy(p, lookup.name, dir_len);
+	memcpy(p, dir, dir_len);
 	p += dir_len;
 	if (top->path_len && !dir_len && name_len)
 		*p++ = '/';
 	memcpy(p, name, name_len);
 	p += name_len;
+	*p = '\0';
 
 	*path_len_ret = path_len;
 
 	return BTRFS_UTIL_OK;
 }
 
-PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct btrfs_util_subvolume_iterator *iter,
-								char **path_ret,
-								uint64_t *id_ret)
+static enum btrfs_util_error build_subvol_path_privileged(struct btrfs_util_subvolume_iterator *iter,
+							  const struct btrfs_ioctl_search_header *header,
+							  const struct btrfs_root_ref *ref,
+							  const char *name,
+							  size_t *path_len_ret)
+{
+	struct btrfs_ioctl_ino_lookup_args lookup = {
+		.treeid = header->objectid,
+		.objectid = le64_to_cpu(ref->dirid),
+	};
+	int ret;
+
+	ret = ioctl(iter->fd, BTRFS_IOC_INO_LOOKUP, &lookup);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_INO_LOOKUP_FAILED;
+
+	return build_subvol_path(iter, name, le16_to_cpu(ref->name_len),
+				 lookup.name, strlen(lookup.name),
+				 path_len_ret);
+}
+
+static enum btrfs_util_error build_subvol_path_unprivileged(struct btrfs_util_subvolume_iterator *iter,
+							    uint64_t treeid,
+							    uint64_t dirid,
+							    size_t *path_len_ret)
+{
+	struct btrfs_ioctl_ino_lookup_user_args args = {
+		.treeid = treeid,
+		.dirid = dirid,
+	};
+	int ret;
+
+	ret = ioctl(iter->cur_fd, BTRFS_IOC_INO_LOOKUP_USER, &args);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_INO_LOOKUP_USER_FAILED;
+
+	return build_subvol_path(iter, args.name, strlen(args.name),
+				 args.path, strlen(args.path), path_len_ret);
+}
+
+static enum btrfs_util_error subvolume_iterator_next_tree_search(struct btrfs_util_subvolume_iterator *iter,
+								 char **path_ret,
+								 uint64_t *id_ret)
 {
 	struct search_stack_entry *top;
 	const struct btrfs_ioctl_search_header *header;
@@ -1273,7 +1430,10 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct btrfs_uti
 				top->buf_off = 0;
 
 				if (top->search.key.nr_items == 0) {
-					iter->search_stack_len--;
+					/*
+					 * This never fails for use_tree_search.
+					 */
+					pop_search_stack(iter);
 					if ((iter->flags & BTRFS_UTIL_SUBVOLUME_ITERATOR_POST_ORDER) &&
 					    iter->search_stack_len)
 						goto out;
@@ -1293,7 +1453,8 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct btrfs_uti
 
 		ref = (struct btrfs_root_ref *)(header + 1);
 		name = (const char *)(ref + 1);
-		err = build_subvol_path(iter, header, ref, name, &path_len);
+		err = build_subvol_path_privileged(iter, header, ref, name,
+						   &path_len);
 		if (err)
 			return err;
 
@@ -1320,6 +1481,100 @@ out:
 	return BTRFS_UTIL_OK;
 }
 
+static enum btrfs_util_error subvolume_iterator_next_unprivileged(struct btrfs_util_subvolume_iterator *iter,
+								  char **path_ret,
+								  uint64_t *id_ret)
+{
+	struct search_stack_entry *top;
+	uint64_t treeid, dirid;
+	enum btrfs_util_error err;
+	size_t path_len;
+	int ret;
+
+	for (;;) {
+		for (;;) {
+			if (iter->search_stack_len == 0)
+				return BTRFS_UTIL_ERROR_STOP_ITERATION;
+
+			top = top_search_stack_entry(iter);
+			if (top->items_pos < top->rootref_args.num_items) {
+				break;
+			} else {
+				ret = ioctl(iter->cur_fd,
+					    BTRFS_IOC_GET_SUBVOL_ROOTREF,
+					    &top->rootref_args);
+				if (ret == -1 && errno != EOVERFLOW)
+					return BTRFS_UTIL_ERROR_GET_SUBVOL_ROOTREF_FAILED;
+				top->items_pos = 0;
+
+				if (top->rootref_args.num_items == 0) {
+					err = pop_search_stack(iter);
+					if (err)
+						return err;
+					if ((iter->flags & BTRFS_UTIL_SUBVOLUME_ITERATOR_POST_ORDER) &&
+					    iter->search_stack_len)
+						goto out;
+				}
+			}
+		}
+
+		treeid = top->rootref_args.rootref[top->items_pos].treeid;
+		dirid = top->rootref_args.rootref[top->items_pos].dirid;
+		top->items_pos++;
+		err = build_subvol_path_unprivileged(iter, treeid, dirid,
+						     &path_len);
+		if (err) {
+			/* Skip the subvolume if we can't access it. */
+			if (errno == EACCES)
+				continue;
+			return err;
+		}
+
+		err = append_to_search_stack(iter, treeid, path_len);
+		if (err) {
+			/*
+			 * Skip the subvolume if it does not exist (which can
+			 * happen if there is another filesystem mounted over a
+			 * parent directory) or we don't have permission to
+			 * access it.
+			 */
+			if (errno == ENOENT || errno == EACCES)
+				continue;
+			return err;
+		}
+
+		if (!(iter->flags & BTRFS_UTIL_SUBVOLUME_ITERATOR_POST_ORDER)) {
+			top = top_search_stack_entry(iter);
+			goto out;
+		}
+	}
+
+out:
+	if (path_ret) {
+		*path_ret = malloc(top->path_len + 1);
+		if (!*path_ret)
+			return BTRFS_UTIL_ERROR_NO_MEMORY;
+		memcpy(*path_ret, iter->cur_path, top->path_len);
+		(*path_ret)[top->path_len] = '\0';
+	}
+	if (id_ret)
+		*id_ret = top->id;
+	return BTRFS_UTIL_OK;
+}
+
+PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct btrfs_util_subvolume_iterator *iter,
+								char **path_ret,
+								uint64_t *id_ret)
+{
+	if (iter->use_tree_search) {
+		return subvolume_iterator_next_tree_search(iter, path_ret,
+							   id_ret);
+	} else {
+		return subvolume_iterator_next_unprivileged(iter, path_ret,
+							    id_ret);
+	}
+}
+
 PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next_info(struct btrfs_util_subvolume_iterator *iter,
 								     char **path_ret,
 								     struct btrfs_util_subvolume_info *subvol)
@@ -1331,7 +1586,10 @@ PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next_info(struct btrf
 	if (err)
 		return err;
 
-	return btrfs_util_subvolume_info_fd(iter->fd, id, subvol);
+	if (iter->use_tree_search)
+		return btrfs_util_subvolume_info_fd(iter->fd, id, subvol);
+	else
+		return btrfs_util_subvolume_info_fd(iter->cur_fd, 0, subvol);
 }
 
 PUBLIC enum btrfs_util_error btrfs_util_deleted_subvolumes(const char *path,
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/10] libbtrfsutil: bump version to 1.1.0
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (7 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 08/10] libbtrfsutil: relax the privileges of subvolume iterator Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-14  7:47 ` [PATCH 10/10] libbtrfsutil: document API in README Omar Sandoval
  2018-11-26 16:18 ` [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue David Sterba
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

With the previous few fixes and features, we should bump the minor
version.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/btrfsutil.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index d88c39e5..ad4f043e 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -26,7 +26,7 @@
 #include <sys/time.h>
 
 #define BTRFS_UTIL_VERSION_MAJOR 1
-#define BTRFS_UTIL_VERSION_MINOR 0
+#define BTRFS_UTIL_VERSION_MINOR 1
 #define BTRFS_UTIL_VERSION_PATCH 0
 
 #ifdef __cplusplus
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/10] libbtrfsutil: document API in README
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (8 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 09/10] libbtrfsutil: bump version to 1.1.0 Omar Sandoval
@ 2018-11-14  7:47 ` Omar Sandoval
  2018-11-26 16:18 ` [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue David Sterba
  10 siblings, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-14  7:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: kernel-team, Misono Tomohiro

From: Omar Sandoval <osandov@fb.com>

btrfsutil.h and the Python docstrings are thorough, but I've gotten a
couple of requests for a high-level overview of the available interfaces
and example usages. Add them to README.md.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/README.md | 422 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 421 insertions(+), 1 deletion(-)

diff --git a/libbtrfsutil/README.md b/libbtrfsutil/README.md
index 0c8eba44..30ae39b6 100644
--- a/libbtrfsutil/README.md
+++ b/libbtrfsutil/README.md
@@ -6,6 +6,425 @@ the LGPL. libbtrfsutil provides interfaces for a subset of the operations
 offered by the `btrfs` command line utility. It also includes official Python
 bindings (Python 3 only).
 
+API Overview
+------------
+
+This section provides an overview of the interfaces available in libbtrfsutil
+as well as example usages. Detailed documentation for the C API can be found in
+[`btrfsutil.h`](btrfsutil.h). Detailed documentation for the Python bindings is
+available with `pydoc3 btrfsutil` or in the interpreter:
+
+```
+>>> import btrfsutil
+>>> help(btrfsutil)
+```
+
+Many functions in the C API have a variant taking a path and a variant taking a
+file descriptor. The latter has the same name as the former with an `_fd`
+suffix. The Python bindings for these functions can take a path, a file object,
+or a file descriptor.
+
+Error handling is omitted from most of these examples for brevity. Please
+handle errors in production code.
+
+### Error Handling
+
+In the C API, all functions that can return an error return an `enum
+btrfs_util_error` and set `errno`. `BTRFS_UTIL_OK` (zero) is returned on
+success. `btrfs_util_strerror()` converts an error code to a string
+description suitable for human-friendly error reporting.
+
+```c
+enum btrfs_util_err err;
+
+err = btrfs_util_sync("/");
+if (err)
+	fprintf("stderr, %s: %m\n", btrfs_util_strerror(err));
+```
+
+In the Python bindings, functions may raise a `BtrfsUtilError`, which is a
+subclass of `OSError` with an added `btrfsutilerror` error code member. Error
+codes are available as `ERROR_*` constants.
+
+```python
+try:
+    btrfsutil.sync('/')
+except btrfsutil.BtrfsUtilError as e:
+    print(e, file=sys.stderr)
+```
+
+### Filesystem Operations
+
+There are several operations which act on the entire filesystem.
+
+#### Sync
+
+Btrfs can commit all caches for a specific filesystem to disk.
+
+`btrfs_util_sync()` forces a sync on the filesystem containing the given file
+and waits for it to complete.
+
+`btrfs_wait_sync()` waits for a previously started transaction to complete. The
+transaction is specified by ID, which may be zero to indicate the current
+transaction.
+
+`btrfs_start_sync()` asynchronously starts a sync and returns a transaction ID
+which can then be passed to `btrfs_wait_sync()`.
+
+```c
+uint64_t transid;
+btrfs_util_sync("/");
+btrfs_util_start_sync("/", &transid);
+btrfs_util_wait_sync("/", &transid);
+btrfs_util_wait_sync("/", 0);
+```
+
+```python
+btrfsutil.sync('/')
+transid = btrfsutil.start_sync('/')
+btrfsutil.wait_sync('/', transid)
+btrfsutil.wait_sync('/')  # equivalent to wait_sync('/', 0)
+```
+
+All of these functions have `_fd` variants.
+
+The equivalent `btrfs-progs` command is `btrfs filesystem sync`.
+
+### Subvolume Operations
+
+Functions which take a file and a subvolume ID can be used in two ways. If zero
+is given as the subvolume ID, then the given file is used as the subvolume.
+Otherwise, the given file can be any file in the filesystem, and the subvolume
+with the given ID is used.
+
+#### Subvolume Information
+
+`btrfs_util_is_subvolume()` returns whether a given file is a subvolume.
+
+`btrfs_util_subvolume_id()` returns the ID of the subvolume containing the
+given file.
+
+```c
+enum btrfs_util_error err;
+err = btrfs_util_is_subvolume("/subvol");
+if (!err)
+	printf("Subvolume\n");
+else if (err == BTRFS_UTIL_ERROR_NOT_BTRFS || err == BTRFS_UTIL_ERROR_NOT_SUBVOLUME)
+	printf("Not subvolume\n");
+uint64_t id;
+btrfs_util_subvolume_id("/subvol", &id);
+```
+
+```python
+if btrfsutil.is_subvolume('/subvol'):
+    print('Subvolume')
+else:
+    print('Not subvolume')
+id_ = btrfsutil.subvolume_id('/subvol')
+```
+
+`btrfs_util_subvolume_path()` returns the path of the subvolume with the given
+ID relative to the filesystem root. This requires `CAP_SYS_ADMIN`. The path
+must be freed with `free()`.
+
+```c
+char *path;
+btrfs_util_subvolume_path("/", 256, &path);
+free(path);
+btrfs_util_subvolume_path("/subvol", 0, &path);
+free(path);
+```
+
+```python
+path = btrfsutil.subvolume_path('/', 256)
+path = btrfsutil.subvolume_path('/subvol')  # equivalent to subvolume_path('/subvol', 0)
+```
+
+`btrfs_util_subvolume_info()` returns information (including ID, parent ID,
+UUID) about a subvolume. In the C API, this is returned as a `struct
+btrfs_util_subvolume_info`. The Python bindings use a `SubvolumeInfo` object.
+
+This requires `CAP_SYS_ADMIN` unless the given subvolume ID is zero and the
+kernel supports the `BTRFS_IOC_GET_SUBVOL_INFO` ioctl (added in 4.18).
+
+The equivalent `btrfs-progs` command is `btrfs subvolume show`.
+
+```c
+struct btrfs_util_subvolume_info info;
+btrfs_util_subvolume_info("/", 256, &info);
+btrfs_util_subvolume_info("/subvol", 0, &info);
+```
+
+```python
+info = btrfsutil.subvolume_info('/', 256)
+info = btrfsutil.subvolume_info('/subvol')  # equivalent to subvolume_info('/subvol', 0)
+```
+
+All of these functions have `_fd` variants.
+
+#### Enumeration
+
+An iterator interface is provided for enumerating subvolumes on a filesystem.
+In the C API, a `struct btrfs_util_subvolume_iterator` is initialized by
+`btrfs_util_create_subvolume_iterator()`, which takes a top subvolume to
+enumerate under and flags. Currently, the only flag is to specify post-order
+traversal instead of the default pre-order. This function has an `_fd` variant.
+
+`btrfs_util_destroy_subvolume_iterator()` must be called to free a previously
+created `struct btrfs_util_subvolume_iterator`.
+
+`btrfs_util_subvolume_iterator_fd()` returns the file descriptor opened by
+`btrfs_util_create_subvolume_iterator()` which can be used for other functions.
+
+`btrfs_util_subvolume_iterator_next()` returns the path (relative to the top
+subvolume that the iterator was created with) and ID of the next subvolume.
+`btrfs_util_subvolume_iterator_next_info()` returns a `struct
+btrfs_subvolume_info` instead of the ID. It is slightly more efficient than
+doing separate `btrfs_util_subvolume_iterator_next()` and
+`btrfs_util_subvolume_info()` calls if the subvolume information is needed. The
+path returned by these functions must be freed with `free()`. When there are no
+more subvolumes, they return `BTRFS_UTIL_ERROR_STOP_ITERATION`.
+
+```c
+struct btrfs_util_subvolume_iterator *iter;
+enum btrfs_util_error err;
+char *path;
+uint64_t id;
+struct btrfs_util_subvolume_info info;
+
+btrfs_util_create_subvolume_iterator("/", 256, 0, &iter);
+/*
+ * This is just an example use-case for btrfs_util_subvolume_iterator_fd(). It
+ * is not necessary.
+ */
+btrfs_util_sync_fd(btrfs_util_subvolume_iterator_fd(iter));
+while (!(err = btrfs_util_subvolume_iterator_next(iter, &path, &id))) {
+	printf("%" PRIu64 " %s\n", id, path);
+	free(path);
+}
+btrfs_util_destroy_subvolume_iterator(iter);
+
+btrfs_util_create_subvolume_iterator("/subvol", 0,
+				     BTRFS_UTIL_SUBVOLUME_ITERATOR_POST_ORDER,
+				     &iter);
+while (!(err = btrfs_util_subvolume_iterator_next_info(iter, &path, &info))) {
+	printf("%" PRIu64 " %" PRIu64 " %s\n", info.id, info.parent_id, path);
+	free(path);
+}
+btrfs_util_destroy_subvolume_iterator(iter);
+```
+
+The Python bindings provide this interface as an iterable `SubvolumeIterator`
+class. It should be used as a context manager to ensure that the underlying
+file descriptor is closed. Alternatively, it has a `close()` method for closing
+explicitly. It also has a `fileno()` method to get the underlying file
+descriptor.
+
+```python
+with btrfsutil.SubvolumeIterator('/', 256) as it:
+    # This is just an example use-case for fileno(). It is not necessary.
+    btrfsutil.sync(it.fileno())
+    for path, id_ in it:
+        print(id_, path)
+
+it = btrfsutil.SubvolumeIterator('/subvol', info=True, post_order=True)
+try:
+    for path, info in it:
+        print(info.id, info.parent_id, path)
+finally:
+    it.close()
+```
+
+This interface requires `CAP_SYS_ADMIN` unless the given top subvolume ID is
+zero and the kernel supports the `BTRFS_IOC_GET_SUBVOL_ROOTREF` and
+`BTRFS_IOC_INO_LOOKUP_USER` ioctls (added in 4.18). In the unprivileged case,
+subvolumes which cannot be accessed are skipped.
+
+The equivalent `btrfs-progs` command is `btrfs subvolume list`.
+
+#### Creation
+
+`btrfs_util_create_subvolume()` creates a new subvolume at the given path. The
+subvolume can be created asynchronously and inherit from quota groups
+(qgroups).
+
+Qgroups to inherit are specified with a `struct btrfs_util_qgroup_inherit`,
+which is created by `btrfs_util_create_qgroup_inherit()` and freed by
+`btrfs_util_destroy_qgroup_inherit()`. Qgroups are added with
+`btrfs_util_qgroup_inherit_add_group()`. The list of added groups can be
+retrieved with `btrfs_util_qgroup_inherit_get_groups()`; note that the returned
+array does not need to be freed and is no longer valid when the `struct
+btrfs_util_qgroup_inherit` is freed.
+
+The Python bindings provide a `QgroupInherit` class. It has an `add_group()`
+method and a `groups` member, which is a list of ints.
+
+```c
+btrfs_util_create_subvolume("/subvol2", 0, NULL, NULL);
+
+uint64_t async_transid;
+btrfs_util_create_subvolume("/subvol2", 0, &async_transid, NULL);
+btrfs_util_wait_sync("/", async_transid);
+
+struct btrfs_util_qgroup_inherit *qgroups;
+btrfs_util_create_qgroup_inherit(0, &qgroups);
+btrfs_util_qgroup_inherit_add_group(&qgroups, 256);
+btrfs_util_create_subvolume("/subvol2", 0, NULL, qgroups);
+btrfs_util_destroy_qgroup_inherit(qgroups);
+```
+
+```python
+btrfsutil.create_subvolume('/subvol2')
+
+async_transid = btrfsutil.create_subvolume('/subvol2', async_=True)
+btrfsutil.wait_sync('/', async_transid)
+
+qgroups = btrfsutil.QgroupInherit()
+qgroups.add_group(256)
+btrfsutil.create_subvolume('/subvol2', qgroup_inherit=qgroups)
+```
+
+The C API has an `_fd` variant which takes a name and a file descriptor
+referring to the parent directory.
+
+The equivalent `btrfs-progs` command is `btrfs subvolume create`.
+
+#### Snapshotting
+
+Snapshots are created with `btrfs_util_create_snapshot()`, which takes a source
+path, a destination path, and flags. It can also be asynchronous and inherit
+from quota groups; see [subvolume creation](#Creation).
+
+Snapshot creation can be recursive, in which case subvolumes underneath the
+subvolume being snapshotted will also be snapshotted onto the same location in
+the new snapshot (note that this is implemented in userspace non-atomically and
+has the same capability requirements as a [subvolume iterator](#Enumeration)).
+The newly created snapshot can also be read-only, but not if doing a recursive
+snapshot. 
+
+```c
+btrfs_util_create_snapshot("/subvol", "/snapshot", 0, NULL, NULL);
+btrfs_util_create_snapshot("/nested_subvol", "/nested_snapshot",
+			   BTRFS_UTIL_CREATE_SNAPSHOT_RECURSIVE, NULL, NULL);
+btrfs_util_create_snapshot("/subvol", "/rosnapshot",
+			   BTRFS_UTIL_CREATE_SNAPSHOT_READ_ONLY, NULL, NULL);
+```
+
+```python
+btrfsutil.create_snapshot('/subvol', '/snapshot')
+btrfsutil.create_snapshot('/nested_subvol', '/nested_snapshot', recursive=True)
+btrfsutil.create_snapshot('/subvol', '/rosnapshot', read_only=True)
+```
+
+The C API has two `_fd` variants. `btrfs_util_create_snapshot_fd()` takes the
+source subvolume as a file descriptor. `btrfs_util_create_snapshot_fd2()` takes
+the source subvolume as a file descriptor and the destination as a name and
+parent file descriptor.
+
+The equivalent `btrfs-progs` command is `btrfs subvolume snapshot`.
+
+#### Deletion
+
+`btrfs_util_delete_subvolume()` takes a subvolume to delete and flags. This
+requires `CAP_SYS_ADMIN` if the filesystem was not mounted with
+`user_subvol_rm_allowed`. Deletion may be recursive, in which case all
+subvolumes beneath the given subvolume are deleted before the given subvolume
+is deleted. This is implemented in user-space non-atomically and has the same
+capability requirements as a [subvolume iterator](#Enumeration).
+
+```c
+btrfs_util_delete_subvolume("/subvol", 0);
+btrfs_util_delete_subvolume("/nested_subvol",
+			    BTRFS_UTIL_DELETE_SUBVOLUME_RECURSIVE);
+```
+
+```python
+btrfsutil.delete_subvolume('/subvol')
+btrfsutil.delete_subvolume('/nested_subvol', recursive=True)
+```
+
+The C API has an `_fd` variant which takes a name and a file descriptor
+referring to the parent directory.
+
+The equivalent `btrfs-progs` command is `btrfs subvolume delete`.
+
+#### Deleted Subvolumes
+
+Btrfs lazily cleans up deleted subvolumes. `btrfs_util_deleted_subvolumes()`
+returns an array of subvolume IDs which have been deleted but not yet cleaned
+up. The returned array should be freed with `free()`.
+```c
+uint64_t *ids;
+size_t n; /* Number of returned IDs. */
+btrfs_util_deleted_subvolumes("/", &ids, &n);
+free(ids);
+```
+
+The Python binding returns a list of ints.
+
+```python
+ids = btrfsutil.deleted_subvolumes('/')
+```
+
+This function also has an `_fd` variant. It requires `CAP_SYS_ADMIN`.
+
+The closest `btrfs-progs` command is `btrfs subvolume sync`, which waits for
+deleted subvolumes to be cleaned up.
+
+#### Read-Only Flag
+
+Subvolumes can be set to read-only. `btrfs_util_get_subvolume_read_only()`
+returns whether a subvolume is read-only.
+`btrfs_util_set_subvolume_read_only()` sets the read-only flag to the desired
+value.
+
+```c
+bool read_only;
+btrfs_util_get_subvolume_read_only("/subvol", &read_only);
+btrfs_util_set_subvolume_read_only("/subvol", true);
+btrfs_util_set_subvolume_read_only("/subvol", false);
+```
+
+```python
+read_only = btrfsutil.get_subvolume_read_only('/subvol')
+btrfsutil.set_subvolume_read_only('/subvol', True)
+btrfsutil.set_subvolume_read_only('/subvol', False)
+```
+
+Both of these functions have `_fd` variants.
+
+The equivalent `btrfs-progs` commands are `btrfs property get` and `btrfs
+property set` with the `ro` property.
+
+#### Default Subvolume
+
+The default subvolume of a filesystem is the subvolume which is mounted when no
+`subvol` or `subvolid` mount option is passed.
+
+`btrfs_util_get_default_subvolume()` gets the ID of the default subvolume for
+the filesystem containing the given file.
+
+`btrfs_util_set_default_subvolume()` sets the default subvolume.
+
+```c
+uint64_t id;
+btrfs_util_get_default_subvolume("/", &id);
+btrfs_util_set_default_subvolume("/", 256);
+btrfs_util_set_default_subvolume("/subvol", 0);
+```
+
+```python
+id = btrfsutil.get_default_subvolume('/')
+btrfsutil.set_default_subvolume('/', 256)
+btrfsutil.set_default_subvolume('/subvol')  # equivalent to set_default_subvolume('/subvol', 0)
+```
+
+Both of these functions have an `_fd` variant. They both require
+`CAP_SYS_ADMIN`.
+
+The equivalent `btrfs-progs` commands are `btrfs subvolume get-default` and
+`btrfs subvolume set-default`.
+
 Development
 -----------
 
@@ -24,7 +443,8 @@ release of btrfs-progs).
 
 A few guidelines:
 
-* All interfaces must be documented in `btrfsutil.h` using the kernel-doc style
+* All interfaces must be documented in this README and in `btrfsutil.h` using
+  the kernel-doc style
 * Error codes should be specific about what _exactly_ failed
 * Functions should have a path and an fd variant whenever possible
 * Spell out terms in function names, etc. rather than abbreviating whenever
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
  2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
                   ` (9 preceding siblings ...)
  2018-11-14  7:47 ` [PATCH 10/10] libbtrfsutil: document API in README Omar Sandoval
@ 2018-11-26 16:18 ` David Sterba
  2018-11-26 17:15   ` Omar Sandoval
  2018-11-27  2:51   ` misono.tomohiro
  10 siblings, 2 replies; 14+ messages in thread
From: David Sterba @ 2018-11-26 16:18 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: linux-btrfs, kernel-team, Misono Tomohiro

On Tue, Nov 13, 2018 at 11:46:55PM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Hi,
> 
> This series contains my backlog of libbtrfsutil changes which I've been
> collecting over the past few weeks.
> 
> Patches 1-4 are fixes. Patches 5-6 add functionality to the unit tests
> which is needed for patches 7-8. Patches 7-8 add support for the
> unprivileged ioctls added in Linux 4.18; more on those below. Patch 9
> bumps the library version. Patch 10 adds documentation for the available
> API along with examples.
> 
> Patches 7-8 are based on Misono Tomohiro's previous patch series [1],
> with a few important changes.
> 
> - Both subvolume_info() and create_subvolume_iterator() now have unit
>   tests for the unprivileged case.
> - Both no longer explicitly check that top == 0 in the unprivileged
>   case, since that will already fail with a clear permission error.
> - Unprivileged iteration is much simpler: it uses openat() instead of
>   fchdir() and is based more closely on the original tree search
>   variant. This fixes a bug in post-order iteration in Misono's version.
> - Unprivileged iteration does _not_ support passing in a non-subvolume
>   path; if this behavior is desired, I'd like it to be a separate change
>   with an explicit flag.

Series merged to devel, thanks. I've added link from the main README now
that there's the API documentation.

The test-libbtrfsutil is missing from the travis CI for some reason, I
was about to add it.  So far the testing environment does not provide
'umount' that knows about '-R' so the tests fail. I'll have a look if
there's a newer base image provided, otherwise a workaround would be
necessary.

As for the unprivileged subvolume listing ioctls, the functionality in
the util library is self-contained and the interface is up to you to
design properly, so this does not depend on the 'btrfs subvolume list'
command. That one has unfortunately not bubbled high enough in my todo.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
  2018-11-26 16:18 ` [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue David Sterba
@ 2018-11-26 17:15   ` Omar Sandoval
  2018-11-27  2:51   ` misono.tomohiro
  1 sibling, 0 replies; 14+ messages in thread
From: Omar Sandoval @ 2018-11-26 17:15 UTC (permalink / raw)
  To: dsterba, linux-btrfs, kernel-team, Misono Tomohiro

On Mon, Nov 26, 2018 at 05:18:12PM +0100, David Sterba wrote:
> On Tue, Nov 13, 2018 at 11:46:55PM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Hi,
> > 
> > This series contains my backlog of libbtrfsutil changes which I've been
> > collecting over the past few weeks.
> > 
> > Patches 1-4 are fixes. Patches 5-6 add functionality to the unit tests
> > which is needed for patches 7-8. Patches 7-8 add support for the
> > unprivileged ioctls added in Linux 4.18; more on those below. Patch 9
> > bumps the library version. Patch 10 adds documentation for the available
> > API along with examples.
> > 
> > Patches 7-8 are based on Misono Tomohiro's previous patch series [1],
> > with a few important changes.
> > 
> > - Both subvolume_info() and create_subvolume_iterator() now have unit
> >   tests for the unprivileged case.
> > - Both no longer explicitly check that top == 0 in the unprivileged
> >   case, since that will already fail with a clear permission error.
> > - Unprivileged iteration is much simpler: it uses openat() instead of
> >   fchdir() and is based more closely on the original tree search
> >   variant. This fixes a bug in post-order iteration in Misono's version.
> > - Unprivileged iteration does _not_ support passing in a non-subvolume
> >   path; if this behavior is desired, I'd like it to be a separate change
> >   with an explicit flag.
> 
> Series merged to devel, thanks.

Thanks!

> I've added link from the main README now
> that there's the API documentation.

Ah, great idea.

> The test-libbtrfsutil is missing from the travis CI for some reason, I
> was about to add it.  So far the testing environment does not provide
> 'umount' that knows about '-R' so the tests fail. I'll have a look if
> there's a newer base image provided, otherwise a workaround would be
> necessary.

It looks like it was added to util-linux in v2.23 back in 2013. Or maybe
the base image uses busybox? I believe that umount from busybox doesn't
have -R.

> As for the unprivileged subvolume listing ioctls, the functionality in
> the util library is self-contained and the interface is up to you to
> design properly, so this does not depend on the 'btrfs subvolume list'
> command. That one has unfortunately not bubbled high enough in my todo.

That comment is mostly for Misono, since the original version had that
functionality, probably for the subvolume list command.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
  2018-11-26 16:18 ` [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue David Sterba
  2018-11-26 17:15   ` Omar Sandoval
@ 2018-11-27  2:51   ` misono.tomohiro
  1 sibling, 0 replies; 14+ messages in thread
From: misono.tomohiro @ 2018-11-27  2:51 UTC (permalink / raw)
  To: 'dsterba@suse.cz', Omar Sandoval; +Cc: linux-btrfs, kernel-team

> -----Original Message-----
> From: David Sterba [mailto:dsterba@suse.cz]
> Sent: Tuesday, November 27, 2018 1:18 AM
> To: Omar Sandoval <osandov@osandov.com>
> Cc: linux-btrfs@vger.kernel.org; kernel-team@fb.com; Misono, Tomohiro
> <misono.tomohiro@fujitsu.com>
> Subject: Re: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
> 
> On Tue, Nov 13, 2018 at 11:46:55PM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > Hi,
> >
> > This series contains my backlog of libbtrfsutil changes which I've been
> > collecting over the past few weeks.
> >
> > Patches 1-4 are fixes. Patches 5-6 add functionality to the unit tests
> > which is needed for patches 7-8. Patches 7-8 add support for the
> > unprivileged ioctls added in Linux 4.18; more on those below. Patch
> 9
> > bumps the library version. Patch 10 adds documentation for the available
> > API along with examples.
> >
> > Patches 7-8 are based on Misono Tomohiro's previous patch series [1],
> > with a few important changes.
> >
> > - Both subvolume_info() and create_subvolume_iterator() now have unit
> >   tests for the unprivileged case.
> > - Both no longer explicitly check that top == 0 in the unprivileged
> >   case, since that will already fail with a clear permission error.
> > - Unprivileged iteration is much simpler: it uses openat() instead of
> >   fchdir() and is based more closely on the original tree search
> >   variant. This fixes a bug in post-order iteration in Misono's version.
> > - Unprivileged iteration does _not_ support passing in a non-subvolume
> >   path; if this behavior is desired, I'd like it to be a separate change
> >   with an explicit flag.
> 
> Series merged to devel, thanks. I've added link from the main README now
> that there's the API documentation.
> 
> The test-libbtrfsutil is missing from the travis CI for some reason, I
> was about to add it.  So far the testing environment does not provide
> 'umount' that knows about '-R' so the tests fail. I'll have a look if
> there's a newer base image provided, otherwise a workaround would be
> necessary.
> 
> As for the unprivileged subvolume listing ioctls, the functionality in
> the util library is self-contained and the interface is up to you to
> design properly, so this does not depend on the 'btrfs subvolume list'
> command. That one has unfortunately not bubbled high enough in my todo.

Hello,
I missed the mails and am sorry for late response.

As mentioned libbtrfsuitl and other progs are mostly independent,
the patches I submitted (once in devel with your review and some modification)
can be cleanly applied to this version of libbtrfsutil.

I will resend them for easier review/comment.

Thanks,
Misono

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-11-27  3:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14  7:46 [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue Omar Sandoval
2018-11-14  7:46 ` [PATCH 01/10] libbtrfsutil: use top=0 as default for SubvolumeIterator() Omar Sandoval
2018-11-14  7:46 ` [PATCH 02/10] libbtrfsutil: change async parameters to async_ in Python bindings Omar Sandoval
2018-11-14  7:46 ` [PATCH 03/10] libbtrfsutil: document qgroup_inherit parameter " Omar Sandoval
2018-11-14  7:46 ` [PATCH 04/10] libbtrfsutil: use SubvolumeIterator as context manager in tests Omar Sandoval
2018-11-14  7:47 ` [PATCH 05/10] libbtrfsutil: add test helpers for dropping privileges Omar Sandoval
2018-11-14  7:47 ` [PATCH 06/10] libbtrfsutil: allow tests to create multiple Btrfs instances Omar Sandoval
2018-11-14  7:47 ` [PATCH 07/10] libbtrfsutil: relax the privileges of subvolume_info() Omar Sandoval
2018-11-14  7:47 ` [PATCH 08/10] libbtrfsutil: relax the privileges of subvolume iterator Omar Sandoval
2018-11-14  7:47 ` [PATCH 09/10] libbtrfsutil: bump version to 1.1.0 Omar Sandoval
2018-11-14  7:47 ` [PATCH 10/10] libbtrfsutil: document API in README Omar Sandoval
2018-11-26 16:18 ` [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue David Sterba
2018-11-26 17:15   ` Omar Sandoval
2018-11-27  2:51   ` misono.tomohiro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.