From owner-svn-src-all@freebsd.org Mon Feb 18 14:31:25 2019 Return-Path: Delivered-To: svn-src-all@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 6708014DD807 for ; Mon, 18 Feb 2019 14:31:25 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound3d.ore.mailhop.org (outbound3d.ore.mailhop.org [54.186.57.195]) (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 CDA9F6A6D3 for ; Mon, 18 Feb 2019 14:31:24 +0000 (UTC) (envelope-from ian@freebsd.org) ARC-Seal: i=1; a=rsa-sha256; t=1550500250; cv=none; d=outbound.mailhop.org; s=arc-outbound20181012; b=Xrg/m/lGJwtL3EtJzKWwQbrJ4s8qjPIL21hv/46T2Ic1wNd1B8g5ecrQhuAxi57c8aEw28SeUanKk 432BuNeKmCRUHFu4JF2LUJP1CsJfFaG33datLOEwm4MzCPRIYKeQX6GkUZMaN6IA/S6H9Oy6iN3MeA BToE3vmkYl3k4DFSxOWNFSesrIXvD02Hvb3DzS2teiCAR166Law66pLZEwysr3OCtRqgqbkoMYtOw0 IB2tRuB8eAyCM/E8Or1DDbgog6aWoObFZamQEztymEdORwTPwI2OuziHU888uM/z+dBIT06pZFFtmX eKGHwJ1fpeGa/6MOm298I1NMC90kLog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=arc-outbound20181012; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:dkim-signature:from; bh=fWSqC37P5KAebJ7w2S/guvAEiOlVYUL4ahocWFLLBe0=; b=qrrD5kkZrRxm/T3Agp+OhQxi0vqLhiXaMhyQcnT3jbS+w2N6zigp5gCLHgpJMum0MuLskCeBw+X8w QTC56i8VVx1MucszJpUGrxYre4IndR6tz9UExnUuPPbLlCSBkcRBuFlzzGyjDyyN25ntC1FNJtuiOQ xtDYQrwubmhUx0ZL05vJ+TZSrRpJCjlLrUMzAGtvwrtQrhj0KmewApoSDLCziPdFeH5f36o/LX46yw YoZXes9ykYYyPfxJskqyWikUzt4gMKb+4KTZ9MnFC7ALn1kaL+nm0UjhRZxyeO0Ebz0o6j4jLz/9IK dmKgE+FnauotzGKB6yyTIuVS3OJlf0A== ARC-Authentication-Results: i=1; outbound3.ore.mailhop.org; spf=softfail smtp.mailfrom=freebsd.org smtp.remote-ip=67.177.211.60; dmarc=none header.from=freebsd.org; arc=none header.oldest-pass=0; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=dkim-high; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:from; bh=fWSqC37P5KAebJ7w2S/guvAEiOlVYUL4ahocWFLLBe0=; b=fRlWFcTbhxNJApcaixu6nihuEtuw7eS7x+iOBxTRwHcucJtonjpUdvV+B3XtLC0Pfg40GG+Wq74kZ pUiZHIEfMmFEwOgVYowElh/MSQt6F0hcuPNfLCbVxf9OHKyQ1O88Porl/aAG6epbA9S0iL6N9tgHkI zbZhWtMGm1UccM6/ZrhekpN5j8FnuE3FoYCwCCTTf7jUCtIWPR05Hu+EVpXL0pmr0mhkA8x3lKybp2 Amge7n7TxJoiObITT0HnoGcnoYKVST/VHMCHudq+c9lHpOx6UH5njav9RP3PF+2EW3ZfwUIGKeyiZM 8a/JRdJ4xQTQZbAeZvV3MieQHc/2YCA== X-MHO-RoutePath: aGlwcGll X-MHO-User: c85eba61-3389-11e9-9789-75353a1f43cf X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound3.ore.mailhop.org (Halon) with ESMTPSA id c85eba61-3389-11e9-9789-75353a1f43cf; Mon, 18 Feb 2019 14:30:48 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id x1IEVKMO078152; Mon, 18 Feb 2019 07:31:20 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: <4283f17c1841561a41a03a9203ab295564aa0ba5.camel@freebsd.org> Subject: Re: svn commit: r344238 - head/stand/common From: Ian Lepore To: Toomas Soome Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 18 Feb 2019 07:31:20 -0700 In-Reply-To: <2EAE5156-2C63-47FC-977F-0D9676CABF7F@me.com> References: <201902172332.x1HNW9ut059440@repo.freebsd.org> <2EAE5156-2C63-47FC-977F-0D9676CABF7F@me.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 FreeBSD GNOME Team Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: CDA9F6A6D3 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org 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-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Feb 2019 14:31:25 -0000 On Mon, 2019-02-18 at 15:09 +0200, Toomas Soome wrote: > > On 18 Feb 2019, at 01:32, Ian Lepore wrote: > > > > Author: ian > > Date: Sun Feb 17 23:32:09 2019 > > New Revision: 344238 > > URL: https://svnweb.freebsd.org/changeset/base/344238 > > > > Log: > > Restore loader(8)'s ability for lsdev to show partitions within a bsd slice. > > > > 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. > > > > 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. > > > > 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). > > Of course I can see the appeal for something like “boot disk0:” 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. > I have no idea. As is so often the case, the commit message for the revert said what the commit did ("revert to historic behavior") without any hint of why the change was made. One has to assume that it broke some working cases and people complained. Part of the problem for the disk_open() "api" is that there is not even a comment block with some hints in it. I was thinking one potential solution might instead of using "if (partition < 0)" we could define a couple magical partition number values, PNUM_GETBEST = -1, PNUM_RAWSLICE = -2, that sort of thing. But it would require carefully combing through the existing code looking at all calls to disk_open() and all usage of disk_devdesc.d_partition. -- Ian