This allows us to differentiate between filters and nodes with COW
backing files: Filters cannot be used as overlays at all (for this
function).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index b5c0fd3c49..0f0cf0d9ae 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1665,7 +1665,12 @@ static void external_snapshot_prepare(BlkActionState *common,
         goto out;
     }
 
-    if (state->new_bs->backing != NULL) {
+    if (state->new_bs->drv->is_filter) {
+        error_setg(errp, "Filters cannot be used as overlays");
+        goto out;
+    }
+
+    if (bdrv_filtered_cow_child(state->new_bs)) {
         error_setg(errp, "The overlay already has a backing image");
         goto out;
     }
-- 
2.21.0

13.06.2019 1:09, Max Reitz wrote:
> This allows us to differentiate between filters and nodes with COW
> backing files: Filters cannot be used as overlays at all (for this
> function).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Overlay created in snapshot operation assumed to consume following writes
and it's filtered child becomes readonly.. And filter works in completely another
way.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

[hmm, I start to like using "filtered child" collocation when I say about this thing.
  didn't you think about renaming backing chain to filtered chain?]

> ---
>   blockdev.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index b5c0fd3c49..0f0cf0d9ae 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1665,7 +1665,12 @@ static void external_snapshot_prepare(BlkActionState *common,
>           goto out;
>       }
>   
> -    if (state->new_bs->backing != NULL) {
> +    if (state->new_bs->drv->is_filter) {
> +        error_setg(errp, "Filters cannot be used as overlays");
> +        goto out;
> +    }
> +
> +    if (bdrv_filtered_cow_child(state->new_bs)) {
>           error_setg(errp, "The overlay already has a backing image");
>           goto out;
>       }
> 


-- 
Best regards,
Vladimir

On 14.06.19 17:46, Vladimir Sementsov-Ogievskiy wrote:
> 13.06.2019 1:09, Max Reitz wrote:
>> This allows us to differentiate between filters and nodes with COW
>> backing files: Filters cannot be used as overlays at all (for this
>> function).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Overlay created in snapshot operation assumed to consume following writes
> and it's filtered child becomes readonly.. And filter works in completely another
> way.
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> [hmm, I start to like using "filtered child" collocation when I say about this thing.
>   didn't you think about renaming backing chain to filtered chain?]

Hm.  There are backing chains and there are backing chains.  There are
qemu-internal backing chains that consist of a healthy mix of filters
and COW overlays, and then there are the more high-level backing chains
the user actually manages, where only the overlays are important.

I think it would make sense to rename the “qemu-internal backing chains"
to “filter chains” or something.  But that makes it sound a bit like it
would only mean R/W filters...  Maybe just “chain”?

Actually, the only functions I find are is_backing_chain_frozen & Co,
and they could simply become is_chain_frozen.  Is there anything else?

Max

14.06.2019 19:20, Max Reitz wrote:
> On 14.06.19 17:46, Vladimir Sementsov-Ogievskiy wrote:
>> 13.06.2019 1:09, Max Reitz wrote:
>>> This allows us to differentiate between filters and nodes with COW
>>> backing files: Filters cannot be used as overlays at all (for this
>>> function).
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>
>> Overlay created in snapshot operation assumed to consume following writes
>> and it's filtered child becomes readonly.. And filter works in completely another
>> way.
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> [hmm, I start to like using "filtered child" collocation when I say about this thing.
>>    didn't you think about renaming backing chain to filtered chain?]
> 
> Hm.  There are backing chains and there are backing chains.  There are
> qemu-internal backing chains that consist of a healthy mix of filters
> and COW overlays, and then there are the more high-level backing chains
> the user actually manages, where only the overlays are important.
> 
> I think it would make sense to rename the “qemu-internal backing chains"
> to “filter chains” or something.  But that makes it sound a bit like it
> would only mean R/W filters...  Maybe just “chain”?
> 
> Actually, the only functions I find are is_backing_chain_frozen & Co,
> and they could simply become is_chain_frozen.  Is there anything else?

Chain is too general, may be, blockchain? :)))

And to be serious, one more reason to rename it is yours
bdrv_backing_chain_next which is about user-backing-chain and differs from
frozen-chain related functions.

However, I don't think that these series is good place for this renaming,
it's rather big already.

-- 
Best regards,
Vladimir