[Top][All Lists]

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

Re: [Qemu-block] [PATCH 0/9] major rework of drive-mirror

From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH 0/9] major rework of drive-mirror
Date: Wed, 15 Jun 2016 12:25:15 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 15.06.2016 um 11:34 hat Denis V. Lunev geschrieben:
> On 06/15/2016 12:06 PM, Kevin Wolf wrote:
> >The second big thing is that I don't want to see new users of the
> >notifiers in I/O functions. Let's try if we can't add a filter
> >BlockDriver instead. Then we'd add an option to set the filter node-name
> >in the mirror QMP command so that the management tool is aware of the
> >node and can refer to it.
> this will be much more difficult to implement at my experience.

I agree that it will be more difficult, though I'm not sure whether it's
much more.

> Can you share more details why filters are bad?

Basically, notifiers (and other code for supporting other features in
the common I/O path) just don't fit the block layer design very well.
They are more hacked on instead of designed in.

This shows in different ways in different places, so I can't fully tell
you the problems that using notifiers in the mirror code would cause
(which is a problem in itself), but for example the copy-on-read code
that has been added to the core block layer instead of a separate filter
driver had trouble with ignoring its own internal requests that it made
to the BDS, which was hacked around with a new request flag, i.e. making
the core block layer even more complicated.

An example that I can think of with respect to mirroring is taking a
snapshot during the operation. Op blockers prevent this from happening
at the moment, but it seems to be a very reasonable thing for a user to
want. Let me use the filter node approach to visualise this:

    (raw-posix)    (qcow2)
    source_file <- source <- mirror <- virtio-blk
    target_file <- target <----+

There are two ways to create a snapshot: Either you put it logically
between the mirror and virtio-blk, so that only source (now a backing
file), but no new writes will be mirrored. This is easy in both
approaches, but maybe the less commonly wanted thing.

The other option is putting the new overlay between source and mirror:

    (raw-posix)    (qcow2)
    source_file <- source <- snap <- mirror <- virtio-blk
    target_file <- target <------------+

With the mirror intercept being its own BDS node, making this change is
very easy and doesn't involve any code that is specific to mirroring.

If it doesn't have a filter driver and uses notifiers, however, the
mirror is directly tied to the source BDS, and now it doesn't just work,
but you need extra code that explicitly moves the notifier from the
source BDS to the snap BDS. And you need such code not only for
mirroring, but for everything that potentially hooks into the I/O

Maybe these two examples give you an idea why I want to use the concepts
that are designed into the block layer with much thought rather than
hacked on notifiers. Notifiers may be simpler to implement in the first
place, but they lead to a less consistent and harder to maintain block
layer in the long run.

> >If we don't do this now, we'll have to introduce it later and can't be
> >sure that the management tool knows about it. This would complicate
> >things quite a bit because we would have to make sure that the added
> >node stays invisible to the management tool.
> >
> >
> >I think these two things are the big architectural questions. The rest
> >is hopefully more or less implementation details.
> I completely agree with you.
> We have the following choices:
> 1. implement parameter to use 'active'/'passive' mode from the very
> beginning
> 2. switch to 'active' mode upon receiving "block-job-complete"
> command unconditionally
> 3. switch to 'active' mode upon receiving "block-job-complete"
> command with proper parameter
> 4. switch to 'active' mode after timeout (I personally do not like
> this option)
> I think that choices 1 and 3 do not contradict each other and
> could be implemented to gather.

I think we definitely want 1. and some way to switch after the fact.
That you suggest three different conditions for doing the switch
suggests that it is policy and doesn't belong in qemu, but in the
management tools. So how about complementing 1. with 5.?

5. Provide a QMP command to switch between active and passive mode


reply via email to

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