[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared bas
From: |
Andrey Shinkevich |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node |
Date: |
Thu, 21 Mar 2019 17:05:47 +0000 |
On 21/03/2019 13:53, Kevin Wolf wrote:
> Am 20.03.2019 um 18:02 hat Alberto Garcia geschrieben:
>> On Wed 20 Mar 2019 10:16:10 AM CET, Kevin Wolf wrote:
>>>> Oh, I see. Let's use a shorter chain for simplicity:
>>>>
>>>> A <- B <- C <- D <- E
>>>
>>> Written from right to left, i.e. E being the base and A the top layer?
>>> We usually write things the other write round, I hope this doesn't get
>>> too confusing later.
>>
>> Oh my... yes, of course you're right, I should have written it the other
>> way around:
>>
>> E <- D <- C <- B <- A
>>
>>>> 1) If we stream first from E to C we add a filter F:
>>>>
>>>> A <- B <- F <- C <- D <- E
>>
>> ( which should have been written E <- D <- C <- F <- B <- A )
>>
>>>> Now we can't stream from C to A because F is on the way, and the F-C
>>>> link is frozen.
>>>
>>> Why is a frozen link a problem? The streaming operation isn't going to
>>> change this link, it just copies data from the subchain (including F
>>> and C) to A. This is not something that a frozen link should prevent.
>>
>> Not the operation itself, but the first thing that block-stream does is
>> freeze the chain from top (A) to base (C), so this would fail if there's
>> already a frozen link on the way (C <- F on this case?).
>
> Oh, I see. I think this is why I suggested a counter originally instead
> of a bool.
>
>>> So it seems frozen links allow the wrong case, but block the correct
>>> one? :-(
>>
>> Yes, we probably need to rethink this scenario a bit.
>
> But yes, even with a counter, the other problem would still remain (that
> the first job can't remove the filter on completion because the second
> one has frozen its link to the filter).
With this example E <- D <- C <- F <- B <- A,
In the current implementation of the copy-on-read filter,
its bs->backing is not initialized (while it is not true for the filter
in block-commit job). So, bdrv_freeze_backing_chain() doesn't go beyond
the cor-filter node. With the two parallel block-stream jobs, we get the
following sub-chains frozen:
F <- B <- A
E <- D <- C
as C <- F backing BdrvChild link doesn't exist.
If the cor-filter is inserted with the bdrv_append(), we get two
BdrvChild links (file and backing) pointed to the same BlockDriverState
'C' and additionally some child-permissions issues that I have not
resolved yet...
Due to the fact mentioned above, freezing the backing chain works with
the filter inserted. But, with the one BdrvChild *file link only in the
BlockDriverState of the cor-filter, we encounter a broken chain each
time we iterate through it with the backing_bs(F) (=NULL) in many other
pieces of the code. In turn, it breaks the existing model.
That's the point! (((
What can we do with that?
In my patch Stream-block-job-involves-copy-on-read-filter.patch :
static BlockDriverState *child_file_bs(BlockDriverState *bs)
{
return bs->file ? bs->file->bs : NULL;
}
static BlockDriverState *skip_filter(BlockDriverState *bs)
{
BlockDriverState *ret_bs = bs;
while (ret_bs && ret_bs->drv && ret_bs->drv->is_filter) {
ret_bs = child_file_bs(ret_bs);
}
return ret_bs;
}
But the solution above looks clumsy to me.
I would appreciate to hear any other ideas from you.
Andrey
>
> I don't think that's a case we want to just forbid because nobody needs
> this anyway. It's a sign of a more fundamental problem in our design,
> and I'm sure it will bite us in other places, too.
>
> Kevin
>
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, (continued)
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Alberto Garcia, 2019/03/20
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Vladimir Sementsov-Ogievskiy, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Alberto Garcia, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Kevin Wolf, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Alberto Garcia, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Alberto Garcia, 2019/03/20
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Kevin Wolf, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Vladimir Sementsov-Ogievskiy, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Alberto Garcia, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Andrey Shinkevich, 2019/03/21
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node,
Andrey Shinkevich <=
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Vladimir Sementsov-Ogievskiy, 2019/03/22
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Kevin Wolf, 2019/03/22
- Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Andrey Shinkevich, 2019/03/21
Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node, Andrey Shinkevich, 2019/03/19