* [PATCH 1/5] libbtrfsutil: don't return free space cache inodes from deleted_subvolumes()
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
@ 2018-03-29 7:53 ` Omar Sandoval
2018-03-29 7:53 ` [PATCH 2/5] libbtrfsutil: fix memory leak in deleted_subvolumes() Omar Sandoval
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-03-29 7:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Deleted free space cache inodes also get an orphan item in the root
tree, but we shouldn't report those as deleted subvolumes. Deleted
subvolumes will still have the root item, so we can just do an extra
tree search.
Reported-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
libbtrfsutil/subvolume.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index d9728281..d6c0ced8 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1339,21 +1339,31 @@ PUBLIC enum btrfs_util_error btrfs_util_deleted_subvolumes_fd(int fd,
}
header = (struct btrfs_ioctl_search_header *)(search.buf + buf_off);
- if (*n >= capacity) {
- size_t new_capacity = capacity ? capacity * 2 : 1;
- uint64_t *new_ids;
- new_ids = reallocarray(*ids, new_capacity,
- sizeof(**ids));
- if (!new_ids)
- return BTRFS_UTIL_ERROR_NO_MEMORY;
-
- *ids = new_ids;
- capacity = new_capacity;
+ /*
+ * The orphan item might be for a free space cache inode, so
+ * check if there's a matching root item.
+ */
+ err = btrfs_util_subvolume_info_fd(fd, header->offset, NULL);
+ if (!err) {
+ if (*n >= capacity) {
+ size_t new_capacity;
+ uint64_t *new_ids;
+
+ new_capacity = capacity ? capacity * 2 : 1;
+ new_ids = reallocarray(*ids, new_capacity,
+ sizeof(**ids));
+ if (!new_ids)
+ return BTRFS_UTIL_ERROR_NO_MEMORY;
+
+ *ids = new_ids;
+ capacity = new_capacity;
+ }
+ (*ids)[(*n)++] = header->offset;
+ } else if (err != BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND) {
+ goto out;
}
- (*ids)[(*n)++] = header->offset;
-
items_pos++;
buf_off += sizeof(*header) + header->len;
search.key.min_offset = header->offset + 1;
--
2.16.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/5] libbtrfsutil: fix memory leak in deleted_subvolumes()
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
2018-03-29 7:53 ` [PATCH 1/5] libbtrfsutil: don't return free space cache inodes from deleted_subvolumes() Omar Sandoval
@ 2018-03-29 7:53 ` Omar Sandoval
2018-03-29 7:53 ` [PATCH 3/5] libbtrfsutil: use local mkfs.btrfs for tests if it exists Omar Sandoval
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-03-29 7:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
If we fail to reallocate the ID array, we still need to free it.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
libbtrfsutil/subvolume.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index d6c0ced8..867b3e10 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1353,8 +1353,10 @@ PUBLIC enum btrfs_util_error btrfs_util_deleted_subvolumes_fd(int fd,
new_capacity = capacity ? capacity * 2 : 1;
new_ids = reallocarray(*ids, new_capacity,
sizeof(**ids));
- if (!new_ids)
- return BTRFS_UTIL_ERROR_NO_MEMORY;
+ if (!new_ids) {
+ err = BTRFS_UTIL_ERROR_NO_MEMORY;
+ goto out;
+ }
*ids = new_ids;
capacity = new_capacity;
--
2.16.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/5] libbtrfsutil: use local mkfs.btrfs for tests if it exists
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
2018-03-29 7:53 ` [PATCH 1/5] libbtrfsutil: don't return free space cache inodes from deleted_subvolumes() Omar Sandoval
2018-03-29 7:53 ` [PATCH 2/5] libbtrfsutil: fix memory leak in deleted_subvolumes() Omar Sandoval
@ 2018-03-29 7:53 ` Omar Sandoval
2018-03-29 7:53 ` [PATCH 4/5] libbtrfsutil: always build libbtrfsutil.so.$MAJOR Omar Sandoval
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-03-29 7:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
The system might not have mkfs installed at all.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Makefile | 2 +-
libbtrfsutil/python/tests/__init__.py | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 8afc8f6e..699f864d 100644
--- a/Makefile
+++ b/Makefile
@@ -357,7 +357,7 @@ testsuite: btrfs-corrupt-block fssum
$(Q)cd tests && ./export-testsuite.sh
ifeq ($(PYTHON_BINDINGS),1)
-test-libbtrfsutil: libbtrfsutil_python
+test-libbtrfsutil: libbtrfsutil_python mkfs.btrfs
$(Q)cd libbtrfsutil/python; \
LD_LIBRARY_PATH=../.. $(PYTHON) -m unittest discover -v tests
diff --git a/libbtrfsutil/python/tests/__init__.py b/libbtrfsutil/python/tests/__init__.py
index d2c6ff28..35550e0a 100644
--- a/libbtrfsutil/python/tests/__init__.py
+++ b/libbtrfsutil/python/tests/__init__.py
@@ -37,8 +37,12 @@ class BtrfsTestCase(unittest.TestCase):
os.rmdir(self.mountpoint)
raise e
+ if os.path.exists('../../mkfs.btrfs'):
+ mkfs = '../../mkfs.btrfs'
+ else:
+ mkfs = 'mkfs.btrfs'
try:
- subprocess.check_call(['mkfs.btrfs', '-q', self.image])
+ subprocess.check_call([mkfs, '-q', self.image])
subprocess.check_call(['mount', '-o', 'loop', '--', self.image, self.mountpoint])
except Exception as e:
os.remove(self.image)
--
2.16.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/5] libbtrfsutil: always build libbtrfsutil.so.$MAJOR
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
` (2 preceding siblings ...)
2018-03-29 7:53 ` [PATCH 3/5] libbtrfsutil: use local mkfs.btrfs for tests if it exists Omar Sandoval
@ 2018-03-29 7:53 ` Omar Sandoval
2018-03-29 7:53 ` [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume Omar Sandoval
2018-03-30 21:58 ` [PATCH 0/5] libbtrfsutil: misc fixes David Sterba
5 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-03-29 7:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Otherwise, make test-libbtrfsutil from a fresh checkout fails.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 699f864d..fbd6677a 100644
--- a/Makefile
+++ b/Makefile
@@ -417,7 +417,7 @@ libbtrfsutil.so.$(libbtrfsutil_major) libbtrfsutil.so: libbtrfsutil.so.$(libbtrf
$(Q)$(LN_S) -f $< $@
ifeq ($(PYTHON_BINDINGS),1)
-libbtrfsutil_python: libbtrfsutil.so libbtrfsutil/btrfsutil.h
+libbtrfsutil_python: libbtrfsutil.so.$(libbtrfsutil_major) libbtrfsutil.so libbtrfsutil/btrfsutil.h
@echo " [PY] libbtrfsutil"
$(Q)cd libbtrfsutil/python; \
CFLAGS= LDFLAGS= $(PYTHON) setup.py $(SETUP_PY_Q) build_ext -i build
--
2.16.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
` (3 preceding siblings ...)
2018-03-29 7:53 ` [PATCH 4/5] libbtrfsutil: always build libbtrfsutil.so.$MAJOR Omar Sandoval
@ 2018-03-29 7:53 ` Omar Sandoval
2018-03-30 7:39 ` Misono Tomohiro
2018-03-30 21:58 ` [PATCH 0/5] libbtrfsutil: misc fixes David Sterba
5 siblings, 1 reply; 8+ messages in thread
From: Omar Sandoval @ 2018-03-29 7:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Since "btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of, FS_TREE",
the top-level subvolume has a non-zero UUID, ctime, and otime. Fix the
subvolume_info() test to not check for zero.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
libbtrfsutil/python/tests/test_subvolume.py | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index a46d4a34..93396cba 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -95,7 +95,8 @@ class TestSubvolume(BtrfsTestCase):
self.assertEqual(info.parent_id, 0)
self.assertEqual(info.dir_id, 0)
self.assertEqual(info.flags, 0)
- self.assertEqual(info.uuid, bytes(16))
+ self.assertIsInstance(info.uuid, bytes)
+ self.assertEqual(len(info.uuid), 16)
self.assertEqual(info.parent_uuid, bytes(16))
self.assertEqual(info.received_uuid, bytes(16))
self.assertNotEqual(info.generation, 0)
@@ -103,8 +104,8 @@ class TestSubvolume(BtrfsTestCase):
self.assertEqual(info.otransid, 0)
self.assertEqual(info.stransid, 0)
self.assertEqual(info.rtransid, 0)
- self.assertEqual(info.ctime, 0)
- self.assertEqual(info.otime, 0)
+ self.assertIsInstance(info.ctime, float)
+ self.assertIsInstance(info.otime, float)
self.assertEqual(info.stime, 0)
self.assertEqual(info.rtime, 0)
@@ -117,6 +118,7 @@ class TestSubvolume(BtrfsTestCase):
self.assertEqual(info.dir_id, 256)
self.assertEqual(info.flags, 0)
self.assertIsInstance(info.uuid, bytes)
+ self.assertEqual(len(info.uuid), 16)
self.assertEqual(info.parent_uuid, bytes(16))
self.assertEqual(info.received_uuid, bytes(16))
self.assertNotEqual(info.generation, 0)
--
2.16.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume
2018-03-29 7:53 ` [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume Omar Sandoval
@ 2018-03-30 7:39 ` Misono Tomohiro
0 siblings, 0 replies; 8+ messages in thread
From: Misono Tomohiro @ 2018-03-30 7:39 UTC (permalink / raw)
To: Omar Sandoval, linux-btrfs; +Cc: kernel-team
On 2018/03/29 16:53, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Since "btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of, FS_TREE",
> the top-level subvolume has a non-zero UUID, ctime, and otime. Fix the
> subvolume_info() test to not check for zero.
Sorry, I didn't notice this.
I checked this works in devel branch:
Reviewed-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> libbtrfsutil/python/tests/test_subvolume.py | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
> index a46d4a34..93396cba 100644
> --- a/libbtrfsutil/python/tests/test_subvolume.py
> +++ b/libbtrfsutil/python/tests/test_subvolume.py
> @@ -95,7 +95,8 @@ class TestSubvolume(BtrfsTestCase):
> self.assertEqual(info.parent_id, 0)
> self.assertEqual(info.dir_id, 0)
> self.assertEqual(info.flags, 0)
> - self.assertEqual(info.uuid, bytes(16))
> + self.assertIsInstance(info.uuid, bytes)
> + self.assertEqual(len(info.uuid), 16)
> self.assertEqual(info.parent_uuid, bytes(16))
> self.assertEqual(info.received_uuid, bytes(16))
> self.assertNotEqual(info.generation, 0)
> @@ -103,8 +104,8 @@ class TestSubvolume(BtrfsTestCase):
> self.assertEqual(info.otransid, 0)
> self.assertEqual(info.stransid, 0)
> self.assertEqual(info.rtransid, 0)
> - self.assertEqual(info.ctime, 0)
> - self.assertEqual(info.otime, 0)
> + self.assertIsInstance(info.ctime, float)
> + self.assertIsInstance(info.otime, float)> self.assertEqual(info.stime, 0)
> self.assertEqual(info.rtime, 0)
>
> @@ -117,6 +118,7 @@ class TestSubvolume(BtrfsTestCase):
> self.assertEqual(info.dir_id, 256)
> self.assertEqual(info.flags, 0)
> self.assertIsInstance(info.uuid, bytes)
> + self.assertEqual(len(info.uuid), 16)
> self.assertEqual(info.parent_uuid, bytes(16))
> self.assertEqual(info.received_uuid, bytes(16))
> self.assertNotEqual(info.generation, 0)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] libbtrfsutil: misc fixes
2018-03-29 7:53 [PATCH 0/5] libbtrfsutil: misc fixes Omar Sandoval
` (4 preceding siblings ...)
2018-03-29 7:53 ` [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume Omar Sandoval
@ 2018-03-30 21:58 ` David Sterba
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2018-03-30 21:58 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-btrfs, kernel-team
On Thu, Mar 29, 2018 at 12:53:52AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> This is a handful of minor fixes for libbtrfsutil that would be good to
> get in before the release. Patch 1 fixes an issue reported by Tomohiro a
> few weeks back. Patch 2 fixes a memory leak I noticed while writing
> patch 1. Patch 3 makes us use the local mkfs.btrfs so that the tests can
> run on the CI system. Patch 4 fixes an issue I noticed while testing
> that fix. Patch 5 fixes a test failure caused by "btrfs-progs: mkfs: add
> uuid and otime to ROOT_ITEM of, FS_TREE".
>
> Based on your devel branch. Please apply!
Done, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread