Re: [Qemu-block] [PATCH v8 4/7] block: introduce backup-top filter driver

[Vladimir Sementsov-Ogievskiy]

18.06.2019 11:20, Kevin Wolf wrote:
> Am 18.06.2019 um 09:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 17.06.2019 19:25, Kevin Wolf wrote:
>>> The first step would be making the block drivers full replacements of
>>> the old things, which unfortunately isn't true today.
>>>
>>> After your "deal with filters" series, we should be a lot closer for the
>>> core infrastructure at least.
>>>
>>> Not sure about copy-on-read, but I know that throttle still doesn't have
>>> feature parity with -drive throttling. At least I'm pretty sure that
>>> something about changing the group or group options at runtime (and not
>>> just dropping the x-) was missing.
>>
>> OK, thanks, I understand now that implicit filters are not just a
>> feature but compatibility mechanism.
>>
>> So, can we at some point deprecate "optionality" of filter-node-name
>> parameters of jobs, in addition to deprecation of things like old
>> copy-on-read option?  And actually deprecate implicit filters by this?
> 
> Hm, I'm not sure if we have ever moved an optional feature to required,
> and how to communicate this to libvirt, but this would be ideal, yes.
> 
>>>>>>>>> But should really filter do that job, or it is here only to do CBW? 
>>>>>>>>> Maybe target
>>>>>>>>> must have another parent which will care about flushing.
>>>>>>>>>
>>>>>>>>> Ok, I think I'll flush target here too for safety, and leave a 
>>>>>>>>> comment, that something
>>>>>>>>> smarter would be better.
>>>>>>>>> (or, if we decide to flush all children by default in generic code, 
>>>>>>>>> I'll drop this handler)
>>>>>>>>
>>>>>>>> [1] Thinking more about this, for normal backups the target file does
>>>>>>>> not reflect a valid disk state until the backup is done; just like for
>>>>>>>> qemu-img convert.  Just like qemu-img convert, there is therefore no
>>>>>>>> reason to flush the target until the job is done.
>>>>>
>>>>> Depends, the target could have the source as its backing file (like
>>>>> fleecing, but without exporting it right now), and then you could always
>>>>> have a consistent view on the target. Well, almost.
>>>>>
>>>>> Almost because to guarantee this, we'd have to flush between each CBW
>>>>> and the corresponding write to the same block, to make sure that the old
>>>>> data is backed up before it is overwritten.
>>>>
>>>> Yes, that’s what I meant by “enforce on the host that the target is
>>>> always flushed before the source”.  Well, I meant to say there is no
>>>> good way of doing that, and I kind of don’t consider this a good way.
>>>>
>>>>> Of course, this would perform about as badly as internal COW in qcow2...
>>>>> So probably not a guarantee we should be making by default. But it might
>>>>> make sense as an option.
>>>>
>>>> I don’t know.  “Use this option so your data isn’t lost in case of a
>>>> power failure, but it makes everything pretty slow”?  Who is going to do
>>>> explicitly enable that (before their first data loss)?
>>>
>>> Maybe the backup job wasn't that clever after all. At least if you care
>>> about keeping the point-in-time snapshot at the start of the backup job
>>> instead of just retrying and getting a snapshot of a different point in
>>> time that is just as good.
>>>
>>> If you do care about the specific point in time, the safe way to do it
>>> is to take a snapshot and copy that away, and then delete the snapshot
>>> again.
>>>
>>> The only problem is that merging external snapshots is slow and you end
>>> up writing the new data twice. If only we had a COW image format... :-)
>>
>> So, I still don't understand the reason of flushing backup target in a
>> meantime.  Backup target remains invalid until the successful end of
>> the job, at which point we definitely flush target, but only once.
>>
>> If node crashes during backup, backup would be invalid independently
>> of were there flushes after each write (or better just enable
>> O_DIRECT) or not.
> 
> Say your VM is running on disk.qcow2 and you use a backup job to copy
> data to backup.qcow2. At some point, the host crashes. If we made sure
> that every CBW to backup.qcow2 has completed before we write new data to
> disk.qcow2, backup.qcow2 contains valid data that represents the state
> at the start of the backup job as long as you use disk.qcow2 as its
> backing file.
> 
> The only way to ensure the right order is flushing between the two
> operations. If you don't do that, then yes, your backing is invalid
> before it has completed.
> 
> O_DIRECT doesn't guarantee that the data is on disk, only a flush does
> that. Maybe your chances that things actually make it to the disk in
> case of a host crash are higher if it sits in the disk cache rather than
> in the host's RAM, but there is no guarantee without a flush.
> 
> Now flushing the target when the guest flushes its disk doesn't give you
> new guarantees. Maybe it increases your chances that you're lucky and
> your data is correct, but you won't be able to tell. So... not really a
> reason not to do it, but it's probably kind of useless.
> 

All clear now, thanks!


-- 
Best regards,
Vladimir