From owner-svn-src-head@freebsd.org Mon Feb 18 13:09:15 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 9366014DBBE8 for ; Mon, 18 Feb 2019 13:09:15 +0000 (UTC) (envelope-from tsoome@me.com) Received: from pv50p00im-zteg10021301.me.com (pv50p00im-zteg10021301.me.com [17.58.6.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 2C00E8E097 for ; Mon, 18 Feb 2019 13:09:15 +0000 (UTC) (envelope-from tsoome@me.com) Received: from [192.168.150.41] (148-52-235-80.sta.estpak.ee [80.235.52.148]) by pv50p00im-zteg10021301.me.com (Postfix) with ESMTPSA id C0B895800E8; Mon, 18 Feb 2019 13:09:05 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: svn commit: r344238 - head/stand/common From: Toomas Soome In-Reply-To: <201902172332.x1HNW9ut059440@repo.freebsd.org> Date: Mon, 18 Feb 2019 15:09:02 +0200 Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <2EAE5156-2C63-47FC-977F-0D9676CABF7F@me.com> References: <201902172332.x1HNW9ut059440@repo.freebsd.org> To: Ian Lepore X-Mailer: Apple Mail (2.3445.102.3) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-02-18_10:, , signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1812120000 definitions=main-1902180099 X-Rspamd-Queue-Id: 2C00E8E097 X-Spamd-Bar: ------ X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.985,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Feb 2019 13:09:15 -0000 > On 18 Feb 2019, at 01:32, Ian Lepore wrote: >=20 > Author: ian > Date: Sun Feb 17 23:32:09 2019 > New Revision: 344238 > URL: https://svnweb.freebsd.org/changeset/base/344238 >=20 > Log: > Restore loader(8)'s ability for lsdev to show partitions within a bsd = slice. >=20 > I'm pretty sure this used to work at one time, perhaps long ago. It = has > been failing recently because if you call disk_open() with = dev->d_partition > set to -1 when d_slice refers to a bsd slice, it assumes you want it = to > open the first partition within that slice. When you then pass that = open > dev instance to ptable_open(), it tries to read the start of the 'a' > partition and decides there is no recognizable partition type there. >=20 > This restores the old functionality by resetting d_offset to the = start > of the raw slice after disk_open() returns. For good measure, = d_partition > is also set back to -1, although that doesn't currently affect = anything. >=20 > I would have preferred to make disk_open() avoid such rude = assumptions and > if you ask for partition -1 you get the raw slice. But the commit = history > shows that someone already did that once (r239058), and had to revert = it > (r239232), so I didn't even try to go down that road. What was the reason for the revert? I still do think the current = disk_open() approach is not good because it does break the promise to = get MBR partition (see common/disk.h).=20 Of course I can see the appeal for something like =E2=80=9Cboot = disk0:=E2=80=9D but case like that should be solved by iterating = partition table, and not by making API to do wrong thing - if I did ask = to for disk0s0: I really would expect to get disk0s0: and not disk0s0a: But anyhow, it would be good to understand the actual reasoning behind = that decision. rgds, toomas >=20 > Modified: > head/stand/common/disk.c >=20 > Modified: head/stand/common/disk.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/stand/common/disk.c Sun Feb 17 20:25:07 2019 = (r344237) > +++ head/stand/common/disk.c Sun Feb 17 23:32:09 2019 = (r344238) > @@ -133,6 +133,13 @@ ptable_print(void *arg, const char *pname, const = struc > dev.d_partition =3D -1; > if (disk_open(&dev, part->end - part->start + 1, > od->sectorsize) =3D=3D 0) { > + /* > + * disk_open() for partition -1 on a bsd slice = assumes > + * you want the first bsd partition. Reset = things so > + * that we're looking at the start of the raw = slice. > + */ > + dev.d_partition =3D -1; > + dev.d_offset =3D part->start; > table =3D ptable_open(&dev, part->end - = part->start + 1, > od->sectorsize, ptblread); > if (table !=3D NULL) { >=20