* [PATCH 1/2] wic: Fix permissions when using exclude or include path
@ 2020-03-04 8:34 Ricardo Ribalda Delgado
2020-03-04 8:34 ` [PATCH 2/2] wic: Add --embed-rootfs argument Ricardo Ribalda Delgado
2020-03-04 9:53 ` [PATCH 1/2] wic: Fix permissions when using exclude or include path Paul Barker
0 siblings, 2 replies; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 8:34 UTC (permalink / raw)
To: openembedded-core; +Cc: Ricardo Ribalda Delgado
When parameters include_path or exclude_path are passed to the rootfs
plugin, it will copy the partition content into a folder and make all
the modifications there.
This is done using copyhardlinktree(), which does not take into
consideration the content of the pseudo folder, which contains the
information about the right permissions and ownership of the folders.
This results in a rootfs owned by the user that is running the wic
command (usually UID 1000), which makes some rootfs unbootable.
To fix this we copy the content of the pseudo folders to the new folder
and modify the pseudo database using the "pseudo -B" command.
Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
---
scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 705aeb5563..40419a64b3 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -16,11 +16,11 @@ import os
import shutil
import sys
-from oe.path import copyhardlinktree
+from oe.path import copyhardlinktree, copytree
from wic import WicError
from wic.pluginbase import SourcePlugin
-from wic.misc import get_bitbake_var
+from wic.misc import get_bitbake_var, exec_native_cmd
logger = logging.getLogger('wic')
@@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
return os.path.realpath(image_rootfs_dir)
+ @staticmethod
+ def __get_pseudo(native_sysroot, rootfs):
+ pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
+ pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
+ pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
+ pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
+ pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
+ return pseudo
+
@classmethod
def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
oe_builddir, bootimg_dir, kernel_dir,
@@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
if os.path.lexists(new_rootfs):
shutil.rmtree(os.path.join(new_rootfs))
-
copyhardlinktree(part.rootfs_dir, new_rootfs)
+ if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
+ shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
+ copytree(os.path.join(part.rootfs_dir, "../pseudo"),
+ os.path.join(new_rootfs, "../pseudo"))
+ pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
+ part.rootfs_dir, new_rootfs)
+ exec_native_cmd(pseudo_cmd, native_sysroot)
+
for path in part.include_path or []:
copyhardlinktree(path, new_rootfs)
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 8:34 [PATCH 1/2] wic: Fix permissions when using exclude or include path Ricardo Ribalda Delgado
@ 2020-03-04 8:34 ` Ricardo Ribalda Delgado
2020-03-04 9:42 ` Paul Barker
2020-03-04 9:53 ` [PATCH 1/2] wic: Fix permissions when using exclude or include path Paul Barker
1 sibling, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 8:34 UTC (permalink / raw)
To: openembedded-core; +Cc: Ricardo Ribalda Delgado
This option adds the content of a rootfs on a specific location on the
rootfs.
It is very useful for making a partition that contains the rootfs for a
host and a target Eg:
/ -> Roofs for the host
/export/ -> Rootfs for the target (which will netboot)
Although today we support making a partition for "/export" this might
not be compatible with some upgrade systems, or we might be limited by
the number of partitions.
With this patch we can use something like:
part / --source rootfs --embed-rootfs=target-image:/export
on the .wks file.
Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
---
scripts/lib/wic/help.py | 7 +++++++
scripts/lib/wic/ksparser.py | 1 +
scripts/lib/wic/partition.py | 1 +
scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 4d342fcf05..67a33e6a65 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -979,6 +979,13 @@ DESCRIPTION
copies. This option only has an effect with the rootfs
source plugin.
+ --embed-rootfs: This option is specific to wic. It embeds a rootfs into
+ the given path to the resulting image. The option
+ contains two fields, the roofs and the path, separated
+ by a colon. The rootfs follows the same logic as the
+ rootfs-dir argument. This option only has an effect
+ with the rootfs source plugin.
+
--extra-space: This option is specific to wic. It adds extra
space after the space filled by the content
of the partition. The final size can go
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 650b976223..d422e2a6bb 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -138,6 +138,7 @@ class KickStart():
part.add_argument('--align', type=int)
part.add_argument('--exclude-path', nargs='+')
part.add_argument('--include-path', nargs='+')
+ part.add_argument('--embed-rootfs', nargs='+')
part.add_argument("--extra-space", type=sizetype)
part.add_argument('--fsoptions', dest='fsopts')
part.add_argument('--fstype', default='vfat',
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 2d95f78439..13857df82f 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -31,6 +31,7 @@ class Partition():
self.extra_space = args.extra_space
self.exclude_path = args.exclude_path
self.include_path = args.include_path
+ self.embed_rootfs = args.embed_rootfs
self.fsopts = args.fsopts
self.fstype = args.fstype
self.label = args.label
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 40419a64b3..16359601dc 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
new_rootfs = None
# Handle excluded paths.
- if part.exclude_path or part.include_path:
+ if part.exclude_path or part.include_path or part.embed_rootfs:
# 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%d" % part.lineno))
@@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
for path in part.include_path or []:
copyhardlinktree(path, new_rootfs)
+ for embed in part.embed_rootfs or []:
+ [embed_rootfs, path] = embed.split(":")
+ #we need to remove the initial / for os.path.join to work
+ if os.path.isabs(path):
+ path = path[1:]
+ if embed_rootfs in krootfs_dir:
+ embed_rootfs = krootfs_dir[embed_rootfs]
+ embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
+ tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
+ tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
+ embed_rootfs), tar_file, embed_rootfs)
+ exec_native_cmd(tar_cmd, native_sysroot)
+ untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
+ tar_file, os.path.join(new_rootfs, path))
+ exec_native_cmd(untar_cmd, native_sysroot,
+ cls.__get_pseudo(native_sysroot, new_rootfs))
+ os.remove(tar_file)
+
for orig_path in part.exclude_path or []:
path = orig_path
if os.path.isabs(path):
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 8:34 ` [PATCH 2/2] wic: Add --embed-rootfs argument Ricardo Ribalda Delgado
@ 2020-03-04 9:42 ` Paul Barker
2020-03-04 9:56 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-03-04 9:42 UTC (permalink / raw)
To: openembedded-core, Ricardo Ribalda Delgado
On Wed, 4 Mar 2020 09:34:38 +0100
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> This option adds the content of a rootfs on a specific location on the
> rootfs.
>
> It is very useful for making a partition that contains the rootfs for a
> host and a target Eg:
>
> / -> Roofs for the host
> /export/ -> Rootfs for the target (which will netboot)
>
> Although today we support making a partition for "/export" this might
> not be compatible with some upgrade systems, or we might be limited by
> the number of partitions.
>
> With this patch we can use something like:
>
> part / --source rootfs --embed-rootfs=target-image:/export
I considered using a syntax like this for --include-path but I chose not to
as it's not easy to choose a separator. ':' is a valid character in a
directory name.
My approach is to handle this in two steps:
1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
directory and copies 'target-image' to 'embedded-rootfs/export'.
2) Use '--include-path=embedded-rootfs' in the wks file.
>
> on the .wks file.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> ---
> scripts/lib/wic/help.py | 7 +++++++
> scripts/lib/wic/ksparser.py | 1 +
> scripts/lib/wic/partition.py | 1 +
> scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> index 4d342fcf05..67a33e6a65 100644
> --- a/scripts/lib/wic/help.py
> +++ b/scripts/lib/wic/help.py
> @@ -979,6 +979,13 @@ DESCRIPTION
> copies. This option only has an effect with the rootfs
> source plugin.
>
> + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
> + the given path to the resulting image. The option
> + contains two fields, the roofs and the path, separated
> + by a colon. The rootfs follows the same logic as the
> + rootfs-dir argument. This option only has an effect
> + with the rootfs source plugin.
> +
> --extra-space: This option is specific to wic. It adds extra
> space after the space filled by the content
> of the partition. The final size can go
> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> index 650b976223..d422e2a6bb 100644
> --- a/scripts/lib/wic/ksparser.py
> +++ b/scripts/lib/wic/ksparser.py
> @@ -138,6 +138,7 @@ class KickStart():
> part.add_argument('--align', type=int)
> part.add_argument('--exclude-path', nargs='+')
> part.add_argument('--include-path', nargs='+')
> + part.add_argument('--embed-rootfs', nargs='+')
> part.add_argument("--extra-space", type=sizetype)
> part.add_argument('--fsoptions', dest='fsopts')
> part.add_argument('--fstype', default='vfat',
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index 2d95f78439..13857df82f 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -31,6 +31,7 @@ class Partition():
> self.extra_space = args.extra_space
> self.exclude_path = args.exclude_path
> self.include_path = args.include_path
> + self.embed_rootfs = args.embed_rootfs
> self.fsopts = args.fsopts
> self.fstype = args.fstype
> self.label = args.label
> diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> index 40419a64b3..16359601dc 100644
> --- a/scripts/lib/wic/plugins/source/rootfs.py
> +++ b/scripts/lib/wic/plugins/source/rootfs.py
> @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
>
> new_rootfs = None
> # Handle excluded paths.
> - if part.exclude_path or part.include_path:
> + if part.exclude_path or part.include_path or part.embed_rootfs:
> # 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%d" % part.lineno))
> @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
> for path in part.include_path or []:
> copyhardlinktree(path, new_rootfs)
>
> + for embed in part.embed_rootfs or []:
> + [embed_rootfs, path] = embed.split(":")
> + #we need to remove the initial / for os.path.join to work
> + if os.path.isabs(path):
> + path = path[1:]
> + if embed_rootfs in krootfs_dir:
> + embed_rootfs = krootfs_dir[embed_rootfs]
> + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
> + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
> + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
> + embed_rootfs), tar_file, embed_rootfs)
> + exec_native_cmd(tar_cmd, native_sysroot)
> + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
> + tar_file, os.path.join(new_rootfs, path))
> + exec_native_cmd(untar_cmd, native_sysroot,
> + cls.__get_pseudo(native_sysroot, new_rootfs))
> + os.remove(tar_file)
> +
Why are you using an intermediate tar file here?
> for orig_path in part.exclude_path or []:
> path = orig_path
> if os.path.isabs(path):
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-03-04 8:34 [PATCH 1/2] wic: Fix permissions when using exclude or include path Ricardo Ribalda Delgado
2020-03-04 8:34 ` [PATCH 2/2] wic: Add --embed-rootfs argument Ricardo Ribalda Delgado
@ 2020-03-04 9:53 ` Paul Barker
2020-03-04 10:02 ` Ricardo Ribalda Delgado
1 sibling, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-03-04 9:53 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: openembedded-core
On Wed, 4 Mar 2020 09:34:37 +0100
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> When parameters include_path or exclude_path are passed to the rootfs
> plugin, it will copy the partition content into a folder and make all
> the modifications there.
>
> This is done using copyhardlinktree(), which does not take into
> consideration the content of the pseudo folder, which contains the
> information about the right permissions and ownership of the folders.
How are you running wic here? In the do_image_wic task it's executed under
pseudo so all this is handled already. Executing wic outside of bitbake may
need some more testing here.
>
> This results in a rootfs owned by the user that is running the wic
> command (usually UID 1000), which makes some rootfs unbootable.
>
> To fix this we copy the content of the pseudo folders to the new folder
> and modify the pseudo database using the "pseudo -B" command.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> ---
> scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> index 705aeb5563..40419a64b3 100644
> --- a/scripts/lib/wic/plugins/source/rootfs.py
> +++ b/scripts/lib/wic/plugins/source/rootfs.py
> @@ -16,11 +16,11 @@ import os
> import shutil
> import sys
>
> -from oe.path import copyhardlinktree
> +from oe.path import copyhardlinktree, copytree
>
> from wic import WicError
> from wic.pluginbase import SourcePlugin
> -from wic.misc import get_bitbake_var
> +from wic.misc import get_bitbake_var, exec_native_cmd
>
> logger = logging.getLogger('wic')
>
> @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
>
> return os.path.realpath(image_rootfs_dir)
>
> + @staticmethod
> + def __get_pseudo(native_sysroot, rootfs):
> + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> + return pseudo
> +
> @classmethod
> def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> oe_builddir, bootimg_dir, kernel_dir,
> @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
>
> if os.path.lexists(new_rootfs):
> shutil.rmtree(os.path.join(new_rootfs))
> -
> copyhardlinktree(part.rootfs_dir, new_rootfs)
>
> + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> + os.path.join(new_rootfs, "../pseudo"))
I don't like stepping up the directory tree like this. We should be more
explicit with the paths.
> + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> + part.rootfs_dir, new_rootfs)
> + exec_native_cmd(pseudo_cmd, native_sysroot)
> +
> for path in part.include_path or []:
> copyhardlinktree(path, new_rootfs)
^^^^^^^^^^^^^^^^
If this is the right approach I imagine you would also need to fix things up
with pseudo after the copyhardlinktree call above.
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 9:42 ` Paul Barker
@ 2020-03-04 9:56 ` Ricardo Ribalda Delgado
2020-03-04 10:08 ` Paul Barker
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 9:56 UTC (permalink / raw)
To: Paul Barker; +Cc: openembedded-core
Hi Paul
Thanks for your reply
On Wed, Mar 4, 2020 at 10:43 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Wed, 4 Mar 2020 09:34:38 +0100
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > This option adds the content of a rootfs on a specific location on the
> > rootfs.
> >
> > It is very useful for making a partition that contains the rootfs for a
> > host and a target Eg:
> >
> > / -> Roofs for the host
> > /export/ -> Rootfs for the target (which will netboot)
> >
> > Although today we support making a partition for "/export" this might
> > not be compatible with some upgrade systems, or we might be limited by
> > the number of partitions.
> >
> > With this patch we can use something like:
> >
> > part / --source rootfs --embed-rootfs=target-image:/export
>
> I considered using a syntax like this for --include-path but I chose not to
> as it's not easy to choose a separator. ':' is a valid character in a
> directory name.
I think we have to live with not being able to copy files to a
directory with a colon :(
>
> My approach is to handle this in two steps:
>
> 1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
> directory and copies 'target-image' to 'embedded-rootfs/export'.
>
> 2) Use '--include-path=embedded-rootfs' in the wks file.
>
The biggest problem with that approach is permissions. We need files
with UID 0, siticky bits, etc. We lose the pseudo database if we
simply copy to a folder.
Also, if we have a complex system with multiple partitions I prefer to
handle all in a single .wks file instead of a script + wks file
> >
> > on the .wks file.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > ---
> > scripts/lib/wic/help.py | 7 +++++++
> > scripts/lib/wic/ksparser.py | 1 +
> > scripts/lib/wic/partition.py | 1 +
> > scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
> > 4 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> > index 4d342fcf05..67a33e6a65 100644
> > --- a/scripts/lib/wic/help.py
> > +++ b/scripts/lib/wic/help.py
> > @@ -979,6 +979,13 @@ DESCRIPTION
> > copies. This option only has an effect with the rootfs
> > source plugin.
> >
> > + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
> > + the given path to the resulting image. The option
> > + contains two fields, the roofs and the path, separated
> > + by a colon. The rootfs follows the same logic as the
> > + rootfs-dir argument. This option only has an effect
> > + with the rootfs source plugin.
> > +
> > --extra-space: This option is specific to wic. It adds extra
> > space after the space filled by the content
> > of the partition. The final size can go
> > diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> > index 650b976223..d422e2a6bb 100644
> > --- a/scripts/lib/wic/ksparser.py
> > +++ b/scripts/lib/wic/ksparser.py
> > @@ -138,6 +138,7 @@ class KickStart():
> > part.add_argument('--align', type=int)
> > part.add_argument('--exclude-path', nargs='+')
> > part.add_argument('--include-path', nargs='+')
> > + part.add_argument('--embed-rootfs', nargs='+')
> > part.add_argument("--extra-space", type=sizetype)
> > part.add_argument('--fsoptions', dest='fsopts')
> > part.add_argument('--fstype', default='vfat',
> > diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> > index 2d95f78439..13857df82f 100644
> > --- a/scripts/lib/wic/partition.py
> > +++ b/scripts/lib/wic/partition.py
> > @@ -31,6 +31,7 @@ class Partition():
> > self.extra_space = args.extra_space
> > self.exclude_path = args.exclude_path
> > self.include_path = args.include_path
> > + self.embed_rootfs = args.embed_rootfs
> > self.fsopts = args.fsopts
> > self.fstype = args.fstype
> > self.label = args.label
> > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > index 40419a64b3..16359601dc 100644
> > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
> >
> > new_rootfs = None
> > # Handle excluded paths.
> > - if part.exclude_path or part.include_path:
> > + if part.exclude_path or part.include_path or part.embed_rootfs:
> > # 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%d" % part.lineno))
> > @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
> > for path in part.include_path or []:
> > copyhardlinktree(path, new_rootfs)
> >
> > + for embed in part.embed_rootfs or []:
> > + [embed_rootfs, path] = embed.split(":")
> > + #we need to remove the initial / for os.path.join to work
> > + if os.path.isabs(path):
> > + path = path[1:]
> > + if embed_rootfs in krootfs_dir:
> > + embed_rootfs = krootfs_dir[embed_rootfs]
> > + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
> > + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
> > + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
> > + embed_rootfs), tar_file, embed_rootfs)
> > + exec_native_cmd(tar_cmd, native_sysroot)
> > + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
> > + tar_file, os.path.join(new_rootfs, path))
> > + exec_native_cmd(untar_cmd, native_sysroot,
> > + cls.__get_pseudo(native_sysroot, new_rootfs))
> > + os.remove(tar_file)
> > +
>
> Why are you using an intermediate tar file here?
For the permissions :(
>
> > for orig_path in part.exclude_path or []:
> > path = orig_path
> > if os.path.isabs(path):
>
>
>
> --
> Paul Barker
> Konsulko Group
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-03-04 9:53 ` [PATCH 1/2] wic: Fix permissions when using exclude or include path Paul Barker
@ 2020-03-04 10:02 ` Ricardo Ribalda Delgado
2020-03-05 9:28 ` Paul Barker
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 10:02 UTC (permalink / raw)
To: Paul Barker; +Cc: openembedded-core
Hi Paul
On Wed, Mar 4, 2020 at 10:53 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Wed, 4 Mar 2020 09:34:37 +0100
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > When parameters include_path or exclude_path are passed to the rootfs
> > plugin, it will copy the partition content into a folder and make all
> > the modifications there.
> >
> > This is done using copyhardlinktree(), which does not take into
> > consideration the content of the pseudo folder, which contains the
> > information about the right permissions and ownership of the folders.
>
> How are you running wic here? In the do_image_wic task it's executed under
> pseudo so all this is handled already. Executing wic outside of bitbake may
> need some more testing here.
I am running wic outside bitbake. But even if it is run under bitbake,
it should also fail. The pseudo directory needs to be present on the
target image
>
> >
> > This results in a rootfs owned by the user that is running the wic
> > command (usually UID 1000), which makes some rootfs unbootable.
> >
> > To fix this we copy the content of the pseudo folders to the new folder
> > and modify the pseudo database using the "pseudo -B" command.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > ---
> > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > index 705aeb5563..40419a64b3 100644
> > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > @@ -16,11 +16,11 @@ import os
> > import shutil
> > import sys
> >
> > -from oe.path import copyhardlinktree
> > +from oe.path import copyhardlinktree, copytree
> >
> > from wic import WicError
> > from wic.pluginbase import SourcePlugin
> > -from wic.misc import get_bitbake_var
> > +from wic.misc import get_bitbake_var, exec_native_cmd
> >
> > logger = logging.getLogger('wic')
> >
> > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> >
> > return os.path.realpath(image_rootfs_dir)
> >
> > + @staticmethod
> > + def __get_pseudo(native_sysroot, rootfs):
> > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > + return pseudo
> > +
> > @classmethod
> > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > oe_builddir, bootimg_dir, kernel_dir,
> > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> >
> > if os.path.lexists(new_rootfs):
> > shutil.rmtree(os.path.join(new_rootfs))
> > -
> > copyhardlinktree(part.rootfs_dir, new_rootfs)
> >
> > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > + os.path.join(new_rootfs, "../pseudo"))
>
> I don't like stepping up the directory tree like this. We should be more
> explicit with the paths.
You are thinking on:
os.path.dirname(directory)
>
> > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > + part.rootfs_dir, new_rootfs)
> > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > +
> > for path in part.include_path or []:
> > copyhardlinktree(path, new_rootfs)
>
> ^^^^^^^^^^^^^^^^
>
> If this is the right approach I imagine you would also need to fix things up
> with pseudo after the copyhardlinktree call above.
I do not think it is needed. include_path does not contain its own
pseudo directory
>
> --
> Paul Barker
> Konsulko Group
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 9:56 ` Ricardo Ribalda Delgado
@ 2020-03-04 10:08 ` Paul Barker
2020-03-04 10:14 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-03-04 10:08 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: openembedded-core
On Wed, 4 Mar 2020 10:56:20 +0100
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> Hi Paul
>
> Thanks for your reply
>
> On Wed, Mar 4, 2020 at 10:43 AM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Wed, 4 Mar 2020 09:34:38 +0100
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > This option adds the content of a rootfs on a specific location on the
> > > rootfs.
> > >
> > > It is very useful for making a partition that contains the rootfs for a
> > > host and a target Eg:
> > >
> > > / -> Roofs for the host
> > > /export/ -> Rootfs for the target (which will netboot)
> > >
> > > Although today we support making a partition for "/export" this might
> > > not be compatible with some upgrade systems, or we might be limited by
> > > the number of partitions.
> > >
> > > With this patch we can use something like:
> > >
> > > part / --source rootfs --embed-rootfs=target-image:/export
> >
> > I considered using a syntax like this for --include-path but I chose not to
> > as it's not easy to choose a separator. ':' is a valid character in a
> > directory name.
>
> I think we have to live with not being able to copy files to a
> directory with a colon :(
I wouldn't like to place a limitation like that on users and I'd like to
avoid the proliferation of similar arguments here.
How about modifying '--include-path' to take two arguments, a source path and
a destination prefix? The python argument parser should be able to handle
this and you can build a list of tuples (source_path, dest_prefix) when
parsing the arguments.
The '--include-path' argument hasn't been part of a Yocto release yet it's
just on the master branch so I think if we're going to fix it with a breaking
change, now is definitely the time.
>
> >
> > My approach is to handle this in two steps:
> >
> > 1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
> > directory and copies 'target-image' to 'embedded-rootfs/export'.
> >
> > 2) Use '--include-path=embedded-rootfs' in the wks file.
> >
>
> The biggest problem with that approach is permissions. We need files
> with UID 0, siticky bits, etc. We lose the pseudo database if we
> simply copy to a folder.
>
> Also, if we have a complex system with multiple partitions I prefer to
> handle all in a single .wks file instead of a script + wks file
>
> > >
> > > on the .wks file.
> > >
> > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > ---
> > > scripts/lib/wic/help.py | 7 +++++++
> > > scripts/lib/wic/ksparser.py | 1 +
> > > scripts/lib/wic/partition.py | 1 +
> > > scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
> > > 4 files changed, 28 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> > > index 4d342fcf05..67a33e6a65 100644
> > > --- a/scripts/lib/wic/help.py
> > > +++ b/scripts/lib/wic/help.py
> > > @@ -979,6 +979,13 @@ DESCRIPTION
> > > copies. This option only has an effect with the rootfs
> > > source plugin.
> > >
> > > + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
> > > + the given path to the resulting image. The option
> > > + contains two fields, the roofs and the path, separated
> > > + by a colon. The rootfs follows the same logic as the
> > > + rootfs-dir argument. This option only has an effect
> > > + with the rootfs source plugin.
> > > +
> > > --extra-space: This option is specific to wic. It adds extra
> > > space after the space filled by the content
> > > of the partition. The final size can go
> > > diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> > > index 650b976223..d422e2a6bb 100644
> > > --- a/scripts/lib/wic/ksparser.py
> > > +++ b/scripts/lib/wic/ksparser.py
> > > @@ -138,6 +138,7 @@ class KickStart():
> > > part.add_argument('--align', type=int)
> > > part.add_argument('--exclude-path', nargs='+')
> > > part.add_argument('--include-path', nargs='+')
> > > + part.add_argument('--embed-rootfs', nargs='+')
> > > part.add_argument("--extra-space", type=sizetype)
> > > part.add_argument('--fsoptions', dest='fsopts')
> > > part.add_argument('--fstype', default='vfat',
> > > diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> > > index 2d95f78439..13857df82f 100644
> > > --- a/scripts/lib/wic/partition.py
> > > +++ b/scripts/lib/wic/partition.py
> > > @@ -31,6 +31,7 @@ class Partition():
> > > self.extra_space = args.extra_space
> > > self.exclude_path = args.exclude_path
> > > self.include_path = args.include_path
> > > + self.embed_rootfs = args.embed_rootfs
> > > self.fsopts = args.fsopts
> > > self.fstype = args.fstype
> > > self.label = args.label
> > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > index 40419a64b3..16359601dc 100644
> > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
> > >
> > > new_rootfs = None
> > > # Handle excluded paths.
> > > - if part.exclude_path or part.include_path:
> > > + if part.exclude_path or part.include_path or part.embed_rootfs:
> > > # 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%d" % part.lineno))
> > > @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
> > > for path in part.include_path or []:
> > > copyhardlinktree(path, new_rootfs)
> > >
> > > + for embed in part.embed_rootfs or []:
> > > + [embed_rootfs, path] = embed.split(":")
> > > + #we need to remove the initial / for os.path.join to work
> > > + if os.path.isabs(path):
> > > + path = path[1:]
> > > + if embed_rootfs in krootfs_dir:
> > > + embed_rootfs = krootfs_dir[embed_rootfs]
> > > + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
> > > + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
> > > + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
> > > + embed_rootfs), tar_file, embed_rootfs)
> > > + exec_native_cmd(tar_cmd, native_sysroot)
> > > + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
> > > + tar_file, os.path.join(new_rootfs, path))
> > > + exec_native_cmd(untar_cmd, native_sysroot,
> > > + cls.__get_pseudo(native_sysroot, new_rootfs))
> > > + os.remove(tar_file)
> > > +
> >
> > Why are you using an intermediate tar file here?
>
> For the permissions :(
Your previous patch handled this using 'pseudo -B'. It would be good to be
consistent if possible - either use an intermediate tar file in all cases if
it's necessary or use 'pseudo -B' in all cases.
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 10:08 ` Paul Barker
@ 2020-03-04 10:14 ` Ricardo Ribalda Delgado
2020-03-04 13:49 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 10:14 UTC (permalink / raw)
To: Paul Barker; +Cc: openembedded-core
Hi Paul
On Wed, Mar 4, 2020 at 11:09 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Wed, 4 Mar 2020 10:56:20 +0100
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > Hi Paul
> >
> > Thanks for your reply
> >
> > On Wed, Mar 4, 2020 at 10:43 AM Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Wed, 4 Mar 2020 09:34:38 +0100
> > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > >
> > > > This option adds the content of a rootfs on a specific location on the
> > > > rootfs.
> > > >
> > > > It is very useful for making a partition that contains the rootfs for a
> > > > host and a target Eg:
> > > >
> > > > / -> Roofs for the host
> > > > /export/ -> Rootfs for the target (which will netboot)
> > > >
> > > > Although today we support making a partition for "/export" this might
> > > > not be compatible with some upgrade systems, or we might be limited by
> > > > the number of partitions.
> > > >
> > > > With this patch we can use something like:
> > > >
> > > > part / --source rootfs --embed-rootfs=target-image:/export
> > >
> > > I considered using a syntax like this for --include-path but I chose not to
> > > as it's not easy to choose a separator. ':' is a valid character in a
> > > directory name.
> >
> > I think we have to live with not being able to copy files to a
> > directory with a colon :(
>
> I wouldn't like to place a limitation like that on users and I'd like to
> avoid the proliferation of similar arguments here.
>
> How about modifying '--include-path' to take two arguments, a source path and
> a destination prefix? The python argument parser should be able to handle
> this and you can build a list of tuples (source_path, dest_prefix) when
> parsing the arguments.
>
> The '--include-path' argument hasn't been part of a Yocto release yet it's
> just on the master branch so I think if we're going to fix it with a breaking
> change, now is definitely the time.
That sounds good to me. I do not mind having two arguments. for
embed-rootfs. and probably will look better on include-path
>
> >
> > >
> > > My approach is to handle this in two steps:
> > >
> > > 1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
> > > directory and copies 'target-image' to 'embedded-rootfs/export'.
> > >
> > > 2) Use '--include-path=embedded-rootfs' in the wks file.
> > >
> >
> > The biggest problem with that approach is permissions. We need files
> > with UID 0, siticky bits, etc. We lose the pseudo database if we
> > simply copy to a folder.
> >
> > Also, if we have a complex system with multiple partitions I prefer to
> > handle all in a single .wks file instead of a script + wks file
> >
> > > >
> > > > on the .wks file.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > ---
> > > > scripts/lib/wic/help.py | 7 +++++++
> > > > scripts/lib/wic/ksparser.py | 1 +
> > > > scripts/lib/wic/partition.py | 1 +
> > > > scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
> > > > 4 files changed, 28 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> > > > index 4d342fcf05..67a33e6a65 100644
> > > > --- a/scripts/lib/wic/help.py
> > > > +++ b/scripts/lib/wic/help.py
> > > > @@ -979,6 +979,13 @@ DESCRIPTION
> > > > copies. This option only has an effect with the rootfs
> > > > source plugin.
> > > >
> > > > + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
> > > > + the given path to the resulting image. The option
> > > > + contains two fields, the roofs and the path, separated
> > > > + by a colon. The rootfs follows the same logic as the
> > > > + rootfs-dir argument. This option only has an effect
> > > > + with the rootfs source plugin.
> > > > +
> > > > --extra-space: This option is specific to wic. It adds extra
> > > > space after the space filled by the content
> > > > of the partition. The final size can go
> > > > diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> > > > index 650b976223..d422e2a6bb 100644
> > > > --- a/scripts/lib/wic/ksparser.py
> > > > +++ b/scripts/lib/wic/ksparser.py
> > > > @@ -138,6 +138,7 @@ class KickStart():
> > > > part.add_argument('--align', type=int)
> > > > part.add_argument('--exclude-path', nargs='+')
> > > > part.add_argument('--include-path', nargs='+')
> > > > + part.add_argument('--embed-rootfs', nargs='+')
> > > > part.add_argument("--extra-space", type=sizetype)
> > > > part.add_argument('--fsoptions', dest='fsopts')
> > > > part.add_argument('--fstype', default='vfat',
> > > > diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> > > > index 2d95f78439..13857df82f 100644
> > > > --- a/scripts/lib/wic/partition.py
> > > > +++ b/scripts/lib/wic/partition.py
> > > > @@ -31,6 +31,7 @@ class Partition():
> > > > self.extra_space = args.extra_space
> > > > self.exclude_path = args.exclude_path
> > > > self.include_path = args.include_path
> > > > + self.embed_rootfs = args.embed_rootfs
> > > > self.fsopts = args.fsopts
> > > > self.fstype = args.fstype
> > > > self.label = args.label
> > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > index 40419a64b3..16359601dc 100644
> > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > > new_rootfs = None
> > > > # Handle excluded paths.
> > > > - if part.exclude_path or part.include_path:
> > > > + if part.exclude_path or part.include_path or part.embed_rootfs:
> > > > # 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%d" % part.lineno))
> > > > @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
> > > > for path in part.include_path or []:
> > > > copyhardlinktree(path, new_rootfs)
> > > >
> > > > + for embed in part.embed_rootfs or []:
> > > > + [embed_rootfs, path] = embed.split(":")
> > > > + #we need to remove the initial / for os.path.join to work
> > > > + if os.path.isabs(path):
> > > > + path = path[1:]
> > > > + if embed_rootfs in krootfs_dir:
> > > > + embed_rootfs = krootfs_dir[embed_rootfs]
> > > > + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
> > > > + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
> > > > + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
> > > > + embed_rootfs), tar_file, embed_rootfs)
> > > > + exec_native_cmd(tar_cmd, native_sysroot)
> > > > + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
> > > > + tar_file, os.path.join(new_rootfs, path))
> > > > + exec_native_cmd(untar_cmd, native_sysroot,
> > > > + cls.__get_pseudo(native_sysroot, new_rootfs))
> > > > + os.remove(tar_file)
> > > > +
> > >
> > > Why are you using an intermediate tar file here?
> >
> > For the permissions :(
>
> Your previous patch handled this using 'pseudo -B'. It would be good to be
> consistent if possible - either use an intermediate tar file in all cases if
> it's necessary or use 'pseudo -B' in all cases.
Unfortunately, you cannot use pseudo -B in this case.
You use pseudo -B when you move one pseudo-aware partition to
somewhere else. But now we are merging two pseudo-aware partitions.
Also I would not use the intermediate tar on the first place because
it is a waste of space + cpu resources. Copyhardlink is the right
tool
>
> --
> Paul Barker
> Konsulko Group
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 10:14 ` Ricardo Ribalda Delgado
@ 2020-03-04 13:49 ` Ricardo Ribalda Delgado
2020-03-04 14:31 ` Joshua Watt
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-04 13:49 UTC (permalink / raw)
To: Paul Barker; +Cc: openembedded-core
Hi Paul
On Wed, Mar 4, 2020 at 11:14 AM Ricardo Ribalda Delgado
<ricardo@ribalda.com> wrote:
>
> Hi Paul
>
> On Wed, Mar 4, 2020 at 11:09 AM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Wed, 4 Mar 2020 10:56:20 +0100
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > Hi Paul
> > >
> > > Thanks for your reply
> > >
> > > On Wed, Mar 4, 2020 at 10:43 AM Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Wed, 4 Mar 2020 09:34:38 +0100
> > > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > > >
> > > > > This option adds the content of a rootfs on a specific location on the
> > > > > rootfs.
> > > > >
> > > > > It is very useful for making a partition that contains the rootfs for a
> > > > > host and a target Eg:
> > > > >
> > > > > / -> Roofs for the host
> > > > > /export/ -> Rootfs for the target (which will netboot)
> > > > >
> > > > > Although today we support making a partition for "/export" this might
> > > > > not be compatible with some upgrade systems, or we might be limited by
> > > > > the number of partitions.
> > > > >
> > > > > With this patch we can use something like:
> > > > >
> > > > > part / --source rootfs --embed-rootfs=target-image:/export
> > > >
> > > > I considered using a syntax like this for --include-path but I chose not to
> > > > as it's not easy to choose a separator. ':' is a valid character in a
> > > > directory name.
> > >
> > > I think we have to live with not being able to copy files to a
> > > directory with a colon :(
> >
> > I wouldn't like to place a limitation like that on users and I'd like to
> > avoid the proliferation of similar arguments here.
> >
> > How about modifying '--include-path' to take two arguments, a source path and
> > a destination prefix? The python argument parser should be able to handle
> > this and you can build a list of tuples (source_path, dest_prefix) when
> > parsing the arguments.
> >
> > The '--include-path' argument hasn't been part of a Yocto release yet it's
> > just on the master branch so I think if we're going to fix it with a breaking
> > change, now is definitely the time.
>
> That sounds good to me. I do not mind having two arguments. for
> embed-rootfs. and probably will look better on include-path
How do you think that the parameters should look like:
--embed-rootfs target-image /export target-image2 /export2
--embed-rootfs (target-image, /export) (target-image2, /export2)
??
Shall I change it for include-path and embed-rootfs or you want to
take care of include-path?
>
> >
> > >
> > > >
> > > > My approach is to handle this in two steps:
> > > >
> > > > 1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
> > > > directory and copies 'target-image' to 'embedded-rootfs/export'.
> > > >
> > > > 2) Use '--include-path=embedded-rootfs' in the wks file.
> > > >
> > >
> > > The biggest problem with that approach is permissions. We need files
> > > with UID 0, siticky bits, etc. We lose the pseudo database if we
> > > simply copy to a folder.
> > >
> > > Also, if we have a complex system with multiple partitions I prefer to
> > > handle all in a single .wks file instead of a script + wks file
> > >
> > > > >
> > > > > on the .wks file.
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > > ---
> > > > > scripts/lib/wic/help.py | 7 +++++++
> > > > > scripts/lib/wic/ksparser.py | 1 +
> > > > > scripts/lib/wic/partition.py | 1 +
> > > > > scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
> > > > > 4 files changed, 28 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> > > > > index 4d342fcf05..67a33e6a65 100644
> > > > > --- a/scripts/lib/wic/help.py
> > > > > +++ b/scripts/lib/wic/help.py
> > > > > @@ -979,6 +979,13 @@ DESCRIPTION
> > > > > copies. This option only has an effect with the rootfs
> > > > > source plugin.
> > > > >
> > > > > + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
> > > > > + the given path to the resulting image. The option
> > > > > + contains two fields, the roofs and the path, separated
> > > > > + by a colon. The rootfs follows the same logic as the
> > > > > + rootfs-dir argument. This option only has an effect
> > > > > + with the rootfs source plugin.
> > > > > +
> > > > > --extra-space: This option is specific to wic. It adds extra
> > > > > space after the space filled by the content
> > > > > of the partition. The final size can go
> > > > > diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> > > > > index 650b976223..d422e2a6bb 100644
> > > > > --- a/scripts/lib/wic/ksparser.py
> > > > > +++ b/scripts/lib/wic/ksparser.py
> > > > > @@ -138,6 +138,7 @@ class KickStart():
> > > > > part.add_argument('--align', type=int)
> > > > > part.add_argument('--exclude-path', nargs='+')
> > > > > part.add_argument('--include-path', nargs='+')
> > > > > + part.add_argument('--embed-rootfs', nargs='+')
> > > > > part.add_argument("--extra-space", type=sizetype)
> > > > > part.add_argument('--fsoptions', dest='fsopts')
> > > > > part.add_argument('--fstype', default='vfat',
> > > > > diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> > > > > index 2d95f78439..13857df82f 100644
> > > > > --- a/scripts/lib/wic/partition.py
> > > > > +++ b/scripts/lib/wic/partition.py
> > > > > @@ -31,6 +31,7 @@ class Partition():
> > > > > self.extra_space = args.extra_space
> > > > > self.exclude_path = args.exclude_path
> > > > > self.include_path = args.include_path
> > > > > + self.embed_rootfs = args.embed_rootfs
> > > > > self.fsopts = args.fsopts
> > > > > self.fstype = args.fstype
> > > > > self.label = args.label
> > > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > index 40419a64b3..16359601dc 100644
> > > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
> > > > >
> > > > > new_rootfs = None
> > > > > # Handle excluded paths.
> > > > > - if part.exclude_path or part.include_path:
> > > > > + if part.exclude_path or part.include_path or part.embed_rootfs:
> > > > > # 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%d" % part.lineno))
> > > > > @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
> > > > > for path in part.include_path or []:
> > > > > copyhardlinktree(path, new_rootfs)
> > > > >
> > > > > + for embed in part.embed_rootfs or []:
> > > > > + [embed_rootfs, path] = embed.split(":")
> > > > > + #we need to remove the initial / for os.path.join to work
> > > > > + if os.path.isabs(path):
> > > > > + path = path[1:]
> > > > > + if embed_rootfs in krootfs_dir:
> > > > > + embed_rootfs = krootfs_dir[embed_rootfs]
> > > > > + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
> > > > > + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
> > > > > + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
> > > > > + embed_rootfs), tar_file, embed_rootfs)
> > > > > + exec_native_cmd(tar_cmd, native_sysroot)
> > > > > + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
> > > > > + tar_file, os.path.join(new_rootfs, path))
> > > > > + exec_native_cmd(untar_cmd, native_sysroot,
> > > > > + cls.__get_pseudo(native_sysroot, new_rootfs))
> > > > > + os.remove(tar_file)
> > > > > +
> > > >
> > > > Why are you using an intermediate tar file here?
> > >
> > > For the permissions :(
> >
> > Your previous patch handled this using 'pseudo -B'. It would be good to be
> > consistent if possible - either use an intermediate tar file in all cases if
> > it's necessary or use 'pseudo -B' in all cases.
>
> Unfortunately, you cannot use pseudo -B in this case.
>
> You use pseudo -B when you move one pseudo-aware partition to
> somewhere else. But now we are merging two pseudo-aware partitions.
>
> Also I would not use the intermediate tar on the first place because
> it is a waste of space + cpu resources. Copyhardlink is the right
> tool
>
>
> >
> > --
> > Paul Barker
> > Konsulko Group
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] wic: Add --embed-rootfs argument
2020-03-04 13:49 ` Ricardo Ribalda Delgado
@ 2020-03-04 14:31 ` Joshua Watt
0 siblings, 0 replies; 18+ messages in thread
From: Joshua Watt @ 2020-03-04 14:31 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Paul Barker; +Cc: openembedded-core
On 3/4/20 7:49 AM, Ricardo Ribalda Delgado wrote:
> Hi Paul
>
> On Wed, Mar 4, 2020 at 11:14 AM Ricardo Ribalda Delgado
> <ricardo@ribalda.com> wrote:
>> Hi Paul
>>
>> On Wed, Mar 4, 2020 at 11:09 AM Paul Barker <pbarker@konsulko.com> wrote:
>>> On Wed, 4 Mar 2020 10:56:20 +0100
>>> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>>>
>>>> Hi Paul
>>>>
>>>> Thanks for your reply
>>>>
>>>> On Wed, Mar 4, 2020 at 10:43 AM Paul Barker <pbarker@konsulko.com> wrote:
>>>>> On Wed, 4 Mar 2020 09:34:38 +0100
>>>>> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>>>>>
>>>>>> This option adds the content of a rootfs on a specific location on the
>>>>>> rootfs.
>>>>>>
>>>>>> It is very useful for making a partition that contains the rootfs for a
>>>>>> host and a target Eg:
>>>>>>
>>>>>> / -> Roofs for the host
>>>>>> /export/ -> Rootfs for the target (which will netboot)
>>>>>>
>>>>>> Although today we support making a partition for "/export" this might
>>>>>> not be compatible with some upgrade systems, or we might be limited by
>>>>>> the number of partitions.
>>>>>>
>>>>>> With this patch we can use something like:
>>>>>>
>>>>>> part / --source rootfs --embed-rootfs=target-image:/export
>>>>> I considered using a syntax like this for --include-path but I chose not to
>>>>> as it's not easy to choose a separator. ':' is a valid character in a
>>>>> directory name.
>>>> I think we have to live with not being able to copy files to a
>>>> directory with a colon :(
>>> I wouldn't like to place a limitation like that on users and I'd like to
>>> avoid the proliferation of similar arguments here.
>>>
>>> How about modifying '--include-path' to take two arguments, a source path and
>>> a destination prefix? The python argument parser should be able to handle
>>> this and you can build a list of tuples (source_path, dest_prefix) when
>>> parsing the arguments.
>>>
>>> The '--include-path' argument hasn't been part of a Yocto release yet it's
>>> just on the master branch so I think if we're going to fix it with a breaking
>>> change, now is definitely the time.
>> That sounds good to me. I do not mind having two arguments. for
>> embed-rootfs. and probably will look better on include-path
> How do you think that the parameters should look like:
>
> --embed-rootfs target-image /export target-image2 /export2
> --embed-rootfs (target-image, /export) (target-image2, /export2)
> ??
FWIW, it should probably end up looking like this:
--embed-rootfs target-image /export --embed-rootfs target-image2 /export2
The python argparse snippet you would use for this would be something like:
parser.add_argument('--embed-rootfs', nargs=2, action='append',
default=(), help='...')
>
> Shall I change it for include-path and embed-rootfs or you want to
> take care of include-path?
>
>>>>> My approach is to handle this in two steps:
>>>>>
>>>>> 1) Add a new task before do_image_wic which creates an 'embedded-rootfs'
>>>>> directory and copies 'target-image' to 'embedded-rootfs/export'.
>>>>>
>>>>> 2) Use '--include-path=embedded-rootfs' in the wks file.
>>>>>
>>>> The biggest problem with that approach is permissions. We need files
>>>> with UID 0, siticky bits, etc. We lose the pseudo database if we
>>>> simply copy to a folder.
>>>>
>>>> Also, if we have a complex system with multiple partitions I prefer to
>>>> handle all in a single .wks file instead of a script + wks file
>>>>
>>>>>> on the .wks file.
>>>>>>
>>>>>> Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
>>>>>> ---
>>>>>> scripts/lib/wic/help.py | 7 +++++++
>>>>>> scripts/lib/wic/ksparser.py | 1 +
>>>>>> scripts/lib/wic/partition.py | 1 +
>>>>>> scripts/lib/wic/plugins/source/rootfs.py | 20 +++++++++++++++++++-
>>>>>> 4 files changed, 28 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
>>>>>> index 4d342fcf05..67a33e6a65 100644
>>>>>> --- a/scripts/lib/wic/help.py
>>>>>> +++ b/scripts/lib/wic/help.py
>>>>>> @@ -979,6 +979,13 @@ DESCRIPTION
>>>>>> copies. This option only has an effect with the rootfs
>>>>>> source plugin.
>>>>>>
>>>>>> + --embed-rootfs: This option is specific to wic. It embeds a rootfs into
>>>>>> + the given path to the resulting image. The option
>>>>>> + contains two fields, the roofs and the path, separated
>>>>>> + by a colon. The rootfs follows the same logic as the
>>>>>> + rootfs-dir argument. This option only has an effect
>>>>>> + with the rootfs source plugin.
>>>>>> +
>>>>>> --extra-space: This option is specific to wic. It adds extra
>>>>>> space after the space filled by the content
>>>>>> of the partition. The final size can go
>>>>>> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
>>>>>> index 650b976223..d422e2a6bb 100644
>>>>>> --- a/scripts/lib/wic/ksparser.py
>>>>>> +++ b/scripts/lib/wic/ksparser.py
>>>>>> @@ -138,6 +138,7 @@ class KickStart():
>>>>>> part.add_argument('--align', type=int)
>>>>>> part.add_argument('--exclude-path', nargs='+')
>>>>>> part.add_argument('--include-path', nargs='+')
>>>>>> + part.add_argument('--embed-rootfs', nargs='+')
>>>>>> part.add_argument("--extra-space", type=sizetype)
>>>>>> part.add_argument('--fsoptions', dest='fsopts')
>>>>>> part.add_argument('--fstype', default='vfat',
>>>>>> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
>>>>>> index 2d95f78439..13857df82f 100644
>>>>>> --- a/scripts/lib/wic/partition.py
>>>>>> +++ b/scripts/lib/wic/partition.py
>>>>>> @@ -31,6 +31,7 @@ class Partition():
>>>>>> self.extra_space = args.extra_space
>>>>>> self.exclude_path = args.exclude_path
>>>>>> self.include_path = args.include_path
>>>>>> + self.embed_rootfs = args.embed_rootfs
>>>>>> self.fsopts = args.fsopts
>>>>>> self.fstype = args.fstype
>>>>>> self.label = args.label
>>>>>> diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
>>>>>> index 40419a64b3..16359601dc 100644
>>>>>> --- a/scripts/lib/wic/plugins/source/rootfs.py
>>>>>> +++ b/scripts/lib/wic/plugins/source/rootfs.py
>>>>>> @@ -80,7 +80,7 @@ class RootfsPlugin(SourcePlugin):
>>>>>>
>>>>>> new_rootfs = None
>>>>>> # Handle excluded paths.
>>>>>> - if part.exclude_path or part.include_path:
>>>>>> + if part.exclude_path or part.include_path or part.embed_rootfs:
>>>>>> # 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%d" % part.lineno))
>>>>>> @@ -100,6 +100,24 @@ class RootfsPlugin(SourcePlugin):
>>>>>> for path in part.include_path or []:
>>>>>> copyhardlinktree(path, new_rootfs)
>>>>>>
>>>>>> + for embed in part.embed_rootfs or []:
>>>>>> + [embed_rootfs, path] = embed.split(":")
>>>>>> + #we need to remove the initial / for os.path.join to work
>>>>>> + if os.path.isabs(path):
>>>>>> + path = path[1:]
>>>>>> + if embed_rootfs in krootfs_dir:
>>>>>> + embed_rootfs = krootfs_dir[embed_rootfs]
>>>>>> + embed_rootfs = cls.__get_rootfs_dir(embed_rootfs)
>>>>>> + tar_file = os.path.realpath(os.path.join(cr_workdir, "aux.tar"))
>>>>>> + tar_cmd = "%s tar cpf %s -C %s ." % (cls.__get_pseudo(native_sysroot,
>>>>>> + embed_rootfs), tar_file, embed_rootfs)
>>>>>> + exec_native_cmd(tar_cmd, native_sysroot)
>>>>>> + untar_cmd = "%s tar xf %s -C %s ." % (cls.__get_pseudo(native_sysroot, new_rootfs),
>>>>>> + tar_file, os.path.join(new_rootfs, path))
>>>>>> + exec_native_cmd(untar_cmd, native_sysroot,
>>>>>> + cls.__get_pseudo(native_sysroot, new_rootfs))
>>>>>> + os.remove(tar_file)
>>>>>> +
>>>>> Why are you using an intermediate tar file here?
>>>> For the permissions :(
>>> Your previous patch handled this using 'pseudo -B'. It would be good to be
>>> consistent if possible - either use an intermediate tar file in all cases if
>>> it's necessary or use 'pseudo -B' in all cases.
>> Unfortunately, you cannot use pseudo -B in this case.
>>
>> You use pseudo -B when you move one pseudo-aware partition to
>> somewhere else. But now we are merging two pseudo-aware partitions.
>>
>> Also I would not use the intermediate tar on the first place because
>> it is a waste of space + cpu resources. Copyhardlink is the right
>> tool
>>
>>
>>> --
>>> Paul Barker
>>> Konsulko Group
>>
>>
>> --
>> Ricardo Ribalda
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-03-04 10:02 ` Ricardo Ribalda Delgado
@ 2020-03-05 9:28 ` Paul Barker
2020-03-05 9:46 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-03-05 9:28 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: openembedded-core
On Wed, 4 Mar 2020 11:02:47 +0100
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> Hi Paul
>
> On Wed, Mar 4, 2020 at 10:53 AM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Wed, 4 Mar 2020 09:34:37 +0100
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > When parameters include_path or exclude_path are passed to the rootfs
> > > plugin, it will copy the partition content into a folder and make all
> > > the modifications there.
> > >
> > > This is done using copyhardlinktree(), which does not take into
> > > consideration the content of the pseudo folder, which contains the
> > > information about the right permissions and ownership of the folders.
> >
> > How are you running wic here? In the do_image_wic task it's executed under
> > pseudo so all this is handled already. Executing wic outside of bitbake may
> > need some more testing here.
>
> I am running wic outside bitbake. But even if it is run under bitbake,
> it should also fail. The pseudo directory needs to be present on the
> target image
If you're running wic outside of bitbake, is there any guarantee that pseudo
is available?
>
> >
> > >
> > > This results in a rootfs owned by the user that is running the wic
> > > command (usually UID 1000), which makes some rootfs unbootable.
> > >
> > > To fix this we copy the content of the pseudo folders to the new folder
> > > and modify the pseudo database using the "pseudo -B" command.
> > >
> > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > ---
> > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > index 705aeb5563..40419a64b3 100644
> > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > @@ -16,11 +16,11 @@ import os
> > > import shutil
> > > import sys
> > >
> > > -from oe.path import copyhardlinktree
> > > +from oe.path import copyhardlinktree, copytree
> > >
> > > from wic import WicError
> > > from wic.pluginbase import SourcePlugin
> > > -from wic.misc import get_bitbake_var
> > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > >
> > > logger = logging.getLogger('wic')
> > >
> > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > >
> > > return os.path.realpath(image_rootfs_dir)
> > >
> > > + @staticmethod
> > > + def __get_pseudo(native_sysroot, rootfs):
> > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > + return pseudo
> > > +
> > > @classmethod
> > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > oe_builddir, bootimg_dir, kernel_dir,
> > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > >
> > > if os.path.lexists(new_rootfs):
> > > shutil.rmtree(os.path.join(new_rootfs))
> > > -
> > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > >
> > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > > + os.path.join(new_rootfs, "../pseudo"))
> >
> > I don't like stepping up the directory tree like this. We should be more
> > explicit with the paths.
>
> You are thinking on:
> os.path.dirname(directory)
No, I'm wondering why we're taking a step up the directory tree from
`part.rootfs_dir`. I can point that at any path using the `--rootfs-dir`
argument and there's no guarantee that ../pseudo exists or is relevant to the
path I gave to `--rootfs-dir`.
>
> >
> > > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > > + part.rootfs_dir, new_rootfs)
> > > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > > +
> > > for path in part.include_path or []:
> > > copyhardlinktree(path, new_rootfs)
> >
> > ^^^^^^^^^^^^^^^^
> >
> > If this is the right approach I imagine you would also need to fix things up
> > with pseudo after the copyhardlinktree call above.
>
> I do not think it is needed. include_path does not contain its own
> pseudo directory
That's not necessarily true. The include_path could have been built by Yocto
using pseudo.
I can see that there is an issue using these arguments to wic outside of
bitbake but I'm not sure this is the correct solution.
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-03-05 9:28 ` Paul Barker
@ 2020-03-05 9:46 ` Ricardo Ribalda Delgado
2020-04-03 19:53 ` [OE-core] " Ricardo Ribalda
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda Delgado @ 2020-03-05 9:46 UTC (permalink / raw)
To: Paul Barker; +Cc: openembedded-core
Hi Paul
On Thu, Mar 5, 2020 at 10:32 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Wed, 4 Mar 2020 11:02:47 +0100
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > Hi Paul
> >
> > On Wed, Mar 4, 2020 at 10:53 AM Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Wed, 4 Mar 2020 09:34:37 +0100
> > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > >
> > > > When parameters include_path or exclude_path are passed to the rootfs
> > > > plugin, it will copy the partition content into a folder and make all
> > > > the modifications there.
> > > >
> > > > This is done using copyhardlinktree(), which does not take into
> > > > consideration the content of the pseudo folder, which contains the
> > > > information about the right permissions and ownership of the folders.
> > >
> > > How are you running wic here? In the do_image_wic task it's executed under
> > > pseudo so all this is handled already. Executing wic outside of bitbake may
> > > need some more testing here.
> >
> > I am running wic outside bitbake. But even if it is run under bitbake,
> > it should also fail. The pseudo directory needs to be present on the
> > target image
>
> If you're running wic outside of bitbake, is there any guarantee that pseudo
> is available?
Yes, the same guarantee that the ext3_tools are available. So I
believe we are safe here. Actually in my docker pseudo is not
installed and when I invoke with wic, everything is fine.
>
> >
> > >
> > > >
> > > > This results in a rootfs owned by the user that is running the wic
> > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > >
> > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > and modify the pseudo database using the "pseudo -B" command.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > ---
> > > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > index 705aeb5563..40419a64b3 100644
> > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > @@ -16,11 +16,11 @@ import os
> > > > import shutil
> > > > import sys
> > > >
> > > > -from oe.path import copyhardlinktree
> > > > +from oe.path import copyhardlinktree, copytree
> > > >
> > > > from wic import WicError
> > > > from wic.pluginbase import SourcePlugin
> > > > -from wic.misc import get_bitbake_var
> > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > >
> > > > logger = logging.getLogger('wic')
> > > >
> > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > > return os.path.realpath(image_rootfs_dir)
> > > >
> > > > + @staticmethod
> > > > + def __get_pseudo(native_sysroot, rootfs):
> > > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > + return pseudo
> > > > +
> > > > @classmethod
> > > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > > oe_builddir, bootimg_dir, kernel_dir,
> > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > > if os.path.lexists(new_rootfs):
> > > > shutil.rmtree(os.path.join(new_rootfs))
> > > > -
> > > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > >
> > > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > > > + os.path.join(new_rootfs, "../pseudo"))
> > >
> > > I don't like stepping up the directory tree like this. We should be more
> > > explicit with the paths.
> >
> > You are thinking on:
> > os.path.dirname(directory)
>
> No, I'm wondering why we're taking a step up the directory tree from
> `part.rootfs_dir`. I can point that at any path using the `--rootfs-dir`
> argument and there's no guarantee that ../pseudo exists or is relevant to the
> path I gave to `--rootfs-dir`.
Because we are asuming that the rootfs is being generated with
OE/yocto, and there is where the pseudo folder lives.
It is the same asumption part.prepare_rootfs() is taking.
>
> >
> > >
> > > > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > > > + part.rootfs_dir, new_rootfs)
> > > > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > > > +
> > > > for path in part.include_path or []:
> > > > copyhardlinktree(path, new_rootfs)
> > >
> > > ^^^^^^^^^^^^^^^^
> > >
> > > If this is the right approach I imagine you would also need to fix things up
> > > with pseudo after the copyhardlinktree call above.
> >
> > I do not think it is needed. include_path does not contain its own
> > pseudo directory
>
> That's not necessarily true. The include_path could have been built by Yocto
> using pseudo.
Then you need to provide where the pseudo folder is. Eg
--include_path folder/A/B/C/D/file
Where is the pseudo database? in folder/A/B/C/D/pseudo,
folder/A/B/C/pseudo , folder/A/B/C/pseudo /...
>
> I can see that there is an issue using these arguments to wic outside of
> bitbake but I'm not sure this is the correct solution.
>
> --
> Paul Barker
> Konsulko Group
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-03-05 9:46 ` Ricardo Ribalda Delgado
@ 2020-04-03 19:53 ` Ricardo Ribalda
2020-04-07 18:12 ` Paul Barker
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda @ 2020-04-03 19:53 UTC (permalink / raw)
To: Paul Barker, Richard Purdie; +Cc: openembedded-core
ping?
On Thu, Mar 5, 2020 at 10:46 AM Ricardo Ribalda Delgado
<ricardo@ribalda.com> wrote:
>
> Hi Paul
>
> On Thu, Mar 5, 2020 at 10:32 AM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Wed, 4 Mar 2020 11:02:47 +0100
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > Hi Paul
> > >
> > > On Wed, Mar 4, 2020 at 10:53 AM Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Wed, 4 Mar 2020 09:34:37 +0100
> > > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > > >
> > > > > When parameters include_path or exclude_path are passed to the rootfs
> > > > > plugin, it will copy the partition content into a folder and make all
> > > > > the modifications there.
> > > > >
> > > > > This is done using copyhardlinktree(), which does not take into
> > > > > consideration the content of the pseudo folder, which contains the
> > > > > information about the right permissions and ownership of the folders.
> > > >
> > > > How are you running wic here? In the do_image_wic task it's executed under
> > > > pseudo so all this is handled already. Executing wic outside of bitbake may
> > > > need some more testing here.
> > >
> > > I am running wic outside bitbake. But even if it is run under bitbake,
> > > it should also fail. The pseudo directory needs to be present on the
> > > target image
> >
> > If you're running wic outside of bitbake, is there any guarantee that pseudo
> > is available?
>
> Yes, the same guarantee that the ext3_tools are available. So I
> believe we are safe here. Actually in my docker pseudo is not
> installed and when I invoke with wic, everything is fine.
>
> >
> > >
> > > >
> > > > >
> > > > > This results in a rootfs owned by the user that is running the wic
> > > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > > >
> > > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > > and modify the pseudo database using the "pseudo -B" command.
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > > ---
> > > > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > index 705aeb5563..40419a64b3 100644
> > > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > @@ -16,11 +16,11 @@ import os
> > > > > import shutil
> > > > > import sys
> > > > >
> > > > > -from oe.path import copyhardlinktree
> > > > > +from oe.path import copyhardlinktree, copytree
> > > > >
> > > > > from wic import WicError
> > > > > from wic.pluginbase import SourcePlugin
> > > > > -from wic.misc import get_bitbake_var
> > > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > > >
> > > > > logger = logging.getLogger('wic')
> > > > >
> > > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > > >
> > > > > return os.path.realpath(image_rootfs_dir)
> > > > >
> > > > > + @staticmethod
> > > > > + def __get_pseudo(native_sysroot, rootfs):
> > > > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > > + return pseudo
> > > > > +
> > > > > @classmethod
> > > > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > > > oe_builddir, bootimg_dir, kernel_dir,
> > > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > > >
> > > > > if os.path.lexists(new_rootfs):
> > > > > shutil.rmtree(os.path.join(new_rootfs))
> > > > > -
> > > > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > > >
> > > > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > > > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > > > > + os.path.join(new_rootfs, "../pseudo"))
> > > >
> > > > I don't like stepping up the directory tree like this. We should be more
> > > > explicit with the paths.
> > >
> > > You are thinking on:
> > > os.path.dirname(directory)
> >
> > No, I'm wondering why we're taking a step up the directory tree from
> > `part.rootfs_dir`. I can point that at any path using the `--rootfs-dir`
> > argument and there's no guarantee that ../pseudo exists or is relevant to the
> > path I gave to `--rootfs-dir`.
>
> Because we are asuming that the rootfs is being generated with
> OE/yocto, and there is where the pseudo folder lives.
> It is the same asumption part.prepare_rootfs() is taking.
>
> >
> > >
> > > >
> > > > > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > > > > + part.rootfs_dir, new_rootfs)
> > > > > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > > > > +
> > > > > for path in part.include_path or []:
> > > > > copyhardlinktree(path, new_rootfs)
> > > >
> > > > ^^^^^^^^^^^^^^^^
> > > >
> > > > If this is the right approach I imagine you would also need to fix things up
> > > > with pseudo after the copyhardlinktree call above.
> > >
> > > I do not think it is needed. include_path does not contain its own
> > > pseudo directory
> >
> > That's not necessarily true. The include_path could have been built by Yocto
> > using pseudo.
>
> Then you need to provide where the pseudo folder is. Eg
>
> --include_path folder/A/B/C/D/file
>
> Where is the pseudo database? in folder/A/B/C/D/pseudo,
> folder/A/B/C/pseudo , folder/A/B/C/pseudo /...
>
>
>
> >
> > I can see that there is an issue using these arguments to wic outside of
> > bitbake but I'm not sure this is the correct solution.
> >
> > --
> > Paul Barker
> > Konsulko Group
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-04-03 19:53 ` [OE-core] " Ricardo Ribalda
@ 2020-04-07 18:12 ` Paul Barker
2020-04-07 18:40 ` Ricardo Ribalda
0 siblings, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-04-07 18:12 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: Richard Purdie, openembedded-core
On Fri, 3 Apr 2020 21:53:39 +0200
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> ping?
I think that '../pseudo' should not be used here. I'll explain inline...
> >
> > This results in a rootfs owned by the user that is running the wic
> > command (usually UID 1000), which makes some rootfs unbootable.
> >
> > To fix this we copy the content of the pseudo folders to the new folder
> > and modify the pseudo database using the "pseudo -B" command.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > ---
> > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > index 705aeb5563..40419a64b3 100644
> > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > @@ -16,11 +16,11 @@ import os
> > import shutil
> > import sys
> >
> > -from oe.path import copyhardlinktree
> > +from oe.path import copyhardlinktree, copytree
> >
> > from wic import WicError
> > from wic.pluginbase import SourcePlugin
> > -from wic.misc import get_bitbake_var
> > +from wic.misc import get_bitbake_var, exec_native_cmd
> >
> > logger = logging.getLogger('wic')
> >
> > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> >
> > return os.path.realpath(image_rootfs_dir)
> >
> > + @staticmethod
> > + def __get_pseudo(native_sysroot, rootfs):
> > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > + return pseudo
> > +
> > @classmethod
> > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > oe_builddir, bootimg_dir, kernel_dir,
> > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> >
> > if os.path.lexists(new_rootfs):
> > shutil.rmtree(os.path.join(new_rootfs))
> > -
> > copyhardlinktree(part.rootfs_dir, new_rootfs)
> >
> > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
new_rootfs is set by the following statement a few lines above:
new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
Consider that `cr_workdir` may contain multiple rootfs staging directories
corresponding to multiple lines in the wks file, for example if a rootfs
image is duplicated into multiple partitions for redundancy. In that case
`os.path.join(new_rootfs, "../pseudo")` will clash between these different
rootfs copies.
Let's use an explicit path instead, such as:
new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))
> > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
directory given. It's unsafe to step up the directory tree and make
assumptions like this.
> > + os.path.join(new_rootfs, "../pseudo"))
> > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > + part.rootfs_dir, new_rootfs)
> > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > +
> > for path in part.include_path or []:
> > copyhardlinktree(path, new_rootfs)
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-04-07 18:12 ` Paul Barker
@ 2020-04-07 18:40 ` Ricardo Ribalda
2020-04-07 19:02 ` Paul Barker
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda @ 2020-04-07 18:40 UTC (permalink / raw)
To: Paul Barker; +Cc: Richard Purdie, openembedded-core
Hi Paul
Thanks for your review, It has been already merged, but if there is
something wrong we can send a patch fixing it.
https://git.openembedded.org/openembedded-core/commit/?id=36993eea89d1c011397b7692b9b8d61b499d0171
On Tue, Apr 7, 2020 at 8:13 PM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Fri, 3 Apr 2020 21:53:39 +0200
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > ping?
>
> I think that '../pseudo' should not be used here. I'll explain inline...
>
> > >
> > > This results in a rootfs owned by the user that is running the wic
> > > command (usually UID 1000), which makes some rootfs unbootable.
> > >
> > > To fix this we copy the content of the pseudo folders to the new folder
> > > and modify the pseudo database using the "pseudo -B" command.
> > >
> > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > ---
> > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > index 705aeb5563..40419a64b3 100644
> > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > @@ -16,11 +16,11 @@ import os
> > > import shutil
> > > import sys
> > >
> > > -from oe.path import copyhardlinktree
> > > +from oe.path import copyhardlinktree, copytree
> > >
> > > from wic import WicError
> > > from wic.pluginbase import SourcePlugin
> > > -from wic.misc import get_bitbake_var
> > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > >
> > > logger = logging.getLogger('wic')
> > >
> > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > >
> > > return os.path.realpath(image_rootfs_dir)
> > >
> > > + @staticmethod
> > > + def __get_pseudo(native_sysroot, rootfs):
> > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > + return pseudo
> > > +
> > > @classmethod
> > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > oe_builddir, bootimg_dir, kernel_dir,
> > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > >
> > > if os.path.lexists(new_rootfs):
> > > shutil.rmtree(os.path.join(new_rootfs))
> > > -
> > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > >
> > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
>
> new_rootfs is set by the following statement a few lines above:
>
> new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
>
> Consider that `cr_workdir` may contain multiple rootfs staging directories
> corresponding to multiple lines in the wks file, for example if a rootfs
> image is duplicated into multiple partitions for redundancy. In that case
> `os.path.join(new_rootfs, "../pseudo")` will clash between these different
> rootfs copies.
>
> Let's use an explicit path instead, such as:
>
> new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))
The reason to have that path was to follow the same structure as the
real image.bb.
If there are multiple partitions on the .wic file the different
partitions are done one by one, not
in parallel.
So
../pseudo will be created for partition1
then it will be used to generate the partition1
../pseudo will be deleted
../pseudo will be created for partition2
Even if they use the same partition, the code works (and ../pseudo is
useless once the partition is generated)
>
> > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
>
> part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
> is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
> directory given. It's unsafe to step up the directory tree and make
> assumptions like this.
I think that if we do not pass a real rootfs to the rootfs plugin it
is an error from the user.
We can add a more beautiful error message instead of a backtrace.
Or if you believe that it is a valid usecase to not pass a rootfs then
we can continue with a warning/debug message and explicitly telling
the user that the permissions are going to be invalid, because what he
is using as a roofs is an unknow directory for bitbake.
I have no personal preference for any of the two, tell me what do you
prefer (or a different option) and I will implement it.
Thans again for the review.
>
> > > + os.path.join(new_rootfs, "../pseudo"))
> > > + pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > > + part.rootfs_dir, new_rootfs)
> > > + exec_native_cmd(pseudo_cmd, native_sysroot)
> > > +
> > > for path in part.include_path or []:
> > > copyhardlinktree(path, new_rootfs)
>
> --
> Paul Barker
> Konsulko Group
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-04-07 18:40 ` Ricardo Ribalda
@ 2020-04-07 19:02 ` Paul Barker
2020-04-07 19:19 ` Ricardo Ribalda
0 siblings, 1 reply; 18+ messages in thread
From: Paul Barker @ 2020-04-07 19:02 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: Richard Purdie, openembedded-core
On Tue, 7 Apr 2020 20:40:18 +0200
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> Hi Paul
>
> Thanks for your review, It has been already merged, but if there is
> something wrong we can send a patch fixing it.
>
> https://git.openembedded.org/openembedded-core/commit/?id=36993eea89d1c011397b7692b9b8d61b499d0171
>
> On Tue, Apr 7, 2020 at 8:13 PM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Fri, 3 Apr 2020 21:53:39 +0200
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > ping?
> >
> > I think that '../pseudo' should not be used here. I'll explain inline...
> >
> > > >
> > > > This results in a rootfs owned by the user that is running the wic
> > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > >
> > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > and modify the pseudo database using the "pseudo -B" command.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > ---
> > > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > index 705aeb5563..40419a64b3 100644
> > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > @@ -16,11 +16,11 @@ import os
> > > > import shutil
> > > > import sys
> > > >
> > > > -from oe.path import copyhardlinktree
> > > > +from oe.path import copyhardlinktree, copytree
> > > >
> > > > from wic import WicError
> > > > from wic.pluginbase import SourcePlugin
> > > > -from wic.misc import get_bitbake_var
> > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > >
> > > > logger = logging.getLogger('wic')
> > > >
> > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > > return os.path.realpath(image_rootfs_dir)
> > > >
> > > > + @staticmethod
> > > > + def __get_pseudo(native_sysroot, rootfs):
> > > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > + return pseudo
> > > > +
> > > > @classmethod
> > > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > > oe_builddir, bootimg_dir, kernel_dir,
> > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > > if os.path.lexists(new_rootfs):
> > > > shutil.rmtree(os.path.join(new_rootfs))
> > > > -
> > > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > >
> > > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> >
> > new_rootfs is set by the following statement a few lines above:
> >
> > new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
> >
> > Consider that `cr_workdir` may contain multiple rootfs staging directories
> > corresponding to multiple lines in the wks file, for example if a rootfs
> > image is duplicated into multiple partitions for redundancy. In that case
> > `os.path.join(new_rootfs, "../pseudo")` will clash between these different
> > rootfs copies.
> >
> > Let's use an explicit path instead, such as:
> >
> > new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))
>
> The reason to have that path was to follow the same structure as the
> real image.bb.
>
> If there are multiple partitions on the .wic file the different
> partitions are done one by one, not
> in parallel.
>
> So
> ../pseudo will be created for partition1
> then it will be used to generate the partition1
>
> ../pseudo will be deleted
> ../pseudo will be created for partition2
>
> Even if they use the same partition, the code works (and ../pseudo is
> useless once the partition is generated)
>
Having these separate is important for debugging though, it lets you look
through the different copies after wic exits if something is wrong.
> >
> > > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> >
> > part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
> > is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
> > directory given. It's unsafe to step up the directory tree and make
> > assumptions like this.
>
> I think that if we do not pass a real rootfs to the rootfs plugin it
> is an error from the user.
>
> We can add a more beautiful error message instead of a backtrace.
>
> Or if you believe that it is a valid usecase to not pass a rootfs then
> we can continue with a warning/debug message and explicitly telling
> the user that the permissions are going to be invalid, because what he
> is using as a roofs is an unknow directory for bitbake.
This is a valid and existing usecase. This is how data partitions are
populated and how you separate /home or another directory into its own
partiton (e.g.
https://stackoverflow.com/questions/56187209/yocto-create-and-populate-a-separate-home-partition).
>
> I have no personal preference for any of the two, tell me what do you
> prefer (or a different option) and I will implement it.
>
> Thans again for the review.
>
This patch needs reverting from master/dunfell. I hope it hasn't gone into
the M4 build...
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-04-07 19:02 ` Paul Barker
@ 2020-04-07 19:19 ` Ricardo Ribalda
2020-04-07 19:43 ` Paul Barker
0 siblings, 1 reply; 18+ messages in thread
From: Ricardo Ribalda @ 2020-04-07 19:19 UTC (permalink / raw)
To: Paul Barker; +Cc: Richard Purdie, openembedded-core
Hi
On Tue, Apr 7, 2020 at 9:02 PM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Tue, 7 Apr 2020 20:40:18 +0200
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > Hi Paul
> >
> > Thanks for your review, It has been already merged, but if there is
> > something wrong we can send a patch fixing it.
> >
> > https://git.openembedded.org/openembedded-core/commit/?id=36993eea89d1c011397b7692b9b8d61b499d0171
> >
> > On Tue, Apr 7, 2020 at 8:13 PM Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Fri, 3 Apr 2020 21:53:39 +0200
> > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > >
> > > > ping?
> > >
> > > I think that '../pseudo' should not be used here. I'll explain inline...
> > >
> > > > >
> > > > > This results in a rootfs owned by the user that is running the wic
> > > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > > >
> > > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > > and modify the pseudo database using the "pseudo -B" command.
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > > ---
> > > > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > index 705aeb5563..40419a64b3 100644
> > > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > @@ -16,11 +16,11 @@ import os
> > > > > import shutil
> > > > > import sys
> > > > >
> > > > > -from oe.path import copyhardlinktree
> > > > > +from oe.path import copyhardlinktree, copytree
> > > > >
> > > > > from wic import WicError
> > > > > from wic.pluginbase import SourcePlugin
> > > > > -from wic.misc import get_bitbake_var
> > > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > > >
> > > > > logger = logging.getLogger('wic')
> > > > >
> > > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > > >
> > > > > return os.path.realpath(image_rootfs_dir)
> > > > >
> > > > > + @staticmethod
> > > > > + def __get_pseudo(native_sysroot, rootfs):
> > > > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > > + return pseudo
> > > > > +
> > > > > @classmethod
> > > > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > > > oe_builddir, bootimg_dir, kernel_dir,
> > > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > > >
> > > > > if os.path.lexists(new_rootfs):
> > > > > shutil.rmtree(os.path.join(new_rootfs))
> > > > > -
> > > > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > > >
> > > > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > >
> > > new_rootfs is set by the following statement a few lines above:
> > >
> > > new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
> > >
> > > Consider that `cr_workdir` may contain multiple rootfs staging directories
> > > corresponding to multiple lines in the wks file, for example if a rootfs
> > > image is duplicated into multiple partitions for redundancy. In that case
> > > `os.path.join(new_rootfs, "../pseudo")` will clash between these different
> > > rootfs copies.
> > >
> > > Let's use an explicit path instead, such as:
> > >
> > > new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))
> >
> > The reason to have that path was to follow the same structure as the
> > real image.bb.
> >
> > If there are multiple partitions on the .wic file the different
> > partitions are done one by one, not
> > in parallel.
> >
> > So
> > ../pseudo will be created for partition1
> > then it will be used to generate the partition1
> >
> > ../pseudo will be deleted
> > ../pseudo will be created for partition2
> >
> > Even if they use the same partition, the code works (and ../pseudo is
> > useless once the partition is generated)
> >
>
> Having these separate is important for debugging though, it lets you look
> through the different copies after wic exits if something is wrong.
I see your point, I can make a patch for that
>
> > >
> > > > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > >
> > > part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
> > > is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
> > > directory given. It's unsafe to step up the directory tree and make
> > > assumptions like this.
> >
> > I think that if we do not pass a real rootfs to the rootfs plugin it
> > is an error from the user.
> >
> > We can add a more beautiful error message instead of a backtrace.
> >
> > Or if you believe that it is a valid usecase to not pass a rootfs then
> > we can continue with a warning/debug message and explicitly telling
> > the user that the permissions are going to be invalid, because what he
> > is using as a roofs is an unknow directory for bitbake.
>
> This is a valid and existing usecase. This is how data partitions are
> populated and how you separate /home or another directory into its own
> partiton (e.g.
> https://stackoverflow.com/questions/56187209/yocto-create-and-populate-a-separate-home-partition).
Luckily on the usecase on that stackoverflow the patch will still work.
we need to combine partition split +
embed-roots/include_path/exlude_path to make it fail
By the way, by looking at your example I think that all the partition
split executed from outside bitbake will have invalid partitions on
their content.
We need to make something like:
part /home --source rootfs --root-dir=/home --ondisk sda --fstype=ext4
--label home
>
> >
> > I have no personal preference for any of the two, tell me what do you
> > prefer (or a different option) and I will implement it.
> >
> > Thans again for the review.
> >
>
> This patch needs reverting from master/dunfell. I hope it hasn't gone into
> the M4 build...
Instead of reverting I believe it is simpler to prepare a patch that
solves both of your concerns
- pseduoX instead of ../pseudo (although this does not break the code)
- keep the old behaviour if the pseudo folder is not found (wrong
permissions, but no backtrace)
And we can start talking about a way to split partitions and keep the
right permissions/username
Best regards.
>
> --
> Paul Barker
> Konsulko Group
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path
2020-04-07 19:19 ` Ricardo Ribalda
@ 2020-04-07 19:43 ` Paul Barker
0 siblings, 0 replies; 18+ messages in thread
From: Paul Barker @ 2020-04-07 19:43 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Richard Purdie, openembedded-core
On Tue, 7 Apr 2020 21:19:32 +0200
"Ricardo Ribalda" <ricardo.ribalda@gmail.com> wrote:
> Hi
>
> On Tue, Apr 7, 2020 at 9:02 PM Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Tue, 7 Apr 2020 20:40:18 +0200
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > Hi Paul
> > >
> > > Thanks for your review, It has been already merged, but if there is
> > > something wrong we can send a patch fixing it.
> > >
> > > https://git.openembedded.org/openembedded-core/commit/?id=36993eea89d1c011397b7692b9b8d61b499d0171
> > >
> > > On Tue, Apr 7, 2020 at 8:13 PM Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Fri, 3 Apr 2020 21:53:39 +0200
> > > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > > >
> > > > > ping?
> > > >
> > > > I think that '../pseudo' should not be used here. I'll explain inline...
> > > >
> > > > > >
> > > > > > This results in a rootfs owned by the user that is running the wic
> > > > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > > > >
> > > > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > > > and modify the pseudo database using the "pseudo -B" command.
> > > > > >
> > > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > > > ---
> > > > > > scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > > > > 1 file changed, 19 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > > index 705aeb5563..40419a64b3 100644
> > > > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > > > @@ -16,11 +16,11 @@ import os
> > > > > > import shutil
> > > > > > import sys
> > > > > >
> > > > > > -from oe.path import copyhardlinktree
> > > > > > +from oe.path import copyhardlinktree, copytree
> > > > > >
> > > > > > from wic import WicError
> > > > > > from wic.pluginbase import SourcePlugin
> > > > > > -from wic.misc import get_bitbake_var
> > > > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > > > >
> > > > > > logger = logging.getLogger('wic')
> > > > > >
> > > > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > > > >
> > > > > > return os.path.realpath(image_rootfs_dir)
> > > > > >
> > > > > > + @staticmethod
> > > > > > + def __get_pseudo(native_sysroot, rootfs):
> > > > > > + pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > > > + pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > > > + pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > > > + pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > > > + pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > > > + return pseudo
> > > > > > +
> > > > > > @classmethod
> > > > > > def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > > > > oe_builddir, bootimg_dir, kernel_dir,
> > > > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > > > >
> > > > > > if os.path.lexists(new_rootfs):
> > > > > > shutil.rmtree(os.path.join(new_rootfs))
> > > > > > -
> > > > > > copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > > > >
> > > > > > + if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > > >
> > > > new_rootfs is set by the following statement a few lines above:
> > > >
> > > > new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
> > > >
> > > > Consider that `cr_workdir` may contain multiple rootfs staging directories
> > > > corresponding to multiple lines in the wks file, for example if a rootfs
> > > > image is duplicated into multiple partitions for redundancy. In that case
> > > > `os.path.join(new_rootfs, "../pseudo")` will clash between these different
> > > > rootfs copies.
> > > >
> > > > Let's use an explicit path instead, such as:
> > > >
> > > > new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))
> > >
> > > The reason to have that path was to follow the same structure as the
> > > real image.bb.
> > >
> > > If there are multiple partitions on the .wic file the different
> > > partitions are done one by one, not
> > > in parallel.
> > >
> > > So
> > > ../pseudo will be created for partition1
> > > then it will be used to generate the partition1
> > >
> > > ../pseudo will be deleted
> > > ../pseudo will be created for partition2
> > >
> > > Even if they use the same partition, the code works (and ../pseudo is
> > > useless once the partition is generated)
> > >
> >
> > Having these separate is important for debugging though, it lets you look
> > through the different copies after wic exits if something is wrong.
>
> I see your point, I can make a patch for that
>
> >
> > > >
> > > > > > + shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > > > + copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > > >
> > > > part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
> > > > is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
> > > > directory given. It's unsafe to step up the directory tree and make
> > > > assumptions like this.
> > >
> > > I think that if we do not pass a real rootfs to the rootfs plugin it
> > > is an error from the user.
> > >
> > > We can add a more beautiful error message instead of a backtrace.
> > >
> > > Or if you believe that it is a valid usecase to not pass a rootfs then
> > > we can continue with a warning/debug message and explicitly telling
> > > the user that the permissions are going to be invalid, because what he
> > > is using as a roofs is an unknow directory for bitbake.
> >
> > This is a valid and existing usecase. This is how data partitions are
> > populated and how you separate /home or another directory into its own
> > partiton (e.g.
> > https://stackoverflow.com/questions/56187209/yocto-create-and-populate-a-separate-home-partition).
>
> Luckily on the usecase on that stackoverflow the patch will still work.
>
> we need to combine partition split +
> embed-roots/include_path/exlude_path to make it fail
>
> By the way, by looking at your example I think that all the partition
> split executed from outside bitbake will have invalid partitions on
> their content.
>
> We need to make something like:
>
> part /home --source rootfs --root-dir=/home --ondisk sda --fstype=ext4
> --label home
>
Agreed, there is an issue here when running wic outside bitbake that we need
to solve. I think this patch is most of the way towards fixing it but does
introduce other problems as I said.
>
> >
> > >
> > > I have no personal preference for any of the two, tell me what do you
> > > prefer (or a different option) and I will implement it.
> > >
> > > Thans again for the review.
> > >
> >
> > This patch needs reverting from master/dunfell. I hope it hasn't gone into
> > the M4 build...
>
> Instead of reverting I believe it is simpler to prepare a patch that
> solves both of your concerns
>
> - pseduoX instead of ../pseudo (although this does not break the code)
> - keep the old behaviour if the pseudo folder is not found (wrong
> permissions, but no backtrace)
>
> And we can start talking about a way to split partitions and keep the
> right permissions/username
>
We can do that after the 3.1 release. We're supposed to be in feature freeze
right now, this shouldn't have been merged so the best solution is to revert
it.
Thanks,
--
Paul Barker
Konsulko Group
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-04-07 19:47 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 8:34 [PATCH 1/2] wic: Fix permissions when using exclude or include path Ricardo Ribalda Delgado
2020-03-04 8:34 ` [PATCH 2/2] wic: Add --embed-rootfs argument Ricardo Ribalda Delgado
2020-03-04 9:42 ` Paul Barker
2020-03-04 9:56 ` Ricardo Ribalda Delgado
2020-03-04 10:08 ` Paul Barker
2020-03-04 10:14 ` Ricardo Ribalda Delgado
2020-03-04 13:49 ` Ricardo Ribalda Delgado
2020-03-04 14:31 ` Joshua Watt
2020-03-04 9:53 ` [PATCH 1/2] wic: Fix permissions when using exclude or include path Paul Barker
2020-03-04 10:02 ` Ricardo Ribalda Delgado
2020-03-05 9:28 ` Paul Barker
2020-03-05 9:46 ` Ricardo Ribalda Delgado
2020-04-03 19:53 ` [OE-core] " Ricardo Ribalda
2020-04-07 18:12 ` Paul Barker
2020-04-07 18:40 ` Ricardo Ribalda
2020-04-07 19:02 ` Paul Barker
2020-04-07 19:19 ` Ricardo Ribalda
2020-04-07 19:43 ` Paul Barker
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.