Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names

[Stefan Hajnoczi]

On Thu, Jun 19, 2014 at 12:26:00PM -0400, Jeff Cody wrote:
> On Thu, Jun 19, 2014 at 05:17:16PM +0800, Stefan Hajnoczi wrote:
> > On Tue, Jun 17, 2014 at 05:53:48PM -0400, Jeff Cody wrote:
> > Let's discuss this topic in a sub-thread and figure out what to do for
> > QEMU 2.1.  This is an important issue to solve before the release
> > because we can't change QMP command semantics easily later.
> > 
> > My questions are:
> > a. How do we fix resize, snapshot-sync, etc?  It seems like we need to
> >    propagate child op blockers.
> > 
> > b. Is it a good idea to perform op blocker checks on the root node?
> >    It's inconsistent with resize, snapshot-sync, etc.  Permissions in
> >    BDS graphs with multiple root nodes (e.g. guest device and NBD
> >    run-time server) will be different depending on which root you
> >    specify.
> 
> I don't think (b) is the ultimate solution.  It is used as a stop-gap
> because op blockers in the current implementation is essentially
> analogous to the in-use flag.  But is it good enough for 2.1?  If
> *everything* checks the topmost node in 2.1, then I think we are OK in
> all cases except where images files share a common BDS.

Checking op blockers on the root node as a stop-gap is a good idea.
Let's apply it across all commands (e.g. snapshot-sync, resize).

Fam pointed out that this approach is vulnerable to blockdev-add, where
blockers could be set/checked on an incomplete BDS graph (since you can
add new nodes on top).  Do we need to move the blockers up the graph if
a new root node is inserted?

Besides this issue, your approach seems like the quickest safe solution
for 2.1.

> The ability for internal BDSs to share a common base BDS makes some
> block jobs unsafe currently, I believe.  A crude and ugly fix is to
> only allow a single block-job to occur at any given time, but that
> doesn't seem feasible, so let's ignore that.

Right now I don't think we share BDS chains.

Stefan

pgpyamPBOQcft.pgp
Description: PGP signature