Re: [Qemu-devel] [PATCH 07/12] block/backup: add 'always' bitmap sync policy

[John Snow]

On 6/21/19 5:48 PM, Max Reitz wrote:
> On 21.06.19 22:58, John Snow wrote:
>>
>>
>> On 6/21/19 9:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
> [...]
> 
> Just chiming in on this:
> 
>>> "So on cancel and abort you synchronize bitmap too?"
>>
>> I will concede that this means that if you ask for a bitmap backup with
>> the 'always' policy and, for whatever reason change your mind about
>> this, there's no way to "cancel" the job in a manner that does not edit
>> the bitmap at this point.
>>
>> I do agree that this seems to go against the wishes of the user, because
>> we have different "kinds" of cancellations:
>>
>> A) Cancellations that actually represent failures in transactions
>> B) Cancellations that represent genuine user intention
>>
>> It might be nice to allow the user to say "No wait, please don't edit
>> that bitmap, I made a mistake!"
> 
> So that “always” doesn’t mean “always”?  To me, that seems like not so
> good an idea.
> 
> If the user uses always, they have to live with that.  I had to live
> with calling “rm” on the wrong file before.  Life’s tough.
> 

I actually agree, but I was making a concession in the ONE conceivable
case where you would theoretically want to abort "always".

> In all seriousness: “Always” is not something a user would use, is it?
> It’s something for management tools.  Why would they cancel because
> “They made a mistake”?
> 

A user might use it -- it's an attractive mode. It's basically
Incremental with retry ability. It is designed for use by a management
utility though, yes.

> Second, what’s the worst thing that may come out of such a mistake?
> Having to perform a full backup?  If so, that doesn’t seem so bad to me.
>  It certainly doesn’t seem so bad to make an unrelated mechanic have an
> influence on whether “always” means “always”.
> 

No, if you "accidentally" issue always (and you change your mind for
whatever reason), the correct way to fix this is:

(1) If the job completes successfully, nothing. Everything is situation
normal. This behaves exactly like "incremental" mode.

(2) If the job fails so hard you don't succeed in writing data anywhere
at all, nothing. Everything is fine. This behaves exactly like a failure
in "incremental" mode. The only way to reliably tell if this happened is
if job never even succeeded in creating a target for you, or your target
is still verifiably empty. (Even so: good practice would be to never
delete a target if you used 'always' mode.)

(3) If the job fails after writing SOME data, you simply issue another
mode=bitmap policy=always against the same target. (Presumably after
fixing your network or clearing room on the target storage.)

The worst mistake you can make is this:

- Issue sync=bitmap policy=always
- Cancel the job because it's taking too long, and you are impatient
- Forget that you used "always", delete the incomplete backup target

Oops! That had data that QEMU was counting on having written out
already. Your bitmap is now garbage.

You fix this with a full backup, yes.

> Also, this cancel idea would only work for jobs where the bitmap mode
> does not come into play until the job is done, i.e. backup.  I suppose
> if we want to have bitmap modes other than 'always' for mirror, that too
> would have to make a copy of the user-supplied bitmap, so there the
> bitmap mode would make a difference only at the end of the job, too, but
> who knows.
> 

Reasonable point; at the moment I modeled bitmap support for mirror to
only do synchronization at the end of the job as well. In this case,
"soft cancels" are modeled (upstream, today) as ret == 0, so those won't
count as failures at all.

(And, actually, force cancels will count as real failures. So maybe it
IS best not to overload this already hacky semantic we have on cancel.)

> And if it only makes a difference at the end of the job, you might as
> well just add a way to change a running job’s bitmap-mode.
> 

This is prescient. I have wanted a "completion-mode" for mirror that you
can change during its runtime (and to deprecate cancel as a way to
"complete" the job) for a very long time.

It's just that the QAPI for it always seems ugly so I shy away from it.

> Max
> 

So, I will say this:

1) I think the implementation of "always" is perfectly correct, in
single, transaction, or grouped-completion transaction modes.

2) Some of these combinations don't make much practical sense, but it's
more work to disallow them, and past experience reminds me that it's not
my job to save the user from themselves at the primitive level.

3) Adding nicer features like "I want a different completion mode since
i started this job" don't exist for any other mode or any other job
right now, and I don't think I will add them to this series.