Re: [Qemu-devel] [PATCH 3/3] block: remove legacy_dinfo at blk_detach_dev time

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 21.03.2016 um 19:14 hat Markus Armbruster geschrieben:
>> Paolo Bonzini <address@hidden> writes:
>> 
>> > On 21/03/2016 18:30, Markus Armbruster wrote:
>> >> However, I've now tested my expectation, and it turned out to be wrong.
>> >> I'm inclined to call that a bug.
>> >
>> > --verbose, what is wrong and what was your expectation?
>> 
>> x-blockdev-del should refuse to touch anything not created with
>> blockdev-add.
>> 
>> >> > In other words, you said "This looks like DriveInfo now owns a reference
>> >> > to BlockBackend, even though the pointer still goes in the other
>> >> > direction".  I say, "I thought this was the idea all along"...
>> >> 
>> >> For me, the DriveInfo doesn't own anything, but a BlockBackend may have
>> >> a DriveInfo.  Evidence:
>> >> 
>> >> * The pointer goes from the BlockBackend to the DriveInfo
>> >> 
>> >> * To go back, you search the blk_backends for the one that has the
>> >>   DriveInfo.  See blk_by_legacy_dinfo().
>> >> 
>> >> * There is no list of DriveInfo.  If you want to find one, you search
>> >>   blk_backends.  See drive_get() & friends.
>> >
>> > That's from the point of view of the code.  But from the point of view
>> > of the user, he specifies a drive=... and the device converts that under
>> > the hood to a BlockBackend; and when he calls drive_del on an unassigned
>> > drive, the BlockBackend is destroyed.
>> >
>> > There is no action on a BlockBackend that destroys the
>> > DriveInfo---except auto-deletion on unplug, but even then the user in
>> > the first place had provided a DriveInfo.  So from the point of view of
>> > the user it's always been the DriveInfo that owned a BlockBackend.  The
>> > lack of a list of DriveInfo is just an implementation detail.
>> 
>> From the user's point of view, neither BlockBackend nor DriveInfo are
>> visible :)
>> 
>> A BlockBackend may have a DriveInfo.  If it has one, then destroying the
>> BlockBackend also destroys its DriveInfo.
>> 
>> DriveInfo exists only to capture a -drive in a more convenient form than
>> its QemuOpts.  We use it for creating a BlockBackend.  It lives on after
>> that only because -drive mixes up front- and backend matter.  Keeping
>> DriveInfo around hanging off BlockBackend lets us keep frontend matter
>> out of BlockBackend: if need to access mixed up frontend matter for
>> back-compat, we find it in the BlockBackend's DriveInfo.
>> 
>> Imagine a future where we drop -drive / drive_add, or at least its mixed
>> up aspects (doesn't have to be practical for imagining it).  In that
>> future, we'd also drop DriveInfo.
>
> While we're dreaming up things... Imagine a future where users don't
> have to know about BlockBackend, but devices automatically create their
> BlockBackend. Which happens to be something that I'd really like to have
> and at least I haven't seen the show stopper for it yet. Which might
> just be because we never really looked much into the details, but
> anyway...
>
> In this case, the relationship between DriveInfo and BlockBackend
> couldn't be "BB owns DriveInfo" any more, because the BB would be
> created later than the DriveInfo. (DriveInfo controls the creation of
> the guest device which would create the BB during its initialisation.)

I don't think that would necessarily affect ownership.

Regardless of how and when we create BlockBackend, we'll want to keep
the clean separation between frontend and backend internally and at the
user interface.

DriveInfo has no role in cleanly separate creation of frontend and
backend now, and it shouldn't get one in the future.  Its purpose is to
support the legacy user interface that has frontend and backend matters
mixed up.  Two things, actually:

* Letting board code find legacy drive configuration information, so it
  can create their frontends.  This code lingers because we still
  haven't reduced legacy drives to sugar for modern configuration.  I
  believe we haven't tried because we got a number of boards nobody
  wants to touch.  That's our old "stuff is too precious to retire, yet
  too worthless for anybody spending the time it takes to update it to
  modern interfaces" problem.

* Letting devices fall back to legacy configuration.  Things like "if
  qdev property "serial" isn't given, try getting the serial number from
  DriveInfo.

The way we do this now is to package up the relevant parts of QemuOpts
as a DriveInfo and stick them to the backend when we create the backend
from the QemuOpts.  "The backend" here is a BlockBackend.  If we change
that to be something else, I guess we'll tack the DriveInfo to whatever
that something else may be.

Reverting the "owns" relationship to make DriveInfo own BlockBackend (or
whatever) makes no sense to me, because DriveInfo exists only for *some*
backends.  The others need an owner two.  I think that owner should own
all of them, not just the ones without a DriveInfo.

> By the way, what's the reason again for keeping DriveInfo around even
> after having created the guest device?

Inertia?

I know it's accessed from some realize() methods.  We'd have to review
whether it's accessed after realize().