On 11/17/2017 10:01 AM, Max Reitz wrote: > On 2017-11-17 13:30, Kevin Wolf wrote: >> Am 23.10.2017 um 11:29 hat Vladimir Sementsov-Ogievskiy geschrieben: >>> Snapshot-switch actually changes active state of disk so it should >>> reflect on dirty bitmaps. Otherwise next incremental backup using >>> these bitmaps will be invalid. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> >> We discussed this quite a while ago, and I'm still not convinced that >> this approach makes sense. > > I think it at least makes more sense than not handling this case at all. > >> Can you give just one example of a use case where dirtying the whole >> bitmap while loading a snapshot is the desired behaviour? >> >> I think the most useful behaviour would be something where the bitmaps >> themselves are snapshotted, too. > > Agreed. > >> But for the time being, the easiest and >> safest solution might just be to error out in any snapshot operations >> if any bitmaps are in use. > > Sounds OK, too. I personally don't have an opinion either way. > > But in any case, what we did before this patch was definitely wrong so I > consider it an improvement. >
This is how I feel about it too. Erroring out entirely is an option, but code-wise just dirtying everything is at least verifiably not-wrong and pretty simple to implement. It's an improvement... Don't do it, but at least you won't get something wrong after, just something heinously unoptimal. > Max >
