[PATCH 0/3] fix and test wic exclude-path

[PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

From: Henning Schild <henning.schild@siemens.com>

This series is two cherry-picks from oe plus one test. Before those
cherry-picks the test would fail, with them it works as epected.

Henning Schild (2):
  lib/oe/path: try hardlinking instead of guessing when it might fail
  CI: test the exclude-path feature of wic

Paul Barker (1):
  oe.path: Add copyhardlink() helper function

 meta/lib/oe/path.py | 27 +++++++++++++++++++++++++--
 scripts/ci_build.sh |  7 +++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

-- 
2.24.1

[PATCH 1/3] oe.path: Add copyhardlink() helper function

[Henning Schild]

From: Paul Barker <paul@betafive.co.uk>

This function creates hard links if possible, falling back to copying
the file if the destination is on a different volume to the source.

The docstring for copyhardlinktree() is also updated to make the
difference between the two functions a little clearer.

Signed-off-by: Paul Barker <paul@betafive.co.uk>
Signed-off-by: Ross Burton <ross.burton@intel.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oe/path.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index 90c87f5239..c6bb6042a3 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -94,7 +94,7 @@ def copytree(src, dst):
     subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
 
 def copyhardlinktree(src, dst):
-    """ Make the hard link when possible, otherwise copy. """
+    """Make a tree of hard links when possible, otherwise copy."""
     bb.utils.mkdirhier(dst)
     if os.path.isdir(src) and not len(os.listdir(src)):
         return
@@ -118,6 +118,17 @@ def copyhardlinktree(src, dst):
     else:
         copytree(src, dst)
 
+def copyhardlink(src, dst):
+    """Make a hard link when possible, otherwise copy."""
+
+    # We need to stat the destination directory as the destination file probably
+    # doesn't exist yet.
+    dstdir = os.path.dirname(dst)
+    if os.stat(src).st_dev == os.stat(dstdir).st_dev:
+        os.link(src, dst)
+    else:
+        shutil.copy(src, dst)
+
 def remove(path, recurse=True):
     """
     Equivalent to rm -f or rm -rf
-- 
2.24.1

[PATCH 2/3] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

From: Henning Schild <henning.schild@siemens.com>

The comparison of the stat st_dev is not enough to judge whether
hardlinking will work. One example would be where you try and hardlink
across two bind-mounts of a directory. The st_dev will be the same and
the operation will still fail.

Instead of implementing a check to try and figure out hardlink support
just try hardlinking and fall back to a copy when running into an
exception.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oe/path.py | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index c6bb6042a3..3506e2c989 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -99,7 +99,22 @@ def copyhardlinktree(src, dst):
     if os.path.isdir(src) and not len(os.listdir(src)):
         return
 
-    if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
+    canhard = False
+    testfile = None
+    for root, dirs, files in os.walk(src):
+        if len(files):
+            testfile = os.path.join(root, files[0])
+            break
+
+    if testfile is not None:
+        try:
+            os.link(testfile, os.path.join(dst, 'testfile'))
+            os.unlink(os.path.join(dst, 'testfile'))
+            canhard = True
+        except Exception as e:
+            bb.debug(2, "Hardlink test failed with " + str(e))
+
+    if (canhard):
         # Need to copy directories only with tar first since cp will error if two 
         # writers try and create a directory at the same time
         cmd = "cd %s; find . -type d -print | tar --xattrs --xattrs-include='*' -cf - -S -C %s -p --no-recursion --files-from - | tar --xattrs --xattrs-include='*' -xhf - -C %s" % (src, src, dst)
@@ -121,12 +136,9 @@ def copyhardlinktree(src, dst):
 def copyhardlink(src, dst):
     """Make a hard link when possible, otherwise copy."""
 
-    # We need to stat the destination directory as the destination file probably
-    # doesn't exist yet.
-    dstdir = os.path.dirname(dst)
-    if os.stat(src).st_dev == os.stat(dstdir).st_dev:
+    try:
         os.link(src, dst)
-    else:
+    except OSError:
         shutil.copy(src, dst)
 
 def remove(path, recurse=True):
-- 
2.24.1

[PATCH 3/3] CI: test the exclude-path feature of wic

[Henning Schild]

From: Henning Schild <henning.schild@siemens.com>

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 scripts/ci_build.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
index adc403be18..134fa22015 100755
--- a/scripts/ci_build.sh
+++ b/scripts/ci_build.sh
@@ -193,3 +193,10 @@ echo -e "do_fetch_append() {\n\n}" >> "${ISARROOT}/meta/classes/dpkg-base.bbclas
 bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base
 
 mv "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" "${ISARROOT}/meta/classes/dpkg-base.bbclass"
+
+cp -a "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup"
+sed -i -e 's/part \/ /part \/ --exclude-path usr /g' "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
+
+bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base
+
+mv "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
-- 
2.24.1

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

https://github.com/henning-schild-work/isar/tree/henning/staging1

On Mon, 6 Apr 2020 22:16:45 +0200
Henning Schild <henning.schild@siemens.com> wrote:

> From: Henning Schild <henning.schild@siemens.com>
> 
> This series is two cherry-picks from oe plus one test. Before those
> cherry-picks the test would fail, with them it works as epected.
> 
> Henning Schild (2):
>   lib/oe/path: try hardlinking instead of guessing when it might fail
>   CI: test the exclude-path feature of wic
> 
> Paul Barker (1):
>   oe.path: Add copyhardlink() helper function
> 
>  meta/lib/oe/path.py | 27 +++++++++++++++++++++++++--
>  scripts/ci_build.sh |  7 +++++++
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 

Re: [PATCH 3/3] CI: test the exclude-path feature of wic

[Baurzhan Ismagulov]

Hello Henning,

On Mon, Apr 06, 2020 at 10:16:48PM +0200, Henning Schild wrote:
> diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
...
> @@ -193,3 +193,10 @@ echo -e "do_fetch_append() {\n\n}" >> "${ISARROOT}/meta/classes/dpkg-base.bbclas
...
> +cp -a "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup"
> +sed -i -e 's/part \/ /part \/ --exclude-path usr /g' "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
> +
> +bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base

The series looks good to me. Please help me, why would this test case fail when
the other patches were not applied?

As a side note, pylint complains about the upstream's choice of except
Exception, because catching a specific one like OSError here and failing
completely on unexpected stuff would be better (especially if copying happens
to work, in which case space and performance would silently degrade). But that
is a completely different task, I'd also stick to the upstream code unless we
identify a specific problem.

With kind regards,
Baurzhan.

Re: [PATCH 3/3] CI: test the exclude-path feature of wic

[Henning Schild]

Am Thu, 7 May 2020 21:25:33 +0200
schrieb Baurzhan Ismagulov <ibr@radix50.net>:

> Hello Henning,
> 
> On Mon, Apr 06, 2020 at 10:16:48PM +0200, Henning Schild wrote:
> > diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh  
> ...
> > @@ -193,3 +193,10 @@ echo -e "do_fetch_append() {\n\n}" >>
> > "${ISARROOT}/meta/classes/dpkg-base.bbclas  
> ...
> > +cp -a
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup"
> > +sed -i -e 's/part \/ /part \/ --exclude-path usr /g'
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
> > + +bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base  
> 
> The series looks good to me. Please help me, why would this test case
> fail when the other patches were not applied?

The "--exclude-path" is the real diff. It is a wic feature that never
worked in isar because of bind-mounts that oe does not use. It is an
important feature when splitting a rootfs into multiple partitions.
Say you want a seperate home, you would add that and need to exclude
home from root. Same goes for other directories people like to have in
separate partitions, in fact boot is where i found it.

> As a side note, pylint complains about the upstream's choice of except
> Exception, because catching a specific one like OSError here and
> failing completely on unexpected stuff would be better (especially if
> copying happens to work, in which case space and performance would
> silently degrade). But that is a completely different task, I'd also
> stick to the upstream code unless we identify a specific problem.

Not sure whether my upstream patch introduces that. Upstream merged it
and it was here for review as well. If you see a need to fix that
upstream now, go for it.

Henning

> With kind regards,
> Baurzhan.
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Still having to work around that in a downstream layer. Any more
comments or questions?

Henning

Am Mon, 6 Apr 2020 22:16:45 +0200
schrieb Henning Schild <henning.schild@siemens.com>:

> From: Henning Schild <henning.schild@siemens.com>
> 
> This series is two cherry-picks from oe plus one test. Before those
> cherry-picks the test would fail, with them it works as epected.
> 
> Henning Schild (2):
>   lib/oe/path: try hardlinking instead of guessing when it might fail
>   CI: test the exclude-path feature of wic
> 
> Paul Barker (1):
>   oe.path: Add copyhardlink() helper function
> 
>  meta/lib/oe/path.py | 27 +++++++++++++++++++++++++--
>  scripts/ci_build.sh |  7 +++++++
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Baurzhan Ismagulov]

Hello Henning,

On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:
> Still having to work around that in a downstream layer. Any more
> comments or questions?

I'd like to look at the following issues:

1. I'd like to understand what exactly is failing when the fix is not applied.

2. The test case is failing with the fix applied.


Regarding (1), you responded:

> The "--exclude-path" is the real diff. It is a wic feature that never
> worked in isar because of bind-mounts that oe does not use. It is an
> important feature when splitting a rootfs into multiple partitions.
> Say you want a seperate home, you would add that and need to exclude
> home from root. Same goes for other directories people like to have in
> separate partitions, in fact boot is where i found it.

Thanks for the explanation. The concept is already clear from your patch
description. Do you have an example of a failing ln command and which
filesystems the source and the destination are on, for the test case you added?

Because I interpret --exclude-path usr as "don't copy usr", but
meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks contains only / and /boot.
If it complained about / and /boot, then I'd expect it to fail also without
--exclude-path usr.


With kind regards,
Baurzhan.

Re: [PATCH 0/3] fix and test wic exclude-path

[Jan Kiszka]

On 12.06.20 08:55, Baurzhan Ismagulov wrote:
> Hello Henning,
> 
> On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:
>> Still having to work around that in a downstream layer. Any more
>> comments or questions?
> 
> I'd like to look at the following issues:
> 
> 1. I'd like to understand what exactly is failing when the fix is not applied.
> 
> 2. The test case is failing with the fix applied.
> 

Before we dig too deep on details:

The pressure to update wic to a more recent version is increasing, just
received another request today, so is our backlog of own upstream
patches missing in Isar. I would proppose starting that update and then
focuson  testing and stabilization of the result.

Did anyone look into that already?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Am Fri, 12 Jun 2020 08:55:22 +0200
schrieb Baurzhan Ismagulov <ibr@radix50.net>:

> Hello Henning,
> 
> On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:
> > Still having to work around that in a downstream layer. Any more
> > comments or questions?  
> 
> I'd like to look at the following issues:
> 
> 1. I'd like to understand what exactly is failing when the fix is not
> applied.

You have the code for that. Just revert the fix and you will see.

> 2. The test case is failing with the fix applied.

Just apply the changes and you will see ;).

> 
> Regarding (1), you responded:
> 
> > The "--exclude-path" is the real diff. It is a wic feature that
> > never worked in isar because of bind-mounts that oe does not use.
> > It is an important feature when splitting a rootfs into multiple
> > partitions. Say you want a seperate home, you would add that and
> > need to exclude home from root. Same goes for other directories
> > people like to have in separate partitions, in fact boot is where i
> > found it.  
> 
> Thanks for the explanation. The concept is already clear from your
> patch description. Do you have an example of a failing ln command and
> which filesystems the source and the destination are on, for the test
> case you added?

Yes, use the exclude feature today.

> Because I interpret --exclude-path usr as "don't copy usr", but
> meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks contains only /
> and /boot. If it complained about / and /boot, then I'd expect it to
> fail also without --exclude-path usr.

With / it also contains /usr. You need the exclude if you want to put a
folder into another partition and not still have it in / as well.

For a real wks you would add another partition with /usr and exlude usr
from /. The test is more basic ... making sure the exclude will work in
isar.

Henning

> 
> With kind regards,
> Baurzhan.
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Am Fri, 12 Jun 2020 17:17:19 +0200
schrieb "[ext] Jan Kiszka" <jan.kiszka@siemens.com>:

> On 12.06.20 08:55, Baurzhan Ismagulov wrote:
> > Hello Henning,
> > 
> > On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:  
> >> Still having to work around that in a downstream layer. Any more
> >> comments or questions?  
> > 
> > I'd like to look at the following issues:
> > 
> > 1. I'd like to understand what exactly is failing when the fix is
> > not applied.
> > 
> > 2. The test case is failing with the fix applied.
> >   
> 
> Before we dig too deep on details:
> 
> The pressure to update wic to a more recent version is increasing,
> just received another request today, so is our backlog of own upstream
> patches missing in Isar. I would proppose starting that update and
> then focuson  testing and stabilization of the result.

Agreed, we should make sure to use a recent bitbake and wic from
upstream. But we have a horrible fork with backports right now ...

The history of this one is so that upstream would never have worked.
But now that upstream fixed it we could try.

> Did anyone look into that already?

Briefly ... did not work out of the box. The problem with wic is that
it potentially uses new tool args that we do not have in all distros
yet. But that can be worked around with some nasty magic and wrappers
... did that in the early stages before we decided to not support <
debian 9 with it.

Henning

> Jan
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Am Fri, 12 Jun 2020 20:11:48 +0200
schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:

> Am Fri, 12 Jun 2020 17:17:19 +0200
> schrieb "[ext] Jan Kiszka" <jan.kiszka@siemens.com>:
> 
> > On 12.06.20 08:55, Baurzhan Ismagulov wrote:  
> > > Hello Henning,
> > > 
> > > On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:
> > >  
> > >> Still having to work around that in a downstream layer. Any more
> > >> comments or questions?    
> > > 
> > > I'd like to look at the following issues:
> > > 
> > > 1. I'd like to understand what exactly is failing when the fix is
> > > not applied.
> > > 
> > > 2. The test case is failing with the fix applied.
> > >     
> > 
> > Before we dig too deep on details:
> > 
> > The pressure to update wic to a more recent version is increasing,
> > just received another request today, so is our backlog of own
> > upstream patches missing in Isar. I would proppose starting that
> > update and then focuson  testing and stabilization of the result.  
> 
> Agreed, we should make sure to use a recent bitbake and wic from
> upstream. But we have a horrible fork with backports right now ...

The clear goal should be submodules or something like it. To prevent
any isar specifics and enforce upstream work. Having copies allowed us
to get into this mess. (talking about bitbake and libs here) Those
other backports should never have been merged ... i just follow along
here ...

Henning

> The history of this one is so that upstream would never have worked.
> But now that upstream fixed it we could try.
> 
> > Did anyone look into that already?  
> 
> Briefly ... did not work out of the box. The problem with wic is that
> it potentially uses new tool args that we do not have in all distros
> yet. But that can be worked around with some nasty magic and wrappers
> ... did that in the early stages before we decided to not support <
> debian 9 with it.
> 
> Henning
> 
> > Jan
> >   
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Am Fri, 12 Jun 2020 17:17:19 +0200
schrieb "[ext] Jan Kiszka" <jan.kiszka@siemens.com>:

> On 12.06.20 08:55, Baurzhan Ismagulov wrote:
> > Hello Henning,
> > 
> > On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:  
> >> Still having to work around that in a downstream layer. Any more
> >> comments or questions?  
> > 
> > I'd like to look at the following issues:
> > 
> > 1. I'd like to understand what exactly is failing when the fix is
> > not applied.
> > 
> > 2. The test case is failing with the fix applied.
> >   
> 
> Before we dig too deep on details:
> 
> The pressure to update wic to a more recent version is increasing,
> just received another request today,

Which feature did the request care about? If it is the exclude, we
might me able to stall the inevitable ...

Henning

> so is our backlog of own upstream
> patches missing in Isar. I would proppose starting that update and
> then focuson  testing and stabilization of the result.
> 
> Did anyone look into that already?
> 
> Jan
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Jan Kiszka]

On 12.06.20 20:17, Henning Schild wrote:
> Am Fri, 12 Jun 2020 20:11:48 +0200
> schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:
> 
>> Am Fri, 12 Jun 2020 17:17:19 +0200
>> schrieb "[ext] Jan Kiszka" <jan.kiszka@siemens.com>:
>>
>>> On 12.06.20 08:55, Baurzhan Ismagulov wrote:  
>>>> Hello Henning,
>>>>
>>>> On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:
>>>>  
>>>>> Still having to work around that in a downstream layer. Any more
>>>>> comments or questions?    
>>>>
>>>> I'd like to look at the following issues:
>>>>
>>>> 1. I'd like to understand what exactly is failing when the fix is
>>>> not applied.
>>>>
>>>> 2. The test case is failing with the fix applied.
>>>>     
>>>
>>> Before we dig too deep on details:
>>>
>>> The pressure to update wic to a more recent version is increasing,
>>> just received another request today, so is our backlog of own
>>> upstream patches missing in Isar. I would proppose starting that
>>> update and then focuson  testing and stabilization of the result.  
>>
>> Agreed, we should make sure to use a recent bitbake and wic from
>> upstream. But we have a horrible fork with backports right now ...
> 
> The clear goal should be submodules or something like it. To prevent
> any isar specifics and enforce upstream work. Having copies allowed us
> to get into this mess. (talking about bitbake and libs here) Those
> other backports should never have been merged ... i just follow along
> here ...

We will likely have to carry patches on top of those submodules then. I
don't believe (yet) that this can be cleaner. But such things are best
understood by trying it, at least a bit until it explodes.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] fix and test wic exclude-path

[Jan Kiszka]

On 12.06.20 20:22, Henning Schild wrote:
> Am Fri, 12 Jun 2020 17:17:19 +0200
> schrieb "[ext] Jan Kiszka" <jan.kiszka@siemens.com>:
> 
>> On 12.06.20 08:55, Baurzhan Ismagulov wrote:
>>> Hello Henning,
>>>
>>> On Wed, Jun 10, 2020 at 05:26:27PM +0200, Henning Schild wrote:  
>>>> Still having to work around that in a downstream layer. Any more
>>>> comments or questions?  
>>>
>>> I'd like to look at the following issues:
>>>
>>> 1. I'd like to understand what exactly is failing when the fix is
>>> not applied.
>>>
>>> 2. The test case is failing with the fix applied.
>>>   
>>
>> Before we dig too deep on details:
>>
>> The pressure to update wic to a more recent version is increasing,
>> just received another request today,
> 
> Which feature did the request care about? If it is the exclude, we
> might me able to stall the inevitable ...

--use-uuid for non-rootfs partitions.

In addition, not only /me would like to get rid of
https://github.com/siemens/meta-iot2050/blob/jan/upstream-u-boot/isar-patches/0001-wic-Ensure-that-sourceparams-values-can-contain.patch
in downstream.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] fix and test wic exclude-path

[Baurzhan Ismagulov]

Hello Henning,

On Fri, Jun 12, 2020 at 08:03:35PM +0200, Henning Schild wrote:
> > 1. I'd like to understand what exactly is failing when the fix is not
> > applied.
> 
> You have the code for that. Just revert the fix and you will see.

Fair enough -- just thought it would speed up things.


> > 2. The test case is failing with the fix applied.
> 
> Just apply the changes and you will see ;).

I meant that your patches started to fail in CI, see e.g.
http://ci.isar-build.org:8080/job/isar_ibr_devel_2_fast/12/consoleFull. The
build succeeds, but start_vm fails due to init dying.

The problem seems to be that Debian doesn't support operating without /usr
anymore. We can fix it by saving a backup of the image before generating one
without /usr -- I'll send the updated patch.


I'd be ready to apply this with the modification above. I see that the wic
series also touches this part but doesn't include the CI patch. What would be
the preferred way?

1. Apply this, rework the wic series, OR

2. Include the CI patch in the wic series and apply that.


With kind regards,
Baurzhan.

[PATCH v2 3/3] CI: test the exclude-path feature of wic

[Baurzhan Ismagulov]

From: Henning Schild <henning.schild@siemens.com>

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 scripts/ci_build.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
index d2c707b..af996d1 100755
--- a/scripts/ci_build.sh
+++ b/scripts/ci_build.sh
@@ -212,3 +212,15 @@ echo -e "do_fetch_append() {\n\n}" >> "${ISARROOT}/meta/classes/dpkg-base.bbclas
 bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base
 
 mv "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" "${ISARROOT}/meta/classes/dpkg-base.bbclass"
+
+# Test wic --exclude-path
+cp -a "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup"
+mv ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img \
+    ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup
+sed -i -e 's/part \/ /part \/ --exclude-path usr /g' "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
+
+bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base
+
+mv "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
+mv ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup \
+    ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img
-- 
2.20.1

Re: [PATCH 0/3] fix and test wic exclude-path

[vijaikumar....@gmail.com]

[-- Attachment #1.1: Type: text/plain, Size: 1888 bytes --]

Hi Baurzhan,

On Wednesday, September 2, 2020 at 1:23:23 AM UTC+5:30 i...@radix50.net 
wrote:

> Hello Henning, 
>
> On Fri, Jun 12, 2020 at 08:03:35PM +0200, Henning Schild wrote: 
> > > 1. I'd like to understand what exactly is failing when the fix is not 
> > > applied. 
> > 
> > You have the code for that. Just revert the fix and you will see. 
>
> Fair enough -- just thought it would speed up things. 
>
>
> > > 2. The test case is failing with the fix applied. 
> > 
> > Just apply the changes and you will see ;). 
>
> I meant that your patches started to fail in CI, see e.g. 
> http://ci.isar-build.org:8080/job/isar_ibr_devel_2_fast/12/consoleFull. 
> The 
> build succeeds, but start_vm fails due to init dying. 
>
> The problem seems to be that Debian doesn't support operating without /usr 
> anymore. We can fix it by saving a backup of the image before generating 
> one 
> without /usr -- I'll send the updated patch. 
>

My suggestion would be to exclude one of /srv /media instead of /usr. Since 
those
might not be critical for the system to boot.
 

>
>
> I'd be ready to apply this with the modification above. I see that the wic 
> series also touches this part but doesn't include the CI patch. What would 
> be 
> the preferred way? 
>
> 1. Apply this, rework the wic series, OR 
>

We could apply this and I can rework the wic series based on this.
 

>
> 2. Include the CI patch in the wic series and apply that. 
>

Right now we are testing only --exclude-path which this series adds support 
for.
For a comprehensive wic test, I was thinking to have a custom wks file 
which would
have most of the options exercised and  we could use some tools to validate 
whether the
final image is as expected. It is something in my pipeline. Will try to 
work on it and send a patch
as early as possible.

Thanks,
Vijai Kumar K


>
> With kind regards, 
> Baurzhan. 
>

[-- Attachment #1.2: Type: text/html, Size: 3118 bytes --]

Re: [PATCH v2 3/3] CI: test the exclude-path feature of wic

[vijaikumar....@gmail.com]

[-- Attachment #1.1: Type: text/plain, Size: 2101 bytes --]



On Wednesday, September 2, 2020 at 1:24:13 AM UTC+5:30 i...@radix50.net 
wrote:

> From: Henning Schild <henning...@siemens.com> 
>
> Signed-off-by: Henning Schild <henning...@siemens.com> 
> --- 
> scripts/ci_build.sh | 12 ++++++++++++ 
> 1 file changed, 12 insertions(+) 
>
> diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh 
> index d2c707b..af996d1 100755 
> --- a/scripts/ci_build.sh 
> +++ b/scripts/ci_build.sh 
> @@ -212,3 +212,15 @@ echo -e "do_fetch_append() {\n\n}" >> 
> "${ISARROOT}/meta/classes/dpkg-base.bbclas 
> bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base 
>
> mv "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" 
> "${ISARROOT}/meta/classes/dpkg-base.bbclass" 
> + 
> +# Test wic --exclude-path 
> +cp -a "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" 
> "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" 
>
> +mv 
> ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img 
> \ 
> + 
> ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup 
>
> +sed -i -e 's/part \/ /part \/ --exclude-path usr /g' 
> "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" 
> + 
> +bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base 
> + 
> +mv 
> "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" 
> "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" 
> +mv 
> ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup 
> \ 
> + 
> ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img 
>
>

The only problem I see is the CI actually doesnot validate the image to see 
if /usr is excluded. It validates the code-path for errors but
doesnot validate the feature itself. We could mount and check though.
Once the new wic is integrated this should be easy with the "wic ls" 
feature. We could list the contents of the partition using
"wic ls" and validate if /usr is excluded.

Thanks,
Vijai Kumar K

-- 
> 2.20.1 
>
>

[-- Attachment #1.2: Type: text/html, Size: 2679 bytes --]

Re: [PATCH 0/3] fix and test wic exclude-path

[Henning Schild]

Am Tue, 1 Sep 2020 21:53:14 +0200
schrieb Baurzhan Ismagulov <ibr@radix50.net>:

> Hello Henning,
> 
> On Fri, Jun 12, 2020 at 08:03:35PM +0200, Henning Schild wrote:
> > > 1. I'd like to understand what exactly is failing when the fix is
> > > not applied.  
> > 
> > You have the code for that. Just revert the fix and you will see.  
> 
> Fair enough -- just thought it would speed up things.

Using the exclude will cause our wic to raise an exception

> 
> > > 2. The test case is failing with the fix applied.  
> > 
> > Just apply the changes and you will see ;).  
> 
> I meant that your patches started to fail in CI, see e.g.
> http://ci.isar-build.org:8080/job/isar_ibr_devel_2_fast/12/consoleFull.
> The build succeeds, but start_vm fails due to init dying.
> 
> The problem seems to be that Debian doesn't support operating without
> /usr anymore. We can fix it by saving a backup of the image before
> generating one without /usr -- I'll send the updated patch.

Oh i see. The idea was to just test wic and whether it would still crap
out with its exception. There was no focus on whether that image was in
fact bootable. And i am not surprised it is not ...
 
> I'd be ready to apply this with the modification above. I see that
> the wic series also touches this part but doesn't include the CI
> patch. What would be the preferred way?
> 
> 1. Apply this, rework the wic series, OR
> 
> 2. Include the CI patch in the wic series and apply that.

3. apply the wic series and do not even care about the test

The test was just there to motivate the backport, if we bump that would
be much better.

Henning 
> 
> With kind regards,
> Baurzhan.
> 

Re: [PATCH v2 3/3] CI: test the exclude-path feature of wic

[Henning Schild]

Baurzhan,

thanks for the proposed fix. Looks like you got the idea that this one
was supposed to only test wic, not "bootable". If you think that still
makes sense we can keep it, we could also go to "wic ls".

We could also add a wks file that excludes /var and turns it into a
partition on its own. That would test the exclude, be bootable, and be
a good example. I would actually opt for that or for dropping the
testcase alltogether.

Henning

Am Tue, 1 Sep 2020 22:30:30 -0700 (PDT)
schrieb "vijaikumar....@gmail.com" <vijaikumar.kanagarajan@gmail.com>:

> On Wednesday, September 2, 2020 at 1:24:13 AM UTC+5:30
> i...@radix50.net wrote:
> 
> > From: Henning Schild <henning...@siemens.com> 
> >
> > Signed-off-by: Henning Schild <henning...@siemens.com> 
> > --- 
> > scripts/ci_build.sh | 12 ++++++++++++ 
> > 1 file changed, 12 insertions(+) 
> >
> > diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh 
> > index d2c707b..af996d1 100755 
> > --- a/scripts/ci_build.sh 
> > +++ b/scripts/ci_build.sh 
> > @@ -212,3 +212,15 @@ echo -e "do_fetch_append() {\n\n}" >> 
> > "${ISARROOT}/meta/classes/dpkg-base.bbclas 
> > bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base 
> >
> > mv "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" 
> > "${ISARROOT}/meta/classes/dpkg-base.bbclass" 
> > + 
> > +# Test wic --exclude-path 
> > +cp -a
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks"
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" 
> >
> > +mv 
> > ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img 
> > \ 
> > + 
> > ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup 
> >
> > +sed -i -e 's/part \/ /part \/ --exclude-path usr /g' 
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" 
> > + 
> > +bitbake $BB_ARGS mc:qemuamd64-stretch:isar-image-base 
> > + 
> > +mv 
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks.ci-backup" 
> > "${ISARROOT}/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks" 
> > +mv 
> > ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img.ci-backup 
> > \ 
> > + 
> > ${BUILDDIR}/tmp/deploy/images/qemuamd64/isar-image-base-debian-stretch-qemuamd64.wic.img 
> >
> >  
> 
> The only problem I see is the CI actually doesnot validate the image
> to see if /usr is excluded. It validates the code-path for errors but
> doesnot validate the feature itself. We could mount and check though.
> Once the new wic is integrated this should be easy with the "wic ls" 
> feature. We could list the contents of the partition using
> "wic ls" and validate if /usr is excluded.
> 
> Thanks,
> Vijai Kumar K
> 

Re: [PATCH 0/3] fix and test wic exclude-path

[Baurzhan Ismagulov]

On Mon, Apr 06, 2020 at 10:16:45PM +0200, Henning Schild wrote:
> This series is two cherry-picks from oe plus one test. Before those
> cherry-picks the test would fail, with them it works as epected.

Thanks Henning and Vijai Kumar for your inputs. As there are no major
objections, I'd prefer to keep the test case as the first step of the future
work. Applied to next.

With kind regards,
Baurzhan.