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

[Kevin Wolf]

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.

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

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?

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

Kevin