diff mbox

[RFC] new target: live filesystem

Message ID 1354003953-10875-1-git-send-email-jeremy.rosen@openwide.fr
State Superseded
Headers show

Commit Message

Jeremy Rosen Nov. 27, 2012, 8:12 a.m. UTC
add a new target to deploy a live filesystem to be used by NFS

---

Hello everybody

this patch is not ready for commit, it's a proposal to help me solve the very common use case of
a developement FS deployed on NFS. I am not very happy of how it's done at this point but I would
like to discuss it to see if it has a chance to get merged... What I don't like (and would gladly
have ideas on how to solve this)
* no dependency on host-sudo, i'm not sure how to add that
* using sudo to break out of fakeroot is not clean
* I am abusing the fs building framework, but I don't know of a cleaner way
* filesystem can only be under output/images at this point

so this patch is more to start a discussion than anything, i'll gladly take any comments or plain
'ol rejection if the project doesn't want it... but if there is some interest i'll work it into
something mainline worthy

Regards

Jérémy Rosen
---
 fs/Config.in      |    1 +
 fs/live/Config.in |   16 ++++++++++++++++
 fs/live/live.mk   |   23 +++++++++++++++++++++++
 3 files changed, 40 insertions(+)
 create mode 100644 fs/live/Config.in
 create mode 100644 fs/live/live.mk

Comments

Arnout Vandecappelle Nov. 29, 2012, 11:43 p.m. UTC | #1
On 27/11/12 09:12, Jérémy Rosen wrote:
> add a new target to deploy a live filesystem to be used by NFS
>
> ---
>
> Hello everybody
>
> this patch is not ready for commit, it's a proposal to help me solve the very common use case of
> a developement FS deployed on NFS. I am not very happy of how it's done at this point but I would
> like to discuss it to see if it has a chance to get merged... What I don't like (and would gladly
> have ideas on how to solve this)
> * no dependency on host-sudo, i'm not sure how to add that

  That's impossible. The sudo executable must be setuid root, and to make
it setuid root you have to be root, so sudo must already exist...

  You could instead add a check in live.mk if sudo actually works.  But since
sudo may ask for a password this can get tricky - you should e.g. probably check
if stdin is a terminal as well.

  That said, I don't know if Peter will accept running sudo within buildroot.

> * using sudo to break out of fakeroot is not clean

  I don't see much of a problem there.

> * I am abusing the fs building framework, but I don't know of a cleaner way

  You're not abusing it.

> * filesystem can only be under output/images at this point

  That's not good but easy to solve.

>
> so this patch is more to start a discussion than anything, i'll gladly take any comments or plain
> 'ol rejection if the project doesn't want it... but if there is some interest i'll work it into
> something mainline worthy
>
> Regards
>
> Jérémy Rosen
> ---
>   fs/Config.in      |    1 +
>   fs/live/Config.in |   16 ++++++++++++++++
>   fs/live/live.mk   |   23 +++++++++++++++++++++++
>   3 files changed, 40 insertions(+)
>   create mode 100644 fs/live/Config.in
>   create mode 100644 fs/live/live.mk
>
> diff --git a/fs/Config.in b/fs/Config.in
> index 94154ea..de2377e 100644
> --- a/fs/Config.in
> +++ b/fs/Config.in
> @@ -11,5 +11,6 @@ source "fs/cpio/Config.in"
>   source "fs/iso9660/Config.in"
>   source "fs/initramfs/Config.in"
>   source "fs/romfs/Config.in"
> +source "fs/live/Config.in"

  Sort alphabetically please.

>
>   endmenu
> diff --git a/fs/live/Config.in b/fs/live/Config.in
> new file mode 100644
> index 0000000..bf24754
> --- /dev/null
> +++ b/fs/live/Config.in
> @@ -0,0 +1,16 @@
> +config BR2_TARGET_ROOTFS_LIVE
> +	bool "live root filesystem"
> +	help
> +	  uses sudo to create a live image of the filesystem
> +	  this is mainly usefull if you want to use your filesystem

  useful

> +	  over NFS

  or as a chroot

> +config BR2_TARGET_ROOTFS_LIVE_DEST
> +	string "directory to put the live image"

  "live image location"

> +	depends on BR2_TARGET_ROOTFS_LIVE
> +	default "live"

  Use an empty default, and give an error in live.mk if it's
empty.

> +	help
> +	  The directory where the image should be stored.
> +	  this directory will be emptied and recreated, it is given
> +	  relative to the buildroot output/images directory

  Make it an absolute path instead.

> +
> diff --git a/fs/live/live.mk b/fs/live/live.mk
> new file mode 100644
> index 0000000..73bcd3e
> --- /dev/null
> +++ b/fs/live/live.mk
> @@ -0,0 +1,23 @@
> +#############################################################
> +#
> +# Build the live root filesystem directory
> +#
> +#############################################################
> +
> +LIVE_OPTS :=

  This is unneeded.

> +
> +
> +ROOTFS_LIVE_DEPENDENCIES = #host-sudo

  This as well.

> +
> +define ROOTFS_LIVE_CMD
> + sudo cp -r $(TARGET_DIR)/* $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)/

  That won't work, because device nodes, symlinks etc. are not
copied correctly.  Use cp -a.  Or even better, rsync -a --delete-during
(then you don't need to remove it first).

  For the target, just use $(BR2_TARGET_ROOTFS_LIVE_DEST).


  Regards,
  Arnout


> +endef
> +
> +define ROOTFS_LIVE_INIT
> +	sudo rm -rf $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
> +	sudo mkdir $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
> +endef
> +
> +ROOTFS_LIVE_PRE_GEN_HOOKS += ROOTFS_LIVE_INIT
> +
> +$(eval $(call ROOTFS_TARGET,live))
Steve Calfee Nov. 30, 2012, 12:04 a.m. UTC | #2
I don't understand why you need this. It is a one line "sudo tar -xf
..." to unpack the tarball into your nfs directory. Either type this
line or add a one line script to your ~/bin directory to do it for
each of your buildroot trees/environments. Don't add clutter to the
buildroot makefile.

Regards, Steve

On Tue, Nov 27, 2012 at 12:12 AM, Jérémy Rosen <jeremy.rosen@openwide.fr> wrote:
> add a new target to deploy a live filesystem to be used by NFS
>
> ---
>
> Hello everybody
>
> this patch is not ready for commit, it's a proposal to help me solve the very common use case of
> a developement FS deployed on NFS. I am not very happy of how it's done at this point but I would
> like to discuss it to see if it has a chance to get merged... What I don't like (and would gladly
> have ideas on how to solve this)
> * no dependency on host-sudo, i'm not sure how to add that
> * using sudo to break out of fakeroot is not clean
> * I am abusing the fs building framework, but I don't know of a cleaner way
> * filesystem can only be under output/images at this point
>
> so this patch is more to start a discussion than anything, i'll gladly take any comments or plain
> 'ol rejection if the project doesn't want it... but if there is some interest i'll work it into
> something mainline worthy
>
> Regards
>
> Jérémy Rosen
> ---
>  fs/Config.in      |    1 +
>  fs/live/Config.in |   16 ++++++++++++++++
>  fs/live/live.mk   |   23 +++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
>  create mode 100644 fs/live/Config.in
>  create mode 100644 fs/live/live.mk
>
> diff --git a/fs/Config.in b/fs/Config.in
> index 94154ea..de2377e 100644
> --- a/fs/Config.in
> +++ b/fs/Config.in
> @@ -11,5 +11,6 @@ source "fs/cpio/Config.in"
>  source "fs/iso9660/Config.in"
>  source "fs/initramfs/Config.in"
>  source "fs/romfs/Config.in"
> +source "fs/live/Config.in"
>
>  endmenu
> diff --git a/fs/live/Config.in b/fs/live/Config.in
> new file mode 100644
> index 0000000..bf24754
> --- /dev/null
> +++ b/fs/live/Config.in
> @@ -0,0 +1,16 @@
> +config BR2_TARGET_ROOTFS_LIVE
> +       bool "live root filesystem"
> +       help
> +         uses sudo to create a live image of the filesystem
> +         this is mainly usefull if you want to use your filesystem
> +         over NFS
> +config BR2_TARGET_ROOTFS_LIVE_DEST
> +       string "directory to put the live image"
> +       depends on BR2_TARGET_ROOTFS_LIVE
> +       default "live"
> +       help
> +         The directory where the image should be stored.
> +         this directory will be emptied and recreated, it is given
> +         relative to the buildroot output/images directory
> +
> +
> diff --git a/fs/live/live.mk b/fs/live/live.mk
> new file mode 100644
> index 0000000..73bcd3e
> --- /dev/null
> +++ b/fs/live/live.mk
> @@ -0,0 +1,23 @@
> +#############################################################
> +#
> +# Build the live root filesystem directory
> +#
> +#############################################################
> +
> +LIVE_OPTS :=
> +
> +
> +ROOTFS_LIVE_DEPENDENCIES = #host-sudo
> +
> +define ROOTFS_LIVE_CMD
> + sudo cp -r $(TARGET_DIR)/* $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)/
> +endef
> +
> +define ROOTFS_LIVE_INIT
> +       sudo rm -rf $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
> +       sudo mkdir $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
> +endef
> +
> +ROOTFS_LIVE_PRE_GEN_HOOKS += ROOTFS_LIVE_INIT
> +
> +$(eval $(call ROOTFS_TARGET,live))
> --
> 1.7.10.4
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Jeremy Rosen Nov. 30, 2012, 9 a.m. UTC | #3
> > * no dependency on host-sudo, i'm not sure how to add that
> 
>   That's impossible. The sudo executable must be setuid root, and to
>   make
> it setuid root you have to be root, so sudo must already exist...
> 
>   You could instead add a check in live.mk if sudo actually works.
>    But since
> sudo may ask for a password this can get tricky - you should e.g.
> probably check
> if stdin is a terminal as well.
> 

hmm, good point... I hadn't thought of that i'll add a check for that 

>   That said, I don't know if Peter will accept running sudo within
>   buildroot.
> 

:) yes, that's why I wanted some feedback early...

I think running sudo only for the deployment of the FS is isolated enough to be worth it (it's not like I'm getting rid of fakeroot or anything, it's just to ease the final step of NFS work)

but yes, if that patch is rejected because of philosophical reasons, it would be nice to know early

> 
> > * filesystem can only be under output/images at this point
> 
>   That's not good but easy to solve.
> 

yes that was one of the "first draft not ready for commit" aspect :)

you suggest in the commented patch below to use an absolute path...

I like the idea, if I add a line to the help text hinting that $(WHATEVER) can be used to point to output/image it's probably good enough


only answering the remarks which i'd like to discuss below, I agree with all the others

> > +	depends on BR2_TARGET_ROOTFS_LIVE
> > +	default "live"
> 
>   Use an empty default, and give an error in live.mk if it's
> empty.
> 

I don't like to provide a non-working default (esp since most/all other fs targets work out of the box) 

why not provide one (i'm perfectly fine with another name)

if this project is ok with non-working defaults i'll go that way, but it's simple enough to provide a default under output/images so i'm not sur why not do it...

> > +
> > +define ROOTFS_LIVE_CMD
> > + sudo cp -r $(TARGET_DIR)/*
> > $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)/
> 
>   That won't work, because device nodes, symlinks etc. are not
> copied correctly.  Use cp -a.  Or even better, rsync -a
> --delete-during
> (then you don't need to remove it first).
> 
>   For the target, just use $(BR2_TARGET_ROOTFS_LIVE_DEST).
> 

ok, i'll check all those cp/rsync options I don't know...


V2 of the patch is under way and will be posted soon

Regards

Jérémy
Arnout Vandecappelle Nov. 30, 2012, 9:15 a.m. UTC | #4
On 30/11/12 01:04, Steve Calfee wrote:
> I don't understand why you need this. It is a one line "sudo tar -xf
> ..." to unpack the tarball into your nfs directory. Either type this
> line or add a one line script to your ~/bin directory to do it for
> each of your buildroot trees/environments. Don't add clutter to the
> buildroot makefile.

  If that is the reasoning, we can also remove the compression of the
.tar file, or the -dirclean targets, etc. etc.

  In addition, this target also cleans up the NFS directory first, so
it's slightly more than just untarring.

  Regards,
  Arnout
Steve Calfee Nov. 30, 2012, 9:46 p.m. UTC | #5
On Fri, Nov 30, 2012 at 1:15 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 30/11/12 01:04, Steve Calfee wrote:
>>
>> I don't understand why you need this. It is a one line "sudo tar -xf
>> ..." to unpack the tarball into your nfs directory. Either type this
>> line or add a one line script to your ~/bin directory to do it for
>> each of your buildroot trees/environments. Don't add clutter to the
>> buildroot makefile.
>
>
>  If that is the reasoning, we can also remove the compression of the
> .tar file, or the -dirclean targets, etc. etc.
>
>  In addition, this target also cleans up the NFS directory first, so
> it's slightly more than just untarring.
>
Hi  Arnout,

I guess, but buildroot is all about building, not deploying. What is
next, an automatic copy to my usb or sd drive? Stuff that is highly
user specific should not be in the makefile, especially stuff that can
easily be added to a simple script - my opinion only,

Steve
Jeremy Rosen Dec. 3, 2012, 8:57 a.m. UTC | #6
well, it's more a question of what you consider the live filesystem to be...

I see it a as just another filesystem format (basically a NFS target) which is why my original patch had it in output/images/live 

if you see it as a deployment, it does not really make sense to add it to buildroot (buildroot doesn't flash the filesystem directly) I can understand that both point of view make sense

however having this target makes things simpler in a very common use case (arguably the most common use case since it's the case where you use buildroot repeatedly to prepare the fs, not the case where you run it once to build the final FS)

yes it's a two line script, and yes it's a minor change, but I think it makes sense and it's simple enough for it not to be a big deal...



    Regards

    Jérémy Rosen

fight key loggers : write some perl using vim

----- Mail original -----
> De: "Steve Calfee" <stevecalfee@gmail.com>
> À: "Arnout Vandecappelle" <arnout@mind.be>
> Cc: "Jérémy Rosen" <jeremy.rosen@openwide.fr>, buildroot@busybox.net
> Envoyé: Vendredi 30 Novembre 2012 22:46:53
> Objet: Re: [Buildroot] [PATCH] [PATCH][RFC] new target: live filesystem
> 
> On Fri, Nov 30, 2012 at 1:15 AM, Arnout Vandecappelle
> <arnout@mind.be> wrote:
> > On 30/11/12 01:04, Steve Calfee wrote:
> >>
> >> I don't understand why you need this. It is a one line "sudo tar
> >> -xf
> >> ..." to unpack the tarball into your nfs directory. Either type
> >> this
> >> line or add a one line script to your ~/bin directory to do it for
> >> each of your buildroot trees/environments. Don't add clutter to
> >> the
> >> buildroot makefile.
> >
> >
> >  If that is the reasoning, we can also remove the compression of
> >  the
> > .tar file, or the -dirclean targets, etc. etc.
> >
> >  In addition, this target also cleans up the NFS directory first,
> >  so
> > it's slightly more than just untarring.
> >
> Hi  Arnout,
> 
> I guess, but buildroot is all about building, not deploying. What is
> next, an automatic copy to my usb or sd drive? Stuff that is highly
> user specific should not be in the makefile, especially stuff that
> can
> easily be added to a simple script - my opinion only,
> 
> Steve
>
Samuel Martin Dec. 3, 2012, 9:21 a.m. UTC | #7
Hi Jeremy, all,

2012/12/3 Jeremy Rosen <jeremy.rosen@openwide.fr>:
> well, it's more a question of what you consider the live filesystem to be...
>
> I see it a as just another filesystem format (basically a NFS target) which is why my original patch had it in output/images/live
>
> if you see it as a deployment, it does not really make sense to add it to buildroot (buildroot doesn't flash the filesystem directly) I can understand that both point of view make sense
>
> however having this target makes things simpler in a very common use case (arguably the most common use case since it's the case where you use buildroot repeatedly to prepare the fs, not the case where you run it once to build the final FS)

That's a controversial topic...
But, at least, you cat start filling the gap in the doc ;-),
especially this section:
http://buildroot.org/downloads/manual/manual.html#_beyond_buildroot
diff mbox

Patch

diff --git a/fs/Config.in b/fs/Config.in
index 94154ea..de2377e 100644
--- a/fs/Config.in
+++ b/fs/Config.in
@@ -11,5 +11,6 @@  source "fs/cpio/Config.in"
 source "fs/iso9660/Config.in"
 source "fs/initramfs/Config.in"
 source "fs/romfs/Config.in"
+source "fs/live/Config.in"
 
 endmenu
diff --git a/fs/live/Config.in b/fs/live/Config.in
new file mode 100644
index 0000000..bf24754
--- /dev/null
+++ b/fs/live/Config.in
@@ -0,0 +1,16 @@ 
+config BR2_TARGET_ROOTFS_LIVE
+	bool "live root filesystem"
+	help
+	  uses sudo to create a live image of the filesystem
+	  this is mainly usefull if you want to use your filesystem
+	  over NFS
+config BR2_TARGET_ROOTFS_LIVE_DEST
+	string "directory to put the live image"
+	depends on BR2_TARGET_ROOTFS_LIVE
+	default "live"
+	help
+	  The directory where the image should be stored.
+	  this directory will be emptied and recreated, it is given
+	  relative to the buildroot output/images directory
+
+
diff --git a/fs/live/live.mk b/fs/live/live.mk
new file mode 100644
index 0000000..73bcd3e
--- /dev/null
+++ b/fs/live/live.mk
@@ -0,0 +1,23 @@ 
+#############################################################
+#
+# Build the live root filesystem directory
+#
+#############################################################
+
+LIVE_OPTS :=
+
+
+ROOTFS_LIVE_DEPENDENCIES = #host-sudo
+
+define ROOTFS_LIVE_CMD
+ sudo cp -r $(TARGET_DIR)/* $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)/
+endef
+
+define ROOTFS_LIVE_INIT
+	sudo rm -rf $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
+	sudo mkdir $(BINARIES_DIR)/$(BR2_TARGET_ROOTFS_LIVE_DEST)
+endef
+
+ROOTFS_LIVE_PRE_GEN_HOOKS += ROOTFS_LIVE_INIT
+
+$(eval $(call ROOTFS_TARGET,live))