[RESEND for ML] wic: imager/direct: passno is set to 0 for all entries in the fstab

[RESEND for ML] wic: imager/direct: passno is set to 0 for all entries in the fstab

[Claudius Heine]

Hi,

I looked into periodic file system checks for ext* partitions, in order 
to increase robustness for systems that use those file systems and was 
surprised that the fstab that wic generates that `0` hard coded as the 
passno parameter [1]. This prevents for instance systemd-fsck@(8) [2] 
from running checks on those file systems.

It seems that wic/mic generated a fstab with passno=0 from the beginning 
[3]. What is the reason for that? Was it a conscious decision to do 
that, or just copy&paste for a different project?

How would a way forward look like? Should a patch that changes passno 
from 0 to 2 be enough, or do we rather have this parameter configurable 
somehow?

regards,
Claudius Heine

[1] 
https://git.openembedded.org/openembedded-core/tree/scripts/lib/wic/plugins/imager/direct.py?id=a59c246be05ca284fa27df87b5db09a70f527cc2#n141
[2] 
https://manpages.debian.org/stretch/systemd/systemd-fsck@.service.8.en.html
[3] 
https://git.openembedded.org/openembedded-core/tree/scripts/lib/mic/imager/direct.py?id=f87acc5e59d3c2c39ff171b5557977dab4c8f4a6#n110

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de

            PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
                              Keyserver: hkp://pool.sks-keyservers.net

Re: [RESEND for ML] wic: imager/direct: passno is set to 0 for all entries in the fstab

[Richard Purdie]

On Fri, 2020-11-06 at 10:07 +0100, Claudius Heine wrote:
> I looked into periodic file system checks for ext* partitions, in
> order to increase robustness for systems that use those file systems
> and was  surprised that the fstab that wic generates that `0` hard
> coded as the  passno parameter [1]. This prevents for instance
> systemd-fsck@(8) [2]  from running checks on those file systems.
> 
> It seems that wic/mic generated a fstab with passno=0 from the
> beginning 
> [3]. What is the reason for that? Was it a conscious decision to do 
> that, or just copy&paste for a different project?
> 
> How would a way forward look like? Should a patch that changes passno
> from 0 to 2 be enough, or do we rather have this parameter
> configurable somehow?

I suspect changing things so fsck was run at every boot would upset
existing use cases, particularly on ext2/3 systems without a journal.
The only option would therefore me to make it configurable.

Cheers,

Richard

[PATCH] wic: added fspassno parameter to partition

[Claudius Heine]

The `fspassno` parameter allows to overwrite the value of the last
column (`fs_passno`) in the /etc/fstab of the target root file system.
This allows to have periodic file system checks.

Signed-off-by: Claudius Heine <ch@denx.de>
---
 scripts/lib/wic/help.py                  | 6 ++++++
 scripts/lib/wic/ksparser.py              | 1 +
 scripts/lib/wic/partition.py             | 1 +
 scripts/lib/wic/plugins/imager/direct.py | 3 ++-
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index bd3a2b97df..dd0a02fc12 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -939,6 +939,12 @@ DESCRIPTION
                       quotes.  If not specified, the default string is
                       "defaults".
 
+         --fspassno: Specifies the order in which filesystem checks are done
+                     at boot time by fsck.  See fs_passno parameter of
+                     fstab(5).  This parameter will be copied into the
+                     /etc/fstab file of the installed system.  If not
+                     specified the default value of "0" will be used.
+
          --label label: Specifies the label to give to the filesystem
                         to be made on the partition. If the given
                         label is already in use by another filesystem,
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 913e3283dc..b1ade01ed4 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -155,6 +155,7 @@ class KickStart():
         part.add_argument('--change-directory')
         part.add_argument("--extra-space", type=sizetype("M"))
         part.add_argument('--fsoptions', dest='fsopts')
+        part.add_argument('--fspassno', dest='fspassno')
         part.add_argument('--fstype', default='vfat',
                           choices=('ext2', 'ext3', 'ext4', 'btrfs',
                                    'squashfs', 'vfat', 'msdos', 'swap'))
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index ebe250b00d..44b17b5bd7 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -33,6 +33,7 @@ class Partition():
         self.include_path = args.include_path
         self.change_directory = args.change_directory
         self.fsopts = args.fsopts
+        self.fspassno = args.fspassno
         self.fstype = args.fstype
         self.label = args.label
         self.use_label = args.use_label
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 55db826e93..c2225ba067 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -138,8 +138,9 @@ class DirectPlugin(ImagerPlugin):
                 device_name = "/dev/%s%s%d" % (part.disk, prefix, part.realnum)
 
             opts = part.fsopts if part.fsopts else "defaults"
+            passno = part.fspassno if part.fspassno else "0"
             line = "\t".join([device_name, part.mountpoint, part.fstype,
-                              opts, "0", "0"]) + "\n"
+                              opts, "0", passno]) + "\n"
 
             fstab_lines.append(line)
             updated = True
-- 
2.20.1

Re: [PATCH] wic: added fspassno parameter to partition

[Henning Schild]

Was this ever looked at? ping

regards,
Henning

Am Fri, 6 Nov 2020 11:27:22 +0100
schrieb Claudius Heine <ch@denx.de>:

> The `fspassno` parameter allows to overwrite the value of the last
> column (`fs_passno`) in the /etc/fstab of the target root file system.
> This allows to have periodic file system checks.
> 
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
>  scripts/lib/wic/help.py                  | 6 ++++++
>  scripts/lib/wic/ksparser.py              | 1 +
>  scripts/lib/wic/partition.py             | 1 +
>  scripts/lib/wic/plugins/imager/direct.py | 3 ++-
>  4 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> index bd3a2b97df..dd0a02fc12 100644
> --- a/scripts/lib/wic/help.py
> +++ b/scripts/lib/wic/help.py
> @@ -939,6 +939,12 @@ DESCRIPTION
>                        quotes.  If not specified, the default string
> is "defaults".
>  
> +         --fspassno: Specifies the order in which filesystem checks
> are done
> +                     at boot time by fsck.  See fs_passno parameter
> of
> +                     fstab(5).  This parameter will be copied into
> the
> +                     /etc/fstab file of the installed system.  If not
> +                     specified the default value of "0" will be used.
> +
>           --label label: Specifies the label to give to the filesystem
>                          to be made on the partition. If the given
>                          label is already in use by another
> filesystem, diff --git a/scripts/lib/wic/ksparser.py
> b/scripts/lib/wic/ksparser.py index 913e3283dc..b1ade01ed4 100644
> --- a/scripts/lib/wic/ksparser.py
> +++ b/scripts/lib/wic/ksparser.py
> @@ -155,6 +155,7 @@ class KickStart():
>          part.add_argument('--change-directory')
>          part.add_argument("--extra-space", type=sizetype("M"))
>          part.add_argument('--fsoptions', dest='fsopts')
> +        part.add_argument('--fspassno', dest='fspassno')
>          part.add_argument('--fstype', default='vfat',
>                            choices=('ext2', 'ext3', 'ext4', 'btrfs',
>                                     'squashfs', 'vfat', 'msdos',
> 'swap')) diff --git a/scripts/lib/wic/partition.py
> b/scripts/lib/wic/partition.py index ebe250b00d..44b17b5bd7 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -33,6 +33,7 @@ class Partition():
>          self.include_path = args.include_path
>          self.change_directory = args.change_directory
>          self.fsopts = args.fsopts
> +        self.fspassno = args.fspassno
>          self.fstype = args.fstype
>          self.label = args.label
>          self.use_label = args.use_label
> diff --git a/scripts/lib/wic/plugins/imager/direct.py
> b/scripts/lib/wic/plugins/imager/direct.py index
> 55db826e93..c2225ba067 100644 ---
> a/scripts/lib/wic/plugins/imager/direct.py +++
> b/scripts/lib/wic/plugins/imager/direct.py @@ -138,8 +138,9 @@ class
> DirectPlugin(ImagerPlugin): device_name = "/dev/%s%s%d" % (part.disk,
> prefix, part.realnum) 
>              opts = part.fsopts if part.fsopts else "defaults"
> +            passno = part.fspassno if part.fspassno else "0"
>              line = "\t".join([device_name, part.mountpoint,
> part.fstype,
> -                              opts, "0", "0"]) + "\n"
> +                              opts, "0", passno]) + "\n"
>  
>              fstab_lines.append(line)
>              updated = True

[PATCH v2] wic: added fspassno parameter to partition

[Vijai Kumar K]

From: Claudius Heine <ch@denx.de>

The `fspassno` parameter allows to overwrite the value of the last
column (`fs_passno`) in the /etc/fstab of the target root file system.
This allows to have periodic file system checks.

Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 scripts/lib/wic/help.py                  | 6 ++++++
 scripts/lib/wic/ksparser.py              | 1 +
 scripts/lib/wic/partition.py             | 1 +
 scripts/lib/wic/plugins/imager/direct.py | 3 ++-
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 4ff7470a6a..73e3380cde 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -940,6 +940,12 @@ DESCRIPTION
                       quotes.  If not specified, the default string is
                       "defaults".
 
+         --fspassno: Specifies the order in which filesystem checks are done
+                     at boot time by fsck.  See fs_passno parameter of
+                     fstab(5).  This parameter will be copied into the
+                     /etc/fstab file of the installed system.  If not
+                     specified the default value of "0" will be used.
+
          --label label: Specifies the label to give to the filesystem
                         to be made on the partition. If the given
                         label is already in use by another filesystem,
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 0df9eb0d05..a49b7b97c4 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -155,6 +155,7 @@ class KickStart():
         part.add_argument('--change-directory')
         part.add_argument("--extra-space", type=sizetype("M"))
         part.add_argument('--fsoptions', dest='fsopts')
+        part.add_argument('--fspassno', dest='fspassno')
         part.add_argument('--fstype', default='vfat',
                           choices=('ext2', 'ext3', 'ext4', 'btrfs',
                                    'squashfs', 'vfat', 'msdos', 'erofs',
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 09e491dd49..e50871b8d7 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -33,6 +33,7 @@ class Partition():
         self.include_path = args.include_path
         self.change_directory = args.change_directory
         self.fsopts = args.fsopts
+        self.fspassno = args.fspassno
         self.fstype = args.fstype
         self.label = args.label
         self.use_label = args.use_label
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 4d0b836ef6..da483daed5 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -138,8 +138,9 @@ class DirectPlugin(ImagerPlugin):
                 device_name = "/dev/%s%s%d" % (part.disk, prefix, part.realnum)
 
             opts = part.fsopts if part.fsopts else "defaults"
+            passno = part.fspassno if part.fspassno else "0"
             line = "\t".join([device_name, part.mountpoint, part.fstype,
-                              opts, "0", "0"]) + "\n"
+                              opts, "0", passno]) + "\n"
 
             fstab_lines.append(line)
             updated = True
-- 
2.25.1

Re: [PATCH] wic: added fspassno parameter to partition

[Kanagarajan, Vijaikumar]

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

Just sent a V2. Rebased on laster master.

https://lists.openembedded.org/g/openembedded-core/message/165597

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

Re: [PATCH v2] wic: added fspassno parameter to partition

[Schmidl, Tobias]

Hi all,

On Sun, May 15, 2022 at 08:36 AM, Kanagarajan, Vijaikumar wrote:
>
> From: Claudius Heine <ch@denx.de>
> 
> The `fspassno` parameter allows to overwrite the value of the last
> column (`fs_passno`) in the /etc/fstab of the target root file system.
> This allows to have periodic file system checks.
> 

I've found this to be ineffective for `/`, as the fstab update is
explicitly omitted in `scripts/lib/wic/plugins/imager/direct.py:120`:

```
for part in self.parts:
    if not part.realnum or not part.mountpoint \
       or part.mountpoint == "/" or not part.mountpoint.startswith('/'):
       ^^^^^^^^^^^^^^^^^^^^^^^^^
        continue
```

I've created a patch, originally for different reasons, that will also
address this issue.

Kind regards,

Tobias

RE: [PATCH v2] wic: added fspassno parameter to partition

[Kanagarajan, Vijaikumar]

> -----Original Message-----
> From: Schmidl, Tobias (T CED SES-DE) <tobiasschmidl@siemens.com>
> Sent: 17 May 2022 23:46
> To: openembedded-core@lists.openembedded.org; Kanagarajan,
> Vijaikumar <Vijaikumar_Kanagarajan@mentor.com>
> Cc: ch@denx.de; Schild, Henning (T CED SES-DE)
> <henning.schild@siemens.com>
> Subject: Re: [PATCH v2] wic: added fspassno parameter to partition
> 
Hi Tobias,

> Hi all,
> 
> On Sun, May 15, 2022 at 08:36 AM, Kanagarajan, Vijaikumar wrote:
> >
> > From: Claudius Heine <ch@denx.de>
> >
> > The `fspassno` parameter allows to overwrite the value of the last
> > column (`fs_passno`) in the /etc/fstab of the target root file system.
> > This allows to have periodic file system checks.
> >
> 
> I've found this to be ineffective for `/`, as the fstab update is explicitly
> omitted in `scripts/lib/wic/plugins/imager/direct.py:120`:

I believe wic has changed much since the first appearance of this patch.

> 
> ```
> for part in self.parts:
>     if not part.realnum or not part.mountpoint \
>        or part.mountpoint == "/" or not part.mountpoint.startswith('/'):
>        ^^^^^^^^^^^^^^^^^^^^^^^^^
>         continue
> ```
> 
> I've created a patch, originally for different reasons, that will also address this
> issue.

If you can send that, we can drop this patch in favour of it.

Thanks,
Vijai Kumar K

> 
> Kind regards,
> 
> Tobias

Re: [PATCH v2] wic: added fspassno parameter to partition

[Henning Schild]

Am Tue, 17 May 2022 18:21:27 +0000
schrieb "Kanagarajan, Vijaikumar" <Vijaikumar_Kanagarajan@mentor.com>:

> > -----Original Message-----
> > From: Schmidl, Tobias (T CED SES-DE) <tobiasschmidl@siemens.com>
> > Sent: 17 May 2022 23:46
> > To: openembedded-core@lists.openembedded.org; Kanagarajan,
> > Vijaikumar <Vijaikumar_Kanagarajan@mentor.com>
> > Cc: ch@denx.de; Schild, Henning (T CED SES-DE)
> > <henning.schild@siemens.com>
> > Subject: Re: [PATCH v2] wic: added fspassno parameter to partition
> >   
> Hi Tobias,
> 
> > Hi all,
> > 
> > On Sun, May 15, 2022 at 08:36 AM, Kanagarajan, Vijaikumar wrote:  
> > >
> > > From: Claudius Heine <ch@denx.de>
> > >
> > > The `fspassno` parameter allows to overwrite the value of the last
> > > column (`fs_passno`) in the /etc/fstab of the target root file
> > > system. This allows to have periodic file system checks.
> > >  
> > 
> > I've found this to be ineffective for `/`, as the fstab update is
> > explicitly omitted in
> > `scripts/lib/wic/plugins/imager/direct.py:120`:  
> 
> I believe wic has changed much since the first appearance of this
> patch.

Yes, some bits have changed but i also do not recall all the details.

> > 
> > ```
> > for part in self.parts:
> >     if not part.realnum or not part.mountpoint \
> >        or part.mountpoint == "/" or not
> > part.mountpoint.startswith('/'): ^^^^^^^^^^^^^^^^^^^^^^^^^
> >         continue
> > ```
> > 
> > I've created a patch, originally for different reasons, that will
> > also address this issue.  
> 
> If you can send that, we can drop this patch in favour of it.

AFAIK Tobias will be aiming at allowing wic to create fstab lines for
/, so it will not replace but instead boil down to merge order.

I can only assume that / might for some people for sure be a prime
target for --fspassno.

regards,
Henning

> Thanks,
> Vijai Kumar K
> 
> > 
> > Kind regards,
> > 
> > Tobias  
>