diff mbox

Create /dev/null device node when building a cpio rootfs

Message ID 1398784378-20251-2-git-send-email-dan.moulding@rackwareinc.com
State Rejected
Headers show

Commit Message

Dan Moulding April 29, 2014, 3:12 p.m. UTC
This allows mdev to be used for automatic device creation, even if the
kernel doesn't support devtmpfs. The programs that run before mdev
(e.g. init) expect /dev/null to already exist.

Signed-off-by: Dan Moulding <dan.moulding@rackwareinc.com>
---
 fs/cpio/cpio.mk | 1 +
 1 file changed, 1 insertion(+)

Comments

Arnout Vandecappelle April 29, 2014, 6:37 p.m. UTC | #1
On 29/04/14 17:12, Dan Moulding wrote:
> This allows mdev to be used for automatic device creation, even if the
> kernel doesn't support devtmpfs. The programs that run before mdev
> (e.g. init) expect /dev/null to already exist.
> 
> Signed-off-by: Dan Moulding <dan.moulding@rackwareinc.com>
> ---
>  fs/cpio/cpio.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
> index 771306c..8648ed9 100644
> --- a/fs/cpio/cpio.mk
> +++ b/fs/cpio/cpio.mk
> @@ -22,6 +22,7 @@ define ROOTFS_CPIO_ADD_INIT
>  endef
>  
>  PACKAGES_PERMISSIONS_TABLE += /dev/console c 622 0 0 5 1 - - -$(sep)
> +PACKAGES_PERMISSIONS_TABLE += /dev/null c 666 0 0 1 3 - - -$(sep)

 How is this issue limited to the cpio rootfs? You'll have the same issue
with any rootfs, right?

 So perhaps a better solution is to move /dev/console and /dev/null from
device_table_dev.txt to device_table.txt.

 In fact, on December 13, 2011, Thomas Petazzoni wrote:

> Le Tue, 13 Dec 2011 09:39:34 +0100,
> Arnout Vandecappelle <arnout@mind.be> a écrit :
> 
>>  If you ask me, it's OK to add /dev entries in the
>> BR2_ROOTFS_DEVICE_TABLE. In fact, I think /dev/console and /dev/null
>> should be put in there.  But I've never gotten around to roll a patch
>> for it.
> 
> FWIW, when I initially introduced (based on prior patches) this
> static /dev vs. devtmpfs vs. mdev vs. udev selection, the mdev and udev
> choices were independent of devtmpfs, i.e there were using a minimal
> device tables with /dev/null and /dev/console. But after discussion on
> the mailing-list, it was decided that it was better to make devtmpfs a
> requirement when mdev and udev were choosen.
> 
> See
> 
>   Message-Id: <f92465f9de8bcd24cf2c974b158a600a84e96422.1291582352.git.thomas.petazzoni@free-electrons.com>
> 
> for the initial patch I proposed on December, 5th 2010.
> 
> And
> 
>  Message-ID: <87r5dgfldw.fsf@macbook.be.48ers.dk>
> 
> for Peter's answer (December, 17th 2010), saying :
> 
> """
>  Thomas> At compile time, only a minimal /dev is created in the filesystem,
>  Thomas> with only "console" and "null". This is done thanks to a small device
>  Thomas> table in target/generic/device_table_mdev_udev.txt. This is done
>  Thomas> directly at the configuration level (fs/Config.in).  
> 
> While I agree we need the minimal device table for /etc/shadow and
> similar permissions, do we really need to support mdev/udev without
> devtmpfs? It's been in the kernel now for close to 2 years, it's very
> small and it simplifies (and speeds up) the boot sequence quite a lot.
> """


 IOW this approach was suggested before but not really picked up.


 Note that if we do that, we can revert e1ebae70 and perhaps even
10a130f9. Actually, 10a130f9 can probably already be reverted, since
e1ebae70 should have the same effect.

 Regards,
 Arnout

>  
>  endif # BR2_ROOTFS_DEVICE_CREATION_STATIC
>  
>
Dan Moulding April 29, 2014, 7:18 p.m. UTC | #2
On Tuesday, April 29, 2014 12:37pm, "Arnout Vandecappelle" <arnout@mind.be> said:

> 
>  How is this issue limited to the cpio rootfs? You'll have the same issue
> with any rootfs, right?

Absolutely. But it was only making /dev/console for cpio... so I decided to follow suit, thinking maybe there was a good reason for it.

> 
>  So perhaps a better solution is to move /dev/console and /dev/null from
> device_table_dev.txt to device_table.txt.
> 

Agreed.

>  IOW this approach was suggested before but not really picked up.
>

Yes. And in fact, I had stumbled upon these past discussions when Googling about this. I just wasn't sure why it never got picked up.
 
Cheers,

-- Dan
Thomas Petazzoni July 16, 2014, 7:07 p.m. UTC | #3
Dear Arnout Vandecappelle,

On Tue, 29 Apr 2014 20:37:44 +0200, Arnout Vandecappelle wrote:

> > diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
> > index 771306c..8648ed9 100644
> > --- a/fs/cpio/cpio.mk
> > +++ b/fs/cpio/cpio.mk
> > @@ -22,6 +22,7 @@ define ROOTFS_CPIO_ADD_INIT
> >  endef
> >  
> >  PACKAGES_PERMISSIONS_TABLE += /dev/console c 622 0 0 5 1 - - -$(sep)
> > +PACKAGES_PERMISSIONS_TABLE += /dev/null c 666 0 0 1 3 - - -$(sep)
> 
>  How is this issue limited to the cpio rootfs? You'll have the same issue
> with any rootfs, right?

Yes. The mdev support makes the assumption that devtmpfs support is
available in the kernel.

>  So perhaps a better solution is to move /dev/console and /dev/null from
> device_table_dev.txt to device_table.txt.

Not really:

 * In the initramfs case, /dev/console is needed. For static /dev, it's
   already there. For non-static /dev, we have to have a
   static /dev/console until our init script mounts devtmpfs. So it's
   really an initramfs specific issue.

 * For the non-initramfs case. For static /dev, it's already there. For
   non-static /dev, devtmpfs is mounted *before* init is started,
   so /dev/console is also already there.

>  Note that if we do that, we can revert e1ebae70 and perhaps even
> 10a130f9. Actually, 10a130f9 can probably already be reverted, since
> e1ebae70 should have the same effect.

I agree. Would you mind sending a patch reverting 10a130f9 ?

Also, since we seem to agree that the patch proposed by Dan was not
needed, I've marked http://patchwork.ozlabs.org/patch/343880/ as
Rejected in our patch tracking system.

Best regards,

Thomas
diff mbox

Patch

diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
index 771306c..8648ed9 100644
--- a/fs/cpio/cpio.mk
+++ b/fs/cpio/cpio.mk
@@ -22,6 +22,7 @@  define ROOTFS_CPIO_ADD_INIT
 endef
 
 PACKAGES_PERMISSIONS_TABLE += /dev/console c 622 0 0 5 1 - - -$(sep)
+PACKAGES_PERMISSIONS_TABLE += /dev/null c 666 0 0 1 3 - - -$(sep)
 
 endif # BR2_ROOTFS_DEVICE_CREATION_STATIC