Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 0/4] qmp dirty bitmap API

[Max Reitz]

On 2017-12-07 23:47, John Snow wrote:
> 
> 
> On 12/07/2017 06:56 AM, Kashyap Chamarthy wrote:
>> On Tue, Nov 21, 2017 at 07:10:19PM -0500, John Snow wrote:
>>> On 11/21/2017 12:23 PM, Kevin Wolf wrote:
>>
>> [...] # Snip lot of good discussion.
>>
>>>> On another note, I would double check before adding a new block job type
>>>> that this is the right way to go. We have recently noticed that many, if
>>>> not all, of the existing block jobs (at least mirror, commit and backup)
>>>> are so similar that they implement the same things multiple times and
>>>> are just lacking different options and have different bugs. I'm
>>>> seriously considering merging all of them into a single job type
>>>> internally that just provides options that effectively turn it into one
>>>> of the existing job types.
>>>>
>>>
>>> I'm not particularly opposed. At the very, very least "backup" and
>>> "mirror" are pretty much the same thing and "stream" and "commit" are
>>> basically the same.
>>>
>>> Forcing the backuppy-job and the consolidatey-job together seems like an
>>> ever-so-slightly harder case to make, but I suppose the truth of the
>>> matter in all cases is that we're copying data from one node to another...
>>
>> So from the above interesting discussion, it seems like Kevin is leaning
>> towards a single job type that offers 'stream', 'commit', 'backup', and
>> 'mirror' functionality as part of a single command / job type.  Based on
>> an instinct, this sounds a bit too stuffy and complex to me.
>>
>> And John seems to be leaning towards two block device job types:
>>
>>   - 'blockdev-foo' that offers both current 'stream' and 'commit'
>>     functionality as two different options to the same QMP command; and
>>
>>   - 'blockdev-bar' will offer both 'mirror' and 'backup' functionality
>>     as part of the same QMP command
>>
>> FWIW, this seems a bit more palatable, as it is unifying
>> similar-functionality-that-differ-slightly into two distinct commands.
>>
>> (/me is still wrapping his head around the bitmaps and incremental
>> backups infrastructure.)
>>
> 
> The discussion you're honing in on is tangential to the one at the heart
> of this topic, which is:
> 
> "What's the right API for pull model incremental backups?" which digs up
> the question "What is the right API for our existing data copying commands?"
> 
> Kevin is probably right in the end, though; he usually is. We do need to
> unify our data moving logic to avoid fixing the same bugs in four
> different but nearly identical jobs.
> 
> My only concern is that the special casing will grow too complex for
> that one unified job and the job will become so complicated nobody knows
> how to work on it anymore without breaking other cases. Maybe this is a
> reasonable concern, maybe it isn't.

The thing is that mirror is becoming more complicated anyway.  For one
thing, we already do commits through it and I don't see why intermediate
commits shouldn't just work the same way (the only thing is that we
would have to automatically complete the job instead of emitting a ready
event).

The other lies in the active mirror thingy.  If we have active mirror,
then we should be able to implement backup pretty easily, too.  We just
need to copy the old data to the target instead of the new one (I think).

So the thing is that commit and backup are going to be pretty much
included in mirror for free by accident.  There is no real reason to
have own code for them, then.

Max

signature.asc
Description: OpenPGP digital signature