[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names |
Date: |
Tue, 24 Jun 2014 17:30:53 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
The Tuesday 24 Jun 2014 à 16:08:57 (+0200), Kevin Wolf wrote :
> Am 24.06.2014 um 15:32 hat Jeff Cody geschrieben:
> > On Tue, Jun 24, 2014 at 10:48:52AM +0800, Fam Zheng wrote:
> > > On Mon, 06/23 21:08, Stefan Hajnoczi wrote:
> > > > 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?
> > >
> > > My concern is if we allow adding new root on top, it's not easy to know
> > > the
> > > real root then.
> > >
> > > To give an example:
> > >
> > > If we have
> > >
> > > [base id=""] <- [active id="drive0" blockers=...]
> > >
> > > When user does
> > >
> > > (QMP) block-commit device="drive0" ...
> > >
> > > We should check drive0, which is OK.
> > >
> > > Then, assume user adds a new root on top, we would take care of moving the
> > > blockers:
> > >
> > > [base id=""] <- [active id="drive0"] <- [active id="drive1" blockers=]
> > >
> > > At this point, what if user does something on drive0 again?
> > >
> > > (QMP) block-commit device="drive0" ...
> > >
> > > The right thing to do is to check blockers on "drive1", since it's the
> > > real
> > > root now. But how do we know? Do we need to add a back reference pointer
> > > ->overlap_hd in BDS, or do we maintain a look up table, or do we search
> > > all BDS
> > > graphs to figure out?
> > >
> > > None is easier than if we put the blockers in the bottom BDS, in the first
> > > place:
> > >
> > > [base id="" blockers=...] <- [active id="drive0"]
> > > ^^^^^^^^^^^^
> > >
> >
> > I think you are right. If we place the blocker at the bottom-most
> > BDS, then that would be a more restrictive blocker. This may end up
> > being more restrictive than needed, but more importantly it should
> > make everything safe.
> >
> > Also, it is an easy change for 2.1 - just call bdrv_find_base(bs), and
> > set/check/clear blockers on the returned BDS.
>
> What does bdrv_find_base() return for e.g. quorum?
This will not work when unblocking a BDS loop like the one formed by
drive-mirror
when replacing an arbitrary node.
>
> Kevin
>
> > > Even if user adds a new root, we don't need to worry about moving
> > > blockers,
> > > because the bottom is not changed.
> > >
> > > [base id="" blockers=...] <- [active id="drive0"] <- [active
> > > id="drive1"]
> > >
> > > Checking the blockers are easy, either for drive0 or drive1: just follow
> > > the
> > > backing chain until getting to the end.
> > >
> > > Fam
>
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, (continued)
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Jeff Cody, 2014/06/19
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Stefan Hajnoczi, 2014/06/23
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Benoît Canet, 2014/06/23
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Fam Zheng, 2014/06/23
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Jeff Cody, 2014/06/24
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Kevin Wolf, 2014/06/24
- Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names,
Benoît Canet <=
Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Benoît Canet, 2014/06/19
Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names, Jeff Cody, 2014/06/24