Bug 1079000 - Missing readonly option in fstab
Missing readonly option in fstab
Status: RESOLVED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Kubic
Current
Other Other
: P1 - Urgent : Major (vote)
: ---
Assigned To: Imobach Gonzalez Sosa
E-mail List
https://trello.com/c/lLGFKXxh
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-02-02 08:40 UTC by Martin Kravec
Modified: 2022-02-24 14:47 UTC (History)
8 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
YaST logs (177.72 KB, application/x-xz)
2018-02-05 08:50 UTC, Martin Kravec
Details
installed fstab (1.07 KB, text/plain)
2018-03-05 10:29 UTC, Martin Kravec
Details
inst-sys fstab (117 bytes, text/plain)
2018-03-05 10:29 UTC, Martin Kravec
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Kravec 2018-02-02 08:40:55 UTC
Kubic have ro property only on btrfs /, but not as mount option. It should be on both places.

# /etc/fstab
UUID=9dab592f-910f-421f-9b3e-029dda8e4e3e  /  btrfs  defaults  0  0

# mount | grep ' / '
/dev/vda2 on / type btrfs (rw,relatime,space_cache,subvolid=266,subvol=/@/.snapshots/1/snapshot)

# btrfs property get / ro
ro=true
Comment 1 Ancor Gonzalez Sosa 2018-02-02 09:31:19 UTC
Added to the storage-ng queue for it to be prioritized in relation to other tasks.
Comment 2 Thorsten Kukuk 2018-02-02 12:16:12 UTC
That's one of the bugs making it currently impossible to update MicroOS/Kubic.
Comment 3 Lukas Ocilka 2018-02-02 15:34:59 UTC
Martin, could you please add more information for those that are not that
well informed about Kubic to understand the whole thing?

Such as: ... after $installation using $method ... etc?
Thanks
Comment 4 Thorsten Kukuk 2018-02-02 15:54:27 UTC
Kubic has a read-only root filesystem.
So after installation, you should have "<dev> / btrfs ro 0 0" in /etc/fstab and /proc/mounts should show a "ro" and not "rw" for the root filesystem, too.
Comment 5 Martin Kravec 2018-02-02 15:57:06 UTC
(In reply to Lukas Ocilka from comment #3)
Tested image: openSUSE-Tumbleweed-Kubic-DVD-x86_64-Snapshot20180130-Media.iso

We have openQA test checking that MicroOS has read-only root filesystem.
http://dhcp91.suse.cz/tests/1174#step/filesystem_ro/15

This check passes when trying :
> btrfs property get / ro | grep "ro=true"

But fails when trying:
> grep '/ btrfs ro' /etc/fstab
> mount | grep 'on / type btrfs (ro,'

So read-only flag is set only as btrfs property, not as mount option.

Richard Brown pointed me to this code, but I don't know any details:
https://github.com/yast/yast-installation/blob/2366e20a4bf30a4ec3b29f50828e9332cc08aa84/src/lib/installation/clients/umount_finish.rb#L422-L428

I tried this also manually - after installation with default options.
Comment 6 Lukas Ocilka 2018-02-02 16:48:56 UTC
Thanks, that helped. Any YaST logs available as attachment?
Comment 7 Martin Kravec 2018-02-05 08:50:15 UTC
Created attachment 758790 [details]
YaST logs
Comment 9 Josef Reidinger 2018-02-27 15:41:00 UTC
yeah, problem is that it does not detect properly product features. I will try to debug it.
Comment 10 Josef Reidinger 2018-02-27 16:04:10 UTC
ah, it is Kubic which have currently lower priority then SLE15 bugs, so returning back to queue, as we have P1s for SLE15.
Comment 11 Richard Brown 2018-03-02 10:52:09 UTC
(In reply to Josef Reidinger from comment #10)
> ah, it is Kubic which have currently lower priority then SLE15 bugs, so
> returning back to queue, as we have P1s for SLE15.

If this bug isn't fixed in SLE 15, then we aer going to need to fork YaST-installation for CaaSP 4 (which will be crippled by this bug)

We do not want to do this

Please reconsider treating this as SLE 15 bug in order to ensure CaaSP4 is not burdened with such inconveniences in the future.
Comment 12 Lukas Ocilka 2018-03-05 09:54:05 UTC
Can I have the /etc/fstab from the installed system?
And also from the inst-sys?
Thanks in advance
Comment 13 Martin Kravec 2018-03-05 10:29:12 UTC
Created attachment 762655 [details]
installed fstab
Comment 14 Martin Kravec 2018-03-05 10:29:40 UTC
Created attachment 762656 [details]
inst-sys fstab
Comment 15 Stefan Hundhammer 2018-03-05 11:23:28 UTC
Just to clarify some things here:

We had implemented this "root read-only" feature for CaaSP 1.0 as a nasty hack after it had been established that this didn't work with the available infrastructure (yast-storage-old and libstorage-old) of the time. 

It was accepted that this was a hack; the CaaSP 1.0 release manager / architect was well aware of it. So we took the least disruptive approach: At the end of the installation, hack up /etc/fstab with a crude "sed" command and issue a "btrfs subvol" command to set the properties of the root subvolume to read-only.

This was never meant to be a mainstream feature. It was never meant to be used outside of the CaaSP installation which consisted of a great number of nasty hacks.

Since that "sed" call needed to be so simplistic, it stopped working in recent versions because it relied on "defaults" being present in the mount options in /etc/fstab - this was its (only!) way to identify the correct column. It was not a full-fledged /etc/fstab parser (and it was never meant to be one).

We now no longer add "defaults" to the mount options because that doesn't make any sense whatsoever: "defaults" is just a placeholder to maintain correct syntax in /etc/fstab when no specific mount options are set. And now that "defaults" is no longer there, that "sed" call fails.
Comment 16 Ancor Gonzalez Sosa 2018-03-05 11:24:46 UTC
On IRC it was mentioned that it's possible that this is wanted also for Leap15 and/or Tumbleweed (not only CaaSP/Kubic), although I personally have no clue what's the rationale of having that in a non-transactional system.

If that's the case, the "hack" linked in comment#5 (modifying fstab via sed at the end of installation) is probably not the way to go anymore.

We will implement support for this as a first class citizen, meaning the proposal and the expert partitioner should be aware of the goal of ending with the read-only subvolume, already managing that setting since the beginning of the installation.

Raised priority in the YaST team
Comment 17 Stefan Hundhammer 2018-03-05 11:25:55 UTC
On the positive side, with storage-ng, we now have much better infrastructure in place to do this right; we no longer need that nasty "sed" hack. But we will need to do this consistently in several places:

- In the storage proposal

- In the expert partitioner if the user does not explicitly choose specific mount options

- In the mount options dialog of the experte partitioner

...all taking the corresponding setting in the control file into account.
Comment 19 Imobach Gonzalez Sosa 2018-03-09 11:58:22 UTC
We have a proof of concept that should work with guided proposals and AutoYaST, but support in the expert partitioner is still missing.

Anyway, I have some questions that I would like to ask:

* Does it make sense to honor this setting when snapshots are disabled?
* Can the user change this value, for instance, using the expert partitioner?

On the other hand, just to be clear, we need to:

* set the btrfs "ro" property to true for the default subvolume (@/.snapshots/1/snapshot).
* set the "ro" mount option only to the / mount point.

Finall, instead of using a global "root_subvolume_read_only", we are using a "btrfs_read_only" setting in the "volumes" section. The idea is to be consistent with other settings like "btrfs_default_subvolume", which are defined in a per-volume basis.
Comment 20 Richard Brown 2018-03-09 13:05:39 UTC
(In reply to Imobach Gonzalez Sosa from comment #19)
> We have a proof of concept that should work with guided proposals and
> AutoYaST, but support in the expert partitioner is still missing.
> 
> Anyway, I have some questions that I would like to ask:
> 
> * Does it make sense to honor this setting when snapshots are disabled?

Yes, though in that case the user would be expected to have an alternate way of updating; like they must already have if they are using the supported Read-only root features in SLE 12

> * Can the user change this value, for instance, using the expert partitioner?

At the moment I do not believe the expert partitioner exposes that value.
I believe it is only set as part of system_roles.

I do not know what was agreed in this area for libstorage-ng, Thorsten is best to answer that question if my answer isn't clear enough.

> On the other hand, just to be clear, we need to:
> 
> * set the btrfs "ro" property to true for the default subvolume
> (@/.snapshots/1/snapshot).

Yes

> * set the "ro" mount option only to the / mount point.

In the /etc/fstab written to the system, yes

> Finall, instead of using a global "root_subvolume_read_only", we are using a
> "btrfs_read_only" setting in the "volumes" section. The idea is to be
> consistent with other settings like "btrfs_default_subvolume", which are
> defined in a per-volume basis.

Sounds like this will mean we have to change all of the skelcd-control-* for CAASP, Kubic, openSUSE(15|TW) to use this new setting?

Can I see the docs for the new setting so I can prepare those changes?
Comment 21 Imobach Gonzalez Sosa 2018-03-09 13:26:49 UTC
(In reply to Richard Brown from comment #20)

Thanks Richard!

[..]

> Yes, though in that case the user would be expected to have an alternate way
> of updating; like they must already have if they are using the supported
> Read-only root features in SLE 12

Ok.

> > * Can the user change this value, for instance, using the expert partitioner?
> 
> At the moment I do not believe the expert partitioner exposes that value.
> I believe it is only set as part of system_roles.
> 
> I do not know what was agreed in this area for libstorage-ng, Thorsten is
> best to answer that question if my answer isn't clear enough.

Sorry, my question was not clear enough. In the expert partitioner you can set/unset the "read-only" (ro) mount option (fstab options). So my question is: can the user unset that option? In that case, I guess we should not set the btrfs "ro" property at the end, right?

> > On the other hand, just to be clear, we need to:
> > 
> > * set the btrfs "ro" property to true for the default subvolume
> > (@/.snapshots/1/snapshot).
> 
> Yes

Obviously, if snapshots are not enabled, the subvolume to set as "ro" will not be "@/.snapshots/1/snapshot" but the default one, right?

> 
> > * set the "ro" mount option only to the / mount point.
> 
> In the /etc/fstab written to the system, yes

Ok (that's the option which is exposed in the expert partitioner).
 
> > Finall, instead of using a global "root_subvolume_read_only", we are using a
> > "btrfs_read_only" setting in the "volumes" section. The idea is to be
> > consistent with other settings like "btrfs_default_subvolume", which are
> > defined in a per-volume basis.
> 
> Sounds like this will mean we have to change all of the skelcd-control-* for
> CAASP, Kubic, openSUSE(15|TW) to use this new setting?
> 
> Can I see the docs for the new setting so I can prepare those changes?

Docs are not ready yet, but I plan to 1) document the setting and 2) create PRs for these products. Anyway, it should be something like this:

<volumes config:type="list">
  <volume>
    <mount_point>/</mount_point>
    <btrfs_read_only config:type="boolean">true</btrfs_read_only> <!-- this line -->
    <btrfs_default_subvolume>@</btrfs_default_subvolume>
    <!-- ... -->
  </volume>
</volumes>
Comment 22 Richard Brown 2018-03-09 13:35:38 UTC
(In reply to Imobach Gonzalez Sosa from comment #21)

> Sorry, my question was not clear enough. In the expert partitioner you can
> set/unset the "read-only" (ro) mount option (fstab options). So my question
> is: can the user unset that option? In that case, I guess we should not set
> the btrfs "ro" property at the end, right?

Technically speaking, the "ro" fstab option and the "ro" btrfs property are two different things

But only "ro" fstab is presented in YaST as an option

I personally think it makes sense that if you're creating a btrfs partition and setting it's fstab to mount "ro", then we should also set that property

And visa versa, if the user is overriding the "ro" fstab mount, then I suppose it makes sense to override the btrfs property.

> > > On the other hand, just to be clear, we need to:
> > > 
> > > * set the btrfs "ro" property to true for the default subvolume
> > > (@/.snapshots/1/snapshot).
> > 
> > Yes
> 
> Obviously, if snapshots are not enabled, the subvolume to set as "ro" will
> not be "@/.snapshots/1/snapshot" but the default one, right?

Right, if the user requires a read-only root filesystem, we should set ro in fstab+property for wherever the root filesystem is being installed.
  

> Docs are not ready yet, but I plan to 1) document the setting and 2) create
> PRs for these products. Anyway, it should be something like this:
> 
> <volumes config:type="list">
>   <volume>
>     <mount_point>/</mount_point>
>     <btrfs_read_only config:type="boolean">true</btrfs_read_only> <!-- this
> line -->
>     <btrfs_default_subvolume>@</btrfs_default_subvolume>
>     <!-- ... -->
>   </volume>
> </volumes>

Thanks! I have a bunch of other changes pending of all the skelcd's that are blocked by this bug and the /home subvol one, this heads up gives me a chance to get all the required changes right first time :)
Comment 24 Imobach Gonzalez Sosa 2018-03-13 17:27:26 UTC
yast2-storage 4.0.131 and yast2-installation 4.0.39 (or even 4.0.37) should fix the issue.
Comment 25 Imobach Gonzalez Sosa 2018-03-14 08:48:34 UTC
All the code have been already merged. So I am closing this bug report.

Please, let me know if it does not work for you.