Re: [Qemu-devel] [PATCH v3 03/23] block: Connect BlockBackend to BlockDriverState

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 23.09.2014 um 14:52 hat Markus Armbruster geschrieben:
>> Kevin Wolf <address@hidden> writes:
>> 
>> > Am 22.09.2014 um 18:34 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf <address@hidden> writes:
>> >> 
>> >> > Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben:
>> >> >> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> >> >> index 8d86a6c..14e0b7c 100644
>> >> >> --- a/include/block/block_int.h
>> >> >> +++ b/include/block/block_int.h
>> >> >> @@ -324,6 +324,8 @@ struct BlockDriverState {
>> >> >>      BlockDriver *drv; /* NULL means no media */
>> >> >>      void *opaque;
>> >> >>  
>> >> >> +    BlockBackend *blk;          /* owning backend, if any */
>> >> >> +
>> >> >>      void *dev;                  /* attached device model, if any */
>> >> >>      /* TODO change to DeviceState when all users are qdevified */
>> >> >>      const BlockDevOps *dev_ops;
>> >> >
>> >> > Just to make sure that we agree on where we're going: This makes the
>> >> > assumption that a BDS has at most one BB that owns it.
>> >> 
>> >> Yes.
>> >> 
>> >> >                                                        Which is not the
>> >> > final state that we want to have, so this will have to go away later.
>> >> 
>> >> I don't know.  Can you explain why you think we're going to want
>> >> multiple BBs?
>> >
>> > We already agreed that we'll have multiple parents for a BDS, for
>> > scenarios like having an NBD server on a snapshot or sharing backing
>> > files, potentially also some block jobs.
>> 
>> We certainly want to provide for multiple "users" (intentionally vague
>> language here), such NBD server, block jobs, device models.  Should they
>> share a BB, or does each one need its own BB?
>
> I think they should have their own BB, but I can still be convinced
> otherwise.
>
> The first reason is that frontends (= BB users, more or less) and
> backends are easiest to understand when they come in pairs. Having
> multiple frontends for a single backend might be confusion.
>
> Second, if the NBD server doesn't sit at the root but accesses a
> backing file, it already has to get its own BB with its own name and
> with no device model attached.  Doing the same at the root helps with
> consistency.

These are valid, but fairly weak.

> Third, we'll probably want to have some things like werror/rerror or I/O
> accounting handled separately for device models and NBD servers.

This one's pretty convincing.

> If we look at another type of users, we might easily find more reasons,
> but for me this is already a pretty strong indicator that shared BBs are
> probably not a good idea.
>
>> > The question is whether among these multiple parents we want to have a
>> > limitation to one BlockBackend, forbidding e.g. an NBD server on the
>> > active layer. This would be a problem for live storage migration if we
>> > don't want the NBD server to reuse the same BB as the guest device.
>> >
>> > More generally, if we can indirectly have multiple BBs on a single
>> > BDS by putting a filter in between, do we have good reasons to forbid
>> > having them attached directly?
>> 
>> Keeping code simple?
>> 
>> Not a valid argument when we *need* multiple BBs, i.e. when the answer
>> to my prior question is "each one needs its own BB".
>
> Are there more places than just the callbacks that would be complicated
> by multiple BBs per BDS?

We'll know when we're done lifting stuff from BDS into BB.

>> >> > What I would consider, however, is adding a TODO comment that tells
>> >> > people that this field needs to go and if you need to use it, something
>> >> > is wrong with your design (which happens to be true for the existing
>> >> > design of some code).
>> >> 
>> >> For the device callbacks, we need a way to find the BB.  If multiple BBs
>> >> can sit on top of the same BDS, we need to find the one with a device
>> >> models attached.  Ot even the ones, if we permit that.
>> >> 
>> >> Let's discuss this a bit, and depending on what we learn, add a suitable
>> >> comment.  Possibly on top.
>> >
>> > Are you sure that nothing else than device models can be interested in
>> > callbacks? I expect that whatever block layer user we have, they will
>> > always be interested in resizes, for example. Media change might also
>> > not be entirely uninteresting, though in most cases what other users
>> > want is probably a blocker.
>> 
>> I designed BlockDevOps for device models only.  If other users emerge,
>> it needs a rename, and possibly a rethink.
>
> Very likely to happen in the long run. Block jobs are today blocking
> resizes, but that's mostly because they don't have an easy way to
> respond to a resize.  Sooner or later someone will want to grow their
> images while they are being mirrored (which is a completely reasonable,
> even if not trivial, thing to want).
>
> Do we have a KVM Forum block layer agenda yet? I think this thread could
> already contain a few topics to discuss there.

No agenda yet, as far as I know.