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.

Sign up for GitHub

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

Jump to bottom

Fix layout mapping #2126

Merged
merged 3 commits into from
Apr 30, 2019
Merged

Fix layout mapping #2126

merged 3 commits into from
Apr 30, 2019

Conversation

OliverO2
Copy link
Contributor

@OliverO2 OliverO2 commented Apr 28, 2019 •
edited

  • Type: Bug Fix

  • Impact: Normal

  • Reference to related issue (URL): none

  • How was this pull request tested? Ran rear mkrescue and rear recover on Ubuntu 18.04.2 LTS server before and after patching. Verified fix works as intended.

  • Brief description of the changes in this pull request:

This PR corrects a mapping error when TCG Opal 2 self-encrypting disks were present but had to be excluded due to a non-existent disk during recovery. This exclusion was not handled properly.

(Edit: dropped second change)

jsmeix
jsmeix requested changes Apr 29, 2019
View reviewed changes
Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/rear/rear/pull/2126/files#r279284701

@jsmeix jsmeix self-assigned this Apr 29, 2019
@jsmeix jsmeix added this to the ReaR v2.6 milestone Apr 29, 2019
@jsmeix
Copy link
Member

jsmeix commented Apr 29, 2019 •
edited

@OliverO2
I assume this issue hapens only when you are in MIGRATION_MODE
because the apply_layout_mappings function is only called there.

I would like to better understand what actually goes on on your
particular system because 'an unmapped disk was present' looks like
things are wrong because - as far as I can imagine offhandedly - there
must not be any unmapped disk left, see my comments in the
apply_layout_mappings function that show an example where
the _REAR*_ string disk _REAR3_ is left which is then
correctly detected and reported by "Step 3".

@OliverO2
Revert "Fix 'Failed to apply layout mappings' complaining about unmap…
cff1fce 
…ped disks"

This reverts commit b3f1e02

Reason: The fix is unnecessary as the problem seemed to be caused
by a bug concerning the exclusion of of TCG Opal 2 self-encrypting
disks (now fixed by commit fbfb9d6).
jsmeix
jsmeix approved these changes Apr 30, 2019
View reviewed changes
Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is only addons for "opaldisk" type of devices and
from plain looking at the code I see no possible regressions
so that from my point of view it looks o.k. now.

@jsmeix jsmeix requested a review from gdha April 30, 2019 07:37
@jsmeix
Copy link
Member

jsmeix commented Apr 30, 2019

@gdha
could you also have a look here and review it?

Do you think we can add it to ReaR 2.5?

@jsmeix jsmeix added the bug The code does not do what it is meant to do label Apr 30, 2019
@gdha gdha modified the milestones: ReaR v2.6, ReaR v2.5 Apr 30, 2019
@gdha
Copy link
Member

gdha commented Apr 30, 2019

@jsmeix I'm fine for release 2.5,
@OliverO2 Thank you for improving rear code base.

@jsmeix jsmeix merged commit 02612fa into rear:master Apr 30, 2019
@jsmeix
Copy link
Member

jsmeix commented Apr 30, 2019

Merged and listed in the ReaR 2.5 release notes via
rear/rear.github.com@a78033b

@OliverO2 OliverO2 deleted the fix-layout-mapping branch November 15, 2019 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsmeix jsmeix jsmeix approved these changes

@gdha gdha gdha approved these changes

Assignees

@jsmeix jsmeix

Labels
bug The code does not do what it is meant to do fixed / solved / done
Projects
None yet
Milestone
ReaR v2.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@OliverO2 @jsmeix @gdha