[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 6/6] block: Fix permissions after b
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 6/6] block: Fix permissions after bdrv_reopen() |
Date: |
Mon, 18 Sep 2017 11:35:57 +0200 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
Am 15.09.2017 um 21:06 hat Eric Blake geschrieben:
> On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> > If we switch between read-only and read-write, the permissions that
> > image format drivers need on bs->file change, too. Make sure to update
> > the permissions during bdrv_reopen().
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > include/block/block.h | 1 +
> > block.c | 64
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 65 insertions(+)
> >
>
> > +static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue
> > *q,
> > + BdrvChild *c)
> > +{
> > + BlockReopenQueueEntry *entry;
> > +
> > + QSIMPLEQ_FOREACH(entry, q, entry) {
> > + BlockDriverState *bs = entry->state.bs;
> > + BdrvChild *child;
> > +
> > + QLIST_FOREACH(child, &bs->children, next) {
> > + if (child == c) {
> > + return entry;
>
> An O(n^2) loop. Is it going to bite us at any point in the future, or
> are we generally dealing with a small enough queue size and BDS graph to
> not worry about it?
The loops you're quoting aren't O(n^2), they don't loop over the same
thing. This part is O(n) in terms of BdrvChild elements looked at.
The thing that worried me a bit more is the caller:
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ parent = find_parent_in_reopen_queue(q, c);
This is indeed O(n^2) (again with n = number of BdrvChild elements) in
the pathological worst case of a quorum node where all children point to
the same node.
As soon as all parents of the node are distinct - and I don't see any
reason why they wouldn't in practice - we're back to O(n) because each
BdrvChild belongs to only one parent. Even if we ever introduce a driver
where having the same node as a child in a constant number of different
roles makes sense for a parent (i.e. anything that doesn't involve an
(unbounded) array of children), we would still be O(n) with an additional
small constant factor.
So I think in practice we should be okay.
Kevin
signature.asc
Description: PGP signature
- [Qemu-block] [PATCH 3/6] block: Add reopen queue to bdrv_check_perm(), (continued)
- [Qemu-block] [PATCH 3/6] block: Add reopen queue to bdrv_check_perm(), Kevin Wolf, 2017/09/15
- [Qemu-block] [PATCH 4/6] block: Base permissions on rw state after reopen, Kevin Wolf, 2017/09/15
- [Qemu-block] [PATCH 2/6] block: Add reopen_queue to bdrv_child_perm(), Kevin Wolf, 2017/09/15
- [Qemu-block] [PATCH 5/6] block: reopen: Queue children after their parents, Kevin Wolf, 2017/09/15
- [Qemu-block] [PATCH 6/6] block: Fix permissions after bdrv_reopen(), Kevin Wolf, 2017/09/15
- [Qemu-block] [PATCH 7/6] qemu-iotests: Test change-backing-file command, Kevin Wolf, 2017/09/15
- Re: [Qemu-block] [PATCH 0/6] block: Fix permissions after ro/rw reopen, Fam Zheng, 2017/09/18
Re: [Qemu-block] [PATCH 0/6] block: Fix permissions after ro/rw reopen, Kevin Wolf, 2017/09/20