Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1033778 - Resizing of volumes with unknown or no filesystem is allowed in guided partitioning with no special warning
Summary: Resizing of volumes with unknown or no filesystem is allowed in guided partit...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: python-blivet
Version: 25
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: David Lehman
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: RejectedBlocker https://fedoraproject...
Depends On: 1039724
Blocks: F26AlphaFreezeException
TreeView+ depends on / blocked
 
Reported: 2013-11-22 21:11 UTC by Chris Murphy
Modified: 2019-08-19 07:37 UTC (History)
15 users (show)

Fixed In Version: python-blivet-2.1.7-8.fc26
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-03-29 05:05:23 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
screen shot showing resizeable core storage partition (78.51 KB, image/png)
2013-11-22 21:11 UTC, Chris Murphy
no flags Details
anaconda.log (5.04 KB, text/plain)
2013-11-22 21:11 UTC, Chris Murphy
no flags Details
program.log (25.63 KB, text/plain)
2013-11-22 21:12 UTC, Chris Murphy
no flags Details
storage.log (205.44 KB, text/plain)
2013-11-22 21:12 UTC, Chris Murphy
no flags Details
storage.state (24.00 KB, application/octet-stream)
2013-11-22 21:12 UTC, Chris Murphy
no flags Details
anaconda-23 screenshot of problem (73.65 KB, image/png)
2016-02-08 19:04 UTC, Chris Murphy
no flags Details
screenshot install alongside Windows 10 Bitlocker'd (118.11 KB, image/png)
2017-03-07 00:36 UTC, Chris Murphy
no flags Details

Description Chris Murphy 2013-11-22 21:11:18 UTC
Created attachment 828006 [details]
screen shot showing resizeable core storage partition

Description of problem:


Version-Release number of selected component (if applicable):
anaconda-20.25.8-1.fc20.x86_64

How reproducible:
Always

Steps to Reproduce:
1. On OS X, enabled FileVault 2 and allow the encryption process to complete.
2. Reboot and launch Fedora installer, use guided partitioning.

Actual results:
sda2 is listed as resizeable, although the resizing slider isn't available

Expected results:
Should be listed as "not resizeable"

Additional info:
- Manual Partitioning doesn't see it as resizeable.
- If the GPT entry for an existing btrfs formatted partition has partitiontypeGUID altered to match Apple Core Storage GUID, anaconda still doesn't see it as resizeable.
- If the encryption process isn't allowed to complete, anaconda doesn't see the volume as resizeable.

So there's some metadata change at the end of the encryption process that anaconda interprets as making it a resizeable volume. Instead if probably should filter for the Apple Core Storage partitiontypeGUID and always flag it as Not resizeable until such time as there's support for resizing them.

Apple Core Storage is analogous to LVM. They have an additional (optional) layer, Logical Volume Family, which presently only supports encryption. So an Apple Core Storage LV isn't necessarily encrypted. Their Fusion Drive implementation combines an SSD and HDD as PV's, and presents them as a single LV, unencrypted by default. But the same partitiontypeGUID is used in all of these cases.

The partitiontypeGUID for Apple Core Storage is 53746F72-6167-11AA-AA11-00306543ECAC

Comment 1 Chris Murphy 2013-11-22 21:11:52 UTC
Created attachment 828007 [details]
anaconda.log

Comment 2 Chris Murphy 2013-11-22 21:12:05 UTC
Created attachment 828008 [details]
program.log

Comment 3 Chris Murphy 2013-11-22 21:12:15 UTC
Created attachment 828009 [details]
storage.log

Comment 4 Chris Murphy 2013-11-22 21:12:37 UTC
Created attachment 828010 [details]
storage.state

Comment 5 Vratislav Podzimek 2013-11-28 07:47:17 UTC
The proposed patch has been rejected due to errors it would probably cause in other places. However, there are patches that should result in better information being displayed in the resize dialog. The goal is to inform user about the type based on the blkid output. Since Anaconda doesn't support the encrypted Apple Core storage, they will be marked as resizable (treated as unknown formats that can be resized), but users will have additional info helping them to avoid issues with resizing those partitions.

Comment 6 Vratislav Podzimek 2013-11-28 08:03:41 UTC
Patches that should make the behaviour described in the comment #5 work have been pushed to master branches of the anaconda and python-blivet packages. They should appear in the next rawhide builds of the packages.

Comment 7 Chris Murphy 2013-12-03 21:27:19 UTC
It seems highly risky that unknown formats are considered resizable. If they're unknown, how to we know they're resizeable? Resize≠destroy and create. There's a perception that resize operations preserve data.

Comment 8 David Lehman 2013-12-03 21:51:30 UTC
(In reply to Chris Murphy from comment #7)
> It seems highly risky that unknown formats are considered resizable. If
> they're unknown, how to we know they're resizeable? Resize≠destroy and

It is impossible to discern unknown formatting from a lack of formatting with total accuracy.

Yes, it is highly risky -- you are choosing to destroy and/or relocate data on your storage devices. I cannot make it into something without potential consequences. If you see a device that says it contains unknown formatting and you choose to resize that device without knowing what is actually on it I will ask that you take any complaints elsewhere. You did it. People have been telling you to look before you leap since you were four years old. It is not my fault if you still have not learned.

> create. There's a perception that resize operations preserve data.

Seriously? There's a perception that we know how to resize something we're calling "Unknown"? Such a perception is something I cannot fix.

Comment 9 Chris Murphy 2013-12-03 22:51:05 UTC
Why is a resize UI appropriate instead of only offering delete in the context of format unknown or none? Seems there's no advantage, yet there is are a couple disadvantages.

To you and I, a resize operation of an unknown format means any underlying data has a 100% chance of being destroyed. Therefore, I advise not enabling a resize UI if you don't know the volume can be non-destructively resized. Enabling a delete option unambiguously conveys the actual outcome to the user.

And it doesn't matter whether the (widespread) perception is flawed, or that you can't change that perception even by subjecting users to data loss as a result of their own ignorance. You could change the UI to be simpler, and very literal, so my suggestion is to be literal rather than invite ambiguous outcomes the user is unprepared for.

Comment 10 Vratislav Podzimek 2013-12-04 07:22:52 UTC
(In reply to Chris Murphy from comment #9)
> Why is a resize UI appropriate instead of only offering delete in the
> context of format unknown or none? Seems there's no advantage, yet there is
> are a couple disadvantages.
Because as dlehman has already pointed out: "It is impossible to discern unknown formatting from a lack of formatting with total accuracy." and a lack of formatting means we can resize that partition.

If you, e.g. have a crazy setup with some DBMS using let say first 2 GB of space directly on your partition without a format (known to the Anaconda installer), you can "safely" resize that partition if it's bigger. Crazy example, I know, but I don't think we should treat users as "blindly clicking monkeys" doing decisions about their data based on what the installer allows them to do or not. Especially users having those uknown formats and installing Fedora next to them will, highly probably, know, what they are doing. Just as you did.

Comment 11 Chris Murphy 2013-12-06 20:34:39 UTC
Let me qualify this better. For this bug, the main concern is that the partition in question is not unknown formatting. It has a partitiontypeGUID that clearly implies the formatting, which is formatting that isn't supported at all, let alone for resizing. Therefore a resize UI is inappropriate with this partition. That seems uncontroversial. Require a two step delete and create new for such a partition.

A secondary concern is the notion of unknown formatting inside of partitioned space with a known partitiontypeGUID. This is still not unpartitioned space. Example (gdisk short hand) type codes 0700 and 8300. This may be a gray area, but my view is the safety gained by requiring a two step delete-create new, is less intrusive than data loss. A huge, huge amount of data loss in the world is user induced.

You either want an installer UX that's akin to a chain saw that requires the user to wear safety goggles. Or you want to help keep them keep their existing data safe, especially from their own ignorance. Why else have a GUI installer? 

Allowing shrink UI for unknown formatting saves a step for some people. And it will induce data loss for others. And I think it's better to make people more clearly indicate their intention to destroy existing data, and only offer resize UI when data is being preserved.

Comment 12 Brian Lane 2013-12-09 18:23:54 UTC
While I don't completely disagree with you here, Anaconda's job is to write stuff to disk, so users had better have made backups and have at least a minimal idea of what they are doing and that bad things could happen.

In this case it looks to me like a big part of the issue is that blkid doesn't understand that the GUID on sda2 means Apple Core Storage. Blivet depends on blkid setting ID_FS_TYPE for its detection of formats, without that it looks like any other unformatted partition to us.

So the first step here is to teach blkid about it.

Comment 13 Chris Murphy 2013-12-09 21:03:30 UTC
Filed an RFE bug against util-linux.

It's possible I underestimate the knowledge, and overestimate the response, of the anaconda target market. But my experience with most users who use graphical installers, don't distinguish in any meaningful way, between OS and application installers. To them it's "installing software" and writing stuff to disk in either case. They essentially never experience data loss when installing applications. We know why, in general they don't. Minimal knowledge of what on disk structures are modified in an app install vs OS install is like the minimal knowledge of CPU pinout voltages. It's that far out of scope.

It's reasonable (i.e. not a scare tactic) for an OS installer to have a disclosure dialog stating something like "this software modifies critical on-disk structures, be sure valued data is backed up before proceeding."

Comment 14 Vratislav Podzimek 2013-12-10 08:46:18 UTC
Thanks! Once bug #1039724 is fixed, we can simply add a new format to blivet and mark it as unresizable. That's the right way of fixing things.

Comment 15 Karel Zak 2013-12-10 11:54:03 UTC
(In reply to Brian C. Lane from comment #12)
> In this case it looks to me like a big part of the issue is that blkid
> doesn't understand that the GUID on sda2 means Apple Core Storage. Blivet
> depends on blkid setting ID_FS_TYPE for its detection of formats, without
> that it looks like any other unformatted partition to us.

blkid returns ID_PART_ENTRY_TYPE= with GUID of the partition, I don't think we want to mix filesystem type (ID_FS_TYPE) and partition type in the blkid output.

> So the first step here is to teach blkid about it.

Please, try

  blkid -p -o udev <device>

Comment 16 Chris Murphy 2013-12-11 02:55:16 UTC
# blkid -p -o udev /dev/sda2
ID_PART_ENTRY_SCHEME=gpt
ID_PART_ENTRY_NAME=Fugue
ID_PART_ENTRY_UUID=9a5448a1-d9dd-48af-9655-05f7a1bf2de8
ID_PART_ENTRY_TYPE=53746f72-6167-11aa-aa11-00306543ecac
ID_PART_ENTRY_NUMBER=2
ID_PART_ENTRY_OFFSET=411648
ID_PART_ENTRY_SIZE=469762048
ID_PART_ENTRY_DISK=8:0

Comment 17 Chris Murphy 2013-12-11 03:49:41 UTC
c16 is the result on an Apple Core Storage partition. 

This is the result on an unencrypted LVM partition:

ID_FS_UUID=xrq27w-KKNv-pp0Y-kIId-Csvm-2XM9-GnVW4i
ID_FS_UUID_ENC=xrq27w-KKNv-pp0Y-kIId-Csvm-2XM9-GnVW4i
ID_FS_VERSION=LVM2\x20001
ID_FS_TYPE=LVM2_member
ID_FS_USAGE=raid
ID_PART_ENTRY_SCHEME=gpt
ID_PART_ENTRY_UUID=3c47c68a-ffdc-4a04-9f03-eba4cdbed44f
ID_PART_ENTRY_TYPE=e6d6d379-f507-44c2-a23c-238f2a3df928
ID_PART_ENTRY_NUMBER=3
ID_PART_ENTRY_OFFSET=1028096
ID_PART_ENTRY_SIZE=10736388096
ID_PART_ENTRY_DISK=252:0

And this is on an encrypted LVM partition:

ID_FS_UUID=65305ed5-337f-4422-82d2-fe3157df9fc1
ID_FS_UUID_ENC=65305ed5-337f-4422-82d2-fe3157df9fc1
ID_FS_VERSION=1
ID_FS_TYPE=crypto_LUKS
ID_FS_USAGE=crypto
ID_PART_ENTRY_SCHEME=gpt
ID_PART_ENTRY_UUID=38107f41-4aaa-4718-ae6a-783cee1a93c2
ID_PART_ENTRY_TYPE=ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
ID_PART_ENTRY_NUMBER=3
ID_PART_ENTRY_OFFSET=1028096
ID_PART_ENTRY_SIZE=10736388096
ID_PART_ENTRY_DISK=252:0

So the output already mixes ID_FS_TYPE and ID_PART_ENTRY_TYPE for those. I find it weird they're referred to as file systems, but whatever. 

It seems blkid could just spit out ID_FS_TYPE=apple_corestorage without needing to go looking for the underlying metadata in that partition. That particular partitionetypeGUID is only used for this purpose. If this is what's needed, I can reopen bug 1039724 and clarify that request.

Comment 18 Karel Zak 2013-12-12 09:06:53 UTC
(In reply to Chris Murphy from comment #17)
> So the output already mixes ID_FS_TYPE and ID_PART_ENTRY_TYPE for those. I
> find it weird they're referred to as file systems, but whatever. 

Ah, that's misunderstanding -- I thought that we don't want to set ID_FS_TYPE= according to partition type (mix up FS type with GUID). It's fine that there is ID_FS_TYPE= and ID_PART_ENTRY_TYPE= in the same output. The filesystem type is always set according to fs/raid/etc superblock, and ID_PART_ENTRY_TYPE= depends on partition table.

> It seems blkid could just spit out ID_FS_TYPE=apple_corestorage without
> needing to go looking for the underlying metadata in that partition. 

I'd like to avoid this. The information about the device content is important and it's pretty common that people use mkfs-like programs without care about partition types.

If you have software (e.g. Blivet) that depends on partition type than use ID_PART_ENTRY_TYPE=. (BTW, we already use the same in systemd and gummiboot to detect UEFI system partition.)

Comment 19 Chris Murphy 2013-12-12 18:41:18 UTC
(In reply to Karel Zak from comment #18)
> > It seems blkid could just spit out ID_FS_TYPE=apple_corestorage without
> > needing to go looking for the underlying metadata in that partition. 
> 
> I'd like to avoid this. The information about the device content is
> important and it's pretty common that people use mkfs-like programs without
> care about partition types.

This never happens in normal usage with Apple CLI or GUI tools. Apple Core Storage partitiontypeGUID only is set if the underlying formatting is Apple Core Storage, and if destroyed or converted the partitiontypeGUID is changed accordingly. If I use mkfs.msdos on OS X, it changes the partitiontypeGUID to Windows Basic data. If I use mkfs.hfs, it changes it to (gdisk code) AF00. If I create a Recovery HD volume, it gets the GUID for Apple Boot (gdisk AB00). So on OS X, the partitiontypeGUID is reliable.

Now, on linux, you're right it's distinctly unreliable for two reasons. mkfs programs don't set partitiontypeGUID, which is arguably wrong behavior. And also, parted developers bizarrely chose to use the Windows partitiontypeGUID for linux partitions, out of 600 million * 6 billion other choices available to them. So the Windows basic data partitiontypeGUID on linux is completely untrustworthy, and you have no choice but to look deeper to find out what it really is, because statistically its content probably isn't Windows related at all.

Apple Core Storage partitions are basically PVs, but their actual usage can differ in three ways currently: Fusion (similar to bcache), FileVault2 (similar to LUKS), and plain (similar to ordinary LVM2 PV). So if you find distinguishing between these three things really of value, and blivet really wants to know about all three types, none of which it supports, and at any moment Apple could add other flavors which blkid would then apparently have to add support for in order to inform blivet to keep its hands off, I'll reopen bug 1039724, change the summary, and I'll help sort out the on disk metadata that indicates these things. As an unpredictably sometimes open sometimes closed company, I don't know if Apple documents Apple Core Storage under Darwin/XNU, but there is a ~128MB unencrypted section of each Apple Core Storage partition that contains unencrypted metadata that probably can be figured out.


> If you have software (e.g. Blivet) that depends on partition type than use
> ID_PART_ENTRY_TYPE=. (BTW, we already use the same in systemd and gummiboot
> to detect UEFI system partition.)

See and I think this makes a lot more sense as a light weight solution in the case of Apple Core Storage. Now if parted developers had borrowed this GUID to use for LVM2, then we'd be screwed again like with Windows basic data GUID. But that isn't the case, maybe only because LVM2 pre-dates Apple Core Storage by quite a number of years.

So figure out how simple or overly complicated a solution you want, and let me know how I can help.

Comment 20 Karel Zak 2013-12-13 12:10:13 UTC
(In reply to Chris Murphy from comment #19)
> (In reply to Karel Zak from comment #18)
> > > It seems blkid could just spit out ID_FS_TYPE=apple_corestorage without
> > > needing to go looking for the underlying metadata in that partition. 
> > 
> > I'd like to avoid this. The information about the device content is
> > important and it's pretty common that people use mkfs-like programs without
> > care about partition types.

[...]
> Now, on linux, you're right it's distinctly unreliable for two reasons. mkfs
> programs don't set partitiontypeGUID, which is arguably wrong behavior.

Well, Linux (kernel) usually don't care about partition types (and see all the undocumented mess around MBR, it was good decision to ignore the types 20 years ago:-).

IMHO it's better to care about a real content on the devices to keep things more robust and modern Linux userspace is based on this paradigm (udev db, udev rules). It's also way how to keep mkfs programs relatively simple. (...but yes, I understand your point of view.)

[...]
> moment Apple could add other flavors which blkid would then apparently have
> to add support for in order to inform blivet to keep its hands off, I'll
> reopen bug 1039724, change the summary, and I'll help sort out the on disk
> metadata that indicates these things.

It would be the best long-term solution to identify the real content on the devices to have proper ID_FS_TYPE=

> As an unpredictably sometimes open
> sometimes closed company, I don't know if Apple documents Apple Core Storage
> under Darwin/XNU, but there is a ~128MB unencrypted section of each Apple
> Core Storage partition that contains unencrypted metadata that probably can
> be figured out.

OK, send me some dd(1) stuff from the begin of the devices.

> > If you have software (e.g. Blivet) that depends on partition type than use
> > ID_PART_ENTRY_TYPE=. (BTW, we already use the same in systemd and gummiboot
> > to detect UEFI system partition.)
> 
> See and I think this makes a lot more sense as a light weight solution in
> the case of Apple Core Storage. Now if parted developers had borrowed this
> GUID to use for LVM2, then we'd be screwed again like with Windows basic
> data GUID. But that isn't the case, maybe only because LVM2 pre-dates Apple
> Core Storage by quite a number of years.
> 
> So figure out how simple or overly complicated a solution you want, and let
> me know how I can help.

I think now (as temporary solution) can blivet try to use the GUID from ID_PART_ENTRY_TYPE=, the long-term solution depends on the metadata..

Comment 21 Chris Murphy 2014-10-24 20:50:41 UTC
OK this bug wrongly depends on bug 1039724. 

"Blivet depends on blkid setting ID_FS_TYPE for its detection of formats, without that it looks like any other unformatted partition to us."

It's been 11 months for bug 1039724, and libblkid still doesn't know what Core Storage is. And in the meantime, Apple now defaults to using Core Storage on internal/boot volumes – new volumes, and they convert existing volumes at OS upgrade time.

I think it's the wrong policy to say we first must actually recognize the world's arbitrary volume formats before the UI will not present a shrink button to the user. Instead, the deference should be to partitiontypeGUID, is it recognized/supported for modification? And if so, THEN defer to libblkid to isolate the specific volume formatting.

It's easier/safer/correct UX, to only present Shrink when it's something we can actually shrink. And right now anaconda presents bogus UI for something it doesn't even recognize. And in the lifetime of Fedora 21, it'll increasingly suggest default OS X installs are resizable which they are not.

Comment 22 Fedora End Of Life 2015-11-04 15:59:18 UTC
This message is a reminder that Fedora 21 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 21. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '21'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 21 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 23 Fedora End Of Life 2015-12-02 03:03:07 UTC
Fedora 21 changed to end-of-life (EOL) status on 2015-12-01. Fedora 21 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 24 Chris Murphy 2015-12-02 20:41:08 UTC
anaconda-24.7-1 offers to shrink Apple Core Storage partitions, and presents the shrink slider when the "shrink" button is clicked. I haven't tested how the attempt will fail, if it results in data loss or not. At best it's misleading though.

Comment 25 Raul Gutierrez S. 2015-12-13 01:20:56 UTC
i got hit by this today when installing fedora 23 on a mac air. surely i could have been a bit more careful and have looked up if core storage volumes are  resizable. so i am +1 in not considering core storage volumes resizeable...

Comment 26 Chris Murphy 2016-02-05 08:39:33 UTC
I tested this with Fedora 23 on an actual system. Using the shrink option obliterates OS X and all user data without warning. Total data loss.

This partition type GUID is well documented for some time and is only used by one company for one purpose, it's not "Unknown". If there's no time to decipher Apple's on-disk format, then forget it and just go with the partition type GUID in the meantime. Eating user's data in the meantime is worse.

Comment 27 David Lehman 2016-02-05 17:22:39 UTC
I am not about to add code to blivet that pretends that partition type GUID is actually equivalent to partition data. If you want the installer to recognize your partitions' type you should get the required support into blkid. That's where such logic belongs. Feel free to reassign this or, if you prefer to leave this open for tracking, open a new one for blkid. If you do the latter, please make a note of the blkid in this bug's "Blocks" field. Thanks.

Comment 28 Chris Murphy 2016-02-05 19:23:08 UTC
(In reply to David Lehman from comment #27)

I hardly think this is a satisfactory answer to installer induced data loss. Shrink implies data preservation.

1. The same shrink UI is presented for Windows NTFS volumes, where user data is preserved. There are two kinds of shrink not distinguished by the UI: destructive partition shrink; and non-destructive volume resize. The former doesn't even really exist as a parted, fdisk, or gdisk function; in practice the partition is deleted and then recreated with the same start value and a different end value. That's properly called delete and recreate, not shrink. Shrink is widely considered a filesystem function that does not involve data loss.

2. blkid bug 1039724 is open for two years. The policy of deference clearly isn't working out very well. Therefore in the meantime, since the partition type code could only be there for one reason, and willfully ignoring the provenance of that type code is folly that can lead to data loss, it's reasonable to stop indicating it's resizable/shrinkable.

3. The PartitionTypeGUID field identifies the contents of the partition. That's the language used in the spec. The creator of the filesystem is under zero obligation to put any additional identifying information in the partition space at all. What of plain dmcrypt partitions. Does Anaconda consider those shrinkable too? If so that's a bug as well.

The partition type GUID is course information. And blkid might be able to provide more detailed information. But the absence of detailed information does not invalidate available course information, although you appear to argue it does. There's a widely agreed upon and published spec for one of these things, not the other. And yet you callously disregard the one that's based on a widely agreed upon published spec. Quite frankly it's bizarre logic, not just bad logic.

The reality is, when Anaconda does not recognize the partition, either with course or detailed information, it should not present a shrink UI. Shrink implies data preservation, especially when contrasted with the button right next to it, delete.

Comment 29 David Lehman 2016-02-05 22:19:51 UTC
(In reply to Chris Murphy from comment #28)
> (In reply to David Lehman from comment #27)
> 2. blkid bug 1039724 is open for two years. The policy of deference clearly
> isn't working out very well. Therefore in the meantime, since the partition
> type code could only be there for one reason, and willfully ignoring the
> provenance of that type code is folly that can lead to data loss, it's
> reasonable to stop indicating it's resizable/shrinkable.

The fact that the blkid maintainer doesn't care enough to fix it does not make it my problem. In my opinion you should be expending your effort in that bug and not this one.

> 
> 3. The PartitionTypeGUID field identifies the contents of the partition.
> That's the language used in the spec. The creator of the filesystem is under
> zero obligation to put any additional identifying information in the
> partition space at all. What of plain dmcrypt partitions. Does Anaconda
> consider those shrinkable too? If so that's a bug as well.

Partition type is not a reliable indicator of what type of data is on the device. However, if you think I should pursue some scheme related to partition types, you're going to have to get that partition GUID added to parted because blivet is never going to have a database of partition type GUIDs.

> The reality is, when Anaconda does not recognize the partition, either with
> course or detailed information, it should not present a shrink UI. Shrink
> implies data preservation, especially when contrasted with the button right
> next to it, delete.

The reality is also that if the installer shows your partition's formatting as "Unknown", a wise person would think twice before letting the installer resize it. I might be willing to add a warning along the lines of "WARNING: You are asking to resize a partition whose formatting I do not understand. It is your responsibility to ensure that it does not contain any important data. Are you sure you want to proceed?" but good grief. How is that warning not self-evident?

Comment 30 Chris Murphy 2016-02-06 21:02:52 UTC
(In reply to David Lehman from comment #29)

Let me walk this back, and better qualify what I'm suggesting.

All users consider a shrink in a GUI to be a non-destructive operation; a handful might know it entails risk. But zero associate shrink with data loss or deletion. This can’t be changed.

The current approach uses libblkid as a blacklist to inhibit what’s ultimately a design flaw serving no valid use case, is fail danger, and has unacceptable lag. The flaw is two fold: the idea there is such a thing as partition shrink, and the idea there is any user wanting let alone interpreting the shrink UI as leaving unused free space in the middle of the drive. This isn’t fail safe because it entices the user into unintended data loss. It sets them up to fail. It’s rather malicious, honestly. And then there’s the lag, demonstrated by the fact it’s been two years and libblkid still doesn’t know about Apple core storage. It likely doesn’t know about Microsoft storage spaces, or who knows what else. At any time Apple or Microsoft can change their on-disk metadata format and without notice. It would surprise me if they didn’t. I can’t even tell you what or where the Apple core storage PV magic even is, it’s not documented anywhere as far as I can tell. And the obscurity of other formats is not a defense for a design that isn’t fail safe, serves no use case, and this bug proves the design is unreliable because it has lead to unintended data loss. And that makes the installer untrustworthy. No one will blame libblkid for this data loss, which is done at the hand of the installer, not libblkid.

You can keep on blaming libblkid's lag all you want, but the current design approach is inherently flawed. And blivet getting its own database doesn't solve the problem either, it'll still be fail danger because avoiding danger depends on an up to date blacklist.

What I’m proposing is a fail safe, white list method, that asks libblkid+blivet logic to prove non-destructive shrink support exists for a partition’s contents. That way the shrink UI is only presented when it’s a data safe operation. The user already believes a shrink operation is data safe, and that a delete operation is data unsafe. The software should not betray them by fighting with them on this point and creating ambiguous, inconsistent UI. You can choose to have contempt for these users who trust that shrink is data safe, and proceed to mislead and punish them anyway, which is what the current UI does, but I'm suggesting you stop doing that.

> The reality is also that if the installer shows your partition's formatting
> as "Unknown", a wise person would think twice before letting the installer
> resize it.

The reality is, the installer considers the partition formatting as unknown, a wise installer would not allow the user resize it.

> I might be willing to add a warning along the lines of "WARNING:
> You are asking to resize a partition whose formatting I do not understand.
> It is your responsibility to ensure that it does not contain any important
> data. Are you sure you want to proceed?" but good grief. How is that warning
> not self-evident?

Let’s pretend it’s ridiculous to put up a placard admitting the thing that comes before it is giving bad advice. Instead of the placard, just remove the thing that’s giving bad advice.

“Unknown” is not a warning by any stretch of the imagination.

Comment 31 Karel Zak 2016-02-08 11:30:59 UTC
> The fact that the blkid maintainer doesn't care enough to fix it

Nice.

The on-disk PV format of Apple Core Storage is nothing public, or at least I don't know about any documentation or open source implementation. 

Chris provided the begin and the end of the device, but all I can say is that first and last sector is the same. From my point of view it is not enough to make any decision based on this. I'm not Apple user, I have no any Apple HW/SW and I have no time and motivation to do any reverse engineering for any Apple stuff. 

If you want to invest your time and money to this issue then go ahead and send me I patch.

Currently, we known than Core Storage partition type is "53746F72-6167-11AA-AA11-00306543ECAC" and it's probably better to follow this GUID than do nothing.

Comment 32 Chris Murphy 2016-02-08 19:04:52 UTC
Created attachment 1122211 [details]
anaconda-23 screenshot of problem

Comment 33 Fedora Blocker Bugs Application 2016-02-15 08:24:36 UTC
Proposed as a Blocker for 24-final by Fedora user chrismurphy using the blocker tracking app because:

 "Storage volume resize:Any installer mechanism for resizing storage volumes must correctly attempt the requested operation.
This means that if the installer offers mechanisms for resizing storage volumes, then it must run the appropriate resizing tool with the appropriate parameters for the resize the user chooses."

Technical summary of bug:
a. This bug applies to the Guided/autopart path in Anaconda.
b. Installer always obliterates all data on the OS X PV when the user chooses the "shrink" option in Reclaim Space.
c . Installer does not use an appropriate resizing tool; it doesn't exist.
d. The user has every reason to expect "shrink" is a data-safe operation, in contrast to "delete". See test case Expected Results Step 2 "The pre-existing contents of the resized volume should not be disturbed." is violated by this bug.
https://fedoraproject.org/wiki/QA:Testcase_partitioning_guided_shrink?rd=QA:Testcase_Anaconda_autopart_(shrink)_install
e. The identical UI is used for safe shrink for ext34, and NTFS. It's a trap to use this same UI for a data destructive operation.

If you really want to read comments, it's sufficient to read 30, 31, and 32. And 25 is a user who was hit by this, not wanting to.

Comment 34 Petr Schindler 2016-02-15 17:56:30 UTC
Discussed at 2016-02-15 blocker review meeting: [1]. 

We decided to delay the decision - on the face of it we're inclined to agree that this is a blocker, but we suspect there may be hidden depths, so we want to discuss with anaconda devs before making a decision (none present at meeting)

[1] http://meetbot.fedoraproject.org/fedora-blocker-review/2016-02-15/f24-blocker-review.2016-02-15-17.00.html

Comment 35 Petr Schindler 2016-02-22 19:23:54 UTC
Discussed at 2016-02-22 blocker review meeting: [1]. 

We decided to delay the decision again: this bug does quite clearly violate the criteria as written, but anaconda devs are strongly of the opinion that it does not qualify as a blocker on technical/architectural grounds. we will discuss this in the coming days and decide its status later

[1] http://meetbot.fedoraproject.org/fedora-blocker-review/2016-02-22/f24-blocker-review.2016-02-22-17.00.html

Comment 36 Jan Kurik 2016-02-24 13:13:50 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase

Comment 37 Petr Schindler 2016-02-29 18:48:32 UTC
Discussed at 2016-02-29 blocker review meeting: [1]. 

This bug was rejected as final blocker: anaconda team presented a reasonable argument that the release criteria should not be worded such that this issue is a blocker, so the criterion will be amended to only cover known filesystems, and this bug is rejected as a blocker. See bug comment for more details

[1] http://meetbot.fedoraproject.org/fedora-blocker-review/2016-02-29/f24-blocker-review.2016-02-29-17.00.html

Comment 38 Adam Williamson 2016-02-29 20:21:26 UTC
Bit more detail on the above - you can find the thread here:

https://www.redhat.com/archives/anaconda-devel-list/2016-February/msg00010.html

paraphrased, anaconda team's position is that this is a typical "foot shooting" UI conundrum, and thus it's not appropriate for the criteria to effectively prescribe how anaconda ought to behave in this case.

anaconda cannot distinguish between a volume with no filesystem and a volume with an unknown filesystem. There are believed to be use cases for destructively resizing the former. There is also a clear argument for only allowing resize of known volumes. Thus this is kind of a subjective decision and it's not appropriate for the release criteria to intervene.

The release criterion will be reworded to apply specifically to resizing of volumes formatted with known filesystems, and it's left up to anaconda team to decide the most appropriate behaviour for handling volumes which do not have known filesystems, both in this 'guided' workflow and in the custom workflow. Once that's decided we could potentially cover the intended behaviour in the criteria if that seems appropriate, but this case was not really considered in initially drafting the current criterion, and it was not intended to dictate anaconda's behaviour in this situation.

This decision doesn't preclude any change or mean this bug report is invalid or closed, it simply means this is not a release blocking bug.

Comment 39 Adam Williamson 2016-02-29 20:24:12 UTC
hmm, sorry, I just realized after I made that summary edit that it may not be what Chris wants; there are really kinda two issues being tracked here, the fact that we don't recognize Apply Core Storage volumes (whoever's responsibility that turns out to be) and the fact that we allow resizing of unknown volumes with no warning. Chris, if you'd prefer this to be the bug for the former and have a new bug for the latter, just change the summary back and we can file a new one.

Comment 40 Adam Williamson 2016-02-29 20:58:12 UTC
It's not destructive in the case the volume contains *no* data, which is - as I understand it - the case for allowing this operation at all.

Comment 41 Chris Murphy 2016-02-29 23:09:02 UTC
There is no valid use case for this misfeature, even excluding the data loss. No one is asking for a feature that divides a partition, and leaves the first portion a.) as unused dead space in the middle of the drive and b.) with the wrong partition type GUID set (i.e. that's a new bug if this feature holds). There is simply no use case for guided partitioning creating space that will not be used for anything, including Fedora, at the expect of data loss.

Comment 42 David Lehman 2016-03-01 14:00:10 UTC
There are more important things that need work than preventing every possible misstep a careless user can make. Could this be nicer? Sure, it could. Is this a blocker? No, and the notion that it is is laughable. Patches are absolutely welcome, as always.

Comment 43 Chris Murphy 2016-06-10 20:10:44 UTC
Seems sufficiently nasty for users who misplace their trust to put it in commonbugs and hopefully some will avoid data loss.

Draft write up for commonbugs page:

The installer appears to support volume shrink for OS X volumes by offering a Shrink button and sizing slider in Automatic partitioning; and likewise allow numeric resizing in Manual partitioning. Setting the installer to shrink and proceeding with installation will result in complete data loss of the volume. Resize the volume in OS X's Disk Utility to create free space before proceeding with the installation of Fedora.

Comment 44 Kevin Fenzi 2017-02-19 19:25:36 UTC
Perhaps we could refocus here and see if we can just solve the immediate problem?

ie, what would it take to be able to identify macos FileVault partitions so we know we cannot resize them?

Comment 45 Chris Murphy 2017-02-19 21:59:23 UTC
(In reply to Kevin Fenzi from comment #44)
> Perhaps we could refocus here and see if we can just solve the immediate
> problem?

Apple is in the process of changing file systems entirely from HFS Plus to Apple File System; so the immediate problem is still that unrecognized partitions should only be deletable, not resizeable.

I've tested Gparted, Blivet GUI, YaST (opensuse installer), Ubuntu installer, macOS's Disk Utility, and Windows Disk Manager and its OS installer and not a single one of them present shrink UI for a partition with unrecognized content. The immediate problem is this is a design flaw that only exists in the Fedora installer.

> 
> ie, what would it take to be able to identify macos FileVault partitions so
> we know we cannot resize them?

Can you rephrase in the context of comment 31? Karel Zak and I have already looked into it and it's sufficiently obfuscated and non-documented that I don't see a way forward, but that is also the wrong strategy in a world with proliferating partition contents that are obfuscated (including Chrome OS's use of partitions with binary blobs, that Anaconda also considers shrinkable).

Comment 46 Adam Williamson 2017-02-20 16:53:57 UTC
Kevin: https://bugzilla.redhat.com/show_bug.cgi?id=1033778#c12 - it is apparently something that has to happen in util-linux (whence blkid).

Comment 47 Kevin Fenzi 2017-02-20 21:02:21 UTC
So, there's now a fuse library to mount encrypted File Vault volumes. They have: https://github.com/libyal/libfvde/blob/master/documentation/FileVault%20Drive%20Encryption%20(FVDE).asciidoc

Perhaps thats enough info to get blkid working?

Or perhaps blkid could just use the GUID that fdisk does? 
/dev/sda2     409640 244240007 243830368 116.3G Apple Core storage

Comment 48 Chris Murphy 2017-03-07 00:31:12 UTC
This bug affects Windows Bitlocker volumes as well. Bitlocker is the built-in Windows volume encryption mechanism. Windows enables it by default when the hardware meets certain requirements, increasingly new hardware meets those requirements.

Two platforms are affected by this bug; one of which is always affected in it's out of the box state. And so far I don't get any shrink UI by any other installer or partitioner on Linux, Windows, or macOS; this problem seems exclusive to Anaconda.

Comment 49 Chris Murphy 2017-03-07 00:36:59 UTC
Created attachment 1260665 [details]
screenshot install alongside Windows 10 Bitlocker'd

anaconda 26.20-1

This is the gdisk and blkid output for this drive.

Number  Start (sector)    End (sector)  Size       Code  Name
   1            2048          923647   450.0 MiB   2700  Basic data partition
   2          923648         1126399   99.0 MiB    EF00  EFI system partition
   3         1126400         1159167   16.0 MiB    0C01  Microsoft reserved ...
   4         1159168        52426751   24.4 GiB    0700  Basic data partition

/dev/sda1: LABEL="Recovery" UUID="62164A58164A2CFB" TYPE="ntfs" PARTLABEL="Basic data partition" PARTUUID="adaaa993-3365-4eff-9cb3-707c2023f619"
/dev/sda2: UUID="A84A-9ABB" TYPE="vfat" PARTLABEL="EFI system partition" PARTUUID="1550a32c-05d9-474f-b7eb-c6b9126bdd90"
/dev/sda3: PARTLABEL="Microsoft reserved partition" PARTUUID="6eecb3ed-7653-4610-bce5-c61cd79db1fd"
/dev/sda4: PARTLABEL="Basic data partition" PARTUUID="65f375ed-6308-41e6-904d-abf03531bbea"

Comment 50 David Lehman 2017-03-09 14:52:55 UTC
I looked at this again a few weeks ago and am planning to fix it within the next couple of weeks. By "fix" I mean I will change blivet to disallow resize of block devices with no/unrecognized formatting. I'd rather err on the side of caution here.

Comment 51 Kamil Páral 2017-03-09 15:16:15 UTC
Sounds reasonable, thanks, David. At least for the guided dialog. For custom/blivet-gui dialogs, it might make sense to allow it, but clearly warn the user first ("we don't recognize the filesystem, if there's any data, it will most probably be destroyed").

Comment 52 David Lehman 2017-03-09 15:32:32 UTC
(In reply to Kamil Páral from comment #51)
> Sounds reasonable, thanks, David. At least for the guided dialog. For
> custom/blivet-gui dialogs, it might make sense to allow it, but clearly warn
> the user first ("we don't recognize the filesystem, if there's any data, it
> will most probably be destroyed").

The "stern warning" approach would be entirely within the applications and would not involve any changes to blivet whereas the change I proposed is only in blivet. Which approach do people prefer? (I'm proposing a survey/vote in this bug report.)

Comment 53 Adam Williamson 2017-03-09 16:37:37 UTC
Personally I'd vote for the simple approach (just disallow it in blivet). If apps on top of blivet really want to allow 'resize' of block devices with unknown filesystems, they can just delete the filesystem before calling blivet to do the resize...

Comment 54 David Lehman 2017-03-09 16:53:36 UTC
(In reply to Adam Williamson from comment #53)
> apps on top of blivet really want to allow 'resize' of block devices with
> unknown filesystems, they can just delete the filesystem before calling
> blivet to do the resize...

That won't work once the proposed change has been made :)

(The trick being that blivet cannot differentiate between a lack of formatting and the formats mentioned in this bug. Even if we could whip up a detection method for one or both of those, it seems clear this is going to be a constant game of add-new-format-detection-hack since blkid has chosen not to play it for us. No, thanks.)

Comment 55 Adam Williamson 2017-03-09 17:11:05 UTC
oh, right, forgot about that wrinkle. then they can just delete it and create a new one. :P It really doesn't seem like a very important case to me, not important enough to be worth implementing warning dialogs about. Maybe the error you get can say 'just delete it and create a new one'.

Comment 56 Chris Murphy 2017-03-09 17:55:31 UTC
The problem is not blkid's as much as that there's a proliferation of partition contents that don't have a signature. It's hardly any different than a plain dmcrypt volume, which lacks a header and thus no signature.

Comment 57 David Lehman 2017-03-16 18:33:30 UTC
Upstream pull request: https://github.com/rhinstaller/blivet/pull/560

Comment 58 Adam Williamson 2017-03-17 16:24:19 UTC
+1 FE for me, this is a prioritized bug and worth fixing for Alpha. The fix looks reasonable to me.

Comment 59 Kevin Fenzi 2017-03-17 18:10:32 UTC
+1 FE for me.

Comment 60 Mike Ruckman 2017-03-20 18:08:06 UTC
+1 FE. Updated the Whiteboard.

Comment 61 Fedora Update System 2017-03-21 16:36:24 UTC
python-blivet-2.1.7-8.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-1d70efd57a

Comment 62 Fedora Update System 2017-03-22 15:27:17 UTC
python-blivet-2.1.7-8.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-1d70efd57a

Comment 63 Jan Kurik 2017-03-23 16:15:26 UTC
This issue does not seem to affect many users. As such, we are not including this bug on the Prioritized bugs and issues list: https://meetbot.fedoraproject.org/fedora-meeting/2017-03-22/fedora_prioritized_bugs_and_issues.2017-03-22-14.06.html

Comment 64 Fedora Update System 2017-03-29 05:05:23 UTC
python-blivet-2.1.7-8.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 65 Kamil Páral 2017-04-11 09:50:19 UTC
Chris, could you please re-test whether this is fixed now? Thanks a lot.

Comment 66 Chris Murphy 2017-04-11 15:04:04 UTC
Appears to be fixed.

Comment 67 Kamil Páral 2017-04-12 07:24:49 UTC
Great, thanks for confirmation.


Note You need to log in before you can comment on or make changes to this bug.