Re: [PATCH v5 5/8] blockdev: Add a new IF type IF_OTHER

[Kevin Wolf]

Am 28.07.2022 um 16:50 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben:
> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote:
> >> >
> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben:
> >> > > An OTP device isn't really a parallel flash, and neither are eFuses.
> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of
> >> > > other interface types, too.
> >> > >
> >> > > This patch introduces IF_OTHER.  The patch after next uses it for an
> >> > > EEPROM device.
> >> > >
> >> > > Do we want IF_OTHER?
> >> >
> >> > What would the semantics even be? Any block device that doesn't pick up
> >> > a different category may pick up IF_OTHER backends?
> >> >
> >> > It certainly feels like a strange interface to ask for "other" disk and
> >> > then getting as surprise what this other thing might be. It's
> >> > essentially the same as having an explicit '-device other', and I
> >> > suppose most people would find that strange.
> >> >
> >> > > If no, I guess we get to abuse IF_PFLASH some more.
> >> > >
> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash
> >> > > memory going forward.  Cleaning up existing abuse of IF_PFLASH may not
> >> > > be worth the trouble, though.
> >> > >
> >> > > Thoughts?
> >> >
> >> > If the existing types aren't good enough (I don't have an opinion on
> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific
> >> > new one, not just "other".
> >> 
> >> I think the common thread is "this isn't what anybody actually thinks
> >> of as being a 'disk', but we would like to back it with a block device
> >> anyway". That can cover a fair range of possibilities...
> >
> > How confident are we that no board will ever have two devices of this
> > kind?
> >
> > As long as every board has at most one, if=other is a bad user interface
> > in terms of descriptiveness, but still more or less workable as long as
> > you know what it means for the specific board you use.
> >
> > But if you have more than one device, it becomes hard to predict which
> > device gets which backend - it depends on the initialisation order in
> > the code then,
> 
> Really?  Board code should use IF_OTHER devices just like it uses the
> other interface types, namely connecting each frontend device to a
> backend device with a well-known and fixed interface type and index (or
> bus and unit instead, where appropriate).
> 
> >                and I'm pretty sure that this isn't something that should
> > have significance in external interfaces and therefore become a stable
> > API.
> 
> I agree that "implied by execution order" is a bad idea: commit
> 95fd260f0a "blockdev: Drop unused drive_get_next()".

Ah good, I was indeed thinking of something drive_get_next()-like.

In case the board works with explicit indices, the situation is not as
bad as I was afraid. It will certainly be workable (even if not obvious)
for any boards that have a fixed number of devices with block backends,
which should cover everything we're intending to cover here.

I still consider if=other a bad user interface because what it means is
completely opaque, but if that's okay for you in your board, who am I to
object.

(Of course, the real solution would be having a generic way to set qdev
properties for on-board devices. I'm not expecting that we're getting
this anytime soon, though.)

Kevin