Patchwork [v2] wic: do not overwrite autogenerated /etc/fstab with original too early

login
register
mail settings
Submitter Maciej Borzecki
Date July 24, 2014, 12:27 p.m.
Message ID <1406204836-18090-1-git-send-email-maciej.borzecki@open-rnd.pl>
Download mbox | patch
Permalink /patch/76587/
State Accepted
Commit 09def7a78c8b7488f0b6a4578f4bfa096fc3b0f1
Headers show

Comments

Maciej Borzecki - July 24, 2014, 12:27 p.m.
DirectImageCreator.__write_fstab() generates new /etc/fstab in sysroot
with rootfs contents. The fstab entries are generated base on the
initialn contents of /etc/fstab, plus any extra (other than / or
/boot) partitions listed in *.wks. A backup of original /etc/fstab is
done in a temp location. Subsequent call to __restore_fstab() restores
the backup copy, replacing the autogenerated one.

Calling __restore_fstab() before Wic_PartData.prepare() brings back the
original fstab before the partition image file actually is created. As
such, the autogenerated /etc/fstab will not make it to the partition.

Signed-off-by: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
Signed-off-by: Maciek Borzecki <maciek.borzecki@gmail.com>
---

Notes:
    Previous version got messed up during merges and faulty code was sent
    out. This one works as expected.

 scripts/lib/mic/imager/direct.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Tom Zanussi - July 25, 2014, 12:49 a.m.
On Thu, 2014-07-24 at 14:27 +0200, Maciej Borzecki wrote:
> DirectImageCreator.__write_fstab() generates new /etc/fstab in sysroot
> with rootfs contents. The fstab entries are generated base on the
> initialn contents of /etc/fstab, plus any extra (other than / or
> /boot) partitions listed in *.wks. A backup of original /etc/fstab is
> done in a temp location. Subsequent call to __restore_fstab() restores
> the backup copy, replacing the autogenerated one.
> 
> Calling __restore_fstab() before Wic_PartData.prepare() brings back the
> original fstab before the partition image file actually is created. As
> such, the autogenerated /etc/fstab will not make it to the partition.
> 

OK, I knew there was something funny about this, and it wasn't really
fixing the problem.  I also knew that it had previously worked, and
digging around realized that the problem was that the recent patch 'wic:
Extend --rootfs-dir to connect rootfs-dirs' is what actually broke
things.

So this patch shouldn't be applied - I need to look at it a bit more and
come up with a proper fix..

Tom

> Signed-off-by: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
> Signed-off-by: Maciek Borzecki <maciek.borzecki@gmail.com>
> ---
> 
> Notes:
>     Previous version got messed up during merges and faulty code was sent
>     out. This one works as expected.
> 
>  scripts/lib/mic/imager/direct.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/mic/imager/direct.py b/scripts/lib/mic/imager/direct.py
> index 2cf4c8d..77a118a 100644
> --- a/scripts/lib/mic/imager/direct.py
> +++ b/scripts/lib/mic/imager/direct.py
> @@ -262,10 +262,12 @@ class DirectImageCreator(BaseImageCreator):
>              # when/if we need to actually do package selection we
>              # should modify things to use those objects, but for now
>              # we can avoid that.
> +
> +            fstab = self.__write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> +
>              p.prepare(self, self.workdir, self.oe_builddir, self.rootfs_dir,
>                        self.bootimg_dir, self.kernel_dir, self.native_sysroot)
>  
> -            fstab = self.__write_fstab(p.get_rootfs())
>              self._restore_fstab(fstab)
>  
>              self.__instimage.add_partition(int(p.size),
> @@ -278,6 +280,7 @@ class DirectImageCreator(BaseImageCreator):
>                                             boot = p.active,
>                                             align = p.align,
>                                             part_type = p.part_type)
> +
>          self.__instimage.layout_partitions(self._ptable_format)
>  
>          self.__imgdir = self.workdir
Tom Zanussi - July 25, 2014, 3 p.m.
On Thu, 2014-07-24 at 19:49 -0500, Tom Zanussi wrote:
> On Thu, 2014-07-24 at 14:27 +0200, Maciej Borzecki wrote:
> > DirectImageCreator.__write_fstab() generates new /etc/fstab in sysroot
> > with rootfs contents. The fstab entries are generated base on the
> > initialn contents of /etc/fstab, plus any extra (other than / or
> > /boot) partitions listed in *.wks. A backup of original /etc/fstab is
> > done in a temp location. Subsequent call to __restore_fstab() restores
> > the backup copy, replacing the autogenerated one.
> > 
> > Calling __restore_fstab() before Wic_PartData.prepare() brings back the
> > original fstab before the partition image file actually is created. As
> > such, the autogenerated /etc/fstab will not make it to the partition.
> > 
> 
> OK, I knew there was something funny about this, and it wasn't really
> fixing the problem.  I also knew that it had previously worked, and
> digging around realized that the problem was that the recent patch 'wic:
> Extend --rootfs-dir to connect rootfs-dirs' is what actually broke
> things.
> 
> So this patch shouldn't be applied - I need to look at it a bit more and
> come up with a proper fix..
> 

OK, I take that back - this patch is fine to apply.

Acked-by: Tom Zanussi <tom.zanussi@intel.com>

What confused me was that your first patch was clearly wrong, but you
moved the __write_fstab() to before .prepare in your second patch, which
is correct.

In the future, for v2 and beyond patches, please note what changed from
the previous version.

Thanks,

Tom


> Tom
> 
> > Signed-off-by: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
> > Signed-off-by: Maciek Borzecki <maciek.borzecki@gmail.com>
> > ---
> > 
> > Notes:
> >     Previous version got messed up during merges and faulty code was sent
> >     out. This one works as expected.
> > 
> >  scripts/lib/mic/imager/direct.py | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/lib/mic/imager/direct.py b/scripts/lib/mic/imager/direct.py
> > index 2cf4c8d..77a118a 100644
> > --- a/scripts/lib/mic/imager/direct.py
> > +++ b/scripts/lib/mic/imager/direct.py
> > @@ -262,10 +262,12 @@ class DirectImageCreator(BaseImageCreator):
> >              # when/if we need to actually do package selection we
> >              # should modify things to use those objects, but for now
> >              # we can avoid that.
> > +
> > +            fstab = self.__write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
> > +
> >              p.prepare(self, self.workdir, self.oe_builddir, self.rootfs_dir,
> >                        self.bootimg_dir, self.kernel_dir, self.native_sysroot)
> >  
> > -            fstab = self.__write_fstab(p.get_rootfs())
> >              self._restore_fstab(fstab)
> >  
> >              self.__instimage.add_partition(int(p.size),
> > @@ -278,6 +280,7 @@ class DirectImageCreator(BaseImageCreator):
> >                                             boot = p.active,
> >                                             align = p.align,
> >                                             part_type = p.part_type)
> > +
> >          self.__instimage.layout_partitions(self._ptable_format)
> >  
> >          self.__imgdir = self.workdir
>

Patch

diff --git a/scripts/lib/mic/imager/direct.py b/scripts/lib/mic/imager/direct.py
index 2cf4c8d..77a118a 100644
--- a/scripts/lib/mic/imager/direct.py
+++ b/scripts/lib/mic/imager/direct.py
@@ -262,10 +262,12 @@  class DirectImageCreator(BaseImageCreator):
             # when/if we need to actually do package selection we
             # should modify things to use those objects, but for now
             # we can avoid that.
+
+            fstab = self.__write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
+
             p.prepare(self, self.workdir, self.oe_builddir, self.rootfs_dir,
                       self.bootimg_dir, self.kernel_dir, self.native_sysroot)
 
-            fstab = self.__write_fstab(p.get_rootfs())
             self._restore_fstab(fstab)
 
             self.__instimage.add_partition(int(p.size),
@@ -278,6 +280,7 @@  class DirectImageCreator(BaseImageCreator):
                                            boot = p.active,
                                            align = p.align,
                                            part_type = p.part_type)
+
         self.__instimage.layout_partitions(self._ptable_format)
 
         self.__imgdir = self.workdir