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

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 22/03/2016 11:25, Markus Armbruster wrote:
>> 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.
>
> This means that the BlockBackend should not own the DriveInfo.  The
> backend and frontend need not know of the object that mixes concepts
> from both of them.  Instead, the DriveInfo can instantiate itself into a
> BlockBackend and the board can (if required) use the frontend parts of
> DriveInfo to instantiate a device and connect it to the BlocKBackend.

You missed or are glossing over the "letting devices fall back to legacy
configuration" part.  Let me explain it in more detail, using frontend
property "serial" as example.  You can use -drive parameter serial to
control it for many devices.

Example: -drive if=ide,index=3,file=tmp.qcow2,serial=Stockhausen

The board examines the if=ide drives and creates frontends for them.  It
could certainly recognize -drive parameter serial and configure the
frontend accordingly.  However, it doesn't.  To show you why, I need
another example.

Example: -drive if=none,id=ide3,file=tmp.qcow2,serial=Stockhausen \
-device ide-hd,bus=ide.1,unit=1,drive=ide3

The board is not involved here.  Instead, the *frontend* implements the
legacy fallback: if its property "serial" isn't set, it checks whether
its backend's DriveInfo has a serial, and if yes, it uses that.  Only
some frontends do that, namely the ones where the legacy configuration
actually needs to be preserved.  Newer ones don't.  Look for
blkconf_serial().

> In Kevin's idea there would be no ownership either way.  Until then, I
> think my patch actually gets us closer to the ideal.

I'm afraid it gets us closer to where we used to be six years ago :)

Qdev drive properties used to point to a DriveInfo, and the DriveInfo
pointed to BlockDriverState.  Commit f8b6cc0 cut out the DriveInfo
middleman.  This was a tiny step towards DriveInfo-less blockdev-add.

DriveInfo is legacy configuration.  Tacking it to BlockBackend is simple
and convenient.  If it ceases to be simple and convenient, we can try to
find another home.  But it really has no life of its own!  It's
ancillary information for whatever -drive creates (currently:
BlockBackend), and therefore should simply die with whatever it is
ancillary to.  It's owned by that, not the other way round.

Now, you can certainly take a reference without being the owner.  But my
review comment wasn't so much about ownership, it was more about the
oddness of taking a reference (in the sense of incrementing the
reference count) without actually *having* a reference (in the sense of
a pointer to the reference-counted object).  I find that confusing.

Confusion can be countered with a comment.  However, I still don't
understand why we need to take this new reference.  Can you explain?

>> 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.