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

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

[Maciek Borzecki]

25 lip 2014 18:30 Richard Purdie <richard.purdie@linuxfoundation.org> napisał(a):
>
> On Fri, 2014-07-25 at 10:57 -0500, Tom Zanussi wrote: 
> > On Fri, 2014-07-25 at 17:48 +0200, Maciek Borzecki wrote: 
> > > One question though. How do I get the patches into master? I'm not 
> > > @intel.com perhaps that's why I was a bit confused by the wiki 
> > > instructions. Would you be able to push all of these changes into your 
> > > tree, so that they would get to master with later merge? 
> > > 
> > 
> > They get pulled into master by Richard and Saul, no need to do anything 
> > else on your side (except ping them if you don't see them get merged in 
> > a reasonable timeframe ;-) 
>
> They're in FWIW.

Great. Thanks.

Maciek

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

[Richard Purdie]

On Fri, 2014-07-25 at 10:57 -0500, Tom Zanussi wrote:
> On Fri, 2014-07-25 at 17:48 +0200, Maciek Borzecki wrote:
> > One question though. How do I get the patches into master? I'm not
> > @intel.com perhaps that's why I was a bit confused by the wiki
> > instructions. Would you be able to push all of these changes into your
> > tree, so that they would get to master with later merge?
> > 
> 
> They get pulled into master by Richard and Saul, no need to do anything
> else on your side (except ping them if you don't see them get merged in
> a reasonable timeframe ;-)

They're in FWIW.

Cheers,

Richard

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

[Tom Zanussi]

On Fri, 2014-07-25 at 17:48 +0200, Maciek Borzecki wrote:
> 25 lip 2014 17:00 Tom Zanussi <tom.zanussi@intel.com> napisał(a):
> >
> > 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.
> 
> Sure.
> 
> One question though. How do I get the patches into master? I'm not
> @intel.com perhaps that's why I was a bit confused by the wiki
> instructions. Would you be able to push all of these changes into your
> tree, so that they would get to master with later merge?
> 

They get pulled into master by Richard and Saul, no need to do anything
else on your side (except ping them if you don't see them get merged in
a reasonable timeframe ;-)

Tom


> Thanks,
> Maciek

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

[Maciek Borzecki]

25 lip 2014 17:00 Tom Zanussi <tom.zanussi@intel.com> napisał(a):
>
> 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.

Sure.

One question though. How do I get the patches into master? I'm not @intel.com perhaps that's why I was a bit confused by the wiki instructions. Would you be able to push all of these changes into your tree, so that they would get to master with later merge?

Thanks,
Maciek

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

[Tom Zanussi]

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
> 

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

[Tom Zanussi]

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

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

[Maciej Borzecki]

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(-)

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
-- 
1.9.0