* [PATCH v2 1/5] wic: copy rootfs directory before changing fstab
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
@ 2017-06-27 13:28 ` Ed Bartosh
2017-07-04 22:16 ` Burton, Ross
2017-06-27 13:28 ` [PATCH v2 2/5] wic: use absolute paths in rootfs plugin Ed Bartosh
` (4 subsequent siblings)
5 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-27 13:28 UTC (permalink / raw)
To: openembedded-core
wic updates /etc/fstab on root partition if there are
valid mount points in .wks
When wic runs from bitbake this can cause incorrect results
or even breakage of other tasks working with rootfs directory
in parallel with do_image_wic.
Implemented copying rootfs directory to a temporary location
using copyhardlinktree before updating fstab to avoid conflicts with
other tasks working with rootfs.
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
scripts/lib/wic/plugins/imager/direct.py | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index aa9cc9f..f707365 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -32,6 +32,8 @@ import uuid
from time import strftime
+from oe.path import copyhardlinktree
+
from wic import WicError
from wic.filemap import sparse_copy
from wic.ksparser import KickStart, KickStartError
@@ -115,12 +117,16 @@ class DirectPlugin(ImagerPlugin):
fstab_lines = fstab.readlines()
if self._update_fstab(fstab_lines, self.parts):
- shutil.copyfile(fstab_path, fstab_path + ".orig")
+ # copy rootfs dir to workdir to update fstab
+ # as rootfs can be used by other tasks and can't be modified
+ new_rootfs = os.path.realpath(os.path.join(self.workdir, "rootfs_copy"))
+ copyhardlinktree(image_rootfs, new_rootfs)
+ fstab_path = os.path.join(new_rootfs, 'etc/fstab')
with open(fstab_path, "w") as fstab:
fstab.writelines(fstab_lines)
- return fstab_path
+ return new_rootfs
def _update_fstab(self, fstab_lines, parts):
"""Assume partition order same as in wks"""
@@ -156,7 +162,10 @@ class DirectPlugin(ImagerPlugin):
filesystems from the artifacts directly and combine them into
a partitioned image.
"""
- fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
+ new_rootfs = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
+ if new_rootfs:
+ # rootfs was copied to update fstab
+ self.rootfs_dir['ROOTFS_DIR'] = new_rootfs
for part in self.parts:
# get rootfs size from bitbake variable if it's not set in .ks file
@@ -172,12 +181,7 @@ class DirectPlugin(ImagerPlugin):
if rsize_bb:
part.size = int(round(float(rsize_bb)))
- try:
- self._image.prepare(self)
- finally:
- if fstab_path:
- shutil.move(fstab_path + ".orig", fstab_path)
-
+ self._image.prepare(self)
self._image.layout_partitions()
self._image.create()
--
2.1.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/5] wic: copy rootfs directory before changing fstab
2017-06-27 13:28 ` [PATCH v2 1/5] wic: copy rootfs directory before changing fstab Ed Bartosh
@ 2017-07-04 22:16 ` Burton, Ross
2017-07-05 8:01 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Burton, Ross @ 2017-07-04 22:16 UTC (permalink / raw)
To: Ed Bartosh; +Cc: OE-core
[-- Attachment #1: Type: text/plain, Size: 3853 bytes --]
Bad news: the failure this was meant to fix happened again.
| tar: ./opt/ltp/testcases/bin/dup01: file changed as we read it
| ERROR: Function failed: do_image_tar (log file is located at
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-arm/build/build/tmp/work/beaglebone-poky-linux-gnueabi/core-image-sato-sdk-ptest/1.0-r0/temp/log.do_image_tar.5535)
As before, image_wic was also running.
Is there any chance this could still be wic, or should we look at the jffs2
image generator?
Ross
On 27 June 2017 at 14:28, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
>
> wic updates /etc/fstab on root partition if there are
> valid mount points in .wks
>
> When wic runs from bitbake this can cause incorrect results
> or even breakage of other tasks working with rootfs directory
> in parallel with do_image_wic.
>
> Implemented copying rootfs directory to a temporary location
> using copyhardlinktree before updating fstab to avoid conflicts with
> other tasks working with rootfs.
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> ---
> scripts/lib/wic/plugins/imager/direct.py | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/lib/wic/plugins/imager/direct.py
b/scripts/lib/wic/plugins/imager/direct.py
> index aa9cc9f..f707365 100644
> --- a/scripts/lib/wic/plugins/imager/direct.py
> +++ b/scripts/lib/wic/plugins/imager/direct.py
> @@ -32,6 +32,8 @@ import uuid
>
> from time import strftime
>
> +from oe.path import copyhardlinktree
> +
> from wic import WicError
> from wic.filemap import sparse_copy
> from wic.ksparser import KickStart, KickStartError
> @@ -115,12 +117,16 @@ class DirectPlugin(ImagerPlugin):
> fstab_lines = fstab.readlines()
>
> if self._update_fstab(fstab_lines, self.parts):
> - shutil.copyfile(fstab_path, fstab_path + ".orig")
> + # copy rootfs dir to workdir to update fstab
> + # as rootfs can be used by other tasks and can't be modified
> + new_rootfs = os.path.realpath(os.path.join(self.workdir,
"rootfs_copy"))
> + copyhardlinktree(image_rootfs, new_rootfs)
> + fstab_path = os.path.join(new_rootfs, 'etc/fstab')
>
> with open(fstab_path, "w") as fstab:
> fstab.writelines(fstab_lines)
>
> - return fstab_path
> + return new_rootfs
>
> def _update_fstab(self, fstab_lines, parts):
> """Assume partition order same as in wks"""
> @@ -156,7 +162,10 @@ class DirectPlugin(ImagerPlugin):
> filesystems from the artifacts directly and combine them into
> a partitioned image.
> """
> - fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> + new_rootfs = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> + if new_rootfs:
> + # rootfs was copied to update fstab
> + self.rootfs_dir['ROOTFS_DIR'] = new_rootfs
>
> for part in self.parts:
> # get rootfs size from bitbake variable if it's not set in
.ks file
> @@ -172,12 +181,7 @@ class DirectPlugin(ImagerPlugin):
> if rsize_bb:
> part.size = int(round(float(rsize_bb)))
>
> - try:
> - self._image.prepare(self)
> - finally:
> - if fstab_path:
> - shutil.move(fstab_path + ".orig", fstab_path)
> -
> + self._image.prepare(self)
> self._image.layout_partitions()
> self._image.create()
>
> --
> 2.1.4
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
[-- Attachment #2: Type: text/html, Size: 4934 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/5] wic: copy rootfs directory before changing fstab
2017-07-04 22:16 ` Burton, Ross
@ 2017-07-05 8:01 ` Ed Bartosh
2017-07-05 10:38 ` Burton, Ross
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-07-05 8:01 UTC (permalink / raw)
To: Burton, Ross; +Cc: OE-core
On Tue, Jul 04, 2017 at 11:16:23PM +0100, Burton, Ross wrote:
> Bad news: the failure this was meant to fix happened again.
>
> | tar: ./opt/ltp/testcases/bin/dup01: file changed as we read it
> | ERROR: Function failed: do_image_tar (log file is located at
> /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-arm/build/build/tmp/work/beaglebone-poky-linux-gnueabi/core-image-sato-sdk-ptest/1.0-r0/temp/log.do_image_tar.5535)
>
> As before, image_wic was also running.
>
> Is there any chance this could still be wic, or should we look at the jffs2
> image generator?
>
There is always a chance. Which commit is this? I'll try to reproduce this.
Regards,
Ed
> Ross
>
> On 27 June 2017 at 14:28, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> >
> > wic updates /etc/fstab on root partition if there are
> > valid mount points in .wks
> >
> > When wic runs from bitbake this can cause incorrect results
> > or even breakage of other tasks working with rootfs directory
> > in parallel with do_image_wic.
> >
> > Implemented copying rootfs directory to a temporary location
> > using copyhardlinktree before updating fstab to avoid conflicts with
> > other tasks working with rootfs.
> >
> > Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> > ---
> > scripts/lib/wic/plugins/imager/direct.py | 22 +++++++++++++---------
> > 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/scripts/lib/wic/plugins/imager/direct.py
> b/scripts/lib/wic/plugins/imager/direct.py
> > index aa9cc9f..f707365 100644
> > --- a/scripts/lib/wic/plugins/imager/direct.py
> > +++ b/scripts/lib/wic/plugins/imager/direct.py
> > @@ -32,6 +32,8 @@ import uuid
> >
> > from time import strftime
> >
> > +from oe.path import copyhardlinktree
> > +
> > from wic import WicError
> > from wic.filemap import sparse_copy
> > from wic.ksparser import KickStart, KickStartError
> > @@ -115,12 +117,16 @@ class DirectPlugin(ImagerPlugin):
> > fstab_lines = fstab.readlines()
> >
> > if self._update_fstab(fstab_lines, self.parts):
> > - shutil.copyfile(fstab_path, fstab_path + ".orig")
> > + # copy rootfs dir to workdir to update fstab
> > + # as rootfs can be used by other tasks and can't be modified
> > + new_rootfs = os.path.realpath(os.path.join(self.workdir,
> "rootfs_copy"))
> > + copyhardlinktree(image_rootfs, new_rootfs)
> > + fstab_path = os.path.join(new_rootfs, 'etc/fstab')
> >
> > with open(fstab_path, "w") as fstab:
> > fstab.writelines(fstab_lines)
> >
> > - return fstab_path
> > + return new_rootfs
> >
> > def _update_fstab(self, fstab_lines, parts):
> > """Assume partition order same as in wks"""
> > @@ -156,7 +162,10 @@ class DirectPlugin(ImagerPlugin):
> > filesystems from the artifacts directly and combine them into
> > a partitioned image.
> > """
> > - fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> > + new_rootfs = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> > + if new_rootfs:
> > + # rootfs was copied to update fstab
> > + self.rootfs_dir['ROOTFS_DIR'] = new_rootfs
> >
> > for part in self.parts:
> > # get rootfs size from bitbake variable if it's not set in
> .ks file
> > @@ -172,12 +181,7 @@ class DirectPlugin(ImagerPlugin):
> > if rsize_bb:
> > part.size = int(round(float(rsize_bb)))
> >
> > - try:
> > - self._image.prepare(self)
> > - finally:
> > - if fstab_path:
> > - shutil.move(fstab_path + ".orig", fstab_path)
> > -
> > + self._image.prepare(self)
> > self._image.layout_partitions()
> > self._image.create()
> >
> > --
> > 2.1.4
> >
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
--
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 2/5] wic: use absolute paths in rootfs plugin
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 1/5] wic: copy rootfs directory before changing fstab Ed Bartosh
@ 2017-06-27 13:28 ` Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 3/5] wic: rootfs: fix rootfs path reporting Ed Bartosh
` (3 subsequent siblings)
5 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-06-27 13:28 UTC (permalink / raw)
To: openembedded-core
Using relative paths can cause copyhardlinktree API to fail as
it changes current directory when working. Converted all paths
to absolute paths using os.path.realpath.
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
scripts/lib/wic/plugins/source/rootfs.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 4dc4cb8..c08f760 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -48,7 +48,7 @@ class RootfsPlugin(SourcePlugin):
@staticmethod
def __get_rootfs_dir(rootfs_dir):
if os.path.isdir(rootfs_dir):
- return rootfs_dir
+ return os.path.realpath(rootfs_dir)
image_rootfs_dir = get_bitbake_var("IMAGE_ROOTFS", rootfs_dir)
if not os.path.isdir(image_rootfs_dir):
@@ -56,7 +56,7 @@ class RootfsPlugin(SourcePlugin):
"named %s has been found at %s, exiting." %
(rootfs_dir, image_rootfs_dir))
- return image_rootfs_dir
+ return os.path.realpath(image_rootfs_dir)
@classmethod
def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
--
2.1.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 3/5] wic: rootfs: fix rootfs path reporting
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 1/5] wic: copy rootfs directory before changing fstab Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 2/5] wic: use absolute paths in rootfs plugin Ed Bartosh
@ 2017-06-27 13:28 ` Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 4/5] wic: rootfs: make copied rootfs unique Ed Bartosh
` (2 subsequent siblings)
5 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-06-27 13:28 UTC (permalink / raw)
To: openembedded-core
wic gets rootfs paths from partition object property
'rootfs_dir' and shows them in final report.
rootfs plugin sets this property to the temporary path,
which causes temporary paths appearing in the report.
wic includes rootfs path in the final report. When fstab needs to be
updated wic copies rootfs to a temporary location. This causes path
to the copy of rootfs to appear in the final report.
Changed the code to prevent storing temporary rootfs path
in part.rootfs_dir. This should fix the report.
Saved original rootfs path when rootfs is copied to include original
path to the report.
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
scripts/lib/wic/plugins/imager/direct.py | 7 ++++++-
scripts/lib/wic/plugins/source/rootfs.py | 10 ++++------
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index f707365..e90f9f9 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -62,6 +62,7 @@ class DirectPlugin(ImagerPlugin):
# parse possible 'rootfs=name' items
self.rootfs_dir = dict(rdir.split('=') for rdir in rootfs_dir.split(' '))
+ self.rootfs_dir_orig = None
self.bootimg_dir = bootimg_dir
self.kernel_dir = kernel_dir
self.native_sysroot = native_sysroot
@@ -165,6 +166,7 @@ class DirectPlugin(ImagerPlugin):
new_rootfs = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
if new_rootfs:
# rootfs was copied to update fstab
+ self.rootfs_dir_orig = self.rootfs_dir['ROOTFS_DIR']
self.rootfs_dir['ROOTFS_DIR'] = new_rootfs
for part in self.parts:
@@ -234,11 +236,14 @@ class DirectPlugin(ImagerPlugin):
for part in self.parts:
if part.rootfs_dir is None:
continue
+ rootfs_dir = part.rootfs_dir
if part.mountpoint == '/':
suffix = ':'
+ if self.rootfs_dir_orig:
+ rootfs_dir = self.rootfs_dir_orig
else:
suffix = '["%s"]:' % (part.mountpoint or part.label)
- msg += ' ROOTFS_DIR%s%s\n' % (suffix.ljust(20), part.rootfs_dir)
+ msg += ' ROOTFS_DIR%s%s\n' % (suffix.ljust(20), rootfs_dir)
msg += ' BOOTIMG_DIR: %s\n' % self.bootimg_dir
msg += ' KERNEL_DIR: %s\n' % self.kernel_dir
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index c08f760..e438158 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -81,8 +81,9 @@ class RootfsPlugin(SourcePlugin):
raise WicError("Couldn't find --rootfs-dir=%s connection or "
"it is not a valid path, exiting" % part.rootfs_dir)
- real_rootfs_dir = cls.__get_rootfs_dir(rootfs_dir)
+ part.rootfs_dir = cls.__get_rootfs_dir(rootfs_dir)
+ new_rootfs = None
# Handle excluded paths.
if part.exclude_path is not None:
# We need a new rootfs directory we can delete files from. Copy to
@@ -92,9 +93,7 @@ class RootfsPlugin(SourcePlugin):
if os.path.lexists(new_rootfs):
shutil.rmtree(os.path.join(new_rootfs))
- copyhardlinktree(real_rootfs_dir, new_rootfs)
-
- real_rootfs_dir = new_rootfs
+ copyhardlinktree(part.rootfs_dir, new_rootfs)
for orig_path in part.exclude_path:
path = orig_path
@@ -123,6 +122,5 @@ class RootfsPlugin(SourcePlugin):
# Delete whole directory.
shutil.rmtree(full_path)
- part.rootfs_dir = real_rootfs_dir
part.prepare_rootfs(cr_workdir, oe_builddir,
- real_rootfs_dir, native_sysroot)
+ new_rootfs or part.rootfs_dir, native_sysroot)
--
2.1.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 4/5] wic: rootfs: make copied rootfs unique
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
` (2 preceding siblings ...)
2017-06-27 13:28 ` [PATCH v2 3/5] wic: rootfs: fix rootfs path reporting Ed Bartosh
@ 2017-06-27 13:28 ` Ed Bartosh
2017-06-27 13:28 ` [PATCH v2 5/5] wic: add /boot mount point to fstab by default Ed Bartosh
2017-06-27 20:35 ` [PATCH v2 0/5] #11662 - wic should mount /boot Otavio Salvador
5 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-06-27 13:28 UTC (permalink / raw)
To: openembedded-core
Used unique suffix (line number from .wks file) for the
copied rootfs directory to avoid possible conflicts.
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
scripts/lib/wic/plugins/source/rootfs.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index e438158..aec720f 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -88,7 +88,7 @@ class RootfsPlugin(SourcePlugin):
if part.exclude_path is not None:
# We need a new rootfs directory we can delete files from. Copy to
# workdir.
- new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs"))
+ new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
if os.path.lexists(new_rootfs):
shutil.rmtree(os.path.join(new_rootfs))
--
2.1.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 5/5] wic: add /boot mount point to fstab by default
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
` (3 preceding siblings ...)
2017-06-27 13:28 ` [PATCH v2 4/5] wic: rootfs: make copied rootfs unique Ed Bartosh
@ 2017-06-27 13:28 ` Ed Bartosh
2017-06-27 20:35 ` [PATCH v2 0/5] #11662 - wic should mount /boot Otavio Salvador
5 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-06-27 13:28 UTC (permalink / raw)
To: openembedded-core
wic avoided adding /boot to fstab for no reason.
This exception was hardcoded in the wic code.
There is no need for this as mountpoint in .wks file is an optional
field. It can be used only if user wants to have partitions
automatically mounted on system boot.
Fixed test_quemu test case failure caused by changed
amount of mounted partitions.
[YOCTO #11662]
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
meta/lib/oeqa/selftest/cases/wic.py | 2 +-
scripts/lib/wic/plugins/imager/direct.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py
index dd72db5..e27dbe9 100644
--- a/meta/lib/oeqa/selftest/cases/wic.py
+++ b/meta/lib/oeqa/selftest/cases/wic.py
@@ -622,7 +622,7 @@ part /etc --source rootfs --ondisk mmcblk0 --fstype=ext4 --exclude-path bin/ --r
cmd = "mount |grep '^/dev/' | cut -f1,3 -d ' '"
status, output = qemu.run_serial(cmd)
self.assertEqual(1, status, 'Failed to run command "%s": %s' % (cmd, output))
- self.assertEqual(output, '/dev/root /\r\n/dev/sda3 /mnt')
+ self.assertEqual(output, '/dev/root /\r\n/dev/sda1 /boot\r\n/dev/sda3 /mnt')
@only_for_arch(['i586', 'i686', 'x86_64'])
@OETestID(1852)
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index e90f9f9..f6f29aa 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -134,7 +134,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 == "/":
continue
# mmc device partitions are named mmcblk0p1, mmcblk0p2..
--
2.1.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-27 13:28 [PATCH v2 0/5] #11662 - wic should mount /boot Ed Bartosh
` (4 preceding siblings ...)
2017-06-27 13:28 ` [PATCH v2 5/5] wic: add /boot mount point to fstab by default Ed Bartosh
@ 2017-06-27 20:35 ` Otavio Salvador
2017-06-27 20:41 ` Fabio Berton
5 siblings, 1 reply; 26+ messages in thread
From: Otavio Salvador @ 2017-06-27 20:35 UTC (permalink / raw)
To: Ed Bartosh, Fabio Berton, Burton, Ross
Cc: Patches and discussions about the oe-core layer
On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> The patchset also fixes long standing bug: wic updated fstab
> inplace in rootfs directory. This causes other tasks working with
> rootfs directory to produce incorrect results or crash. This is
> fixed by copying rootfs to the temporary directory before updating
> fstab.
As you is working on this, please also include Fabio's patch on the
patchset. It includes a command like option to disable fstab change at
all. For delta-based updates this is imperative.
Fabio, could you point him the last patch revision?
--
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] 26+ messages in thread* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-27 20:35 ` [PATCH v2 0/5] #11662 - wic should mount /boot Otavio Salvador
@ 2017-06-27 20:41 ` Fabio Berton
2017-06-28 7:31 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Fabio Berton @ 2017-06-27 20:41 UTC (permalink / raw)
To: Otavio Salvador, Ed Bartosh, Burton, Ross
Cc: Patches and discussions about the oe-core layer
The last patch I sent is here:
https://patchwork.openembedded.org/patch/139252/
We're using this patch internally with Pyro branch. I can rework to
apply on master.
On 06/27/2017 05:35 PM, Otavio Salvador wrote:
> On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
>> The patchset also fixes long standing bug: wic updated fstab
>> inplace in rootfs directory. This causes other tasks working with
>> rootfs directory to produce incorrect results or crash. This is
>> fixed by copying rootfs to the temporary directory before updating
>> fstab.
>
> As you is working on this, please also include Fabio's patch on the
> patchset. It includes a command like option to disable fstab change at
> all. For delta-based updates this is imperative.
>
> Fabio, could you point him the last patch revision?
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-27 20:41 ` Fabio Berton
@ 2017-06-28 7:31 ` Ed Bartosh
2017-06-28 13:32 ` Otavio Salvador
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-28 7:31 UTC (permalink / raw)
To: Fabio Berton
Cc: Patches and discussions about the oe-core layer, Otavio Salvador
On Tue, Jun 27, 2017 at 05:41:45PM -0300, Fabio Berton wrote:
> The last patch I sent is here:
> https://patchwork.openembedded.org/patch/139252/
>
> We're using this patch internally with Pyro branch. I can rework to
> apply on master.
>
> On 06/27/2017 05:35 PM, Otavio Salvador wrote:
> >On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> >>The patchset also fixes long standing bug: wic updated fstab
> >>inplace in rootfs directory. This causes other tasks working with
> >>rootfs directory to produce incorrect results or crash. This is
> >>fixed by copying rootfs to the temporary directory before updating
> >>fstab.
> >
> >As you is working on this, please also include Fabio's patch on the
> >patchset. It includes a command like option to disable fstab change at
> >all. For delta-based updates this is imperative.
> >
> >Fabio, could you point him the last patch revision?
> >
Do we really need that?
JFYI: Mount point in .wks is an optional field. It makes sense to use it only
if partition needs to be mounted on boot. fstab will not be updated
unless it's explicitly requested by specifying mount points in .wks
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-28 7:31 ` Ed Bartosh
@ 2017-06-28 13:32 ` Otavio Salvador
2017-06-29 8:39 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Otavio Salvador @ 2017-06-28 13:32 UTC (permalink / raw)
To: Ed Bartosh; +Cc: Patches and discussions about the oe-core layer
On Wed, Jun 28, 2017 at 4:31 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Tue, Jun 27, 2017 at 05:41:45PM -0300, Fabio Berton wrote:
>> The last patch I sent is here:
>> https://patchwork.openembedded.org/patch/139252/
>>
>> We're using this patch internally with Pyro branch. I can rework to
>> apply on master.
>>
>> On 06/27/2017 05:35 PM, Otavio Salvador wrote:
>> >On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
>> >>The patchset also fixes long standing bug: wic updated fstab
>> >>inplace in rootfs directory. This causes other tasks working with
>> >>rootfs directory to produce incorrect results or crash. This is
>> >>fixed by copying rootfs to the temporary directory before updating
>> >>fstab.
>> >
>> >As you is working on this, please also include Fabio's patch on the
>> >patchset. It includes a command like option to disable fstab change at
>> >all. For delta-based updates this is imperative.
>> >
>> >Fabio, could you point him the last patch revision?
>> >
>
> Do we really need that?
>
> JFYI: Mount point in .wks is an optional field. It makes sense to use it only
> if partition needs to be mounted on boot. fstab will not be updated
> unless it's explicitly requested by specifying mount points in .wks
It should have support to not touch it. For images which we intend to
do delta updates, the content cannot be changed besides the original
rootfs generation. So yes, we need that.
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-28 13:32 ` Otavio Salvador
@ 2017-06-29 8:39 ` Ed Bartosh
2017-06-30 8:37 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-29 8:39 UTC (permalink / raw)
To: Otavio Salvador; +Cc: Patches and discussions about the oe-core layer
On Wed, Jun 28, 2017 at 10:32:27AM -0300, Otavio Salvador wrote:
> On Wed, Jun 28, 2017 at 4:31 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > On Tue, Jun 27, 2017 at 05:41:45PM -0300, Fabio Berton wrote:
> >> The last patch I sent is here:
> >> https://patchwork.openembedded.org/patch/139252/
> >>
> >> We're using this patch internally with Pyro branch. I can rework to
> >> apply on master.
> >>
> >> On 06/27/2017 05:35 PM, Otavio Salvador wrote:
> >> >On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> >> >>The patchset also fixes long standing bug: wic updated fstab
> >> >>inplace in rootfs directory. This causes other tasks working with
> >> >>rootfs directory to produce incorrect results or crash. This is
> >> >>fixed by copying rootfs to the temporary directory before updating
> >> >>fstab.
> >> >
> >> >As you is working on this, please also include Fabio's patch on the
> >> >patchset. It includes a command like option to disable fstab change at
> >> >all. For delta-based updates this is imperative.
> >> >
> >> >Fabio, could you point him the last patch revision?
> >> >
> >
> > Do we really need that?
> >
> > JFYI: Mount point in .wks is an optional field. It makes sense to use it only
> > if partition needs to be mounted on boot. fstab will not be updated
> > unless it's explicitly requested by specifying mount points in .wks
>
> It should have support to not touch it. For images which we intend to
> do delta updates, the content cannot be changed besides the original
> rootfs generation. So yes, we need that.
I'm not sure I understand this. If you don't want fstab to be changed
you should not specify mount points in .wks
There is only one reason to have mount points in .wks: to make wic to
change /etc/fstab, which you apparently don't want. So, don't specify
mount points and you'll have what you want.
Having additional option for this looks redundand to me.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-29 8:39 ` Ed Bartosh
@ 2017-06-30 8:37 ` Ed Bartosh
2017-06-30 9:02 ` Patrick Ohly
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-30 8:37 UTC (permalink / raw)
To: Otavio Salvador; +Cc: Patches and discussions about the oe-core layer
On Thu, Jun 29, 2017 at 11:39:42AM +0300, Ed Bartosh wrote:
> On Wed, Jun 28, 2017 at 10:32:27AM -0300, Otavio Salvador wrote:
> > On Wed, Jun 28, 2017 at 4:31 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > > On Tue, Jun 27, 2017 at 05:41:45PM -0300, Fabio Berton wrote:
> > >> The last patch I sent is here:
> > >> https://patchwork.openembedded.org/patch/139252/
> > >>
> > >> We're using this patch internally with Pyro branch. I can rework to
> > >> apply on master.
> > >>
> > >> On 06/27/2017 05:35 PM, Otavio Salvador wrote:
> > >> >On Tue, Jun 27, 2017 at 10:28 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > >> >>The patchset also fixes long standing bug: wic updated fstab
> > >> >>inplace in rootfs directory. This causes other tasks working with
> > >> >>rootfs directory to produce incorrect results or crash. This is
> > >> >>fixed by copying rootfs to the temporary directory before updating
> > >> >>fstab.
> > >> >
> > >> >As you is working on this, please also include Fabio's patch on the
> > >> >patchset. It includes a command like option to disable fstab change at
> > >> >all. For delta-based updates this is imperative.
> > >> >
> > >> >Fabio, could you point him the last patch revision?
> > >> >
> > >
> > > Do we really need that?
> > >
> > > JFYI: Mount point in .wks is an optional field. It makes sense to use it only
> > > if partition needs to be mounted on boot. fstab will not be updated
> > > unless it's explicitly requested by specifying mount points in .wks
> >
> > It should have support to not touch it. For images which we intend to
> > do delta updates, the content cannot be changed besides the original
> > rootfs generation. So yes, we need that.
>
> I'm not sure I understand this. If you don't want fstab to be changed
> you should not specify mount points in .wks
> There is only one reason to have mount points in .wks: to make wic to
> change /etc/fstab, which you apparently don't want. So, don't specify
> mount points and you'll have what you want.
>
> Having additional option for this looks redundand to me.
After thinking a bit more about it I'd propose to have global wic option
to avoid rootfs content changes. Not just fstab updates, but any
changes. For now this option (--no-rootfs-update ?) should prevent creating
images if either mount points are specified or --exclude-path is used in .wks
In future if any other rootfs changing functionality is added to wic it must
conflict with this option.
Does this make sense?
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 8:37 ` Ed Bartosh
@ 2017-06-30 9:02 ` Patrick Ohly
2017-06-30 12:23 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Patrick Ohly @ 2017-06-30 9:02 UTC (permalink / raw)
To: ed.bartosh
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
> > I'm not sure I understand this. If you don't want fstab to be
> changed
> > you should not specify mount points in .wks
> > There is only one reason to have mount points in .wks: to make wic
> to
> > change /etc/fstab, which you apparently don't want. So, don't
> specify
> > mount points and you'll have what you want.
> >
> > Having additional option for this looks redundand to me.
>
> After thinking a bit more about it I'd propose to have global wic
> option
> to avoid rootfs content changes. Not just fstab updates, but any
> changes. For now this option (--no-rootfs-update ?) should prevent
> creating
> images if either mount points are specified or --exclude-path is used
> in .wks
Why does --exclude-path conflict with --no-rootfs-update? Is that a
conceptual problem or an implementation problem?
If I'm not mistaken, --exclude-path merely means "take this rootfs, but
exclude certain parts". That's in line with --no-rootfs-update == "do
not modify the content of the rootfs", as it just helps with choosing
where content goes (the "single rootfs" -> "different partitions"
approach).
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 9:02 ` Patrick Ohly
@ 2017-06-30 12:23 ` Ed Bartosh
2017-06-30 13:16 ` Patrick Ohly
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-30 12:23 UTC (permalink / raw)
To: Patrick Ohly
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 11:02:13AM +0200, Patrick Ohly wrote:
> On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
> > > I'm not sure I understand this. If you don't want fstab to be
> > changed
> > > you should not specify mount points in .wks
> > > There is only one reason to have mount points in .wks: to make wic
> > to
> > > change /etc/fstab, which you apparently don't want. So, don't
> > specify
> > > mount points and you'll have what you want.
> > >
> > > Having additional option for this looks redundand to me.
> >
> > After thinking a bit more about it I'd propose to have global wic
> > option
> > to avoid rootfs content changes. Not just fstab updates, but any
> > changes. For now this option (--no-rootfs-update ?) should prevent
> > creating
> > images if either mount points are specified or --exclude-path is used
> > in .wks
>
> Why does --exclude-path conflict with --no-rootfs-update? Is that a
> conceptual problem or an implementation problem?
>
I thought that removing directories from original rootfs is a
modification.
> If I'm not mistaken, --exclude-path merely means "take this rootfs, but
> exclude certain parts". That's in line with --no-rootfs-update == "do
> not modify the content of the rootfs", as it just helps with choosing
> where content goes (the "single rootfs" -> "different partitions"
> approach).
That's questionable statement, but I agree it makes sense in some cases.
If nobody objects I'm ok with this. Let's assume that removing part of
the content is not a modifiation :)
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 12:23 ` Ed Bartosh
@ 2017-06-30 13:16 ` Patrick Ohly
2017-06-30 13:58 ` Otavio Salvador
2017-06-30 15:44 ` Ed Bartosh
0 siblings, 2 replies; 26+ messages in thread
From: Patrick Ohly @ 2017-06-30 13:16 UTC (permalink / raw)
To: ed.bartosh
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Fri, 2017-06-30 at 15:23 +0300, Ed Bartosh wrote:
> On Fri, Jun 30, 2017 at 11:02:13AM +0200, Patrick Ohly wrote:
> > On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
> > > > I'm not sure I understand this. If you don't want fstab to be
> > > changed
> > > > you should not specify mount points in .wks
> > > > There is only one reason to have mount points in .wks: to make wic
> > > to
> > > > change /etc/fstab, which you apparently don't want. So, don't
> > > specify
> > > > mount points and you'll have what you want.
> > > >
> > > > Having additional option for this looks redundand to me.
> > >
> > > After thinking a bit more about it I'd propose to have global wic
> > > option
> > > to avoid rootfs content changes. Not just fstab updates, but any
> > > changes. For now this option (--no-rootfs-update ?) should prevent
> > > creating
> > > images if either mount points are specified or --exclude-path is used
> > > in .wks
> >
> > Why does --exclude-path conflict with --no-rootfs-update? Is that a
> > conceptual problem or an implementation problem?
> >
>
> I thought that removing directories from original rootfs is a
> modification.
But it's not actually removed from the original roofs directory, right?
For me, "not modified" refers to that and the files in it.
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 13:16 ` Patrick Ohly
@ 2017-06-30 13:58 ` Otavio Salvador
2017-06-30 15:37 ` Ed Bartosh
2017-06-30 15:44 ` Ed Bartosh
1 sibling, 1 reply; 26+ messages in thread
From: Otavio Salvador @ 2017-06-30 13:58 UTC (permalink / raw)
To: Patrick Ohly; +Cc: Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 10:16 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Fri, 2017-06-30 at 15:23 +0300, Ed Bartosh wrote:
>> On Fri, Jun 30, 2017 at 11:02:13AM +0200, Patrick Ohly wrote:
>> > On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
>> > > > I'm not sure I understand this. If you don't want fstab to be
>> > > changed
>> > > > you should not specify mount points in .wks
>> > > > There is only one reason to have mount points in .wks: to make wic
>> > > to
>> > > > change /etc/fstab, which you apparently don't want. So, don't
>> > > specify
>> > > > mount points and you'll have what you want.
>> > > >
>> > > > Having additional option for this looks redundand to me.
>> > >
>> > > After thinking a bit more about it I'd propose to have global wic
>> > > option
>> > > to avoid rootfs content changes. Not just fstab updates, but any
>> > > changes. For now this option (--no-rootfs-update ?) should prevent
>> > > creating
>> > > images if either mount points are specified or --exclude-path is used
>> > > in .wks
>> >
>> > Why does --exclude-path conflict with --no-rootfs-update? Is that a
>> > conceptual problem or an implementation problem?
>> >
>>
>> I thought that removing directories from original rootfs is a
>> modification.
>
> But it's not actually removed from the original roofs directory, right?
> For me, "not modified" refers to that and the files in it.
My problem is with the fstab change. If I explicitly ask wic to drop
something I know it is doing it so it is under my control.
Adding --no-fstab-change solves in an elegant way my problem.
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 13:58 ` Otavio Salvador
@ 2017-06-30 15:37 ` Ed Bartosh
0 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-06-30 15:37 UTC (permalink / raw)
To: Otavio Salvador; +Cc: Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 10:58:27AM -0300, Otavio Salvador wrote:
> On Fri, Jun 30, 2017 at 10:16 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > On Fri, 2017-06-30 at 15:23 +0300, Ed Bartosh wrote:
> >> On Fri, Jun 30, 2017 at 11:02:13AM +0200, Patrick Ohly wrote:
> >> > On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
> >> > > > I'm not sure I understand this. If you don't want fstab to be
> >> > > changed
> >> > > > you should not specify mount points in .wks
> >> > > > There is only one reason to have mount points in .wks: to make wic
> >> > > to
> >> > > > change /etc/fstab, which you apparently don't want. So, don't
> >> > > specify
> >> > > > mount points and you'll have what you want.
> >> > > >
> >> > > > Having additional option for this looks redundand to me.
> >> > >
> >> > > After thinking a bit more about it I'd propose to have global wic
> >> > > option
> >> > > to avoid rootfs content changes. Not just fstab updates, but any
> >> > > changes. For now this option (--no-rootfs-update ?) should prevent
> >> > > creating
> >> > > images if either mount points are specified or --exclude-path is used
> >> > > in .wks
> >> >
> >> > Why does --exclude-path conflict with --no-rootfs-update? Is that a
> >> > conceptual problem or an implementation problem?
> >> >
> >>
> >> I thought that removing directories from original rootfs is a
> >> modification.
> >
> > But it's not actually removed from the original roofs directory, right?
> > For me, "not modified" refers to that and the files in it.
>
> My problem is with the fstab change. If I explicitly ask wic to drop
> something I know it is doing it so it is under my control.
>
> Adding --no-fstab-change solves in an elegant way my problem.
What if wic at some point will modify rootfs for one or another
reason? We'd need to introduce --no-hosts-change --no-exports-change
--no-whatever-is-changed-change etc. It doesn't look too elegant to me to be
honest. Adding mount points to .wks and then disabling fstab update
(which is the only purpose and outcome of having mount points in .wks)
doesn't look good neither.
Thoughts?
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 13:16 ` Patrick Ohly
2017-06-30 13:58 ` Otavio Salvador
@ 2017-06-30 15:44 ` Ed Bartosh
2017-06-30 17:33 ` Otavio Salvador
1 sibling, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-06-30 15:44 UTC (permalink / raw)
To: Patrick Ohly
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 03:16:33PM +0200, Patrick Ohly wrote:
> On Fri, 2017-06-30 at 15:23 +0300, Ed Bartosh wrote:
> > On Fri, Jun 30, 2017 at 11:02:13AM +0200, Patrick Ohly wrote:
> > > On Fri, 2017-06-30 at 11:37 +0300, Ed Bartosh wrote:
> > > > > I'm not sure I understand this. If you don't want fstab to be
> > > > changed
> > > > > you should not specify mount points in .wks
> > > > > There is only one reason to have mount points in .wks: to make wic
> > > > to
> > > > > change /etc/fstab, which you apparently don't want. So, don't
> > > > specify
> > > > > mount points and you'll have what you want.
> > > > >
> > > > > Having additional option for this looks redundand to me.
> > > >
> > > > After thinking a bit more about it I'd propose to have global wic
> > > > option
> > > > to avoid rootfs content changes. Not just fstab updates, but any
> > > > changes. For now this option (--no-rootfs-update ?) should prevent
> > > > creating
> > > > images if either mount points are specified or --exclude-path is used
> > > > in .wks
> > >
> > > Why does --exclude-path conflict with --no-rootfs-update? Is that a
> > > conceptual problem or an implementation problem?
> > >
> >
> > I thought that removing directories from original rootfs is a
> > modification.
>
> But it's not actually removed from the original roofs directory, right?
> For me, "not modified" refers to that and the files in it.
We're talking about different things here, I guess. For me if target
rootfs content in the image is different from rootfs produced by bitbake
it means it's modified.
BTW, if this patchset is accepted wic will not be changing fstab in original rootfs anymore:
http://lists.openembedded.org/pipermail/openembedded-core/2017-June/138803.html
However, it would not mean that rootfs is not changed.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 15:44 ` Ed Bartosh
@ 2017-06-30 17:33 ` Otavio Salvador
2017-06-30 18:34 ` Patrick Ohly
0 siblings, 1 reply; 26+ messages in thread
From: Otavio Salvador @ 2017-06-30 17:33 UTC (permalink / raw)
To: Ed Bartosh; +Cc: Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 12:44 PM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> We're talking about different things here, I guess. For me if target
> rootfs content in the image is different from rootfs produced by bitbake
> it means it's modified.
>
> BTW, if this patchset is accepted wic will not be changing fstab in original rootfs anymore:
> http://lists.openembedded.org/pipermail/openembedded-core/2017-June/138803.html
> However, it would not mean that rootfs is not changed.
Honestly it is quite confusing to me that wic changes fstab content.
It is confusing as you add something on base-files for fstab and it
may, or not, be what is used.
As I mentioned before, our needs is to have a way to forbid this
behavior and we use standard wks files specially when doing
development so an option makes it easy to adopt one way or another in
a distro for example.
Other possible rootfs changes also should be possible to be disabled
but IMO it should be per-feature (one for fstab, one for exclude,
...).
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 17:33 ` Otavio Salvador
@ 2017-06-30 18:34 ` Patrick Ohly
2017-07-03 7:31 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Patrick Ohly @ 2017-06-30 18:34 UTC (permalink / raw)
To: Otavio Salvador; +Cc: Patches and discussions about the oe-core layer
On Fri, 2017-06-30 at 14:33 -0300, Otavio Salvador wrote:
> Other possible rootfs changes also should be possible to be disabled
> but IMO it should be per-feature (one for fstab, one for exclude,
> ...).
I also think it should be per-feature, it necessary at all.
I still do not fully understand under which circumstances wic modifies
the rootfs. If that happens only when explicitly requested in the wks
file as Ed said, then I don't see a need for any additional flags. Just
don't use the features which result in a rootfs modification.
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-06-30 18:34 ` Patrick Ohly
@ 2017-07-03 7:31 ` Ed Bartosh
2017-07-03 7:53 ` Patrick Ohly
0 siblings, 1 reply; 26+ messages in thread
From: Ed Bartosh @ 2017-07-03 7:31 UTC (permalink / raw)
To: Patrick Ohly
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Fri, Jun 30, 2017 at 08:34:30PM +0200, Patrick Ohly wrote:
> On Fri, 2017-06-30 at 14:33 -0300, Otavio Salvador wrote:
> > Other possible rootfs changes also should be possible to be disabled
> > but IMO it should be per-feature (one for fstab, one for exclude,
> > ...).
>
> I also think it should be per-feature, it necessary at all.
>
> I still do not fully understand under which circumstances wic modifies
> the rootfs. If that happens only when explicitly requested in the wks
> file as Ed said, then I don't see a need for any additional flags.
Yes, that happens only if it explicitly requested, i.e. if there are valid
mount points in .wks file
> then I don't see a need for any additional flags. Just
> don't use the features which result in a rootfs modification.
I also didn't see it till last message from Otavio. Now I do - they
don't want to change .wks files. They're using standard wks from
scripts/lib/wic/canned-wks or from standard layers and they don't want
to duplicate them when they don't want rootfs modifications.
It could be a valid reason to have --no-fstab-update option I think.
However, I'm still not 100% convinced I'm ok with this if nobody else
objects.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-07-03 7:31 ` Ed Bartosh
@ 2017-07-03 7:53 ` Patrick Ohly
2017-07-03 8:59 ` Ed Bartosh
0 siblings, 1 reply; 26+ messages in thread
From: Patrick Ohly @ 2017-07-03 7:53 UTC (permalink / raw)
To: ed.bartosh
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Mon, 2017-07-03 at 10:31 +0300, Ed Bartosh wrote:
> On Fri, Jun 30, 2017 at 08:34:30PM +0200, Patrick Ohly wrote:
> > then I don't see a need for any additional flags. Just
> > don't use the features which result in a rootfs modification.
>
> I also didn't see it till last message from Otavio. Now I do - they
> don't want to change .wks files. They're using standard wks from
> scripts/lib/wic/canned-wks or from standard layers and they don't want
> to duplicate them when they don't want rootfs modifications.
>
> It could be a valid reason to have --no-fstab-update option I think.
> However, I'm still not 100% convinced I'm ok with this if nobody else
> objects.
Okay, now I see what the purpose is.
I prefer a --no-fstab-update over a general --no-rootfs-update because
for each case where wic would normally modify the rootfs, some other
mechanism must be in place which makes that modification redundant (like
using PARTUUID). Having separate parameters forces the developers to
think about it. Just my 2 cents...
--
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] 26+ messages in thread
* Re: [PATCH v2 0/5] #11662 - wic should mount /boot
2017-07-03 7:53 ` Patrick Ohly
@ 2017-07-03 8:59 ` Ed Bartosh
0 siblings, 0 replies; 26+ messages in thread
From: Ed Bartosh @ 2017-07-03 8:59 UTC (permalink / raw)
To: Patrick Ohly
Cc: Otavio Salvador, Patches and discussions about the oe-core layer
On Mon, Jul 03, 2017 at 09:53:32AM +0200, Patrick Ohly wrote:
> On Mon, 2017-07-03 at 10:31 +0300, Ed Bartosh wrote:
> > On Fri, Jun 30, 2017 at 08:34:30PM +0200, Patrick Ohly wrote:
> > > then I don't see a need for any additional flags. Just
> > > don't use the features which result in a rootfs modification.
> >
> > I also didn't see it till last message from Otavio. Now I do - they
> > don't want to change .wks files. They're using standard wks from
> > scripts/lib/wic/canned-wks or from standard layers and they don't want
> > to duplicate them when they don't want rootfs modifications.
> >
> > It could be a valid reason to have --no-fstab-update option I think.
> > However, I'm still not 100% convinced I'm ok with this if nobody else
> > objects.
>
> Okay, now I see what the purpose is.
>
> I prefer a --no-fstab-update over a general --no-rootfs-update because
> for each case where wic would normally modify the rootfs, some other
> mechanism must be in place which makes that modification redundant (like
> using PARTUUID). Having separate parameters forces the developers to
> think about it. Just my 2 cents...
>
Tha makes sense to me.
From other point of view if the goal is to have rootfs unmodified
--no-rootfs-update would make it easier to achive. Moreover it will
guarantee that rootfs is unmodified even if wic introduces new
functionality that modifies rootfs.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 26+ messages in thread