From 2077c72863835e4e5fb8d5c0dc16344253b30a8c Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Thu, 3 Sep 2026 10:27:14 +0200 Subject: [PATCH] loop: Add _from_fd variants for the remaining loop functions Add bd_loop_info_from_fd, bd_loop_teardown_from_fd, bd_loop_set_autoclear_from_fd and bd_loop_set_capacity_from_fd which operate on an already open file descriptor of the loop device instead of opening it by name. This lets consumers open the device early on their side and pass it in, avoiding race conditions. The existing name-based functions now just open the device and delegate to their _from_fd counterparts (mirroring bd_loop_setup/setup_from_fd) to avoid duplicating the ioctl logic. Resolves: https://github.com/storaged-project/libblockdev/issues/1208 Co-Authored-By: Claude Opus 4.8 --- docs/libblockdev-sections.txt | 4 + src/lib/plugin_apis/loop.api | 50 ++++++++++ src/plugins/loop.c | 175 +++++++++++++++++++++++----------- src/plugins/loop.h | 4 + tests/loop_test.py | 92 +++++++++++++++++- 5 files changed, 265 insertions(+), 60 deletions(-) diff --git a/docs/libblockdev-sections.txt b/docs/libblockdev-sections.txt index 2e57e75a4..4a8f538d9 100644 --- a/docs/libblockdev-sections.txt +++ b/docs/libblockdev-sections.txt @@ -271,14 +271,18 @@ BD_LOOP_ERROR BDLoopError BDLoopInfo bd_loop_info +bd_loop_info_from_fd bd_loop_info_copy bd_loop_info_free bd_loop_get_loop_name bd_loop_setup bd_loop_setup_from_fd bd_loop_teardown +bd_loop_teardown_from_fd bd_loop_set_autoclear +bd_loop_set_autoclear_from_fd bd_loop_set_capacity +bd_loop_set_capacity_from_fd BDLoopTech BDLoopTechMode bd_loop_is_tech_avail diff --git a/src/lib/plugin_apis/loop.api b/src/lib/plugin_apis/loop.api index c55e0c56d..e6cac8348 100644 --- a/src/lib/plugin_apis/loop.api +++ b/src/lib/plugin_apis/loop.api @@ -117,6 +117,18 @@ GType bd_loop_info_get_type () { */ BDLoopInfo* bd_loop_info (const gchar *loop, GError **error); +/** + * bd_loop_info_from_fd: + * @fd: file descriptor of an open loop device to get information about + * @error: (out) (optional): place to store error (if any) + * + * Returns: (transfer full): information about the loop device represented by @fd + * or %NULL in case of error + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_QUERY + */ +BDLoopInfo* bd_loop_info_from_fd (gint fd, GError **error); + /** * bd_loop_get_loop_name: * @file: path of the backing file to get loop name for @@ -174,6 +186,17 @@ gboolean bd_loop_setup_from_fd (gint fd, guint64 offset, guint64 size, gboolean */ gboolean bd_loop_teardown (const gchar *loop, GError **error); +/** + * bd_loop_teardown_from_fd: + * @fd: file descriptor of an open loop device to tear down + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the loop device represented by @fd was successfully torn down or not + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_DESTROY + */ +gboolean bd_loop_teardown_from_fd (gint fd, GError **error); + /** * bd_loop_set_autoclear: * @loop: path or name of the loop device @@ -186,6 +209,19 @@ gboolean bd_loop_teardown (const gchar *loop, GError **error); */ gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **error); +/** + * bd_loop_set_autoclear_from_fd: + * @fd: file descriptor of an open loop device + * @autoclear: whether to set or unset the autoclear flag + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the autoclear flag was successfully set on the loop device + * represented by @fd or not + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_MODIFY + */ +gboolean bd_loop_set_autoclear_from_fd (gint fd, gboolean autoclear, GError **error); + /** * bd_loop_set_capacity: * @loop: path or name of the loop device @@ -200,4 +236,18 @@ gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError ** */ gboolean bd_loop_set_capacity (const gchar *loop, GError **error); +/** + * bd_loop_set_capacity_from_fd: + * @fd: file descriptor of an open loop device + * @error: (out) (optional): place to store error (if any) + * + * Force the loop driver to reread the size of the file associated with the loop + * device represented by @fd. + * + * Returns: whether the LOOP_SET_CAPACITY ioctl was successfully issued or not. + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_MODIFY + */ +gboolean bd_loop_set_capacity_from_fd (gint fd, GError **error); + #endif /* BD_LOOP_API */ diff --git a/src/plugins/loop.c b/src/plugins/loop.c index f17e445bb..721b1c5d1 100644 --- a/src/plugins/loop.c +++ b/src/plugins/loop.c @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include #include @@ -114,11 +116,21 @@ BDLoopInfo* bd_loop_info_copy (BDLoopInfo *info) { return new_info; } -static gchar* _loop_get_backing_file (const gchar *dev_name, GError **error) { - gchar *sys_path = g_strdup_printf ("/sys/class/block/%s/loop/backing_file", dev_name); +static gchar* _loop_get_backing_file_from_fd (gint fd, GError **error) { + struct stat statbuf; + gchar *sys_path = NULL; gchar *ret = NULL; gboolean success = FALSE; + if (fstat (fd, &statbuf) != 0) { + g_set_error (error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, + "Failed to stat the loop device: %m"); + return NULL; + } + + sys_path = g_strdup_printf ("/sys/dev/block/%u:%u/loop/backing_file", + major (statbuf.st_rdev), minor (statbuf.st_rdev)); + if (access (sys_path, R_OK) != 0) { g_free (sys_path); return NULL; @@ -145,11 +157,9 @@ static gchar* _loop_get_backing_file (const gchar *dev_name, GError **error) { * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_QUERY */ BDLoopInfo* bd_loop_info (const gchar *loop, GError **error) { - BDLoopInfo *info = NULL; g_autofree gchar *dev_loop = NULL; gint fd = -1; - struct loop_info64 li64; - GError *l_error = NULL; + BDLoopInfo *info = NULL; if (!g_str_has_prefix (loop, "/dev/")) dev_loop = g_strdup_printf ("/dev/%s", loop); @@ -161,17 +171,34 @@ BDLoopInfo* bd_loop_info (const gchar *loop, GError **error) { return NULL; } + info = bd_loop_info_from_fd (fd, error); + close (fd); + return info; +} + +/** + * bd_loop_info_from_fd: + * @fd: file descriptor of an open loop device to get information about + * @error: (out) (optional): place to store error (if any) + * + * Returns: (transfer full): information about the loop device represented by @fd + * or %NULL in case of error + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_QUERY + */ +BDLoopInfo* bd_loop_info_from_fd (gint fd, GError **error) { + BDLoopInfo *info = NULL; + struct loop_info64 li64; + GError *l_error = NULL; + memset (&li64, 0, sizeof (li64)); if (ioctl (fd, LOOP_GET_STATUS64, &li64) < 0) { g_set_error (error, BD_LOOP_ERROR, errno == ENXIO ? BD_LOOP_ERROR_DEVICE : BD_LOOP_ERROR_FAIL, - "Failed to get status of the device %s: %m", loop); - close (fd); + "Failed to get status of the device: %m"); return NULL; } - close (fd); - info = g_new0 (BDLoopInfo, 1); info->offset = li64.lo_offset; if ((li64.lo_flags & LO_FLAGS_AUTOCLEAR) != 0) @@ -183,11 +210,11 @@ BDLoopInfo* bd_loop_info (const gchar *loop, GError **error) { if ((li64.lo_flags & LO_FLAGS_READ_ONLY) != 0) info->read_only = TRUE; - info->backing_file = _loop_get_backing_file (loop, &l_error); + info->backing_file = _loop_get_backing_file_from_fd (fd, &l_error); if (l_error) { bd_loop_info_free (info); g_set_error (error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, - "Failed to get backing file of the device %s: %s", loop, l_error->message); + "Failed to get backing file of the device: %s", l_error->message); g_clear_error (&l_error); return NULL; } @@ -426,34 +453,48 @@ gboolean bd_loop_setup_from_fd (gint fd, guint64 offset, guint64 size, gboolean gboolean bd_loop_teardown (const gchar *loop, GError **error) { gchar *dev_loop = NULL; gint loop_fd = -1; - guint64 progress_id = 0; - GError *l_error = NULL; - - progress_id = bd_utils_report_started ("Started tearing down loop device"); + gboolean ret = FALSE; if (!g_str_has_prefix (loop, "/dev/")) dev_loop = g_strdup_printf ("/dev/%s", loop); loop_fd = open (dev_loop ? dev_loop : loop, O_RDONLY); - g_free (dev_loop); if (loop_fd == -1) { - g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, + g_set_error (error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, "Failed to open the %s device: %m", loop); - bd_utils_report_finished (progress_id, l_error->message); - g_propagate_error (error, l_error); + g_free (dev_loop); return FALSE; } + g_free (dev_loop); - if (ioctl (loop_fd, LOOP_CLR_FD) < 0) { + ret = bd_loop_teardown_from_fd (loop_fd, error); + close (loop_fd); + return ret; +} + +/** + * bd_loop_teardown_from_fd: + * @fd: file descriptor of an open loop device to tear down + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the loop device represented by @fd was successfully torn down or not + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_DESTROY + */ +gboolean bd_loop_teardown_from_fd (gint fd, GError **error) { + guint64 progress_id = 0; + GError *l_error = NULL; + + progress_id = bd_utils_report_started ("Started tearing down loop device"); + + if (ioctl (fd, LOOP_CLR_FD) < 0) { g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, - "Failed to detach the backing file from the %s device: %m", loop); - close (loop_fd); + "Failed to detach the backing file from the loop device: %m"); bd_utils_report_finished (progress_id, l_error->message); g_propagate_error (error, l_error); return FALSE; } - close (loop_fd); bd_utils_report_finished (progress_id, "Completed"); return TRUE; @@ -472,34 +513,46 @@ gboolean bd_loop_teardown (const gchar *loop, GError **error) { gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **error) { gchar *dev_loop = NULL; gint fd = -1; - struct loop_info64 li64; - guint64 progress_id = 0; - gchar *msg = NULL; - GError *l_error = NULL; + gboolean ret = FALSE; if (!g_str_has_prefix (loop, "/dev/")) dev_loop = g_strdup_printf ("/dev/%s", loop); - msg = g_strdup_printf ("Started setting up the autoclear flag on the %s device", - dev_loop ? dev_loop : loop); - progress_id = bd_utils_report_started (msg); - g_free (msg); - fd = open (dev_loop ? dev_loop : loop, O_RDWR); g_free (dev_loop); if (fd < 0) { - g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_DEVICE, + g_set_error (error, BD_LOOP_ERROR, BD_LOOP_ERROR_DEVICE, "Failed to open device %s: %m", loop); - bd_utils_report_finished (progress_id, l_error->message); - g_propagate_error (error, l_error); return FALSE; } + ret = bd_loop_set_autoclear_from_fd (fd, autoclear, error); + close (fd); + return ret; +} + +/** + * bd_loop_set_autoclear_from_fd: + * @fd: file descriptor of an open loop device + * @autoclear: whether to set or unset the autoclear flag + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the autoclear flag was successfully set on the loop device + * represented by @fd or not + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_MODIFY + */ +gboolean bd_loop_set_autoclear_from_fd (gint fd, gboolean autoclear, GError **error) { + struct loop_info64 li64; + guint64 progress_id = 0; + GError *l_error = NULL; + + progress_id = bd_utils_report_started ("Started setting up the autoclear flag on a loop device"); + memset (&li64, 0, sizeof (li64)); if (ioctl (fd, LOOP_GET_STATUS64, &li64) < 0) { g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, - "Failed to get status of the device %s: %m", loop); - close (fd); + "Failed to get status of the device: %m"); bd_utils_report_finished (progress_id, l_error->message); g_propagate_error (error, l_error); return FALSE; @@ -512,14 +565,12 @@ gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError ** if (ioctl (fd, LOOP_SET_STATUS64, &li64) < 0) { g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, - "Failed to set status of the device %s: %m", loop); - close (fd); + "Failed to set status of the device: %m"); bd_utils_report_finished (progress_id, l_error->message); g_propagate_error (error, l_error); return FALSE; } - close (fd); bd_utils_report_finished (progress_id, "Completed"); return TRUE; } @@ -539,30 +590,44 @@ gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError ** gboolean bd_loop_set_capacity (const gchar *loop, GError **error) { gchar *dev_loop = NULL; gint fd = -1; - guint64 progress_id = 0; - gchar *msg = NULL; - guint n_try = 0; - gint status = 0; - GError *l_error = NULL; + gboolean ret = FALSE; if (!g_str_has_prefix (loop, "/dev/")) dev_loop = g_strdup_printf ("/dev/%s", loop); - msg = g_strdup_printf ("Started setting up capacity on the %s device", - dev_loop ? dev_loop : loop); - progress_id = bd_utils_report_started (msg); - g_free (msg); - fd = open (dev_loop ? dev_loop : loop, O_RDWR); g_free (dev_loop); if (fd < 0) { - g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_DEVICE, + g_set_error (error, BD_LOOP_ERROR, BD_LOOP_ERROR_DEVICE, "Failed to open device %s: %m", loop); - bd_utils_report_finished (progress_id, l_error->message); - g_propagate_error (error, l_error); return FALSE; } + ret = bd_loop_set_capacity_from_fd (fd, error); + close (fd); + return ret; +} + +/** + * bd_loop_set_capacity_from_fd: + * @fd: file descriptor of an open loop device + * @error: (out) (optional): place to store error (if any) + * + * Force the loop driver to reread the size of the file associated with the loop + * device represented by @fd. + * + * Returns: whether the LOOP_SET_CAPACITY ioctl was successfully issued or not. + * + * Tech category: %BD_LOOP_TECH_LOOP-%BD_LOOP_TECH_MODE_MODIFY + */ +gboolean bd_loop_set_capacity_from_fd (gint fd, GError **error) { + guint64 progress_id = 0; + guint n_try = 0; + gint status = 0; + GError *l_error = NULL; + + progress_id = bd_utils_report_started ("Started setting up capacity on a loop device"); + for (n_try=10, status=-1; (status != 0) && (n_try > 0); n_try--) { status = ioctl (fd, LOOP_SET_CAPACITY, 0); if (status < 0 && errno == EAGAIN) @@ -573,14 +638,12 @@ gboolean bd_loop_set_capacity (const gchar *loop, GError **error) { if (status != 0) { g_set_error (&l_error, BD_LOOP_ERROR, BD_LOOP_ERROR_FAIL, - "Failed to set capacity of the device %s: %m", loop); - close (fd); + "Failed to set capacity of the device: %m"); bd_utils_report_finished (progress_id, l_error->message); g_propagate_error (error, l_error); return FALSE; } - close (fd); bd_utils_report_finished (progress_id, "Completed"); return TRUE; } diff --git a/src/plugins/loop.h b/src/plugins/loop.h index cfdb7e3ef..3b69a38f7 100644 --- a/src/plugins/loop.h +++ b/src/plugins/loop.h @@ -59,13 +59,17 @@ void bd_loop_close (void); gboolean bd_loop_is_tech_avail (BDLoopTech tech, guint64 mode, GError **error); BDLoopInfo* bd_loop_info (const gchar *loop, GError **error); +BDLoopInfo* bd_loop_info_from_fd (gint fd, GError **error); gchar* bd_loop_get_loop_name (const gchar *file, GError **error); gboolean bd_loop_setup (const gchar *file, guint64 offset, guint64 size, gboolean read_only, gboolean part_scan, guint64 sector_size, const gchar **loop_name, GError **error); gboolean bd_loop_setup_from_fd (gint fd, guint64 offset, guint64 size, gboolean read_only, gboolean part_scan, guint64 sector_size, const gchar **loop_name, GError **error); gboolean bd_loop_teardown (const gchar *loop, GError **error); +gboolean bd_loop_teardown_from_fd (gint fd, GError **error); gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **error); +gboolean bd_loop_set_autoclear_from_fd (gint fd, gboolean autoclear, GError **error); gboolean bd_loop_set_capacity (const gchar *loop, GError **error); +gboolean bd_loop_set_capacity_from_fd (gint fd, GError **error); #endif /* BD_LOOP */ diff --git a/tests/loop_test.py b/tests/loop_test.py index cb7df269d..d5ab8a1db 100644 --- a/tests/loop_test.py +++ b/tests/loop_test.py @@ -31,10 +31,11 @@ def setUp(self): self.loop = None def _clean_up(self): - try: - BlockDev.loop_teardown(self.loop) - except: - pass + if self.loop: + try: + BlockDev.loop_teardown(self.loop) + except: + pass os.unlink(self.dev_file) def _get_loop_size(self): @@ -78,6 +79,89 @@ def test_loop_setup_teardown_basic(self): self.assertTrue(succ) +class LoopTestFromFd(LoopTestCase): + @tag_test(TestTags.CORE) + def test_loop_teardown_from_fd(self): + """Verify that loop_teardown_from_fd works as expected""" + + succ, self.loop = BlockDev.loop_setup(self.dev_file) + self.assertTrue(succ) + self.assertTrue(self.loop) + + fd = os.open("/dev/" + self.loop, os.O_RDONLY) + try: + succ = BlockDev.loop_teardown_from_fd(fd) + self.assertTrue(succ) + except: + raise + else: + self.loop = None + finally: + os.close(fd) + + def test_loop_info_from_fd(self): + """Verify that loop_info_from_fd works as expected""" + + succ, self.loop = BlockDev.loop_setup(self.dev_file, 10 * 1024**2) + self.assertTrue(succ) + self.assertTrue(self.loop) + + fd = os.open("/dev/" + self.loop, os.O_RDONLY) + try: + info = BlockDev.loop_info_from_fd(fd) + self.assertIsNotNone(info) + self.assertEqual(info.backing_file, self.dev_file) + self.assertEqual(info.offset, 10 * 1024**2) + finally: + os.close(fd) + + def test_loop_set_autoclear_from_fd(self): + """Verify that loop_set_autoclear_from_fd works as expected""" + + succ, self.loop = BlockDev.loop_setup(self.dev_file) + self.assertTrue(succ) + self.assertTrue(self.loop) + + # open the loop device so that it doesn't disappear once we set + # autoclear to True + fd = os.open("/dev/" + self.loop, os.O_RDWR) + self.addCleanup(os.close, fd) + + self.assertTrue(BlockDev.loop_set_autoclear_from_fd(fd, True)) + info = BlockDev.loop_info(self.loop) + self.assertIsNotNone(info) + self.assertTrue(info.autoclear) + + self.assertTrue(BlockDev.loop_set_autoclear_from_fd(fd, False)) + info = BlockDev.loop_info(self.loop) + self.assertIsNotNone(info) + self.assertFalse(info.autoclear) + + def test_loop_set_capacity_from_fd(self): + """Verify that loop_set_capacity_from_fd works as expected""" + + succ, self.loop = BlockDev.loop_setup(self.dev_file) + self.assertTrue(succ) + self.assertTrue(self.loop) + self.assertEqual(self._get_loop_size(), self._loop_size) + + # enlarge the backing file + create_sparse_file(self.dev_file, self._loop_size * 2) + + # size shouldn't change without forcing re-read + self.assertEqual(self._get_loop_size(), self._loop_size) + + fd = os.open("/dev/" + self.loop, os.O_RDWR) + try: + succ = BlockDev.loop_set_capacity_from_fd(fd) + self.assertTrue(succ) + finally: + os.close(fd) + + # now the size should be updated + self.assertEqual(self._get_loop_size(), self._loop_size * 2) + + class LoopTestSetupOffset(LoopTestCase): def test_loop_setup_with_offset(self): """Verify that loop_setup with offset specified works as expected"""