libbtrfsutil: misc fixes
Subsystems (2)
cover libbtrfsutil: misc fixes
From: Omar Sandoval <[email protected]>
Hi, Dave,
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!
Omar Sandoval (5):
libbtrfsutil: don't return free space cache inodes from
deleted_subvolumes()
libbtrfsutil: fix memory leak in deleted_subvolumes()
libbtrfsutil: use local mkfs.btrfs for tests if it exists
libbtrfsutil: always build libbtrfsutil.so.$MAJOR
libbtrfsutil: fix test assumptions about top-level subvolume
Makefile | 4 ++-- libbtrfsutil/python/tests/__init__.py | 6 ++++- libbtrfsutil/python/tests/test_subvolume.py | 8 ++++--- libbtrfsutil/subvolume.c | 34 +++++++++++++++++++---------- 4 files changed, 35 insertions(+), 17 deletions(-)
Signature · 1 line
2.16.3
PATCH 1/5 libbtrfsutil: don't return free space cache inodes from deleted_subvolumes()
From: Omar Sandoval <[email protected]>
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.
--- libbtrfsutil/subvolume.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)
libbtrfsutil/subvolume.c +22 -12
@@ -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 ;
Signature · 1 line
2.16.3
PATCH 2/5 libbtrfsutil: fix memory leak in deleted_subvolumes()
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
If we fail to reallocate the ID array, we still need to free it.
--- libbtrfsutil/subvolume.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
libbtrfsutil/subvolume.c +4 -2
@@ -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;
Signature · 1 line
2.16.3
PATCH 3/5 libbtrfsutil: use local mkfs.btrfs for tests if it exists
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
The system might not have mkfs installed at all.
--- Makefile | 2 +- libbtrfsutil/python/tests/__init__.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-)
Makefile +1 -1
@@ -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
libbtrfsutil/python/tests/__init__.py +5 -1
@@ -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)
Signature · 1 line
2.16.3
PATCH 4/5 libbtrfsutil: always build libbtrfsutil.so.$MAJOR
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Otherwise, make test-libbtrfsutil from a fresh checkout fails.
--- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
@@ -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
Signature · 1 line
2.16.3
PATCH 5/5 libbtrfsutil: fix test assumptions about top-level subvolume
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
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.
--- libbtrfsutil/python/tests/test_subvolume.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
libbtrfsutil/python/tests/test_subvolume.py +5 -3
@@ -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)
Signature · 1 line
2.16.3
Re: [PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume
Quoted 5 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
Since "btrfs-progs: mkfs: add uuid and otime to
ROOT_ITEMof,FS_TREE", the top-level subvolume has a non-zero UUID, ctime, and otime. Fix thesubvolume_info()test to not check for zero.
Sorry, I didn't notice this.
I checked this works in devel branch:
Quoted 39 lines
Signed-off-by Omar Sandoval <[email protected]>--- libbtrfsutil/python/tests/test_subvolume.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)libbtrfsutil/python/tests/test_subvolume.py +5
-3@@ -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)
Quoted 10 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
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 toROOT_ITEMof,FS_TREE".Based on your devel branch. Please apply!
Done, thanks.