[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 20/42] block/snapshot: Fix fallback
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v6 20/42] block/snapshot: Fix fallback |
Date: |
Tue, 10 Sep 2019 14:04:47 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 10.09.19 13:56, Kevin Wolf wrote:
> Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
>> If the top node's driver does not provide snapshot functionality and we
>> want to fall back to a node down the chain, we need to snapshot all
>> non-COW children. For simplicity's sake, just do not fall back if there
>> is more than one such child.
>>
>> bdrv_snapshot_goto() becomes a bit weird because we may have to redirect
>> the actual child pointer, so it only works if the fallback child is
>> bs->file or bs->backing (and then we have to find out which it is).
>>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> block/snapshot.c | 100 +++++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 79 insertions(+), 21 deletions(-)
>>
>> diff --git a/block/snapshot.c b/block/snapshot.c
>> index f2f48f926a..35403c167f 100644
>> --- a/block/snapshot.c
>> +++ b/block/snapshot.c
>> @@ -146,6 +146,32 @@ bool bdrv_snapshot_find_by_id_and_name(BlockDriverState
>> *bs,
>> return ret;
>> }
>>
>> +/**
>> + * Return the child BDS to which we can fall back if the given BDS
>> + * does not support snapshots.
>> + * Return NULL if there is no BDS to (safely) fall back to.
>> + */
>> +static BlockDriverState *bdrv_snapshot_fallback(BlockDriverState *bs)
>> +{
>> + BlockDriverState *child_bs = NULL;
>> + BdrvChild *child;
>> +
>> + QLIST_FOREACH(child, &bs->children, next) {
>> + if (child == bdrv_filtered_cow_child(bs)) {
>> + /* Ignore: COW children need not be included in snapshots */
>> + continue;
>> + }
>> +
>> + if (child_bs) {
>> + /* Cannot fall back to a single child if there are multiple */
>> + return NULL;
>> + }
>> + child_bs = child->bs;
>> + }
>> +
>> + return child_bs;
>> +}
>
> Why do we return child->bs here when bdrv_snapshot_goto() then needs to
> reconstruct what the associated BdrvChild was? Wouldn't it make more
> sense to return BdrvChild** from here and maybe have a small wrapper for
> the other functions that only need a BDS?
What would you return instead? &child doesn’t work.
We could limit ourselves to bs->file and bs->backing. It just seemed
like a bit of an artificial limit to me, because we only really have it
for bdrv_snapshot_goto().
Max
signature.asc
Description: OpenPGP digital signature