mirror of
https://git.yoctoproject.org/poky
synced 2026-01-29 21:08:42 +01:00
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>
This commit is contained in:
Trevor Woerner
committed by
Richard Purdie
Richard Purdie
parent
f7c094fce0
commit
fbbcf19f61
@@ -9,7 +9,12 @@ SECTION = "console/utils"
|
||||
LICENSE = "GPL-2.0-only"
|
||||
LIC_FILES_CHKSUM = "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263"
|
||||
|
||||
SRC_URI = "git://github.com/intel/${BPN};branch=main;protocol=https"
|
||||
FILESEXTRAPATHS:prepend := "${THISDIR}/files:"
|
||||
SRC_URI = "git://github.com/intel/${BPN};branch=main;protocol=https \
|
||||
file://0001-BmapCopy.py-fix-error-message.patch \
|
||||
file://0002-CLI.py-fix-block-device-udev-race-condition.patch \
|
||||
file://0003-BmapCopy.py-tweak-suggested-udev-rule.patch \
|
||||
"
|
||||
|
||||
SRCREV = "d84a6fd202fe246a0bc19ed2082e41bcdd75fb13"
|
||||
S = "${WORKDIR}/git"
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
From ad0b0513a46c7d238d0fdabee0267c7084b75e84 Mon Sep 17 00:00:00 2001
|
||||
From: Trevor Woerner <twoerner@gmail.com>
|
||||
Date: Thu, 11 Jan 2024 22:04:23 -0500
|
||||
Subject: [PATCH 1/3] BmapCopy.py: fix error message
|
||||
|
||||
The wrong variable was being used when attempting to print out an informative
|
||||
message to the user. Leading to nonsense messages such as:
|
||||
|
||||
bmaptool: info: failed to enable I/O optimization, expect suboptimal speed (reason: cannot switch to the 1 I/O scheduler: 1 in use. None)
|
||||
|
||||
Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/129]
|
||||
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
|
||||
---
|
||||
bmaptools/BmapCopy.py | 6 +++---
|
||||
1 file changed, 3 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/bmaptools/BmapCopy.py b/bmaptools/BmapCopy.py
|
||||
index 9de7ef434233..b1e8e0fcbdb7 100644
|
||||
--- a/bmaptools/BmapCopy.py
|
||||
+++ b/bmaptools/BmapCopy.py
|
||||
@@ -892,9 +892,9 @@ class BmapBdevCopy(BmapCopy):
|
||||
_log.info(
|
||||
"failed to enable I/O optimization, expect "
|
||||
"suboptimal speed (reason: cannot switch to the "
|
||||
- f"{max_ratio_chg.temp_value} I/O scheduler: "
|
||||
- f"{max_ratio_chg.old_value or 'unknown scheduler'} in use. "
|
||||
- f"{max_ratio_chg.error})"
|
||||
+ f"'{scheduler_chg.temp_value}' I/O scheduler: "
|
||||
+ f"'{scheduler_chg.old_value or 'unknown scheduler'}' in use. "
|
||||
+ f"{scheduler_chg.error})"
|
||||
)
|
||||
if max_ratio_chg.error or scheduler_chg.error:
|
||||
_log.info(
|
||||
--
|
||||
2.43.0.76.g1a87c842ece3
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
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
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
From 2a71e0c1a675e4f30f02c03dd0325944b393c434 Mon Sep 17 00:00:00 2001
|
||||
From: Trevor Woerner <twoerner@gmail.com>
|
||||
Date: Fri, 12 Jan 2024 01:54:26 -0500
|
||||
Subject: [PATCH 3/3] BmapCopy.py: tweak suggested udev rule
|
||||
|
||||
Both bdi/max_ratio and queue/scheduler are only valid for whole block devices,
|
||||
not individual partitions. Therefore, add a
|
||||
|
||||
ENV{DEVTYPE}!="partition",
|
||||
|
||||
to the suggested udev rule so that bmaptool doesn't try to check every
|
||||
partition of the block device, just the whole device.
|
||||
|
||||
Otherwise the following will appear in the logs:
|
||||
|
||||
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
|
||||
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
|
||||
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
|
||||
[... and so on for every partition on your block device ...]
|
||||
|
||||
Upstream-Status: Submitted [https://github.com/intel/bmap-tools/pull/131]
|
||||
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
|
||||
---
|
||||
bmaptools/BmapCopy.py | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/bmaptools/BmapCopy.py b/bmaptools/BmapCopy.py
|
||||
index b1e8e0fcbdb7..a4c1177246a9 100644
|
||||
--- a/bmaptools/BmapCopy.py
|
||||
+++ b/bmaptools/BmapCopy.py
|
||||
@@ -906,7 +906,8 @@ class BmapBdevCopy(BmapCopy):
|
||||
"\n"
|
||||
'ACTION=="add", SUBSYSTEMS=="usb", ATTRS{idVendor}=="xxxx", '
|
||||
'ATTRS{idProduct}=="xxxx", TAG+="uaccess"\n'
|
||||
- 'SUBSYSTEMS=="usb", ATTRS{idVendor}=="xxxx", '
|
||||
+ 'SUBSYSTEMS=="usb", ENV{DEVTYPE}!="partition", '
|
||||
+ 'ATTRS{idVendor}=="xxxx", '
|
||||
'ATTRS{idProduct}=="xxxx", ATTR{bdi/min_ratio}="0", '
|
||||
'ATTR{bdi/max_ratio}="1", ATTR{queue/scheduler}="none"\n'
|
||||
"\n"
|
||||
--
|
||||
2.43.0.76.g1a87c842ece3
|
||||
|
||||
Reference in New Issue
Block a user