From c3ec690318a52adf04d37078f426fbeb4182a60e Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Mon, 7 Mar 2022 23:49:15 -0700 Subject: [PATCH 1/9] Add new ddrescue argument options Addresses #184 --- scripts/wk/cfg/ddrescue.py | 4 ++++ scripts/wk/hw/ddrescue.py | 34 +++++++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/scripts/wk/cfg/ddrescue.py b/scripts/wk/cfg/ddrescue.py index 738a1c8c..6e3459b8 100644 --- a/scripts/wk/cfg/ddrescue.py +++ b/scripts/wk/cfg/ddrescue.py @@ -23,15 +23,18 @@ AUTO_PASS_THRESHOLDS = { DDRESCUE_SETTINGS = { 'Default': { '--binary-prefixes': {'Selected': True, 'Hidden': True, }, + '--complete-only': {'Selected': True, 'Hidden': True, }, '--data-preview': {'Selected': True, 'Value': '5', 'Hidden': True, }, '--idirect': {'Selected': True, }, '--odirect': {'Selected': True, }, + '--input-position': {'Selected': False, 'Value': '0', }, '--max-error-rate': {'Selected': True, 'Value': '100MiB', }, '--max-read-rate': {'Selected': False, 'Value': '1MiB', }, '--min-read-rate': {'Selected': True, 'Value': '64KiB', }, '--reopen-on-error': {'Selected': True, }, '--retry-passes': {'Selected': True, 'Value': '0', }, '--reverse': {'Selected': False, }, + '--skip-size': {'Selected': True, 'Value': '0.0001,0.01', }, # Percentages of source size '--test-mode': {'Selected': False, 'Value': 'test.map', }, '--timeout': {'Selected': True, 'Value': '30m', }, '-vvvv': {'Selected': True, 'Hidden': True, }, @@ -46,6 +49,7 @@ DDRESCUE_SETTINGS = { '--max-read-rate': {'Selected': True, 'Value': '64MiB', }, '--min-read-rate': {'Selected': True, 'Value': '1KiB', }, '--reopen-on-error': {'Selected': True, }, + '--skip-size': {'Selected': True, 'Value': '0.001,0.05', }, # Percentages of source size '--timeout': {'Selected': False, 'Value': '30m', }, }, } diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index 3e441127..391a4dcc 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -67,6 +67,7 @@ DDRESCUE_LOG_REGEX = re.compile( r'.*\(\s*(?P\d+\.?\d*)%\)$', re.IGNORECASE, ) +INITIAL_SKIP_MIN = 64 * 1024 # This is ddrescue's minimum accepted value REGEX_REMAINING_TIME = re.compile( r'remaining time:' r'\s*((?P\d+)d)?' @@ -1202,7 +1203,7 @@ def build_block_pair_report(block_pairs, settings): return report -def build_ddrescue_cmd(block_pair, pass_name, settings): +def build_ddrescue_cmd(block_pair, pass_name, settings_menu): """Build ddrescue cmd using passed details, returns list.""" cmd = ['sudo', 'ddrescue'] if (block_pair.destination.is_block_device() @@ -1216,8 +1217,30 @@ def build_ddrescue_cmd(block_pair, pass_name, settings): elif pass_name == 'scrape': # Allow trimming and scraping pass - cmd.extend(settings) - cmd.append(f'--size={block_pair.size}') + + # Fix domain size based on starting position + domain_size = block_pair.size + if settings_menu.options['--input-position']['Selected']: + settings_menu.options['--reverse']['Selected'] = False + input_position = std.string_to_bytes( + settings_menu.options['--input-position']['Value'], + ) + domain_size -= input_position + cmd.append(f'--size={domain_size}') + + # Determine skip sizes + if settings_menu.options['--skip-size']['Selected']: + skip_sizes = settings_menu.options['--skip-size']['Value'].split(',') + skip_sizes = [float(s) for s in skip_sizes] + initial_skip = min(INITIAL_SKIP_MIN, int(block_pair.size * skip_sizes[0])) + max_skip = min(int(block_pair.size * skip_sizes[1]), domain_size) + cmd.append(f'--skip-size={initial_skip},{max_skip}') + cmd.extend(get_ddrescue_settings(settings_menu)) + + # Add source physical sector size (if possible) + cmd.append(f'--sector-size={block_pair.source.details.get("phy-sec", 512)}') + + # Add block pair and map file if PLATFORM == 'Darwin': # Use Raw disks if possible for dev in (block_pair.source, block_pair.destination): @@ -1569,6 +1592,8 @@ def get_ddrescue_settings(settings_menu): # Check menu selections for name, details in settings_menu.options.items(): + if name == '--skip-size': + continue if details['Selected']: if 'Value' in details: settings.append(f'{name}={details["Value"]}') @@ -2130,7 +2155,6 @@ def run_recovery(state, main_menu, settings_menu, dry_run=True): if 'Retry' in name and details['Selected']: details['Selected'] = False state.retry_all_passes() - settings = get_ddrescue_settings(settings_menu) # Start SMART/Journal state.panes['SMART'] = tmux.split_window( @@ -2158,7 +2182,7 @@ def run_recovery(state, main_menu, settings_menu, dry_run=True): attempted_recovery = True state.mark_started() try: - run_ddrescue(state, pair, pass_name, settings, dry_run=dry_run) + run_ddrescue(state, pair, pass_name, settings_menu, dry_run=dry_run) except (FileNotFoundError, KeyboardInterrupt, std.GenericAbort): is_missing_source_or_destination(state) abort = True From ea9e3b36961fb14e2aeb17fbc0d32e2f95d5d25e Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Tue, 8 Mar 2022 11:52:04 -0700 Subject: [PATCH 2/9] Disable broken --sector-size argument Addresses #184 --- scripts/wk/hw/ddrescue.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index 391a4dcc..a5f0d05d 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -1238,7 +1238,8 @@ def build_ddrescue_cmd(block_pair, pass_name, settings_menu): cmd.extend(get_ddrescue_settings(settings_menu)) # Add source physical sector size (if possible) - cmd.append(f'--sector-size={block_pair.source.details.get("phy-sec", 512)}') + # TODO: Fix + #cmd.append(f'--sector-size={block_pair.source.details.get("phy-sec", 512)}') # Add block pair and map file if PLATFORM == 'Darwin': From 9d2eb8b1754c0a9a15291d92986ae677a3f61191 Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Tue, 8 Mar 2022 11:53:56 -0700 Subject: [PATCH 3/9] Fix initial and max skip sizes ddrescue's minimum is 64KiB so we should respect that. Addresses #184 --- scripts/wk/hw/ddrescue.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index a5f0d05d..2b7d5c4c 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -1232,8 +1232,9 @@ def build_ddrescue_cmd(block_pair, pass_name, settings_menu): if settings_menu.options['--skip-size']['Selected']: skip_sizes = settings_menu.options['--skip-size']['Value'].split(',') skip_sizes = [float(s) for s in skip_sizes] - initial_skip = min(INITIAL_SKIP_MIN, int(block_pair.size * skip_sizes[0])) + initial_skip = max(INITIAL_SKIP_MIN, int(block_pair.size * skip_sizes[0])) max_skip = min(int(block_pair.size * skip_sizes[1]), domain_size) + max_skip = max(INITIAL_SKIP_MIN, max_skip) cmd.append(f'--skip-size={initial_skip},{max_skip}') cmd.extend(get_ddrescue_settings(settings_menu)) From b82493b12b3a6d88c8b0ce56bff55832915ff84a Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Tue, 8 Mar 2022 11:55:23 -0700 Subject: [PATCH 4/9] Generate new map files when starting a recovery This is done to define the domain size and let us use --complete-only. This also enables us to open ddrescueview immediately since that tool requires a valid map file from the start. If you open an empty map file ddrescueview doesn't auto-reload the file correctly. Addresses #184 --- scripts/wk/cfg/ddrescue.py | 4 ++++ scripts/wk/hw/ddrescue.py | 33 +++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/scripts/wk/cfg/ddrescue.py b/scripts/wk/cfg/ddrescue.py index 6e3459b8..f40cf70c 100644 --- a/scripts/wk/cfg/ddrescue.py +++ b/scripts/wk/cfg/ddrescue.py @@ -20,6 +20,10 @@ AUTO_PASS_THRESHOLDS = { 'trim': 98, 'scrape': float('inf'), } +DDRESCUE_MAP_TEMPLATE = '''# Mapfile. Created by {name} +0x0 ? 1 +0x0 {size:#x} ? +''' DDRESCUE_SETTINGS = { 'Default': { '--binary-prefixes': {'Selected': True, 'Hidden': True, }, diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index 2b7d5c4c..b907c871 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -22,7 +22,7 @@ import psutil import pytz from wk import cfg, debug, exe, io, log, net, std, tmux -from wk.cfg.ddrescue import DDRESCUE_SETTINGS +from wk.cfg.ddrescue import DDRESCUE_MAP_TEMPLATE, DDRESCUE_SETTINGS from wk.hw import obj as hw_obj @@ -139,7 +139,7 @@ class BlockPair(): self.view_proc = None - # Set map file + # Set map path # e.g. '(Clone|Image)_Model[_p#]_Size[_Label].map' map_name = model if model else 'None' if source.details['bus'] == 'Image': @@ -148,7 +148,7 @@ class BlockPair(): part_num = re.sub(r"^.*?(\d+)$", r"\1", source.path.name) map_name += f'_p{part_num}' size_str = std.bytes_to_string( - size=source.details["size"], + size=self.size, use_binary=False, ) map_name += f'_{size_str.replace(" ", "")}' @@ -164,7 +164,17 @@ class BlockPair(): else: # Cloning self.map_path = pathlib.Path(f'{working_dir}/Clone_{map_name}.map') - self.map_path.touch() + + # Create map file if needed + # NOTE: We need to set the domain size for --complete-only to work + if not self.map_path.exists(): + self.map_path.write_text( + data=DDRESCUE_MAP_TEMPLATE.format( + name=cfg.main.KIT_NAME_FULL, + size=self.size, + ), + encoding='utf-8', + ) # Set initial status self.set_initial_status() @@ -2044,8 +2054,12 @@ def run_ddrescue(state, block_pair, pass_name, settings, dry_run=True): LOG.info('ddrescue cmd: %s', cmd) return - # Start ddrescue + # Start ddrescue and ddrescueview proc = exe.popen_program(cmd) + exe.popen_program( + ['ddrescueview', '-r', '5s', block_pair.map_path], + pipe=True, + ) # ddrescue loop _i = 0 @@ -2062,15 +2076,6 @@ def run_ddrescue(state, block_pair, pass_name, settings, dry_run=True): std.print_error(warning_message) break - # Open ddrescueview - ## NOTE: This needs to be started a bit into the recovery since it needs - ## a non-zero map file to read - if not block_pair.view_proc and _i > 1: - block_pair.view_proc = exe.popen_program( - ['ddrescueview', '-r', '5s', block_pair.map_path], - pipe=True, - ) - if _i % 60 == 0: # Clear ddrescue pane tmux.clear_pane() From b66f25dfea8446bea9ea93e149093fe029da6944 Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Tue, 8 Mar 2022 12:36:49 -0700 Subject: [PATCH 5/9] Only open ddrescueview if running with a DISPLAY --- scripts/wk/hw/ddrescue.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index b907c871..0e2bebc7 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -133,11 +133,7 @@ class BlockPair(): 'trim': 'Pending', 'scrape': 'Pending', }) - self.view_proc = 'Disabled' - if 'DISPLAY' in os.environ or 'WAYLAND_DISPLAY' in os.environ: - # Enable opening ddrescueview during recovery - self.view_proc = None - + self.view_map = 'DISPLAY' in os.environ or 'WAYLAND_DISPLAY' in os.environ # Set map path # e.g. '(Clone|Image)_Model[_p#]_Size[_Label].map' @@ -2054,12 +2050,13 @@ def run_ddrescue(state, block_pair, pass_name, settings, dry_run=True): LOG.info('ddrescue cmd: %s', cmd) return - # Start ddrescue and ddrescueview + # Start ddrescue and ddrescueview (if enabled) proc = exe.popen_program(cmd) - exe.popen_program( - ['ddrescueview', '-r', '5s', block_pair.map_path], - pipe=True, - ) + if block_pair.view_map: + exe.popen_program( + ['ddrescueview', '-r', '5s', block_pair.map_path], + pipe=True, + ) # ddrescue loop _i = 0 From 4e61025e995545ac3374ec0c2f0a6997e334d535 Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Tue, 8 Mar 2022 13:02:33 -0700 Subject: [PATCH 6/9] Fix --sector-size argument --- scripts/wk/hw/ddrescue.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index 0e2bebc7..fc3724dc 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -117,12 +117,14 @@ TIMEZONE = pytz.timezone(cfg.main.LINUX_TIME_ZONE) # Classes class BlockPair(): """Object for tracking source to dest recovery data.""" + # pylint: disable=too-many-instance-attributes def __init__(self, source, destination, model, working_dir): """Initialize BlockPair() NOTE: source should be a wk.hw.obj.Disk() object and destination should be a pathlib.Path() object. """ + self.sector_size = source.details.get('phy-sec', 512) self.source = source.path self.destination = destination self.map_data = {} @@ -1245,8 +1247,7 @@ def build_ddrescue_cmd(block_pair, pass_name, settings_menu): cmd.extend(get_ddrescue_settings(settings_menu)) # Add source physical sector size (if possible) - # TODO: Fix - #cmd.append(f'--sector-size={block_pair.source.details.get("phy-sec", 512)}') + cmd.append(f'--sector-size={block_pair.sector_size}') # Add block pair and map file if PLATFORM == 'Darwin': From 817cfc3de7bb29a0ddd711e104ecb6294a88f633 Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Fri, 25 Mar 2022 15:48:00 -0600 Subject: [PATCH 7/9] Skip --reopen-on-error by default for all presets --- scripts/wk/cfg/ddrescue.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/wk/cfg/ddrescue.py b/scripts/wk/cfg/ddrescue.py index f40cf70c..27fd5a26 100644 --- a/scripts/wk/cfg/ddrescue.py +++ b/scripts/wk/cfg/ddrescue.py @@ -35,7 +35,7 @@ DDRESCUE_SETTINGS = { '--max-error-rate': {'Selected': True, 'Value': '100MiB', }, '--max-read-rate': {'Selected': False, 'Value': '1MiB', }, '--min-read-rate': {'Selected': True, 'Value': '64KiB', }, - '--reopen-on-error': {'Selected': True, }, + '--reopen-on-error': {'Selected': False, }, '--retry-passes': {'Selected': True, 'Value': '0', }, '--reverse': {'Selected': False, }, '--skip-size': {'Selected': True, 'Value': '0.0001,0.01', }, # Percentages of source size @@ -46,13 +46,11 @@ DDRESCUE_SETTINGS = { 'Fast': { '--max-error-rate': {'Selected': True, 'Value': '32MiB', }, '--min-read-rate': {'Selected': True, 'Value': '1MiB', }, - '--reopen-on-error': {'Selected': False, }, '--timeout': {'Selected': True, 'Value': '5m', }, }, 'Safe': { '--max-read-rate': {'Selected': True, 'Value': '64MiB', }, '--min-read-rate': {'Selected': True, 'Value': '1KiB', }, - '--reopen-on-error': {'Selected': True, }, '--skip-size': {'Selected': True, 'Value': '0.001,0.05', }, # Percentages of source size '--timeout': {'Selected': False, 'Value': '30m', }, }, From 4817fe6d1ff8bcee821595194d3723e52f7e6107 Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Fri, 25 Mar 2022 16:53:29 -0600 Subject: [PATCH 8/9] Use larger --skip-size by default --- scripts/wk/cfg/ddrescue.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/wk/cfg/ddrescue.py b/scripts/wk/cfg/ddrescue.py index 27fd5a26..02b7de36 100644 --- a/scripts/wk/cfg/ddrescue.py +++ b/scripts/wk/cfg/ddrescue.py @@ -38,7 +38,7 @@ DDRESCUE_SETTINGS = { '--reopen-on-error': {'Selected': False, }, '--retry-passes': {'Selected': True, 'Value': '0', }, '--reverse': {'Selected': False, }, - '--skip-size': {'Selected': True, 'Value': '0.0001,0.01', }, # Percentages of source size + '--skip-size': {'Selected': True, 'Value': '0.001,0.05', }, # Percentages of source size '--test-mode': {'Selected': False, 'Value': 'test.map', }, '--timeout': {'Selected': True, 'Value': '30m', }, '-vvvv': {'Selected': True, 'Hidden': True, }, @@ -51,7 +51,6 @@ DDRESCUE_SETTINGS = { 'Safe': { '--max-read-rate': {'Selected': True, 'Value': '64MiB', }, '--min-read-rate': {'Selected': True, 'Value': '1KiB', }, - '--skip-size': {'Selected': True, 'Value': '0.001,0.05', }, # Percentages of source size '--timeout': {'Selected': False, 'Value': '30m', }, }, } From 8dd8701e8d35e2253918c26fba8282cb262c1e7a Mon Sep 17 00:00:00 2001 From: 2Shirt <2xShirt@gmail.com> Date: Fri, 25 Mar 2022 18:45:28 -0600 Subject: [PATCH 9/9] Split read phase into two parts Addresses issue #184 The first read phase will skip a lot more to try to recover more data from the whole source. Then the second read phase will fill in like the previous configuration. --- scripts/wk/cfg/ddrescue.py | 12 ++++++++--- scripts/wk/hw/ddrescue.py | 41 ++++++++++++++++++-------------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/scripts/wk/cfg/ddrescue.py b/scripts/wk/cfg/ddrescue.py index 02b7de36..15f5f46d 100644 --- a/scripts/wk/cfg/ddrescue.py +++ b/scripts/wk/cfg/ddrescue.py @@ -16,9 +16,10 @@ TMUX_LAYOUT = OrderedDict({ # ddrescue AUTO_PASS_THRESHOLDS = { # NOTE: The scrape key is set to infinity to force a break - 'read': 95, - 'trim': 98, - 'scrape': float('inf'), + 'read-skip': 50, + 'read-full': 95, + 'trim': 98, + 'scrape': float('inf'), } DDRESCUE_MAP_TEMPLATE = '''# Mapfile. Created by {name} 0x0 ? 1 @@ -54,6 +55,11 @@ DDRESCUE_SETTINGS = { '--timeout': {'Selected': False, 'Value': '30m', }, }, } +DDRESCUE_SPECIFIC_PASS_SETTINGS = { + 'read-skip': ['--no-scrape', '--no-trim', '--cpass=1,2'], + 'read-full': ['--no-scrape', '--no-trim'], + 'trim': ['--no-scrape'], + } DRIVE_POWEROFF_TIMEOUT = 90 PARTITION_TYPES = { 'GPT': { diff --git a/scripts/wk/hw/ddrescue.py b/scripts/wk/hw/ddrescue.py index fc3724dc..e457dfe2 100644 --- a/scripts/wk/hw/ddrescue.py +++ b/scripts/wk/hw/ddrescue.py @@ -22,7 +22,11 @@ import psutil import pytz from wk import cfg, debug, exe, io, log, net, std, tmux -from wk.cfg.ddrescue import DDRESCUE_MAP_TEMPLATE, DDRESCUE_SETTINGS +from wk.cfg.ddrescue import ( + DDRESCUE_MAP_TEMPLATE, + DDRESCUE_SETTINGS, + DDRESCUE_SPECIFIC_PASS_SETTINGS, + ) from wk.hw import obj as hw_obj @@ -105,11 +109,11 @@ SETTING_PRESETS = ( 'Safe', ) STATUS_COLORS = { - 'Passed': 'GREEN', - 'Aborted': 'YELLOW', - 'Skipped': 'YELLOW', - 'Working': 'YELLOW', - 'ERROR': 'RED', + 'Passed': 'GREEN', + 'Aborted': 'YELLOW', + 'Skipped': 'YELLOW', + 'Working': 'YELLOW', + 'ERROR': 'RED', } TIMEZONE = pytz.timezone(cfg.main.LINUX_TIME_ZONE) @@ -131,9 +135,10 @@ class BlockPair(): self.map_path = None self.size = source.details['size'] self.status = OrderedDict({ - 'read': 'Pending', - 'trim': 'Pending', - 'scrape': 'Pending', + 'read-skip': 'Pending', + 'read-full': 'Pending', + 'trim': 'Pending', + 'scrape': 'Pending', }) self.view_map = 'DISPLAY' in os.environ or 'WAYLAND_DISPLAY' in os.environ @@ -305,10 +310,9 @@ class BlockPair(): # Mark future passes as skipped if applicable if percent == 100: - if pass_name == 'read': - self.status['trim'] = 'Skipped' - if pass_name in ('read', 'trim'): - self.status['scrape'] = 'Skipped' + status_keys = list(self.status.keys()) + for i in status_keys[status_keys.index(pass_name)+1:]: + self.status[status_keys[i]] = 'Skipped' class State(): @@ -1217,14 +1221,7 @@ def build_ddrescue_cmd(block_pair, pass_name, settings_menu): if (block_pair.destination.is_block_device() or block_pair.destination.is_char_device()): cmd.append('--force') - if pass_name == 'read': - cmd.extend(['--no-trim', '--no-scrape']) - elif pass_name == 'trim': - # Allow trimming - cmd.append('--no-scrape') - elif pass_name == 'scrape': - # Allow trimming and scraping - pass + cmd.extend(DDRESCUE_SPECIFIC_PASS_SETTINGS.get(pass_name, [])) # Fix domain size based on starting position domain_size = block_pair.size @@ -2172,7 +2169,7 @@ def run_recovery(state, main_menu, settings_menu, dry_run=True): ) # Run pass(es) - for pass_name in ('read', 'trim', 'scrape'): + for pass_name in ('read-skip', 'read-full', 'trim', 'scrape'): abort = False # Skip to next pass