Signed-off-by: Michael Tremer michael.tremer@ipfire.org --- src/initscripts/system/mountkernfs | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/initscripts/system/mountkernfs b/src/initscripts/system/mountkernfs index 264da24c4..1f1426077 100644 --- a/src/initscripts/system/mountkernfs +++ b/src/initscripts/system/mountkernfs @@ -39,6 +39,11 @@ case "${1}" in mount -t cgroup2 none /sys/fs/cgroup || failed=1 fi
+ if ! mountpoint /sys/firmware/efi/efivars &>/dev/null && [ -d "/sys/firmware/efi" ]; then + boot_mesg -n " /sys/firmware/efi/efivars" ${NORMAL} + mount -t efivarfs efivarfs /sys/firmware/efi/efivars || failed=1 + fi + # create folder for dhcpcd changeroot mkdir -p /run/dhcpcd/chroot chown dhcpcd:dhcpcd /run/dhcpcd/chroot
Signed-off-by: Michael Tremer michael.tremer@ipfire.org --- src/installer/dracut-module/module-setup.sh | 1 + src/installer/dracut-module/run-installer.sh | 5 +++++ 2 files changed, 6 insertions(+)
diff --git a/src/installer/dracut-module/module-setup.sh b/src/installer/dracut-module/module-setup.sh index 29ec4c0d7..9c3a5d03e 100755 --- a/src/installer/dracut-module/module-setup.sh +++ b/src/installer/dracut-module/module-setup.sh @@ -23,6 +23,7 @@ install() {
# Kernel drivers instmods =drivers/hid + instmods efivarfs
# Network drivers instmods =drivers/net/ethernet =drivers/net/usb diff --git a/src/installer/dracut-module/run-installer.sh b/src/installer/dracut-module/run-installer.sh index 33c8c4b10..755de1d3a 100644 --- a/src/installer/dracut-module/run-installer.sh +++ b/src/installer/dracut-module/run-installer.sh @@ -8,6 +8,11 @@ if grep -q "installer.unattended" /proc/cmdline; then unattended=1 fi
+# Mount efivarfs on EFI systems +if [ -d "/sys/firmware/efi" ]; then + mount -t efivarfs efivarfs /sys/firmware/efi/efivars +fi + # Enable Unicode echo -en '\033%G' && kbd_mode -u
Reviewed-by: Peter Müller peter.mueller@ipfire.org
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/installer/dracut-module/module-setup.sh | 1 + src/installer/dracut-module/run-installer.sh | 5 +++++ 2 files changed, 6 insertions(+)
diff --git a/src/installer/dracut-module/module-setup.sh b/src/installer/dracut-module/module-setup.sh index 29ec4c0d7..9c3a5d03e 100755 --- a/src/installer/dracut-module/module-setup.sh +++ b/src/installer/dracut-module/module-setup.sh @@ -23,6 +23,7 @@ install() {
# Kernel drivers instmods =drivers/hid
instmods efivarfs
# Network drivers instmods =drivers/net/ethernet =drivers/net/usb
diff --git a/src/installer/dracut-module/run-installer.sh b/src/installer/dracut-module/run-installer.sh index 33c8c4b10..755de1d3a 100644 --- a/src/installer/dracut-module/run-installer.sh +++ b/src/installer/dracut-module/run-installer.sh @@ -8,6 +8,11 @@ if grep -q "installer.unattended" /proc/cmdline; then unattended=1 fi
+# Mount efivarfs on EFI systems +if [ -d "/sys/firmware/efi" ]; then
- mount -t efivarfs efivarfs /sys/firmware/efi/efivars
+fi
- # Enable Unicode echo -en '\033%G' && kbd_mode -u
Signed-off-by: Michael Tremer michael.tremer@ipfire.org --- src/installer/hw.c | 113 +++++++++++++++++++++++++++++-------------- src/installer/hw.h | 2 +- src/installer/main.c | 2 +- 3 files changed, 78 insertions(+), 39 deletions(-)
diff --git a/src/installer/hw.c b/src/installer/hw.c index 71a1f1cce..265df2d8c 100644 --- a/src/installer/hw.c +++ b/src/installer/hw.c @@ -46,13 +46,6 @@
#include "hw.h"
-const char* other_filesystems[] = { - "/dev", - "/proc", - "/sys", - NULL -}; - static int system_chroot(const char* output, const char* path, const char* cmd) { char chroot_cmd[STRING_SIZE];
@@ -149,14 +142,53 @@ int hw_mount(const char* source, const char* target, const char* fs, int flags) return mount(source, target, fs, flags, NULL); }
-int hw_umount(const char* target) { - int r = umount2(target, 0); +static int hw_bind_mount(const char* source, const char* prefix) { + if (!source || !prefix) { + errno = EINVAL; + return 1; + }
- if (r && errno == EBUSY) { - // Give it a moment to settle - sleep(1); + char target[PATH_MAX]; + int r; + + // Format target + r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source); + if (r < 0) + return 1;
- r = umount2(target, MNT_FORCE); + // Ensure target exists + mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO); + + return hw_mount(source, target, NULL, MS_BIND); +} + +int hw_umount(const char* source, const char* prefix) { + char target[PATH_MAX]; + int r; + + if (prefix) + r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source); + else + r = snprintf(target, sizeof(target) - 1, "%s", source); + if (r < 0) + return r; + + // Perform umount + r = umount2(target, 0); + if (r) { + switch (errno) { + // Try again with force if umount wasn't successful + case EBUSY: + sleep(1); + + r = umount2(target, MNT_FORCE); + break; + + // target wasn't a mountpoint. Ignore. + case EINVAL: + r = 0; + break; + } }
return r; @@ -174,7 +206,7 @@ static int hw_test_source_medium(const char* path) { ret = access(SOURCE_TEST_FILE, R_OK);
// Umount the test device. - hw_umount(SOURCE_MOUNT_PATH); + hw_umount(SOURCE_MOUNT_PATH, NULL);
return ret; } @@ -881,20 +913,21 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { }
// bind-mount misc filesystems - char** otherfs = other_filesystems; - while (*otherfs) { - snprintf(target, sizeof(target), "%s%s", prefix, *otherfs); + r = hw_bind_mount("/dev", prefix); + if (r) + return r;
- mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO); - r = hw_mount(*otherfs, target, NULL, MS_BIND); - if (r) { - hw_umount_filesystems(dest, prefix); + r = hw_bind_mount("/proc", prefix); + if (r) + return r;
- return r; - } + r = hw_bind_mount("/sys", prefix); + if (r) + return r;
- otherfs++; - } + r = hw_bind_mount("/sys/firmware/efi/efivars", prefix); + if (r && errno != ENOENT) + return r;
return 0; } @@ -908,16 +941,14 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) {
// ESP if (*dest->part_boot_efi) { - snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT_EFI); - r = hw_umount(target); + r = hw_umount(HW_PATH_BOOT_EFI, prefix); if (r) return -1; }
// boot if (*dest->part_boot) { - snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT); - r = hw_umount(target); + r = hw_umount(HW_PATH_BOOT, prefix); if (r) return -1; } @@ -928,16 +959,24 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { }
// misc filesystems - char** otherfs = other_filesystems; - while (*otherfs) { - snprintf(target, sizeof(target), "%s%s", prefix, *otherfs++); - r = hw_umount(target); - if (r) - return -1; - } + r = hw_umount("/sys/firmware/efi/efivars", prefix); + if (r) + return -1; + + r = hw_umount("/sys", prefix); + if (r) + return -1; + + r = hw_umount("/proc", prefix); + if (r) + return -1; + + r = hw_umount("/dev", prefix); + if (r) + return -1;
// root - r = hw_umount(prefix); + r = hw_umount(prefix, NULL); if (r) return -1;
diff --git a/src/installer/hw.h b/src/installer/hw.h index 9fe69271e..b11dfa48f 100644 --- a/src/installer/hw.h +++ b/src/installer/hw.h @@ -108,7 +108,7 @@ struct hw* hw_init(); void hw_free(struct hw* hw);
int hw_mount(const char* source, const char* target, const char* fs, int flags); -int hw_umount(const char* target); +int hw_umount(const char* source, const char* prefix);
char* hw_find_source_medium(struct hw* hw);
diff --git a/src/installer/main.c b/src/installer/main.c index bc0fdaa67..fabc0ef52 100644 --- a/src/installer/main.c +++ b/src/installer/main.c @@ -909,7 +909,7 @@ int main(int argc, char *argv[]) { }
// Umount source drive and eject - hw_umount(SOURCE_MOUNT_PATH); + hw_umount(SOURCE_MOUNT_PATH, NULL);
// Free downloaded ISO image if (strcmp(sourcedrive, SOURCE_TEMPFILE) == 0) {
Hello Michael,
lacking C skills, this one looks good to me, but I am not confident enough to tag it as being reviewed.
Acked-by: Peter Müller peter.mueller@ipfire.org
Thanks, and best regards, Peter Müller
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/installer/hw.c | 113 +++++++++++++++++++++++++++++-------------- src/installer/hw.h | 2 +- src/installer/main.c | 2 +- 3 files changed, 78 insertions(+), 39 deletions(-)
diff --git a/src/installer/hw.c b/src/installer/hw.c index 71a1f1cce..265df2d8c 100644 --- a/src/installer/hw.c +++ b/src/installer/hw.c @@ -46,13 +46,6 @@
#include "hw.h"
-const char* other_filesystems[] = {
- "/dev",
- "/proc",
- "/sys",
- NULL
-};
- static int system_chroot(const char* output, const char* path, const char* cmd) { char chroot_cmd[STRING_SIZE];
@@ -149,14 +142,53 @@ int hw_mount(const char* source, const char* target, const char* fs, int flags) return mount(source, target, fs, flags, NULL); }
-int hw_umount(const char* target) {
- int r = umount2(target, 0);
+static int hw_bind_mount(const char* source, const char* prefix) {
- if (!source || !prefix) {
errno = EINVAL;return 1;- }
- if (r && errno == EBUSY) {
// Give it a moment to settlesleep(1);
- char target[PATH_MAX];
- int r;
- // Format target
- r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);
- if (r < 0)
return 1;
r = umount2(target, MNT_FORCE);
- // Ensure target exists
- mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);
- return hw_mount(source, target, NULL, MS_BIND);
+}
+int hw_umount(const char* source, const char* prefix) {
char target[PATH_MAX];
int r;
if (prefix)
r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);else
r = snprintf(target, sizeof(target) - 1, "%s", source);if (r < 0)
return r;// Perform umount
r = umount2(target, 0);
if (r) {
switch (errno) {// Try again with force if umount wasn't successfulcase EBUSY:sleep(1);r = umount2(target, MNT_FORCE);break;// target wasn't a mountpoint. Ignore.case EINVAL:r = 0;break;}}
return r;
@@ -174,7 +206,7 @@ static int hw_test_source_medium(const char* path) { ret = access(SOURCE_TEST_FILE, R_OK);
// Umount the test device.
- hw_umount(SOURCE_MOUNT_PATH);
hw_umount(SOURCE_MOUNT_PATH, NULL);
return ret; }
@@ -881,20 +913,21 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { }
// bind-mount misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs);
- r = hw_bind_mount("/dev", prefix);
- if (r)
return r;
mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);r = hw_mount(*otherfs, target, NULL, MS_BIND);if (r) {hw_umount_filesystems(dest, prefix);
- r = hw_bind_mount("/proc", prefix);
- if (r)
return r;
return r;}
- r = hw_bind_mount("/sys", prefix);
- if (r)
return r;
otherfs++;- }
r = hw_bind_mount("/sys/firmware/efi/efivars", prefix);
if (r && errno != ENOENT)
return r;return 0; }
@@ -908,16 +941,14 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) {
// ESP if (*dest->part_boot_efi) {
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT_EFI);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT_EFI, prefix);if (r) return -1; }
// boot if (*dest->part_boot) {
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT, prefix);@@ -928,16 +959,24 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { }
// misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs++);r = hw_umount(target);if (r)return -1;- }
r = hw_umount("/sys/firmware/efi/efivars", prefix);
if (r)
return -1;r = hw_umount("/sys", prefix);
if (r)
return -1;r = hw_umount("/proc", prefix);
if (r)
return -1;r = hw_umount("/dev", prefix);
if (r)
return -1;// root
- r = hw_umount(prefix);
- r = hw_umount(prefix, NULL); if (r) return -1;
diff --git a/src/installer/hw.h b/src/installer/hw.h index 9fe69271e..b11dfa48f 100644 --- a/src/installer/hw.h +++ b/src/installer/hw.h @@ -108,7 +108,7 @@ struct hw* hw_init(); void hw_free(struct hw* hw);
int hw_mount(const char* source, const char* target, const char* fs, int flags); -int hw_umount(const char* target); +int hw_umount(const char* source, const char* prefix);
char* hw_find_source_medium(struct hw* hw);
diff --git a/src/installer/main.c b/src/installer/main.c index bc0fdaa67..fabc0ef52 100644 --- a/src/installer/main.c +++ b/src/installer/main.c @@ -909,7 +909,7 @@ int main(int argc, char *argv[]) { }
// Umount source drive and eject
- hw_umount(SOURCE_MOUNT_PATH);
hw_umount(SOURCE_MOUNT_PATH, NULL);
// Free downloaded ISO image if (strcmp(sourcedrive, SOURCE_TEMPFILE) == 0) {
Isn’t an Ack a stronger kind of Review?
https://wiki.ipfire.org/devel/git/tags
However, thanks for looking at this.
-Michael
On 17 Nov 2021, at 20:11, Peter Müller peter.mueller@ipfire.org wrote:
Hello Michael,
lacking C skills, this one looks good to me, but I am not confident enough to tag it as being reviewed.
Acked-by: Peter Müller peter.mueller@ipfire.org
Thanks, and best regards, Peter Müller
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/installer/hw.c | 113 +++++++++++++++++++++++++++++-------------- src/installer/hw.h | 2 +- src/installer/main.c | 2 +- 3 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/installer/hw.c b/src/installer/hw.c index 71a1f1cce..265df2d8c 100644 --- a/src/installer/hw.c +++ b/src/installer/hw.c @@ -46,13 +46,6 @@ #include "hw.h" -const char* other_filesystems[] = {
- "/dev",
- "/proc",
- "/sys",
- NULL
-};
static int system_chroot(const char* output, const char* path, const char* cmd) { char chroot_cmd[STRING_SIZE]; @@ -149,14 +142,53 @@ int hw_mount(const char* source, const char* target, const char* fs, int flags) return mount(source, target, fs, flags, NULL); } -int hw_umount(const char* target) {
- int r = umount2(target, 0);
+static int hw_bind_mount(const char* source, const char* prefix) {
- if (!source || !prefix) {
errno = EINVAL;return 1;- }
- if (r && errno == EBUSY) {
// Give it a moment to settlesleep(1);
- char target[PATH_MAX];
- int r;
- // Format target
- r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);
- if (r < 0)
return 1;
r = umount2(target, MNT_FORCE);
- // Ensure target exists
- mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);
- return hw_mount(source, target, NULL, MS_BIND);
+}
+int hw_umount(const char* source, const char* prefix) {
- char target[PATH_MAX];
- int r;
- if (prefix)
r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);- else
r = snprintf(target, sizeof(target) - 1, "%s", source);- if (r < 0)
return r;- // Perform umount
- r = umount2(target, 0);
- if (r) {
switch (errno) {// Try again with force if umount wasn't successfulcase EBUSY:sleep(1);r = umount2(target, MNT_FORCE);break;// target wasn't a mountpoint. Ignore.case EINVAL:r = 0;break;}@@ -174,7 +206,7 @@ static int hw_test_source_medium(const char* path) { ret = access(SOURCE_TEST_FILE, R_OK); // Umount the test device.
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); return ret;
} @@ -881,20 +913,21 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { } // bind-mount misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs);
- r = hw_bind_mount("/dev", prefix);
- if (r)
return r;
mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);r = hw_mount(*otherfs, target, NULL, MS_BIND);if (r) {hw_umount_filesystems(dest, prefix);
- r = hw_bind_mount("/proc", prefix);
- if (r)
return r;
return r;}
- r = hw_bind_mount("/sys", prefix);
- if (r)
return r;
otherfs++;- }
- r = hw_bind_mount("/sys/firmware/efi/efivars", prefix);
- if (r && errno != ENOENT)
return r;} @@ -908,16 +941,14 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { // ESP if (*dest->part_boot_efi) {
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT_EFI);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT_EFI, prefix);
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT, prefix);@@ -928,16 +959,24 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { } // misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs++);r = hw_umount(target);if (r)return -1;- }
- r = hw_umount("/sys/firmware/efi/efivars", prefix);
- if (r)
return -1;- r = hw_umount("/sys", prefix);
- if (r)
return -1;- r = hw_umount("/proc", prefix);
- if (r)
return -1;- r = hw_umount("/dev", prefix);
- if (r)
return -1;
- r = hw_umount(prefix);
- r = hw_umount(prefix, NULL); if (r) return -1;
diff --git a/src/installer/hw.h b/src/installer/hw.h index 9fe69271e..b11dfa48f 100644 --- a/src/installer/hw.h +++ b/src/installer/hw.h @@ -108,7 +108,7 @@ struct hw* hw_init(); void hw_free(struct hw* hw); int hw_mount(const char* source, const char* target, const char* fs, int flags); -int hw_umount(const char* target); +int hw_umount(const char* source, const char* prefix); char* hw_find_source_medium(struct hw* hw); diff --git a/src/installer/main.c b/src/installer/main.c index bc0fdaa67..fabc0ef52 100644 --- a/src/installer/main.c +++ b/src/installer/main.c @@ -909,7 +909,7 @@ int main(int argc, char *argv[]) { } // Umount source drive and eject
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); // Free downloaded ISO image if (strcmp(sourcedrive, SOURCE_TEMPFILE) == 0) {
Hello Michael,
interesting. I understood that wiki page the other way; a "looks good to me" intuitively sounds weaker than "I have reviewed this and am okay with it" to me. Glad you mentioned this.
Thanks, and best regards, Peter Müller
Isn’t an Ack a stronger kind of Review?
https://wiki.ipfire.org/devel/git/tags
However, thanks for looking at this.
-Michael
On 17 Nov 2021, at 20:11, Peter Müller peter.mueller@ipfire.org wrote:
Hello Michael,
lacking C skills, this one looks good to me, but I am not confident enough to tag it as being reviewed.
Acked-by: Peter Müller peter.mueller@ipfire.org
Thanks, and best regards, Peter Müller
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/installer/hw.c | 113 +++++++++++++++++++++++++++++-------------- src/installer/hw.h | 2 +- src/installer/main.c | 2 +- 3 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/installer/hw.c b/src/installer/hw.c index 71a1f1cce..265df2d8c 100644 --- a/src/installer/hw.c +++ b/src/installer/hw.c @@ -46,13 +46,6 @@ #include "hw.h" -const char* other_filesystems[] = {
- "/dev",
- "/proc",
- "/sys",
- NULL
-};
static int system_chroot(const char* output, const char* path, const char* cmd) { char chroot_cmd[STRING_SIZE]; @@ -149,14 +142,53 @@ int hw_mount(const char* source, const char* target, const char* fs, int flags) return mount(source, target, fs, flags, NULL); } -int hw_umount(const char* target) {
- int r = umount2(target, 0);
+static int hw_bind_mount(const char* source, const char* prefix) {
- if (!source || !prefix) {
errno = EINVAL;return 1;- }
- if (r && errno == EBUSY) {
// Give it a moment to settlesleep(1);
- char target[PATH_MAX];
- int r;
- // Format target
- r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);
- if (r < 0)
return 1;
r = umount2(target, MNT_FORCE);
- // Ensure target exists
- mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);
- return hw_mount(source, target, NULL, MS_BIND);
+}
+int hw_umount(const char* source, const char* prefix) {
- char target[PATH_MAX];
- int r;
- if (prefix)
r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);- else
r = snprintf(target, sizeof(target) - 1, "%s", source);- if (r < 0)
return r;- // Perform umount
- r = umount2(target, 0);
- if (r) {
switch (errno) {// Try again with force if umount wasn't successfulcase EBUSY:sleep(1);r = umount2(target, MNT_FORCE);break;// target wasn't a mountpoint. Ignore.case EINVAL:r = 0;break;}@@ -174,7 +206,7 @@ static int hw_test_source_medium(const char* path) { ret = access(SOURCE_TEST_FILE, R_OK); // Umount the test device.
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); return ret;
} @@ -881,20 +913,21 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { } // bind-mount misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs);
- r = hw_bind_mount("/dev", prefix);
- if (r)
return r;
mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);r = hw_mount(*otherfs, target, NULL, MS_BIND);if (r) {hw_umount_filesystems(dest, prefix);
- r = hw_bind_mount("/proc", prefix);
- if (r)
return r;
return r;}
- r = hw_bind_mount("/sys", prefix);
- if (r)
return r;
otherfs++;- }
- r = hw_bind_mount("/sys/firmware/efi/efivars", prefix);
- if (r && errno != ENOENT)
return r;} @@ -908,16 +941,14 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { // ESP if (*dest->part_boot_efi) {
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT_EFI);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT_EFI, prefix);
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT, prefix);@@ -928,16 +959,24 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { } // misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs++);r = hw_umount(target);if (r)return -1;- }
- r = hw_umount("/sys/firmware/efi/efivars", prefix);
- if (r)
return -1;- r = hw_umount("/sys", prefix);
- if (r)
return -1;- r = hw_umount("/proc", prefix);
- if (r)
return -1;- r = hw_umount("/dev", prefix);
- if (r)
return -1;
- r = hw_umount(prefix);
- r = hw_umount(prefix, NULL); if (r) return -1;
diff --git a/src/installer/hw.h b/src/installer/hw.h index 9fe69271e..b11dfa48f 100644 --- a/src/installer/hw.h +++ b/src/installer/hw.h @@ -108,7 +108,7 @@ struct hw* hw_init(); void hw_free(struct hw* hw); int hw_mount(const char* source, const char* target, const char* fs, int flags); -int hw_umount(const char* target); +int hw_umount(const char* source, const char* prefix); char* hw_find_source_medium(struct hw* hw); diff --git a/src/installer/main.c b/src/installer/main.c index bc0fdaa67..fabc0ef52 100644 --- a/src/installer/main.c +++ b/src/installer/main.c @@ -909,7 +909,7 @@ int main(int argc, char *argv[]) { } // Umount source drive and eject
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); // Free downloaded ISO image if (strcmp(sourcedrive, SOURCE_TEMPFILE) == 0) {
Hmm, difficult. Reading it again it is a little bit unclear.
I suppose the Ack-by doesn’t quite fit into a bucket like Reported-by and Tested-by which are very clear.
It is indeed maybe a more relaxed review, but with a “I want this change” in it. So it makes sense that you used that.
Never mind me.
-Michael
On 19 Nov 2021, at 06:30, Peter Müller peter.mueller@ipfire.org wrote:
Hello Michael,
interesting. I understood that wiki page the other way; a "looks good to me" intuitively sounds weaker than "I have reviewed this and am okay with it" to me. Glad you mentioned this.
Thanks, and best regards, Peter Müller
Isn’t an Ack a stronger kind of Review?
https://wiki.ipfire.org/devel/git/tags
However, thanks for looking at this.
-Michael
On 17 Nov 2021, at 20:11, Peter Müller peter.mueller@ipfire.org wrote:
Hello Michael,
lacking C skills, this one looks good to me, but I am not confident enough to tag it as being reviewed.
Acked-by: Peter Müller peter.mueller@ipfire.org
Thanks, and best regards, Peter Müller
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/installer/hw.c | 113 +++++++++++++++++++++++++++++-------------- src/installer/hw.h | 2 +- src/installer/main.c | 2 +- 3 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/installer/hw.c b/src/installer/hw.c index 71a1f1cce..265df2d8c 100644 --- a/src/installer/hw.c +++ b/src/installer/hw.c @@ -46,13 +46,6 @@ #include "hw.h" -const char* other_filesystems[] = {
- "/dev",
- "/proc",
- "/sys",
- NULL
-};
static int system_chroot(const char* output, const char* path, const char* cmd) { char chroot_cmd[STRING_SIZE]; @@ -149,14 +142,53 @@ int hw_mount(const char* source, const char* target, const char* fs, int flags) return mount(source, target, fs, flags, NULL); } -int hw_umount(const char* target) {
- int r = umount2(target, 0);
+static int hw_bind_mount(const char* source, const char* prefix) {
- if (!source || !prefix) {
errno = EINVAL;return 1;- }
- if (r && errno == EBUSY) {
// Give it a moment to settlesleep(1);
- char target[PATH_MAX];
- int r;
- // Format target
- r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);
- if (r < 0)
return 1;
r = umount2(target, MNT_FORCE);
- // Ensure target exists
- mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);
- return hw_mount(source, target, NULL, MS_BIND);
+}
+int hw_umount(const char* source, const char* prefix) {
- char target[PATH_MAX];
- int r;
- if (prefix)
r = snprintf(target, sizeof(target) - 1, "%s/%s", prefix, source);- else
r = snprintf(target, sizeof(target) - 1, "%s", source);- if (r < 0)
return r;- // Perform umount
- r = umount2(target, 0);
- if (r) {
switch (errno) {// Try again with force if umount wasn't successfulcase EBUSY:sleep(1);r = umount2(target, MNT_FORCE);break;// target wasn't a mountpoint. Ignore.case EINVAL:r = 0;break;}@@ -174,7 +206,7 @@ static int hw_test_source_medium(const char* path) { ret = access(SOURCE_TEST_FILE, R_OK); // Umount the test device.
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); return ret;
} @@ -881,20 +913,21 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { } // bind-mount misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs);
- r = hw_bind_mount("/dev", prefix);
- if (r)
return r;
mkdir(target, S_IRWXU|S_IRWXG|S_IRWXO);r = hw_mount(*otherfs, target, NULL, MS_BIND);if (r) {hw_umount_filesystems(dest, prefix);
- r = hw_bind_mount("/proc", prefix);
- if (r)
return r;
return r;}
- r = hw_bind_mount("/sys", prefix);
- if (r)
return r;
otherfs++;- }
- r = hw_bind_mount("/sys/firmware/efi/efivars", prefix);
- if (r && errno != ENOENT)
return r;} @@ -908,16 +941,14 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { // ESP if (*dest->part_boot_efi) {
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT_EFI);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT_EFI, prefix);
snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);r = hw_umount(target);
r = hw_umount(HW_PATH_BOOT, prefix);@@ -928,16 +959,24 @@ int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) { } // misc filesystems
- char** otherfs = other_filesystems;
- while (*otherfs) {
snprintf(target, sizeof(target), "%s%s", prefix, *otherfs++);r = hw_umount(target);if (r)return -1;- }
- r = hw_umount("/sys/firmware/efi/efivars", prefix);
- if (r)
return -1;- r = hw_umount("/sys", prefix);
- if (r)
return -1;- r = hw_umount("/proc", prefix);
- if (r)
return -1;- r = hw_umount("/dev", prefix);
- if (r)
return -1;
- r = hw_umount(prefix);
- r = hw_umount(prefix, NULL); if (r) return -1;
diff --git a/src/installer/hw.h b/src/installer/hw.h index 9fe69271e..b11dfa48f 100644 --- a/src/installer/hw.h +++ b/src/installer/hw.h @@ -108,7 +108,7 @@ struct hw* hw_init(); void hw_free(struct hw* hw); int hw_mount(const char* source, const char* target, const char* fs, int flags); -int hw_umount(const char* target); +int hw_umount(const char* source, const char* prefix); char* hw_find_source_medium(struct hw* hw); diff --git a/src/installer/main.c b/src/installer/main.c index bc0fdaa67..fabc0ef52 100644 --- a/src/installer/main.c +++ b/src/installer/main.c @@ -909,7 +909,7 @@ int main(int argc, char *argv[]) { } // Umount source drive and eject
- hw_umount(SOURCE_MOUNT_PATH);
- hw_umount(SOURCE_MOUNT_PATH, NULL); // Free downloaded ISO image if (strcmp(sourcedrive, SOURCE_TEMPFILE) == 0) {
Reviewed-by: Peter Müller peter.mueller@ipfire.org
Signed-off-by: Michael Tremer michael.tremer@ipfire.org
src/initscripts/system/mountkernfs | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/initscripts/system/mountkernfs b/src/initscripts/system/mountkernfs index 264da24c4..1f1426077 100644 --- a/src/initscripts/system/mountkernfs +++ b/src/initscripts/system/mountkernfs @@ -39,6 +39,11 @@ case "${1}" in mount -t cgroup2 none /sys/fs/cgroup || failed=1 fi
if ! mountpoint /sys/firmware/efi/efivars &>/dev/null && [ -d "/sys/firmware/efi" ]; thenboot_mesg -n " /sys/firmware/efi/efivars" ${NORMAL}mount -t efivarfs efivarfs /sys/firmware/efi/efivars || failed=1fi- # create folder for dhcpcd changeroot mkdir -p /run/dhcpcd/chroot chown dhcpcd:dhcpcd /run/dhcpcd/chroot
-
Michael Tremer -
Peter Müller