Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/libblockdev-sections.txt
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ bd_loop_setup
bd_loop_setup_from_fd
bd_loop_teardown
bd_loop_set_autoclear
bd_loop_set_capacity
BDLoopTech
BDLoopTechMode
bd_loop_is_tech_avail
Expand Down
14 changes: 14 additions & 0 deletions src/lib/plugin_apis/loop.api
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,18 @@ gboolean bd_loop_teardown (const gchar *loop, GError **error);
*/
gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **error);

/**
* bd_loop_set_capacity:
* @loop: path or name of the 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
* specified @loop device.
*
* 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 (const gchar *loop, GError **error);

#endif /* BD_LOOP_API */
61 changes: 61 additions & 0 deletions src/plugins/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -517,3 +517,64 @@ gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **
bd_utils_report_finished (progress_id, "Completed");
return TRUE;
}

/**
* bd_loop_set_capacity:
* @loop: path or name of the 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
* specified @loop device.
*
* 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 (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;

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,
"Failed to open device %s: %m", loop);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So previously you use a complex construct like dev_loop ? dev_loop : loop for messages but in the rest of the code you just use loop... I don't mind, I wonder why the AI didn't spot this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I copied it from two different functions with two different messages :-)

bd_utils_report_finished (progress_id, l_error->message);
g_propagate_error (error, l_error);
return FALSE;
}

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)
g_usleep (100 * 1000); /* microseconds */
else
break;
}
Comment thread
vojtechtrefny marked this conversation as resolved.
Comment on lines +560 to +566
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic in this for loop is a bit dense due to the combined initialization and condition, along with the if/else break structure. A more conventional loop structure would be more readable and easier for future maintenance.

    for (n_try = 10; n_try > 0; n_try--) {
        status = ioctl (fd, LOOP_SET_CAPACITY, 0);
        if (status == 0)
            break;
        if (errno != EAGAIN)
            break;
        g_usleep (100 * 1000); /* microseconds */
    }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better!


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);
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;
}
1 change: 1 addition & 0 deletions src/plugins/loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,6 @@ gboolean bd_loop_setup_from_fd (gint fd, guint64 offset, guint64 size, gboolean
gboolean bd_loop_teardown (const gchar *loop, GError **error);

gboolean bd_loop_set_autoclear (const gchar *loop, gboolean autoclear, GError **error);
gboolean bd_loop_set_capacity (const gchar *loop, GError **error);

#endif /* BD_LOOP */
37 changes: 29 additions & 8 deletions tests/loop_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import unittest
import overrides_hack

from utils import create_sparse_tempfile, TestTags, tag_test, required_plugins
from utils import create_sparse_tempfile, create_sparse_file, TestTags, tag_test, required_plugins

import gi
gi.require_version('BlockDev', '3.0')
Expand All @@ -11,6 +11,7 @@

@required_plugins(("loop",))
class LoopTestCase(unittest.TestCase):
_loop_size = 100 * 1024**2

requested_plugins = BlockDev.plugin_specs_from_names(("loop",))

Expand All @@ -23,7 +24,7 @@ def setUpClass(cls):

def setUp(self):
self.addCleanup(self._clean_up)
self.dev_file = create_sparse_tempfile("loop_test", 1024**3)
self.dev_file = create_sparse_tempfile("loop_test", self._loop_size)
self.loop = None

def _clean_up(self):
Expand All @@ -33,6 +34,10 @@ def _clean_up(self):
pass
os.unlink(self.dev_file)

def _get_loop_size(self):
with open("/sys/block/%s/size" % self.loop, "r") as f:
return int(f.read()) * 512

class LoopPluginVersionCase(LoopTestCase):
@tag_test(TestTags.NOSTORAGE)
def test_plugin_version(self):
Expand Down Expand Up @@ -75,9 +80,7 @@ def test_loop_setup_with_offset(self):
self.assertEqual(info.offset, 10 * 1024**2)

# should have smaller size due to the offset
with open("/sys/block/%s/size" % self.loop, "r") as f:
size = int(f.read()) * 512
self.assertEqual(size, 1024**3 - 10 * 1024 **2)
self.assertEqual(self._get_loop_size(), self._loop_size - 10 * 1024 **2)

succ = BlockDev.loop_teardown(self.loop)
self.assertTrue(succ)
Expand All @@ -93,9 +96,7 @@ def test_loop_setup_with_offset_and_size(self):
self.assertTrue(self.loop)

# should have size as specified
with open("/sys/block/%s/size" % self.loop, "r") as f:
size = int(f.read()) * 512
self.assertEqual(size, 50 * 1024**2)
self.assertEqual(self._get_loop_size(), 50 * 1024**2)

succ = BlockDev.loop_teardown(self.loop)
self.assertTrue(succ)
Expand Down Expand Up @@ -219,3 +220,23 @@ def test_loop_get_set_autoclear(self):
info = BlockDev.loop_info(self.loop)
self.assertIsNotNone(info)
self.assertFalse(info.autoclear)


class LoopTestSetCapacity(LoopTestCase):
def test_loop_set_capacity(self):
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)

succ = BlockDev.loop_set_capacity(self.loop)
self.assertTrue(succ)

# now the size should be updated
self.assertEqual(self._get_loop_size(), self._loop_size * 2)