| From 34f4321dfce28697f830639260076e60d765698b Mon Sep 17 00:00:00 2001 |
| From: Trevor Woerner <twoerner@gmail.com> |
| Date: Fri, 12 Jan 2024 01:16:19 -0500 |
| Subject: [PATCH 2/3] CLI.py: fix block device udev race condition |
| |
| We are encouraged to add a udev rule to change a block device's |
| bdi/max_ratio to '1' and queue/scheduler to 'none', which I did. |
| So I was surprised when, about 50% of the time, I kept seeing: |
| |
| ... |
| bmaptool: info: failed to enable I/O optimization, expect suboptimal speed (reason: cannot switch to the 'none' I/O scheduler: 'bfq' in use. [Errno 13] Permission denied: '/sys/dev/block/8:160/queue/scheduler') |
| bmaptool: info: You may want to set these I/O optimizations through a udev rule like this: |
| ... |
| |
| The strange part is that sometimes it doesn't report a problem and |
| sometimes it does, even if the block device is left plugged in continuously |
| between multiple bmaptool invocations. |
| |
| In all of my tests the bdi/max_ratio is always okay, but the |
| queue/scheduler would sometimes be reported as being the default scheduler, |
| not the one the udev rule was setting (none). Yet no matter how many times |
| I would read the file outside of bmaptool it always would be set to the |
| correct scheduler. |
| |
| It turns out that opening a block device in "wb+" mode, which is what |
| bmaptool is doing at one point, causes the block device to act as though |
| it was just inserted, giving it the default settings, then causing udev to |
| trigger to switch it to the requested settings. However, if udev doesn't |
| finish before bmaptool reads the scheduler value there's a chance it will |
| read the pre-udev value, not the post-udev value, even though the block |
| device was never physically removed and re-inserted. |
| |
| bmaptool was opening every file, then checking for block devices and |
| if found, closing then re-opening the block devices via a special |
| block-opening helper function. This patch re-organizes the code to only |
| open block devices once using the special block-opening helper function |
| that does not open block devices in "wb+" mode. |
| |
| Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/130] |
| Signed-off-by: Trevor Woerner <twoerner@gmail.com> |
| --- |
| bmaptools/CLI.py | 14 +++++++------- |
| 1 file changed, 7 insertions(+), 7 deletions(-) |
| |
| diff --git a/bmaptools/CLI.py b/bmaptools/CLI.py |
| index 82303b7bc398..0a263f05cf43 100644 |
| --- a/bmaptools/CLI.py |
| +++ b/bmaptools/CLI.py |
| @@ -38,6 +38,7 @@ import tempfile |
| import traceback |
| import shutil |
| import io |
| +import pathlib |
| from bmaptools import BmapCreate, BmapCopy, BmapHelpers, TransRead |
| |
| VERSION = "3.7" |
| @@ -440,17 +441,16 @@ def open_files(args): |
| # Try to open the destination file. If it does not exist, a new regular |
| # file will be created. If it exists and it is a regular file - it'll be |
| # truncated. If this is a block device, it'll just be opened. |
| + dest_is_blkdev = False |
| try: |
| - dest_obj = open(args.dest, "wb+") |
| + if pathlib.Path(args.dest).is_block_device(): |
| + dest_is_blkdev = True |
| + dest_obj = open_block_device(args.dest) |
| + else: |
| + dest_obj = open(args.dest, "wb+") |
| except IOError as err: |
| error_out("cannot open destination file '%s':\n%s", args.dest, err) |
| |
| - # Check whether the destination file is a block device |
| - dest_is_blkdev = stat.S_ISBLK(os.fstat(dest_obj.fileno()).st_mode) |
| - if dest_is_blkdev: |
| - dest_obj.close() |
| - dest_obj = open_block_device(args.dest) |
| - |
| return (image_obj, dest_obj, bmap_obj, bmap_path, image_obj.size, dest_is_blkdev) |
| |
| |
| -- |
| 2.43.0.76.g1a87c842ece3 |
| |