NetBSD Problem Report #56354

From tsutsui@ceres.dti.ne.jp  Mon Aug  9 17:48:55 2021
Return-Path: <tsutsui@ceres.dti.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 5C6891A921F
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  9 Aug 2021 17:48:55 +0000 (UTC)
Message-Id: <202108091748.179HmkWn004810@ceres.dti.ne.jp>
Date: Tue, 10 Aug 2021 02:48:46 +0900 (JST)
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Reply-To: tsutsui@ceres.dti.ne.jp
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: sysinst fails to upgrade on miniroot (for sparc)
X-Send-Pr-Version: 3.95

>Number:         56354
>Category:       install
>Synopsis:       sysinst fails to upgrade on miniroot (for sparc)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    martin
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 09 17:50:00 +0000 2021
>Closed-Date:    Fri Aug 20 04:19:13 +0000 2021
>Last-Modified:  Fri Aug 20 04:19:13 +0000 2021
>Originator:     Izumi Tsutsui
>Release:        NetBSD 9.2
>Organization:
>Environment:
System: NetBSD legnum 9.2 NetBSD 9.2 (GENERIC) #0: Wed May 12 13:15:55 UTC 2021  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/sparc/compile/GENERIC sparc
Architecture: sparc, but possibly all with miniroot + sysinst
Machine: sparc
>Description:
On upgrade install of NetBSD 9.2, if the target machine has less than
32 MB (defined as TINY_RAM_SIZE) memory, sysinst silently refuses
to continue installation after selecting target disk.

do_upgrade() in src/usr.sbin/sysinst/upgrade.c calls set_swap_if_low_ram()
 https://nxr.netbsd.org/xref/src/usr.sbin/sysinst/upgrade.c?r=1.17#84
---
     65 	if (find_disks(msg_string(MSG_upgrade), !root_is_read_only()) < 0)
     66 		return;
     67 
 :
     83 
     84 	if (set_swap_if_low_ram(&install) < 0)
     85 		return;
---
after find_disks() and it returns silently if SWAP_ON for swpctl(2)
fails in set_swap() in src/usr.sbin/sysinst/disks.c:
 https://nxr.netbsd.org/xref/src/usr.sbin/sysinst/disks.c?r=1.74#1952
---
   1952 	rval = swapctl(SWAP_ON, swap_dev, 0);
   1953 	if (rval != 0) {
   1954 		swap_dev[0] = 0;
   1955 		return -1;
   1956 	}
---

So sysinst silently returns to the main manu and users cannot see
what's wrong.

>How-To-Repeat:
Upgrade install to NetBSD/sparc 9.2 using miniroot on <32MB RAM sparc.

>Fix:
Don't treat the failure of set_swap_if_low_ram() as fatal,
i.e. sysinst should just print warnings and continue?

Note do_install (i.e. new installation) doesn't check a return value of
the set_swap_if_low_ram():
 https://nxr.netbsd.org/xref/src/usr.sbin/sysinst/install.c?r=1.9.2.4#119
---
    118 	/* phase 3: now we may have a first chance to enable swap space */
    119 	set_swap_if_low_ram(install);
---

---
Izumi Tsutsui

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: install-manager->martin
Responsible-Changed-By: martin@NetBSD.org
Responsible-Changed-When: Mon, 09 Aug 2021 17:52:28 +0000
Responsible-Changed-Why:
take


From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: install/56354: sysinst fails to upgrade on miniroot (for sparc)
Date: Mon, 9 Aug 2021 19:53:23 +0200

 Side question:
 does the upgraded machine have a swap partition or is there a good
 reason why SWAP_ON fails?

 Martin

From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@netbsd.org
Cc: martin@netbsd.org, tsutsui@ceres.dti.ne.jp
Subject: Re: install/56354: sysinst fails to upgrade on miniroot (for sparc)
Date: Tue, 10 Aug 2021 03:04:11 +0900

 >  Side question:
 >  does the upgraded machine have a swap partition

 Yes. It's required to use miniroot.

 > or is there a good
 >  reason why SWAP_ON fails?

 Because /dev/sd0b is already mounted as a root partition on
 the miniroot installation.

 ---
 Izumi Tsutsui

From: Martin Husemann <martin@duskware.de>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@netbsd.org
Subject: Re: install/56354: sysinst fails to upgrade on miniroot (for sparc)
Date: Mon, 9 Aug 2021 20:05:53 +0200

 On Tue, Aug 10, 2021 at 03:04:11AM +0900, Izumi Tsutsui wrote:
 > >  Side question:
 > >  does the upgraded machine have a swap partition
 > 
 > Yes. It's required to use miniroot.
 > 
 > > or is there a good
 > >  reason why SWAP_ON fails?
 > 
 > Because /dev/sd0b is already mounted as a root partition on
 > the miniroot installation.

 Oh duh - that kind of miniroot - got it!

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56354 CVS commit: src/usr.sbin/sysinst
Date: Thu, 12 Aug 2021 09:33:59 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Aug 12 09:33:59 UTC 2021

 Modified Files:
 	src/usr.sbin/sysinst: defs.h disks.c upgrade.c

 Log Message:
 PR 56354: all actions to set up swap space are not guaranteed to gain
 us enough virtual memory anyway, so drop return codes from set_swap*.
 The state for cleanup (which swap dev to unuse) has been made global
 some time ago anyway.

 Previously use of the return values was inconsistent. Error reporting
 will only confuse users and sometimes the situation is hard to fix or
 even impossible (like in miniroots copide to swap space for booting).


 To generate a diff of this commit:
 cvs rdiff -u -r1.71 -r1.72 src/usr.sbin/sysinst/defs.h
 cvs rdiff -u -r1.74 -r1.75 src/usr.sbin/sysinst/disks.c
 cvs rdiff -u -r1.17 -r1.18 src/usr.sbin/sysinst/upgrade.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->pending-pullups
State-Changed-By: martin@NetBSD.org
State-Changed-When: Thu, 12 Aug 2021 09:39:53 +0000
State-Changed-Why:
[pullup-9 #1333]


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56354 CVS commit: [netbsd-9] src/usr.sbin/sysinst
Date: Thu, 19 Aug 2021 04:52:10 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Thu Aug 19 04:52:10 UTC 2021

 Modified Files:
 	src/usr.sbin/sysinst [netbsd-9]: defs.h disks.c upgrade.c

 Log Message:
 Pull up following revision(s) (requested by martin in ticket #1333):
 	usr.sbin/sysinst/defs.h: revision 1.72
 	usr.sbin/sysinst/disks.c: revision 1.75
 	usr.sbin/sysinst/upgrade.c: revision 1.18
 PR 56354: all actions to set up swap space are not guaranteed to gain
 us enough virtual memory anyway, so drop return codes from set_swap*.
 The state for cleanup (which swap dev to unuse) has been made global
 some time ago anyway.
 Previously use of the return values was inconsistent. Error reporting
 will only confuse users and sometimes the situation is hard to fix or
 even impossible (like in miniroots copide to swap space for booting).


 To generate a diff of this commit:
 cvs rdiff -u -r1.42.2.8 -r1.42.2.9 src/usr.sbin/sysinst/defs.h
 cvs rdiff -u -r1.44.2.15 -r1.44.2.16 src/usr.sbin/sysinst/disks.c
 cvs rdiff -u -r1.12.2.3 -r1.12.2.4 src/usr.sbin/sysinst/upgrade.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Fri, 20 Aug 2021 04:19:13 +0000
State-Changed-Why:
pullups completed, thanks all


>Unformatted:

Home	PR Database Search