makedevs: unlink device nodes before they are created
PATCH 1/1 makedevs: unlink device nodes before they are created
We use makedevs to create device nodes in the target rootfs. However, this can be called several times, e.g. when building several filesystem images or when rebuilding. When the script is called the second time, the device node already exists so mknod() errors out.
This wasn't noticed before because fakeroot's mknod() wrapper (incorrectly) does _not_ error out when the file exists already. Now we switched from fakeroot to pseudo, the problem becomes apparent.
The simplest solution is to change makedevs to first remove the target file before creating it. This is simpler than two alternative approaches:
- Removing the target files before makedevs is called is difficult, because we would have to parse the device table file to find out which files have to be deleted.
- Silently skipping device creation if it exists already is also easy, but then we really should also check if the device numbers and mode are correct, and that is more complicated.
The other types don't have to be changed. The 'd' (directory) type is already OK because it already only creates directories if they don't exist yet. The 'f' (file mode) and 'r' (recursive) types only operate on files and directories that exist already.
--- package/makedevs/makedevs.c | 2 ++ 1 file changed, 2 insertions(+)
package/makedevs/makedevs.c +2 -0
@@ -622,6 +622,7 @@ int main(int argc, char **argv)for (i =0 ; i < count; i++) {sprintf (full_name_inc,"%s%d" , full_name, start + i); rdev =makedev (major, minor + i * increment); +unlink (full_name_inc);if (mknod (full_name_inc, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name_inc); ret =EXIT_FAILURE ;
@@ -638,6 +639,7 @@ int main(int argc, char **argv)free (full_name_inc); }else { rdev =makedev (major, minor); +unlink (full_name);if (mknod (full_name, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name); ret =EXIT_FAILURE ;
Signature · 1 line
2.9.3
PATCH 1/1 makedevs: unlink device nodes before they are created
Hi Arnout,
Quoted 21 lines · Arnout Vandecappelle (Essensium/Mind)
We use makedevs to create device nodes in the target rootfs. However, this can be called several times, e.g. when building several filesystem images or when rebuilding. When the script is called the second time, the device node already exists so
mknod()errors out.This wasn't noticed before because fakeroot's
mknod()wrapper (incorrectly) does _not_ error out when the file exists already. Now we switched from fakeroot to pseudo, the problem becomes apparent.The simplest solution is to change makedevs to first remove the target file before creating it. This is simpler than two alternative approaches:
- Removing the target files before makedevs is called is difficult, because we would have to parse the device table file to find out which files have to be deleted.
- Silently skipping device creation if it exists already is also easy, but then we really should also check if the device numbers and mode are correct, and that is more complicated.
Quoted 7 lines
The other types don't have to be changed. The 'd' (directory) type is already OK because it already only creates directories if they don't exist yet. The 'f' (file mode) and 'r' (recursive) types only operate on files and directories that exist already.
Reported-by Fabio Estevam <[email protected]>
This fixes the warp7_defconfig build error, thanks!
Yann E. MORIN
NAKPATCH 1/1 makedevs: unlink device nodes before they are created
In reply to Arnout Vandecappelle (Essensium/Mind)Arnout, All,
On 2016-11-05 01:14 +0100, Arnout Vandecappelle (Essensium/Mind) spake thusly:
Quoted 21 lines
We use makedevs to create device nodes in the target rootfs. However, this can be called several times, e.g. when building several filesystem images or when rebuilding. When the script is called the second time, the device node already exists so
mknod()errors out.This wasn't noticed before because fakeroot's
mknod()wrapper (incorrectly) does _not_ error out when the file exists already. Now we switched from fakeroot to pseudo, the problem becomes apparent.The simplest solution is to change makedevs to first remove the target file before creating it. This is simpler than two alternative approaches:
- Removing the target files before makedevs is called is difficult, because we would have to parse the device table file to find out which files have to be deleted.
- Silently skipping device creation if it exists already is also easy, but then we really should also check if the device numbers and mode are correct, and that is more complicated.
Quoted 7 lines
The other types don't have to be changed. The 'd' (directory) type is already OK because it already only creates directories if they don't exist yet. The 'f' (file mode) and 'r' (recursive) types only operate on files and directories that exist already.
Reported-by Fabio Estevam <[email protected]>
NAK: we really do not want that files exist in the target in case they are configured as device nodes in a dev-table. This should not happen.
That it works now is just an ill side-effect of fakeroot being lenient. That it breaks with pseudo is good.
The fix is far from trivial, though, but we should handle the situation via pseudo, and understand why it does not reload its DB.
If we can't fix our use of pseudo, then we should revertt to using fakeroot for this release cycle.
Regards,
Yann E. MORIN.
Quoted 31 lines
--- package/makedevs/makedevs.c | 2 ++ 1 file changed, 2 insertions(+)package/makedevs/makedevs.c +2
-0@@ -622,6 +622,7 @@ int main(int argc, char **argv)for (i =0 ; i < count; i++) {sprintf (full_name_inc,"%s%d" , full_name, start + i); rdev =makedev (major, minor + i * increment); +unlink (full_name_inc);if (mknod (full_name_inc, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name_inc); ret =EXIT_FAILURE ;@@ -638,6 +639,7 @@ int main(int argc, char **argv)free (full_name_inc); }else { rdev =makedev (major, minor); +unlink (full_name);if (mknod (full_name, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name); ret =EXIT_FAILURE ;Signature · 6 lines
2.9.3 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 makedevs: unlink device nodes before they are created
Quoted 29 lines · Yann E. MORIN
Arnout, All,
On 2016-11-05 01:14 +0100, Arnout Vandecappelle (Essensium/Mind) spake thusly:
Quoted 26 lines
We use makedevs to create device nodes in the target rootfs. However, this can be called several times, e.g. when building several filesystem images or when rebuilding. When the script is called the second time, the device node already exists so
mknod()errors out.This wasn't noticed before because fakeroot's
mknod()wrapper (incorrectly) does _not_ error out when the file exists already. Now we switched from fakeroot to pseudo, the problem becomes apparent.The simplest solution is to change makedevs to first remove the target file before creating it. This is simpler than two alternative approaches:
- Removing the target files before makedevs is called is difficult, because we would have to parse the device table file to find out which files have to be deleted.
- Silently skipping device creation if it exists already is also easy, but then we really should also check if the device numbers and mode are correct, and that is more complicated.
The other types don't have to be changed. The 'd' (directory) type is already OK because it already only creates directories if they don't exist yet. The 'f' (file mode) and 'r' (recursive) types only operate on files and directories that exist already.
Quoted 5 lines
Quoted 2 lines
Reported-by Fabio Estevam <[email protected]>NAK: we really do not want that files exist in the target in case they are configured as device nodes in a dev-table. This should not happen.
So what you are saying is: we should instead go for the second alternative I
mentioned in the commit log, i.e. check if the existing node is the right type?
Quoted 2 lines
That it works now is just an ill side-effect of fakeroot being lenient. That it breaks with pseudo is good.
So you do agree that something needs to be fixed? I.e., if we would run it
under sudo instead of fakeroot/pseudo, we would see the same issue, right?
Quoted 2 lines
The fix is far from trivial, though, but we should handle the situation via pseudo, and understand why it does not reload its DB.
pseudo reloading its DB has nothing to do with it AFAICS. BTW can you describe
what the problem is with pseudo reloading its DB?
Regards,
Arnout
Quoted 39 lines
If we can't fix our use of pseudo, then we should revertt to using fakeroot for this release cycle.
Regards,
Yann E. MORIN.Quoted 31 lines
--- package/makedevs/makedevs.c | 2 ++ 1 file changed, 2 insertions(+)package/makedevs/makedevs.c +2
-0@@ -622,6 +622,7 @@ int main(int argc, char **argv)for (i =0 ; i < count; i++) {sprintf (full_name_inc,"%s%d" , full_name, start + i); rdev =makedev (major, minor + i * increment); +unlink (full_name_inc);if (mknod (full_name_inc, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name_inc); ret =EXIT_FAILURE ;@@ -638,6 +639,7 @@ int main(int argc, char **argv)free (full_name_inc); }else { rdev =makedev (major, minor); +unlink (full_name);if (mknod (full_name, mode, rdev) ==-1 ) {bb_perror_msg ("line %d: Couldnt create node %s" , linenum, full_name); ret =EXIT_FAILURE ;Signature · 6 lines
2.9.3 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 makedevs: unlink device nodes before they are created
Arnout, All,
On 2016-11-05 12:05 +0100, Arnout Vandecappelle spake thusly:
Quoted 28 lines · Yann E. MORIN
Quoted 27 lines
On 2016-11-05 01:14 +0100, Arnout Vandecappelle (Essensium/Mind) spake thusly:
Quoted 26 lines
We use makedevs to create device nodes in the target rootfs. However, this can be called several times, e.g. when building several filesystem images or when rebuilding. When the script is called the second time, the device node already exists so
mknod()errors out.This wasn't noticed before because fakeroot's
mknod()wrapper (incorrectly) does _not_ error out when the file exists already. Now we switched from fakeroot to pseudo, the problem becomes apparent.The simplest solution is to change makedevs to first remove the target file before creating it. This is simpler than two alternative approaches:
- Removing the target files before makedevs is called is difficult, because we would have to parse the device table file to find out which files have to be deleted.
- Silently skipping device creation if it exists already is also easy, but then we really should also check if the device numbers and mode are correct, and that is more complicated.
The other types don't have to be changed. The 'd' (directory) type is already OK because it already only creates directories if they don't exist yet. The 'f' (file mode) and 'r' (recursive) types only operate on files and directories that exist already.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Reported-by Fabio Estevam <[email protected]>NAK: we really do not want that files exist in the target in case they are configured as device nodes in a dev-table. This should not happen.
So what you are saying is: we should instead go for the second alternative I
mentioned in the commit log, i.e. check if the existing node is the right type?
Yes, that would be acceptable, and correct.
Quoted 5 lines
Quoted 2 lines
That it works now is just an ill side-effect of fakeroot being lenient. That it breaks with pseudo is good.
So you do agree that something needs to be fixed? I.e., if we would run it
under sudo instead of fakeroot/pseudo, we would see the same issue, right?
Yup:
# mkdir foo c 5 1
# mkdir foo c 5 1
mknod: foo: File exists
# echo $?
1Quoted 5 lines
Quoted 2 lines
The fix is far from trivial, though, but we should handle the situation via pseudo, and understand why it does not reload its DB.
pseudo reloading its DB has nothing to do with it AFAICS. BTW can you describe
what the problem is with pseudo reloading its DB?
Forget it, I re-ran a few tests now, and it does work.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'