From owner-freebsd-sysinstall@FreeBSD.ORG Tue Nov 23 07:08:13 2010 Return-Path: Delivered-To: freebsd-sysinstall@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4329B106566B; Tue, 23 Nov 2010 07:08:13 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 722408FC15; Tue, 23 Nov 2010 07:08:11 +0000 (UTC) Received: by wyb35 with SMTP id 35so100344wyb.13 for ; Mon, 22 Nov 2010 23:08:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:date:message-id :subject:from:to:cc:content-type; bh=395Wj8FG18Ka7CU5o5fCeRnipHtD0eNRT8bjMesPsME=; b=Dq6w+4S7j9mmSpaX2TIcIMG/GrPnZ3hpMWK3k5Ei489Vm8+KeR4H3qgU2Ywzroh6w4 /dNhSjM8Ts6bbFsz0SDnyyealcHdJxQv0CGXxNFfyhQx8e/umlpRtj0zP21S9WShNcAZ y19H74TQsLJDiHuvDhTH17TrnUeA5tbzK2sbI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:date:message-id:subject:from:to:cc:content-type; b=P3IpeWGfbDOTQd3M+TR+SptAbP59bl3uLrphR7TnjDNAD3/S+BSPQkLMLdIPkjY4h9 zG3GcS6GhE6MIVdkVol+L9INDLtyow8KbsSUI9Dc9iuA8aoHZylwGQED+Y+2eSh1h53n SqysGHUkjMQTwhUk/rap0FuLRmVrmBFo/gebU= MIME-Version: 1.0 Received: by 10.216.46.200 with SMTP id r50mr616179web.45.1290496091024; Mon, 22 Nov 2010 23:08:11 -0800 (PST) Received: by 10.216.198.27 with HTTP; Mon, 22 Nov 2010 23:08:10 -0800 (PST) Date: Mon, 22 Nov 2010 23:08:10 -0800 Message-ID: From: Garrett Cooper To: bug-followup@FreeBSD.org, peter@trumanbrewery.com Content-Type: multipart/mixed; boundary=0016364c7a1316ee150495b3092e Cc: Gavin Atkinson , freebsd-sysinstall@freebsd.org Subject: Re: bin/38854: sysinstall(8): resetting during setup causes the target installation path to change from "/mnt" (new root partition) to "/" (the memory disk) X-BeenThere: freebsd-sysinstall@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Sysinstall Work List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Nov 2010 07:08:13 -0000 --0016364c7a1316ee150495b3092e Content-Type: text/plain; charset=ISO-8859-1 Hi, I prototyped up a solution for this issue by banking on the fact that the *fork(2) system calls actually do a copy on write of several process variables, including the current working directory, etc. Example: $ ./test_forking_and_chdiring [0] getwd = /tmp [1] getwd = /usr/home/gcooper [2] getwd = /tmp [3] getwd = /usr/home/gcooper So at least now sysinstall is starting from a sane state consistently, instead of just reexecuting itself via system(3) with -fakeInit and -restart (ugh). I was concerned about sysinstall closing file descriptors on exec, etc, but this appears to be mostly baseless. Cleaning up these dead options should be done in later commits. I executed sysinstall from the command line in multiuser given the following 4 scenarios: 1. Exit (exit code was 0). 2. ^C: a. Continue (process continued as usual). b. Restart (process `restarted' by continuing on in the loop again -- it was rather amusing when I discovered that it reexecutes itself and can essentially overload the stack eventually if it gets into an infinite loop :/). c. Abort (process exited with 1 as I designed it to). There might be some corner cases with usage, but this focuses on the primary areas. I was hoping to use vfork(2) (because it would have made the logic a bit simpler, but unfortunately vfork isn't isolated enough to do proper signal handling with SIGINT (I think that the parent process and the child process share the same signal vectors, but I could be wrong... I should look). Ideally sysinstall should be run from a custom /etc/rc script that sets up all of these variables and ensures that the sysinstall process is playing by the rules, but that's something else that needs to be resolved down the line for this particular issue. This is just a quick and dirty stopgap fix. Credit goes to Gavin for pointing me to this annoying problem :). Thanks, -Garrett #include #include #include #include #include #include int main(void) { int status; switch (fork()) { case -1: err(1, "fork"); case 0: if (chdir("/tmp") == -1) err(1, "chroot"); printf("[0] getwd = %s\n", getwd(NULL)); exit(0); default: if (wait(&status) == -1) err(1, "wait"); else if (status) errx(1, "exit status != 0"); } printf("[1] getwd = %s\n", getwd(NULL)); switch (vfork()) { case -1: err(1, "vfork"); case 0: if (chdir("/tmp") == -1) err(1, "chroot"); printf("[2] getwd = %s\n", getwd(NULL)); exit(0); default: if (wait(&status) == -1) err(1, "wait"); else if (status) errx(1, "exit status != 0"); } printf("[3] getwd = %s\n", getwd(NULL)); exit(0); } --0016364c7a1316ee150495b3092e Content-Type: text/x-patch; charset=US-ASCII; name="bin.38854.patch" Content-Disposition: attachment; filename="bin.38854.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ggufvsd80 SW5kZXg6IHVzci5zYmluL3N5c2luc3RhbGwvc3lzdGVtLmMKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gdXNyLnNi aW4vc3lzaW5zdGFsbC9zeXN0ZW0uYwkocmV2aXNpb24gMjE1NjYxKQorKysgdXNyLnNiaW4vc3lz aW5zdGFsbC9zeXN0ZW0uYwkod29ya2luZyBjb3B5KQpAQCAtNTgsMjQgKzU4LDEwIEBACiBzdGF0 aWMgaW50CiBpbnRyX3Jlc3RhcnQoZGlhbG9nTWVudUl0ZW0gKnNlbGYpCiB7Ci0gICAgaW50IHJl dCwgZmQsIGZkbWF4OwotICAgIGNoYXIgKmFyZzsKLQogICAgIG1lZGlhQ2xvc2UoKTsKICAgICBm cmVlX3ZhcmlhYmxlcygpOwotICAgIGZkbWF4ID0gZ2V0ZHRhYmxlc2l6ZSgpOwotICAgIGZvciAo ZmQgPSAzOyBmZCA8IGZkbWF4OyBmZCsrKQotCWNsb3NlKGZkKTsKLSAgICAKLSAgICBpZiAoUnVu bmluZ0FzSW5pdCkKLQkgICAgYXJnID0gIi1yZXN0YXJ0IC1mYWtlSW5pdCI7Ci0gICAgZWxzZQot CSAgICBhcmcgPSAiLXJlc3RhcnQiOwotCi0gICAgcmV0ID0gZXhlY2woU3RhcnROYW1lLCBTdGFy dE5hbWUsIGFyZywgTlVMTCk7Ci0gICAgbXNnRGVidWcoImV4ZWNsIGZhaWxlZCAoJXMpXG4iLCBz dHJlcnJvcihlcnJubykpOwotICAgIC8qIE5PVFJFQUNIRUQgKi8KLSAgICByZXR1cm4gLTE7Cisg ICAgLyogV2UnbGwgcmV0dXJuIGJhY2sgdG8gbWFpbiBmcm9tIGhlcmUuICovCisgICAgX2V4aXQo MSk7CiB9CiAKIHN0YXRpYyBkaWFsb2dNZW51SXRlbSBpbnRybWVudVtdID0gewpJbmRleDogdXNy LnNiaW4vc3lzaW5zdGFsbC9tYWluLmMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gdXNyLnNiaW4vc3lzaW5zdGFs bC9tYWluLmMJKHJldmlzaW9uIDIxNTY2MSkKKysrIHVzci5zYmluL3N5c2luc3RhbGwvbWFpbi5j CSh3b3JraW5nIGNvcHkpCkBAIC0zNSwxMCArMzUsMTEgQEAKICAqLwogCiAjaW5jbHVkZSAic3lz aW5zdGFsbC5oIgotI2luY2x1ZGUgPHN5cy9zaWduYWwuaD4KICNpbmNsdWRlIDxzeXMvZmNudGwu aD4KKyNpbmNsdWRlIDxzeXMvcmVzb3VyY2UuaD4KICNpbmNsdWRlIDxzeXMvdGltZS5oPgotI2lu Y2x1ZGUgPHN5cy9yZXNvdXJjZS5oPgorI2luY2x1ZGUgPGVyci5oPgorI2luY2x1ZGUgPHNpZ25h bC5oPgogCiBjb25zdCBjaGFyICpTdGFydE5hbWU7CQkvKiBJbml0aWFsIGNvbnRlbnRzIG9mIGFy Z3ZbMF0gKi8KIGNvbnN0IGNoYXIgKlByb2dOYW1lID0gInN5c2luc3RhbGwiOwpAQCAtNTcsOCAr NTgsOSBAQAogICAgIGNoYXIgdGl0bGVzdHJbODBdLCAqYXJjaCwgKm9zcmVsLCAqb3N0eXBlOwog ICAgIHN0cnVjdCBybGltaXQgcmxpbTsKICAgICBjaGFyICphcmc7Ci0gICAgaW50IGk7CisgICAg aW50IGV4aXRfc3RhdHVzLCBpOwogICAgIGludCBvcHRpb25BcmdzID0gMDsKKyAgICBwaWRfdCBy diwgc3lzaW5zdGFsbF9wcm9wZXI7CiAKICAgICAvKiBSZWNvcmQgbmFtZSB0byBiZSBhYmxlIHRv IHJlc3RhcnQgKi8KICAgICBTdGFydE5hbWUgPSBhcmd2WzBdOwpAQCAtODgsOCArOTAsNzMgQEAK ICAgICB9CiAKICAgICBpZiAoZ2V0cGlkKCkgPT0gMSkKLQkgICAgUnVubmluZ0FzSW5pdCA9IFRS VUU7Ci0gICAKKwlSdW5uaW5nQXNJbml0ID0gVFJVRTsKKworICAgIC8qIFdlIGRvbid0IHdvcmsg dG9vIHdlbGwgd2hlbiBydW5uaW5nIGFzIG5vbi1yb290IGFueW1vcmUgKi8KKyAgICBpZiAoZ2V0 ZXVpZCgpICE9IDApIHsKKwlmcHJpbnRmKHN0ZGVyciwgIkVycm9yOiBUaGlzIHV0aWxpdHkgc2hv dWxkIG9ubHkgYmUgcnVuIGFzIHJvb3QuXG4iKTsKKwlyZXR1cm4gMTsKKyAgICB9CisKKyAgICAv KiAKKyAgICAgKiBUbyBzaW1wbGlmeSB0aGluZ3MgZGVhbGluZyB3aXRoIGNsZWFudXAgYXMgc3lz aW5zdGFsbCBpcyByZWVudHJhbnQKKyAgICAgKiAoc2FkbHkpLCBpdCdzIGp1c3QgZWFzaWVyIHRv IGZvcmsgYSBuZXcgcHJvY2VzcyBvbmNlIHdlIGtub3cgdGhhdAorICAgICAqIHdlJ3JlIHJvb3Qg YW5kIHJ1bm5pbmcgYXMgaW5pdC4KKyAgICAgKgorICAgICAqIFBsZWFzZSBkbyBub3QgbW92ZSB0 aGlzIGxvZ2ljLgorICAgICAqLworICAgIGRvIHsKKworCS8qIFJlc2V0IHRoZSBwcm9jZXNzIGdy b3VwIGp1c3QgaW4gY2FzZS4gKi8KKwlzZXRwZ2lkKDAsIGdldHBpZCgpKTsKKworCXN5c2luc3Rh bGxfcHJvcGVyID0gZm9yaygpOworCisJc3dpdGNoIChzeXNpbnN0YWxsX3Byb3BlcikgeworCWNh c2UgLTE6CisJICAgIGVycigxLCAiZm9yayBmYWlsZWQiKTsKKwljYXNlIDA6CisJICAgIGJyZWFr OworCWRlZmF1bHQ6CisKKwkgICAgLyogTGV0IHRoZSBjaGlsZCB0YWtlIHRoZSBoZWF0LiAqLwor CSAgICBzaWduYWwoU0lHSU5ULCBTSUdfSUdOKTsKKwkgICAgc2V0cGdycCgwLCBzeXNpbnN0YWxs X3Byb3Blcik7CisKKwkgICAgZG8geworCQlydiA9IHdhaXRwaWQoc3lzaW5zdGFsbF9wcm9wZXIs ICZleGl0X3N0YXR1cywgMCk7CisJCWlmIChlcnJubyA9PSBFSU5UUikKKwkJICAgIC8qIEJlIGtp bmQgdG8gdGhlIENQVSAqLworCQkgICAgc2xlZXAoMSk7CisJICAgIH0gd2hpbGUgKHJ2ID09IC0x ICYmIGVycm5vID09IEVJTlRSKTsKKworCSAgICBpZiAocnYgPT0gLTEpIHsKKwkJLyoKKwkJICog SnVzdCBpbiBjYXNlIChzZWUgRUNISUxEL1NBX05PQ0xEV0FJVCBlcnJvciB1bmRlciB3YWl0KDIp LgorCQkgKiBlcnJhbnQgc3lzaW5zdGFsbCBwcm9jZXNzZXMgPT4gYmFkLgorCQkgKi8KKwkJa2ls bCg5LCBzeXNpbnN0YWxsX3Byb3Blcik7CisJCWVycigxLCAid2FpdGluZyBmb3Igc3lzaW5zdGFs bCBwcm9wZXIgZmFpbGVkIik7CisJICAgIH0gZWxzZSBpZiAoZXhpdF9zdGF0dXMgIT0gMCkgewor CQkvKiBGb3IgZGV2ZWxvcGVyIGRlYnVnLiAqLworI2lmIDAKKwkJaWYgKFdJRlNJR05BTEVEKGV4 aXRfc3RhdHVzKSkgeworCQkgICAgZXJyeCgxLCAic3lzaW5zdGFsbCBwcm9wZXIgc2lnbmFsZWQg d2l0aCBzaWduYWwgPSAlZCIsCisJCQlXVEVSTVNJRyhleGl0X3N0YXR1cykpOworCQl9IGVsc2Ug aWYgKFdJRkVYSVRFRChleGl0X3N0YXR1cykpIHsKKwkJICAgIGVycngoMSwgInN5c2luc3RhbGwg cHJvcGVyIGV4aXRlZCB3aXRoIGV4aXQgY29kZSA9ICVkIiwKKwkJCVdFWElUU1RBVFVTKGV4aXRf c3RhdHVzKSk7CisJCX0KKyNlbmRpZgorCQlleGl0KDEpOworCSAgICB9IGVsc2UKKwkJLyogQWxs J3MgZ29vZCEgKi8KKwkJZXhpdCgwKTsKKwkgICAgYnJlYWs7CisJfQorCisgICAgfSB3aGlsZSAo c3lzaW5zdGFsbF9wcm9wZXIgIT0gMCk7CisKICAgICAvKiBDYXRjaCBmYXRhbCBzaWduYWxzIGFu ZCBjb21wbGFpbiBhYm91dCB0aGVtIGlmIHJ1bm5pbmcgYXMgaW5pdCAqLwogICAgIGlmIChSdW5u aW5nQXNJbml0KSB7CiAJc2lnbmFsKFNJR0JVUywgc2NyZWVjaCk7CkBAIC05NywxMiArMTY0LDYg QEAKICAgICB9CiAgICAgc2lnbmFsKFNJR1BJUEUsIFNJR19JR04pOwogCi0gICAgLyogV2UgZG9u J3Qgd29yayB0b28gd2VsbCB3aGVuIHJ1bm5pbmcgYXMgbm9uLXJvb3QgYW55bW9yZSAqLwotICAg IGlmIChnZXRldWlkKCkgIT0gMCkgewotCWZwcmludGYoc3RkZXJyLCAiRXJyb3I6IFRoaXMgdXRp bGl0eSBzaG91bGQgb25seSBiZSBydW4gYXMgcm9vdC5cbiIpOwotCXJldHVybiAxOwotICAgIH0K LQogICAgIC8qCiAgICAgICogR2l2ZW4gd2hhdCBpdCBkb2VzIHN5c2luc3RhbGwgKGFuZCBzdHVm ZiBzeXNpbnN0YWxsIHJ1bnMgbGlrZQogICAgICAqIHBrZ19hZGQpIHNob3VsZG4ndCBiZSBzdWJq ZWN0IHRvIHByb2Nlc3MgbGltaXRzLiAgQmV0dGVyIHRvIGp1c3QK --0016364c7a1316ee150495b3092e--