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

[storage] marks actual devices as protected/unprotected (avoids ancestors) #5243

Closed
wants to merge 1 commit into from

Conversation

T0MASD
Copy link

@T0MASD T0MASD commented Oct 10, 2023

As of F39, the protection of the device where live media is located was hardened. This breaks installation for users with single drives who want to boot their live media from an iso file or from a logical volume, or want to install fedora to the available space on the external drive they booted the live environment from.

This change marks actual devices as protected/unprotected (avoids ancestors)

@poncovka
Copy link
Contributor

For the context, the change happened at 71dda92 and it allowed to unify protection of devices across the installer. That means that no disk that provides an installation source (live images/packages/...) can be used for the installation.

If we want to support this use case, it should be enabled only for live devices. That means that only this part should be modified: https://github.com/rhinstaller/anaconda/blob/553cdb180f26393f024c0c967ef08078c31ce99d/pyanaconda/modules/storage/devicetree/model.py#L301C1-L305

The protection of the live device's parent devices was implemented at e35a921 and the comment at https://bugzilla.redhat.com/show_bug.cgi?id=738964#c38 clearly talks about protecting disks. Therefore, I still think that we never officially supported using one disk for booting the live environment and the installation at the same time.

@T0MASD
Copy link
Author

T0MASD commented Oct 10, 2023

Thanks for taking a look. I don't agree https://github.com/rhinstaller/anaconda/blob/553cdb180f26393f024c0c967ef08078c31ce99d/pyanaconda/modules/storage/devicetree/model.py#L301C1-L305 is the place to make the change because you want to protect live media device(partition) from teardown. The issue here is that _mark_protected_device is making the device(partition) and its ancestors(partitions and disks) protected, imho it should only be making the protection on the actual device(partition), if ancestors need protection from teardown, then they should be collected in _mark_protected_devices, this is only for when media is in hard drive partition because iso protection already happens

to clarify the protection from the teardown in this context means protection from the device being unmounted before installation runs

@T0MASD T0MASD force-pushed the patch-1 branch 3 times, most recently from add3129 to 3dd4545 Compare October 10, 2023 14:05
@T0MASD T0MASD changed the title [storage] skip protection of the last ancestor, which matches the disk. [storage] marks actual disks as protected/unprotected (avoids ancestors) Oct 10, 2023
@T0MASD T0MASD changed the title [storage] marks actual disks as protected/unprotected (avoids ancestors) [storage] marks actual devices as protected/unprotected (avoids ancestors) Oct 10, 2023
…tors)

As of F39, the protection of the device where live media is located  was hardened. This breaks installation for users with single disks who want to boot their live media from an iso file or from a logical volume.

This change skips protection/unprotection of the ancestors and works directly on the passed device
@AdamWill
Copy link
Contributor

if you look through git history, there's a lot of previous history around protecting the live device and its parents. should probably look through all that before changing anything here.

@T0MASD T0MASD closed this Oct 18, 2023
@T0MASD
Copy link
Author

T0MASD commented Oct 18, 2023

I'm closing this PR because I see it's not welcome here. Have fun

@AdamWill
Copy link
Contributor

It's not that it's not wanted, just that it's not a simple change and we would need to think through the consequences and perhaps do it differently to achieve the desired result. The discussion on the bug report is around whether it's a release-blocking bug or not. That's not the same as "do we want to make this work if we safely can".

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

Successfully merging this pull request may close these issues.

3 participants