svn commit: r299371 - in head: sbin/camcontrol sys/cam sys/cam/scsi

[Edward Tomasz Napierała]

On 0517T1158, Warner Losh wrote:
> On Tue, May 10, 2016 at 11:33 AM, Edward Tomasz Napierala <trasz at freebsd.org
> > wrote:
> 
> > On 0510T1020, Alan Somers wrote:
> > > On Tue, May 10, 2016 at 9:46 AM, Edward Tomasz Napierala <
> > trasz at freebsd.org>
> > > wrote:
> > >
> > > > Author: trasz
> > > > Date: Tue May 10 15:46:33 2016
> > > > New Revision: 299371
> > > > URL: https://svnweb.freebsd.org/changeset/base/299371
> > > >
> > > > Log:
> > > >   Add "camcontrol reprobe" subcommand, and implement it for da(4).
> > > >   This makes it possible to manually force updating capacity data
> > > >   after the disk got resized. Without it it might be neccessary to
> > > >   reboot before FreeBSD notices updated disk size under eg VMWare.
> > > >
> > > >   Discussed with:       imp@
> > > >   MFC after:    1 month
> > > >   Sponsored by: The FreeBSD Foundation
> > > >   Differential Revision:        https://reviews.freebsd.org/D6108
> > > >
> > > > Modified:
> > > >   head/sbin/camcontrol/camcontrol.8
> > > >   head/sbin/camcontrol/camcontrol.c
> > > >   head/sys/cam/cam_ccb.h
> > > >   head/sys/cam/cam_xpt.c
> > > >   head/sys/cam/scsi/scsi_da.c
> > > >
> > > >
> > >
> > > I too have been annoyed that "camcontrol rescan" won't update capacity
> > > data.  But could we solve the problem by simply adding logic to
> > "camcontrol
> > > rescan" instead of adding an entirely new command?  Would a user ever
> > want
> > > to rescan a device without reprobing it too?
> >
> > Two reasons.  First, I want to be able to pass the device name (like
> > 'da0') and not the CAM path (like 1:0:0) for usability reasons - it seems
> > easy to figure out the latter from the former, using "camcontrol devlist",
> > but it suddenly becomes complicated when you try to explain it in a man
> > page.
> 
> 
> You can look up one or the other. fwdownload uses the daX name.

Indeed.  But it would mean fixing "camcontrol rescan" first.

> > Second - I don't understand the "camcontrol rescan" logic well
> > enough, and "camcontrol rescan all" sometimes fails for me anyway,
> > in a way I'm not sure how to debug.
> >
> 
> That's a cop-out. CAM is hard, but if you aren't willing to figure itout,
> adding hacks that the other CAM maintainers have to cope with doesn't
> help.

That's true.  However, this hack is pretty non-intrusive - it only adds
a trivial amount of code, and the "reprobe" command could be replaced
with a simple alias to "rescan" if someone steps up to reimplement it.

> Also, to be honest I'm not sure those two are actually that related.
> > Rescanning is about discovering new devices on the bus.  "Reprobe"
> > is about updating... well, mostly updating the capacity.  The former
> > requires enumerating the bus using a mechanism built into XPT; the
> > latter is just notifying the periph driver (in this case da(4)) that
> > it needs to query the capacity and call disk_resize(4).
> >
> 
> The two are very related. Now we have two stupid paths in CAM instead of
> one.

We have two clearly separated code paths, doing completely different
things - one scanning the bus, and only notifying periph drivers if
new device is discovered, and the other one to notify existing periph
driver instances, without scanning anything.  I just don't see how
entangling them with each other would improve things.

> and you didn't do ada like I asked.

As I said in review, the ada(4) driver seems to lack resizing
capability.  It doesn't contain a call to disk_resize(9).  It's been
a few years since I've added resizing to da(4), but it took quite 
some time to make sure it interfaces with existing code in exactly
the right way.  I just don't have time for this kind of side quest
right now.  And I'm not even sure how to test it.  With da(4) it
was easy - I've just added LUN resizing to CTL.

> Not happy with this at all, but not asking for a back out.

Thanks.