Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support future kernel scrub output #2342

Closed
phillxnet opened this issue Jan 7, 2022 · 6 comments
Closed

Support future kernel scrub output #2342

phillxnet opened this issue Jan 7, 2022 · 6 comments
Assignees

Comments

@phillxnet
Copy link
Member

phillxnet commented Jan 7, 2022

Thanks to forum member dunkelfalke for highlighting this issue. If one runs a non default kernel e.g. a Stable backports kernel such as is referenced in the following pull request:
"Remarked Kernel_Head & Kernel_stable_Backport + filesystems repos ..."
See: rockstor/rockstor-installer#88
The btrfs scrub status output is radically changed. This in turn throws our scrub reporting and ends up in a database error akin to:

DataError: integer out of range

As detailed in the following forum thread:
https://forum.rockstor.com/t/integer-out-of-range/8183

[EDIT]
From a mostly functional point of view this issue has been addressed: see the comment copied from below:

Linking to potentially related work-done in the interim:

Scrub UI - Integer Out of Range #2397 #2398 @Hooverdan96

And to a prior adaptation on scrub format changes:

BTRFS SCRUB status enhancements for rockstor #2157 @ubenmackin @FroggyFlox

But is now awaiting the initially missed test coverage: See the following linked pull request below for this connection.
Refactor scrub status parsing to enhance/enable testing #2342 #2493

@phillxnet
Copy link
Member Author

Example scrub output from a default openSUSE Leap 15.3 kernel:

rleap15-3:~ # btrfs scrub status /
scrub status for 475bbc39-01c8-4cdd-bb22-de07b40f7e13
        scrub started at Mon Jan  3 11:50:00 2022 and finished after 00:00:20
        total bytes scrubbed: 4.45GiB with 0 errors
rleap15-3:~ # uname -a
Linux rleap15-3 5.3.18-59.37-default #1 SMP Mon Nov 22 12:29:04 UTC 2021 (d10168e) x86_64 x86_64 x86_64 GNU/Linux

And example 5.15 kernel scrub output, as supplied by dunkelfalke

btrfs scrub status /mnt2/Helm
UUID:             7ad2dd67-7c38-4260-84ae-dce3843b717b
Scrub started:    Mon Jan  3 18:29:55 2022
Status:           finished
Duration:         4:58:07
Total to scrub:   15.18TiB
Rate:             879.79MiB/s
Error summary:    no errors found

As such our parsing function will require an additional 'personality' to switch to if needed:
fs/btrfs.py:def scrub_status(pool):

@phillxnet phillxnet added this to the First Stable Poetry build milestone Jan 17, 2023
@phillxnet
Copy link
Member Author

Linking to potentially related work-done in the interim:

Scrub UI - Integer Out of Range #2397 #2398 @Hooverdan96

And to a prior adaptation on scrub format changes:

BTRFS SCRUB status enhancements for rockstor #2157 @ubenmackin @FroggyFlox

@phillxnet phillxnet self-assigned this Feb 8, 2023
@phillxnet
Copy link
Member Author

Taking a fresh look re my comment here: https://forum.rockstor.com/t/integer-out-of-range/8183/12

...that we don’t have test coverage of the most recent additions to that rather critical function. So I’ll leave the associated issue open where we can also add test data from the latest kernel.

The referenced issue is this one!

Our test coverage is actually likely broken due to the addition of a new run_command added in the last major change reverenced earlier:
The existing code now checks (intra procedure via an additional run_command()) for the btrfs-progs version. This check, in our now outdated tests, will actually be perfomed on what is intended as test data for the prior, and then only, run_command mocking.

self.patch_run_command = patch("fs.btrfs.run_command")

Existing 'personalities' for our existing scrub_status() function:

if parse_version(btrfsProgsVers) < parse_version("v5.1.2"):
...
else
... # newer btrfsProgsVersion

It looks like we need to move-on (deprecate) legacy progs version support if we are not to build up an ever longer list of variations here. At least until we have a json output to process, or post our move to libbtrfsutil.

@phillxnet
Copy link
Member Author

As of comment date:

Stable Kernel Backport (and filesystem) as per: https://rockstor.com/docs/howtos/stable_kernel_backport.html

As performed on a Leap 15.4 base.

btrfs version
btrfs-progs v6.1.3

Default Leap 15.4 fully updated:

btrfs version
btrfs-progs v5.14

Now EOL Leap 15.3:

btrfs version
btrfs-progs v4.19.1

@phillxnet
Copy link
Member Author

Currently re-factoring scrub status code to address the follow-up suggestions in the review of the last major change in this area:

BTRFS SCRUB status enhancements for rockstor #2157 @ubenmackin @FroggyFlox

And developing the unit tests that were not submitted with those changes, in order that we can return, before our next stable update, to having better unit test coverage of this newer btrfs-progs output and our additional newer feature of extracting from non raw output also the ETA etc.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Feb 10, 2023
Previous scrub status enhancements lacked test coverage.
By refactoring our scrub status parsing mechanisms we
can return almost full test coverage to this capability.

Replaces all-in-one scrub_status() with a wrapper that invokes:
1. scrub_status_raw() 'btrfs scrub status -R' parser.
2. scrub_status_extra() non '-R' variant of 1.
Combining the results there-after.

## Includes
- Replacing pkg_resources (setuptools) 'parse_version' with
custom btrfsprogs_legacy() function.
- Tests for btrfsprogs_legacy() and previously missing
scrub status parsing extensions now refactored into
(1.) and (2.) above.
phillxnet added a commit that referenced this issue Feb 16, 2023
…rub_output

Refactor scrub status parsing to enhance/enable testing #2342
@phillxnet
Copy link
Member Author

Closing as having its last outstanding elements: testability & mostly full tests addressed via:
Fixed by #2493

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant