diff options
author | Trevor Woerner <twoerner@gmail.com> | 2024-01-12 09:18:56 -0500 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2024-01-15 21:42:15 +0000 |
commit | fbbcf19f61fc80e77e9054fc5ead8926a45baf8f (patch) | |
tree | 215172e566cf7df132aa921f78397d086103abd5 /meta | |
parent | f7c094fce0b4381a2b1f8fd60ec56d3270d36041 (diff) | |
download | poky-fbbcf19f61fc80e77e9054fc5ead8926a45baf8f.tar.gz |
bmaptool: add 3 fixes
I found a couple issues with bmaptool, including a race condition, which I
have fixed and submitted upstream. This patch adds these fixes to the project
now while waiting for feedback and a new release:
BmapCopy.py: fix error message
CLI.py: fix block device udev race condition
BmapCopy.py: tweak suggested udev rule
(From OE-Core rev: 72ae1304bc8d3db14d0a7cc01d10328e47cf5a09)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Diffstat (limited to 'meta')
4 files changed, 168 insertions, 1 deletions
diff --git a/meta/recipes-support/bmap-tools/bmap-tools_git.bb b/meta/recipes-support/bmap-tools/bmap-tools_git.bb index effba2ecad..9bbd7d51c8 100644 --- a/meta/recipes-support/bmap-tools/bmap-tools_git.bb +++ b/meta/recipes-support/bmap-tools/bmap-tools_git.bb | |||
@@ -9,7 +9,12 @@ SECTION = "console/utils" | |||
9 | LICENSE = "GPL-2.0-only" | 9 | LICENSE = "GPL-2.0-only" |
10 | LIC_FILES_CHKSUM = "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263" | 10 | LIC_FILES_CHKSUM = "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263" |
11 | 11 | ||
12 | SRC_URI = "git://github.com/intel/${BPN};branch=main;protocol=https" | 12 | FILESEXTRAPATHS:prepend := "${THISDIR}/files:" |
13 | SRC_URI = "git://github.com/intel/${BPN};branch=main;protocol=https \ | ||
14 | file://0001-BmapCopy.py-fix-error-message.patch \ | ||
15 | file://0002-CLI.py-fix-block-device-udev-race-condition.patch \ | ||
16 | file://0003-BmapCopy.py-tweak-suggested-udev-rule.patch \ | ||
17 | " | ||
13 | 18 | ||
14 | SRCREV = "d84a6fd202fe246a0bc19ed2082e41bcdd75fb13" | 19 | SRCREV = "d84a6fd202fe246a0bc19ed2082e41bcdd75fb13" |
15 | S = "${WORKDIR}/git" | 20 | S = "${WORKDIR}/git" |
diff --git a/meta/recipes-support/bmap-tools/files/0001-BmapCopy.py-fix-error-message.patch b/meta/recipes-support/bmap-tools/files/0001-BmapCopy.py-fix-error-message.patch new file mode 100644 index 0000000000..ddac322e91 --- /dev/null +++ b/meta/recipes-support/bmap-tools/files/0001-BmapCopy.py-fix-error-message.patch | |||
@@ -0,0 +1,36 @@ | |||
1 | From ad0b0513a46c7d238d0fdabee0267c7084b75e84 Mon Sep 17 00:00:00 2001 | ||
2 | From: Trevor Woerner <twoerner@gmail.com> | ||
3 | Date: Thu, 11 Jan 2024 22:04:23 -0500 | ||
4 | Subject: [PATCH 1/3] BmapCopy.py: fix error message | ||
5 | |||
6 | The wrong variable was being used when attempting to print out an informative | ||
7 | message to the user. Leading to nonsense messages such as: | ||
8 | |||
9 | bmaptool: info: failed to enable I/O optimization, expect suboptimal speed (reason: cannot switch to the 1 I/O scheduler: 1 in use. None) | ||
10 | |||
11 | Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/129] | ||
12 | Signed-off-by: Trevor Woerner <twoerner@gmail.com> | ||
13 | --- | ||
14 | bmaptools/BmapCopy.py | 6 +++--- | ||
15 | 1 file changed, 3 insertions(+), 3 deletions(-) | ||
16 | |||
17 | diff --git a/bmaptools/BmapCopy.py b/bmaptools/BmapCopy.py | ||
18 | index 9de7ef434233..b1e8e0fcbdb7 100644 | ||
19 | --- a/bmaptools/BmapCopy.py | ||
20 | +++ b/bmaptools/BmapCopy.py | ||
21 | @@ -892,9 +892,9 @@ class BmapBdevCopy(BmapCopy): | ||
22 | _log.info( | ||
23 | "failed to enable I/O optimization, expect " | ||
24 | "suboptimal speed (reason: cannot switch to the " | ||
25 | - f"{max_ratio_chg.temp_value} I/O scheduler: " | ||
26 | - f"{max_ratio_chg.old_value or 'unknown scheduler'} in use. " | ||
27 | - f"{max_ratio_chg.error})" | ||
28 | + f"'{scheduler_chg.temp_value}' I/O scheduler: " | ||
29 | + f"'{scheduler_chg.old_value or 'unknown scheduler'}' in use. " | ||
30 | + f"{scheduler_chg.error})" | ||
31 | ) | ||
32 | if max_ratio_chg.error or scheduler_chg.error: | ||
33 | _log.info( | ||
34 | -- | ||
35 | 2.43.0.76.g1a87c842ece3 | ||
36 | |||
diff --git a/meta/recipes-support/bmap-tools/files/0002-CLI.py-fix-block-device-udev-race-condition.patch b/meta/recipes-support/bmap-tools/files/0002-CLI.py-fix-block-device-udev-race-condition.patch new file mode 100644 index 0000000000..ea2749a264 --- /dev/null +++ b/meta/recipes-support/bmap-tools/files/0002-CLI.py-fix-block-device-udev-race-condition.patch | |||
@@ -0,0 +1,83 @@ | |||
1 | From 34f4321dfce28697f830639260076e60d765698b Mon Sep 17 00:00:00 2001 | ||
2 | From: Trevor Woerner <twoerner@gmail.com> | ||
3 | Date: Fri, 12 Jan 2024 01:16:19 -0500 | ||
4 | Subject: [PATCH 2/3] CLI.py: fix block device udev race condition | ||
5 | |||
6 | We are encouraged to add a udev rule to change a block device's | ||
7 | bdi/max_ratio to '1' and queue/scheduler to 'none', which I did. | ||
8 | So I was surprised when, about 50% of the time, I kept seeing: | ||
9 | |||
10 | ... | ||
11 | 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') | ||
12 | bmaptool: info: You may want to set these I/O optimizations through a udev rule like this: | ||
13 | ... | ||
14 | |||
15 | The strange part is that sometimes it doesn't report a problem and | ||
16 | sometimes it does, even if the block device is left plugged in continuously | ||
17 | between multiple bmaptool invocations. | ||
18 | |||
19 | In all of my tests the bdi/max_ratio is always okay, but the | ||
20 | queue/scheduler would sometimes be reported as being the default scheduler, | ||
21 | not the one the udev rule was setting (none). Yet no matter how many times | ||
22 | I would read the file outside of bmaptool it always would be set to the | ||
23 | correct scheduler. | ||
24 | |||
25 | It turns out that opening a block device in "wb+" mode, which is what | ||
26 | bmaptool is doing at one point, causes the block device to act as though | ||
27 | it was just inserted, giving it the default settings, then causing udev to | ||
28 | trigger to switch it to the requested settings. However, if udev doesn't | ||
29 | finish before bmaptool reads the scheduler value there's a chance it will | ||
30 | read the pre-udev value, not the post-udev value, even though the block | ||
31 | device was never physically removed and re-inserted. | ||
32 | |||
33 | bmaptool was opening every file, then checking for block devices and | ||
34 | if found, closing then re-opening the block devices via a special | ||
35 | block-opening helper function. This patch re-organizes the code to only | ||
36 | open block devices once using the special block-opening helper function | ||
37 | that does not open block devices in "wb+" mode. | ||
38 | |||
39 | Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/130] | ||
40 | Signed-off-by: Trevor Woerner <twoerner@gmail.com> | ||
41 | --- | ||
42 | bmaptools/CLI.py | 14 +++++++------- | ||
43 | 1 file changed, 7 insertions(+), 7 deletions(-) | ||
44 | |||
45 | diff --git a/bmaptools/CLI.py b/bmaptools/CLI.py | ||
46 | index 82303b7bc398..0a263f05cf43 100644 | ||
47 | --- a/bmaptools/CLI.py | ||
48 | +++ b/bmaptools/CLI.py | ||
49 | @@ -38,6 +38,7 @@ import tempfile | ||
50 | import traceback | ||
51 | import shutil | ||
52 | import io | ||
53 | +import pathlib | ||
54 | from bmaptools import BmapCreate, BmapCopy, BmapHelpers, TransRead | ||
55 | |||
56 | VERSION = "3.7" | ||
57 | @@ -440,17 +441,16 @@ def open_files(args): | ||
58 | # Try to open the destination file. If it does not exist, a new regular | ||
59 | # file will be created. If it exists and it is a regular file - it'll be | ||
60 | # truncated. If this is a block device, it'll just be opened. | ||
61 | + dest_is_blkdev = False | ||
62 | try: | ||
63 | - dest_obj = open(args.dest, "wb+") | ||
64 | + if pathlib.Path(args.dest).is_block_device(): | ||
65 | + dest_is_blkdev = True | ||
66 | + dest_obj = open_block_device(args.dest) | ||
67 | + else: | ||
68 | + dest_obj = open(args.dest, "wb+") | ||
69 | except IOError as err: | ||
70 | error_out("cannot open destination file '%s':\n%s", args.dest, err) | ||
71 | |||
72 | - # Check whether the destination file is a block device | ||
73 | - dest_is_blkdev = stat.S_ISBLK(os.fstat(dest_obj.fileno()).st_mode) | ||
74 | - if dest_is_blkdev: | ||
75 | - dest_obj.close() | ||
76 | - dest_obj = open_block_device(args.dest) | ||
77 | - | ||
78 | return (image_obj, dest_obj, bmap_obj, bmap_path, image_obj.size, dest_is_blkdev) | ||
79 | |||
80 | |||
81 | -- | ||
82 | 2.43.0.76.g1a87c842ece3 | ||
83 | |||
diff --git a/meta/recipes-support/bmap-tools/files/0003-BmapCopy.py-tweak-suggested-udev-rule.patch b/meta/recipes-support/bmap-tools/files/0003-BmapCopy.py-tweak-suggested-udev-rule.patch new file mode 100644 index 0000000000..2794eeada3 --- /dev/null +++ b/meta/recipes-support/bmap-tools/files/0003-BmapCopy.py-tweak-suggested-udev-rule.patch | |||
@@ -0,0 +1,43 @@ | |||
1 | From 2a71e0c1a675e4f30f02c03dd0325944b393c434 Mon Sep 17 00:00:00 2001 | ||
2 | From: Trevor Woerner <twoerner@gmail.com> | ||
3 | Date: Fri, 12 Jan 2024 01:54:26 -0500 | ||
4 | Subject: [PATCH 3/3] BmapCopy.py: tweak suggested udev rule | ||
5 | |||
6 | Both bdi/max_ratio and queue/scheduler are only valid for whole block devices, | ||
7 | not individual partitions. Therefore, add a | ||
8 | |||
9 | ENV{DEVTYPE}!="partition", | ||
10 | |||
11 | to the suggested udev rule so that bmaptool doesn't try to check every | ||
12 | partition of the block device, just the whole device. | ||
13 | |||
14 | Otherwise the following will appear in the logs: | ||
15 | |||
16 | Jan 10 01:30:31 localhost (udev-worker)[10399]: sdk1: /etc/udev/rules.d/60-bmaptool-optimizations.rules:5 Failed to write ATTR{/sys/devices/pci0000:00/0000:00:14.0/usb2/2-3/2-3.1/2-3.1.2/2-3.1.2:1.0/host14/target14:0:0/14:0:0:0/block/sdk/sdk1/bdi/min_ratio}, ignoring: No such file or directory | ||
17 | Jan 10 01:30:31 localhost (udev-worker)[10399]: sdk1: /etc/udev/rules.d/60-bmaptool-optimizations.rules:5 Failed to write ATTR{/sys/devices/pci0000:00/0000:00:14.0/usb2/2-3/2-3.1/2-3.1.2/2-3.1.2:1.0/host14/target14:0:0/14:0:0:0/block/sdk/sdk1/bdi/max_ratio}, ignoring: No such file or directory | ||
18 | Jan 10 01:30:31 localhost (udev-worker)[10399]: sdk1: /etc/udev/rules.d/60-bmaptool-optimizations.rules:5 Failed to write ATTR{/sys/devices/pci0000:00/0000:00:14.0/usb2/2-3/2-3.1/2-3.1.2/2-3.1.2:1.0/host14/target14:0:0/14:0:0:0/block/sdk/sdk1/queue/scheduler}, ignoring: No such file or directory | ||
19 | [... and so on for every partition on your block device ...] | ||
20 | |||
21 | Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/131] | ||
22 | Signed-off-by: Trevor Woerner <twoerner@gmail.com> | ||
23 | --- | ||
24 | bmaptools/BmapCopy.py | 3 ++- | ||
25 | 1 file changed, 2 insertions(+), 1 deletion(-) | ||
26 | |||
27 | diff --git a/bmaptools/BmapCopy.py b/bmaptools/BmapCopy.py | ||
28 | index b1e8e0fcbdb7..a4c1177246a9 100644 | ||
29 | --- a/bmaptools/BmapCopy.py | ||
30 | +++ b/bmaptools/BmapCopy.py | ||
31 | @@ -906,7 +906,8 @@ class BmapBdevCopy(BmapCopy): | ||
32 | "\n" | ||
33 | 'ACTION=="add", SUBSYSTEMS=="usb", ATTRS{idVendor}=="xxxx", ' | ||
34 | 'ATTRS{idProduct}=="xxxx", TAG+="uaccess"\n' | ||
35 | - 'SUBSYSTEMS=="usb", ATTRS{idVendor}=="xxxx", ' | ||
36 | + 'SUBSYSTEMS=="usb", ENV{DEVTYPE}!="partition", ' | ||
37 | + 'ATTRS{idVendor}=="xxxx", ' | ||
38 | 'ATTRS{idProduct}=="xxxx", ATTR{bdi/min_ratio}="0", ' | ||
39 | 'ATTR{bdi/max_ratio}="1", ATTR{queue/scheduler}="none"\n' | ||
40 | "\n" | ||
41 | -- | ||
42 | 2.43.0.76.g1a87c842ece3 | ||
43 | |||