All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wic: Prevent duplicate entries on fstab
@ 2017-02-23 18:13 Fabio Berton
  2017-02-24 23:02 ` Burton, Ross
  0 siblings, 1 reply; 13+ messages in thread
From: Fabio Berton @ 2017-02-23 18:13 UTC (permalink / raw)
  To: openembedded-core

Add function to read fstab and return all mount points. This function
is useful to _update_fstab doesn't duplicate entries in /etc/fstab.

Signed-off-by: Fabio Berton <fabio.berton@ossystems.com.br>
---
 scripts/lib/wic/plugins/imager/direct.py | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 481d24d5ba..570dbbbf79 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -97,6 +97,25 @@ class DirectPlugin(ImagerPlugin):
         finally:
             self.cleanup()
 
+    def _get_fstab_mountpoints(self):
+        """ Read file /etc/fstab and returns all mountpoints."""
+        image_rootfs = self.rootfs_dir.get("ROOTFS_DIR")
+        if not image_rootfs:
+            return
+
+        fstab_path = image_rootfs + "/etc/fstab"
+        if not os.path.isfile(fstab_path):
+            return
+
+        with open(fstab_path) as fp:
+            mp = []
+            for l in fp:
+                if not l.startswith("#"):
+                    valid_mp = l.replace("\t", " ").split()
+                    if len(valid_mp) >= 2:
+                        mp.append(valid_mp[1])
+            return mp
+
     def _write_fstab(self, image_rootfs):
         """overriden to generate fstab (temporarily) in rootfs. This is called
         from _create, make sure it doesn't get called from
@@ -125,7 +144,7 @@ class DirectPlugin(ImagerPlugin):
         updated = False
         for part in parts:
             if not part.realnum or not part.mountpoint \
-               or part.mountpoint in ("/", "/boot"):
+               or part.mountpoint in self._get_fstab_mountpoints():
                 continue
 
             # mmc device partitions are named mmcblk0p1, mmcblk0p2..
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-02-23 18:13 [PATCH] wic: Prevent duplicate entries on fstab Fabio Berton
@ 2017-02-24 23:02 ` Burton, Ross
  2017-03-03 12:12   ` Fabio Berton
  0 siblings, 1 reply; 13+ messages in thread
From: Burton, Ross @ 2017-02-24 23:02 UTC (permalink / raw)
  To: Fabio Berton; +Cc: OE-core

[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]

On 23 February 2017 at 18:13, Fabio Berton <fabio.berton@ossystems.com.br>
wrote:
>
> Add function to read fstab and return all mount points. This function
> is useful to _update_fstab doesn't duplicate entries in /etc/fstab.


The AB just failed all over with this error message:

| Traceback (most recent call last):
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic",
line 327, in <module>
|     sys.exit(main(sys.argv[1:]))
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic",
line 322, in main
|     return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage,
subcommands)
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/help.py",
line 95, in invoke_subcommand
|     subcommands.get(args[0], subcommand_error)[0](args[1:], usage)
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic",
line 255, in wic_create_subcommand
|     native_sysroot, options)
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/engine.py",
line 199, in wic_create
|     plugin.do_create()
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py",
line 93, in do_create
|     self.create()
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py",
line 176, in create
|     fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py",
line 134, in _write_fstab
|     if self._update_fstab(fstab_lines, self.parts):
|   File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py",
line 156, in _update_fstab
|     opts, "0", "0"]) + "\n"
| TypeError: sequence item 2: expected str instance, NoneType found

Ross

[-- Attachment #2: Type: text/html, Size: 2342 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-02-24 23:02 ` Burton, Ross
@ 2017-03-03 12:12   ` Fabio Berton
  2017-03-03 13:49     ` Burton, Ross
  0 siblings, 1 reply; 13+ messages in thread
From: Fabio Berton @ 2017-03-03 12:12 UTC (permalink / raw)
  To: Burton, Ross; +Cc: OE-core

[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]

Hi Ross,

Sorry for delay. This error for qemux86 is because common.wks.inc and 
directdisk-gpt.wks files in scripts/lib/wic/canned-wks directory doesn't 
have option --fstype= for part /boot.

For qemuarm machines I get the error:

| DEBUG: Executing python function set_image_size
| DEBUG: Python function set_image_size finished
| DEBUG: Executing shell function do_image_wic
| Error: Please build syslinux first

On 02/24/2017 08:02 PM, Burton, Ross wrote:
>
> On 23 February 2017 at 18:13, Fabio Berton 
> <fabio.berton@ossystems.com.br <mailto:fabio.berton@ossystems.com.br>> 
> wrote:
> >
> > Add function to read fstab and return all mount points. This function
> > is useful to _update_fstab doesn't duplicate entries in /etc/fstab.
>
>
> The AB just failed all over with this error message:
>
> | Traceback (most recent call last):
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic", 
> line 327, in <module>
> |     sys.exit(main(sys.argv[1:]))
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic", 
> line 322, in main
> |     return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage, 
> subcommands)
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/help.py", 
> line 95, in invoke_subcommand
> |     subcommands.get(args[0], subcommand_error)[0](args[1:], usage)
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/wic", 
> line 255, in wic_create_subcommand
> |     native_sysroot, options)
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/engine.py", 
> line 199, in wic_create
> |     plugin.do_create()
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py", 
> line 93, in do_create
> |     self.create()
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py", 
> line 176, in create
> |     fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py", 
> line 134, in _write_fstab
> |     if self._update_fstab(fstab_lines, self.parts):
> |   File 
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86/build/scripts/lib/wic/plugins/imager/direct.py", 
> line 156, in _update_fstab
> |     opts, "0", "0"]) + "\n"
> | TypeError: sequence item 2: expected str instance, NoneType found
>
> Ross


[-- Attachment #2: Type: text/html, Size: 3726 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-03 12:12   ` Fabio Berton
@ 2017-03-03 13:49     ` Burton, Ross
  2017-03-06 14:00       ` Fabio Berton
  0 siblings, 1 reply; 13+ messages in thread
From: Burton, Ross @ 2017-03-03 13:49 UTC (permalink / raw)
  To: Fabio Berton; +Cc: OE-core

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On 3 March 2017 at 12:12, Fabio Berton <fabio.berton@ossystems.com.br>
wrote:

> Sorry for delay. This error for qemux86 is because common.wks.inc and
> directdisk-gpt.wks files in scripts/lib/wic/canned-wks directory doesn't
> have option --fstype= for part /boot.
>
> For qemuarm machines I get the error:
>
> | DEBUG: Executing python function set_image_size
> | DEBUG: Python function set_image_size finished
> | DEBUG: Executing shell function do_image_wic
> | Error: Please build syslinux first
>

There's patches queued that will change the behaviour here and likely cause
your patch to not apply - can you rebase on top of poky-contrib:ed/wic/wip
and see what happens?  The build dependency is now wic-tools.

Ross

[-- Attachment #2: Type: text/html, Size: 1206 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-03 13:49     ` Burton, Ross
@ 2017-03-06 14:00       ` Fabio Berton
  2017-03-06 18:13         ` Ed Bartosh
  0 siblings, 1 reply; 13+ messages in thread
From: Fabio Berton @ 2017-03-06 14:00 UTC (permalink / raw)
  To: Burton, Ross; +Cc: OE-core

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

Same error with poky-contrib:ed/wic/wip, we need to add --fstype= to 
part /boot line in all wks files.


On 03/03/2017 10:49 AM, Burton, Ross wrote:
>
> On 3 March 2017 at 12:12, Fabio Berton <fabio.berton@ossystems.com.br 
> <mailto:fabio.berton@ossystems.com.br>> wrote:
>
>     Sorry for delay. This error for qemux86 is because common.wks.inc
>     and directdisk-gpt.wks files in scripts/lib/wic/canned-wks
>     directory doesn't have option --fstype= for part /boot.
>
>     For qemuarm machines I get the error:
>
>     | DEBUG: Executing python function set_image_size
>     | DEBUG: Python function set_image_size finished
>     | DEBUG: Executing shell function do_image_wic
>     | Error: Please build syslinux first
>
>
> There's patches queued that will change the behaviour here and likely 
> cause your patch to not apply - can you rebase on top of 
> poky-contrib:ed/wic/wip and see what happens?  The build dependency is 
> now wic-tools.
>
> Ross


[-- Attachment #2: Type: text/html, Size: 2169 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-06 14:00       ` Fabio Berton
@ 2017-03-06 18:13         ` Ed Bartosh
  2017-03-06 18:48           ` Fabio Berton
  0 siblings, 1 reply; 13+ messages in thread
From: Ed Bartosh @ 2017-03-06 18:13 UTC (permalink / raw)
  To: Fabio Berton; +Cc: OE-core

On Mon, Mar 06, 2017 at 11:00:59AM -0300, Fabio Berton wrote:
> Same error with poky-contrib:ed/wic/wip, we need to add --fstype= to
> part /boot line in all wks files.
> 
Only if we want boot partitions to be added to fstab. Currently / and
/boot partitions are not added. Why do we want to change this?

BTW, your patch will make wic to parse the same fstab file several
times. Can you change this, please?

> 
> On 03/03/2017 10:49 AM, Burton, Ross wrote:
> >
> >On 3 March 2017 at 12:12, Fabio Berton
> ><fabio.berton@ossystems.com.br
> ><mailto:fabio.berton@ossystems.com.br>> wrote:
> >
> >    Sorry for delay. This error for qemux86 is because common.wks.inc
> >    and directdisk-gpt.wks files in scripts/lib/wic/canned-wks
> >    directory doesn't have option --fstype= for part /boot.
> >
> >    For qemuarm machines I get the error:
> >
> >    | DEBUG: Executing python function set_image_size
> >    | DEBUG: Python function set_image_size finished
> >    | DEBUG: Executing shell function do_image_wic
> >    | Error: Please build syslinux first
> >
> >
> >There's patches queued that will change the behaviour here and
> >likely cause your patch to not apply - can you rebase on top of
> >poky-contrib:ed/wic/wip and see what happens?  The build
> >dependency is now wic-tools.
> >
> >Ross
> 

> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-06 18:13         ` Ed Bartosh
@ 2017-03-06 18:48           ` Fabio Berton
  2017-03-06 19:07             ` Ed Bartosh
  0 siblings, 1 reply; 13+ messages in thread
From: Fabio Berton @ 2017-03-06 18:48 UTC (permalink / raw)
  To: ed.bartosh; +Cc: OE-core

Hi Ed,

The main motivation to my patch is prevent to duplicate entries. For 
example, if I add to my fstab line:

LABEL=data           /data                auto defaults              0  1

and add to wsk file:

part /data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192 
--size 500M --extra-space 0

Final fstab will have two entries for /data.

In most Linux distros mount /boot partition, if we have kernel image or 
boot script to update we need to mount /boot partition. Why the reason 
to not mount /boot?

I'll look into parser issue.

Fabio

On 03/06/2017 03:13 PM, Ed Bartosh wrote:
> On Mon, Mar 06, 2017 at 11:00:59AM -0300, Fabio Berton wrote:
>> Same error with poky-contrib:ed/wic/wip, we need to add --fstype= to
>> part /boot line in all wks files.
>>
> Only if we want boot partitions to be added to fstab. Currently / and
> /boot partitions are not added. Why do we want to change this?
>
> BTW, your patch will make wic to parse the same fstab file several
> times. Can you change this, please?
>
>> On 03/03/2017 10:49 AM, Burton, Ross wrote:
>>> On 3 March 2017 at 12:12, Fabio Berton
>>> <fabio.berton@ossystems.com.br
>>> <mailto:fabio.berton@ossystems.com.br>> wrote:
>>>
>>>     Sorry for delay. This error for qemux86 is because common.wks.inc
>>>     and directdisk-gpt.wks files in scripts/lib/wic/canned-wks
>>>     directory doesn't have option --fstype= for part /boot.
>>>
>>>     For qemuarm machines I get the error:
>>>
>>>     | DEBUG: Executing python function set_image_size
>>>     | DEBUG: Python function set_image_size finished
>>>     | DEBUG: Executing shell function do_image_wic
>>>     | Error: Please build syslinux first
>>>
>>>
>>> There's patches queued that will change the behaviour here and
>>> likely cause your patch to not apply - can you rebase on top of
>>> poky-contrib:ed/wic/wip and see what happens?  The build
>>> dependency is now wic-tools.
>>>
>>> Ross
>> -- 
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-06 18:48           ` Fabio Berton
@ 2017-03-06 19:07             ` Ed Bartosh
  2017-03-09 21:11               ` Otavio Salvador
  0 siblings, 1 reply; 13+ messages in thread
From: Ed Bartosh @ 2017-03-06 19:07 UTC (permalink / raw)
  To: Fabio Berton; +Cc: OE-core

On Mon, Mar 06, 2017 at 03:48:00PM -0300, Fabio Berton wrote:
> Hi Ed,
> 
> The main motivation to my patch is prevent to duplicate entries. For
> example, if I add to my fstab line:
> 
> LABEL=data           /data                auto defaults              0  1
> 
> and add to wsk file:
> 
> part /data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192
> --size 500M --extra-space 0
> 
> Final fstab will have two entries for /data.
This can be easily avoided if you remove leading slash:
part data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192 --size 500M --extra-space 0


> 
> In most Linux distros mount /boot partition, if we have kernel image
> or boot script to update we need to mount /boot partition. Why the
> reason to not mount /boot?
>
The code that skips / and /boot was brought to wic codebase more than 4
years ago: https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=75c143a7aef46ecea07cf33edd2b1a0192e10149

I don't know exact reason to be honest. However, I think we need to be careful with this
kind of legacy. It doesn't mean we shouldn't remove it, but it should
not be done as a side effect of the patch addressing absolutely
different issue, I believe.

> >>>    For qemuarm machines I get the error:
> >>>
> >>>    | DEBUG: Executing python function set_image_size
> >>>    | DEBUG: Python function set_image_size finished
> >>>    | DEBUG: Executing shell function do_image_wic
> >>>    | Error: Please build syslinux first

Sorry, missed this. It looks like you're trying to use plugin that
depends on syslinux. For example, directdisk.wks uses bootimg-pcbios,
which uses syslinux. It's x86 specific and should not be used on arm.

--
Regards,
Ed


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-06 19:07             ` Ed Bartosh
@ 2017-03-09 21:11               ` Otavio Salvador
  2017-03-10  7:33                 ` Patrick Ohly
  0 siblings, 1 reply; 13+ messages in thread
From: Otavio Salvador @ 2017-03-09 21:11 UTC (permalink / raw)
  To: Ed Bartosh, Burton, Ross; +Cc: OE-core

On Mon, Mar 6, 2017 at 4:07 PM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Mon, Mar 06, 2017 at 03:48:00PM -0300, Fabio Berton wrote:
>> Hi Ed,
>>
>> The main motivation to my patch is prevent to duplicate entries. For
>> example, if I add to my fstab line:
>>
>> LABEL=data           /data                auto defaults              0  1
>>
>> and add to wsk file:
>>
>> part /data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192
>> --size 500M --extra-space 0
>>
>> Final fstab will have two entries for /data.
> This can be easily avoided if you remove leading slash:
> part data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192 --size 500M --extra-space 0

HACK ALERT!

>> In most Linux distros mount /boot partition, if we have kernel image
>> or boot script to update we need to mount /boot partition. Why the
>> reason to not mount /boot?
>>
> The code that skips / and /boot was brought to wic codebase more than 4
> years ago: https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=75c143a7aef46ecea07cf33edd2b1a0192e10149
>
> I don't know exact reason to be honest. However, I think we need to be careful with this
> kind of legacy. It doesn't mean we shouldn't remove it, but it should
> not be done as a side effect of the patch addressing absolutely
> different issue, I believe.

While discussing this with Fabio here, at O.S. Systems, we ended
agreeing that wic touching the fstab is wrong. The fstab should be
prepare as part of the image and not mangled during the disk
generation.

The mangled fstab is disastrous if someone uses an image upgrade. The
generated tarball or filesystem WILL NOT be the same running on the
device as wic will add entries.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-09 21:11               ` Otavio Salvador
@ 2017-03-10  7:33                 ` Patrick Ohly
  2017-03-10 13:32                   ` Otavio Salvador
  0 siblings, 1 reply; 13+ messages in thread
From: Patrick Ohly @ 2017-03-10  7:33 UTC (permalink / raw)
  To: Otavio Salvador; +Cc: OE-core

On Thu, 2017-03-09 at 18:11 -0300, Otavio Salvador wrote:
> On Mon, Mar 6, 2017 at 4:07 PM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > On Mon, Mar 06, 2017 at 03:48:00PM -0300, Fabio Berton wrote:
> >> Hi Ed,
> >>
> >> The main motivation to my patch is prevent to duplicate entries. For
> >> example, if I add to my fstab line:
> >>
> >> LABEL=data           /data                auto defaults              0  1
> >>
> >> and add to wsk file:
> >>
> >> part /data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192
> >> --size 500M --extra-space 0
> >>
> >> Final fstab will have two entries for /data.
> > This can be easily avoided if you remove leading slash:
> > part data --ondisk mmcblk0 --fstype=ext4 --label data --align 8192 --size 500M --extra-space 0
> 
> HACK ALERT!
> 
> >> In most Linux distros mount /boot partition, if we have kernel image
> >> or boot script to update we need to mount /boot partition. Why the
> >> reason to not mount /boot?
> >>
> > The code that skips / and /boot was brought to wic codebase more than 4
> > years ago: https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=75c143a7aef46ecea07cf33edd2b1a0192e10149
> >
> > I don't know exact reason to be honest. However, I think we need to be careful with this
> > kind of legacy. It doesn't mean we shouldn't remove it, but it should
> > not be done as a side effect of the patch addressing absolutely
> > different issue, I believe.
> 
> While discussing this with Fabio here, at O.S. Systems, we ended
> agreeing that wic touching the fstab is wrong. The fstab should be
> prepare as part of the image and not mangled during the disk
> generation.

I agree that it is a hack, and I also would prefer to not have wic
modify the existing rootfs. That also breaks when IMA is enabled,
because then the content of the /etc/fstab must match the security.ima
xattr that was calculated for the unmodified content.

However, it's a problem that doesn't have a good solution. The image
recipe which describes what goes into the rootfs and thus determines the
content of /etc/fstab has little control over the IMAGE_FSTYPES - that's
typically set by the BSP or the user.

Suppose IMAGE_FSTYPES = "ext4 wic", and the WKS_FILE has multiple
partitions and thus needs more entries in /etc/fstab than the
single-partition "ext4" - the result of do_rootfs simply cannot work for
both.

Right now, all one can do is assume (or perhaps check) that the right
IMAGE_FSTYPES are set.

> The mangled fstab is disastrous if someone uses an image upgrade. The
> generated tarball or filesystem WILL NOT be the same running on the
> device as wic will add entries.

When do you take a snapshot of the rootfs? Is it as another do_image_*
task, via an IMAGE_FSTYPE entry?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-10  7:33                 ` Patrick Ohly
@ 2017-03-10 13:32                   ` Otavio Salvador
  2017-03-10 13:43                     ` Patrick Ohly
  0 siblings, 1 reply; 13+ messages in thread
From: Otavio Salvador @ 2017-03-10 13:32 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: OE-core

On Fri, Mar 10, 2017 at 4:33 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Thu, 2017-03-09 at 18:11 -0300, Otavio Salvador wrote:
...
>> While discussing this with Fabio here, at O.S. Systems, we ended
>> agreeing that wic touching the fstab is wrong. The fstab should be
>> prepare as part of the image and not mangled during the disk
>> generation.
>
> I agree that it is a hack, and I also would prefer to not have wic
> modify the existing rootfs. That also breaks when IMA is enabled,
> because then the content of the /etc/fstab must match the security.ima
> xattr that was calculated for the unmodified content.

Exactly! the generated rootfs should not be modified. This is critical
for IMA but also for root-only fs and delta-based upgrades.

> However, it's a problem that doesn't have a good solution. The image
> recipe which describes what goes into the rootfs and thus determines the
> content of /etc/fstab has little control over the IMAGE_FSTYPES - that's
> typically set by the BSP or the user.

Yes but the /etc/fstab should be modified by the metadata and not
during the image generation. The format used (ext4, fsfs, wic...)
should not impact in the contents inside of the image.

> Suppose IMAGE_FSTYPES = "ext4 wic", and the WKS_FILE has multiple
> partitions and thus needs more entries in /etc/fstab than the
> single-partition "ext4" - the result of do_rootfs simply cannot work for
> both.

That is the point. If the machine requires to use multiple partitions
the ext4 should have the /etc/fstab ready for use when deployed.

> Right now, all one can do is assume (or perhaps check) that the right
> IMAGE_FSTYPES are set.
>
>> The mangled fstab is disastrous if someone uses an image upgrade. The
>> generated tarball or filesystem WILL NOT be the same running on the
>> device as wic will add entries.
>
> When do you take a snapshot of the rootfs? Is it as another do_image_*
> task, via an IMAGE_FSTYPE entry?

We generally use the pristine ext4 filesystem, for example.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-10 13:32                   ` Otavio Salvador
@ 2017-03-10 13:43                     ` Patrick Ohly
  2017-03-10 13:51                       ` Otavio Salvador
  0 siblings, 1 reply; 13+ messages in thread
From: Patrick Ohly @ 2017-03-10 13:43 UTC (permalink / raw)
  To: Otavio Salvador; +Cc: OE-core

On Fri, 2017-03-10 at 10:32 -0300, Otavio Salvador wrote:
> On Fri, Mar 10, 2017 at 4:33 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > Suppose IMAGE_FSTYPES = "ext4 wic", and the WKS_FILE has multiple
> > partitions and thus needs more entries in /etc/fstab than the
> > single-partition "ext4" - the result of do_rootfs simply cannot work for
> > both.
> 
> That is the point. If the machine requires to use multiple partitions
> the ext4 should have the /etc/fstab ready for use when deployed.

My thinking was a bit different. I had in mind a setup where the "ext4"
format as used by qemu has all files of the rootfs in a single
partition, whereas the wic image splits out the data into multiple
partitions (it recently gained such a feature).

This could be made to work if wic knew how to change fstab without
rewriting the root filesystem. But as that can't be done, such a
combination of different image formats is only possible for a rootfs
which always works with just a single partition.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] wic: Prevent duplicate entries on fstab
  2017-03-10 13:43                     ` Patrick Ohly
@ 2017-03-10 13:51                       ` Otavio Salvador
  0 siblings, 0 replies; 13+ messages in thread
From: Otavio Salvador @ 2017-03-10 13:51 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: OE-core

On Fri, Mar 10, 2017 at 10:43 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Fri, 2017-03-10 at 10:32 -0300, Otavio Salvador wrote:
>> On Fri, Mar 10, 2017 at 4:33 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> > Suppose IMAGE_FSTYPES = "ext4 wic", and the WKS_FILE has multiple
>> > partitions and thus needs more entries in /etc/fstab than the
>> > single-partition "ext4" - the result of do_rootfs simply cannot work for
>> > both.
>>
>> That is the point. If the machine requires to use multiple partitions
>> the ext4 should have the /etc/fstab ready for use when deployed.
>
> My thinking was a bit different. I had in mind a setup where the "ext4"
> format as used by qemu has all files of the rootfs in a single
> partition, whereas the wic image splits out the data into multiple
> partitions (it recently gained such a feature).
>
> This could be made to work if wic knew how to change fstab without
> rewriting the root filesystem. But as that can't be done, such a
> combination of different image formats is only possible for a rootfs
> which always works with just a single partition.

So it seems like we need a command line option for wic which says for
it to generate/do not generate the /etc/fstab.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-03-10 13:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-23 18:13 [PATCH] wic: Prevent duplicate entries on fstab Fabio Berton
2017-02-24 23:02 ` Burton, Ross
2017-03-03 12:12   ` Fabio Berton
2017-03-03 13:49     ` Burton, Ross
2017-03-06 14:00       ` Fabio Berton
2017-03-06 18:13         ` Ed Bartosh
2017-03-06 18:48           ` Fabio Berton
2017-03-06 19:07             ` Ed Bartosh
2017-03-09 21:11               ` Otavio Salvador
2017-03-10  7:33                 ` Patrick Ohly
2017-03-10 13:32                   ` Otavio Salvador
2017-03-10 13:43                     ` Patrick Ohly
2017-03-10 13:51                       ` Otavio Salvador

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.