[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: John Snow
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 0/4] qmp dirty bitmap API
Date: Thu, 7 Dec 2017 17:47:19 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

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.

I'm also not in a hurry to personally unify the loops myself, but if
someone did, they could CC me and I'd review it thoroughly.

We'd find out in a hurry from the implementation if unification was a
win for SLOC.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]