All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: "תומר בן אור" <tomer@zertodata.com>,
	"Stefan Hajnoczi" <stefanha@gmail.com>,
	"Jeff Cody" <jcody@redhat.com>,
	dlaor@redhat.com, qemu-devel@nongnu.org,
	"Markus Armbruster" <armbru@redhat.com>,
	"Zhi Yong Wu" <zwu.kernel@gmail.com>,
	"Federico Simoncelli" <fsimonce@redhat.com>,
	"Ori Mamluk" <omamluk@zerto.com>, "עודד קדם" <oded@zerto.com>,
	"Yair Kuszpet" <yairk@zerto.com>
Subject: Re: [Qemu-devel] BlockDriverState stack and BlockListeners
Date: Tue, 21 Feb 2012 12:36:46 +0100	[thread overview]
Message-ID: <4F4381CE.70604@redhat.com> (raw)
In-Reply-To: <4F43773B.6060109@redhat.com>

On 02/21/2012 11:51 AM, Kevin Wolf wrote:
> And even protocols and protocols don't. Compare file to blkdebug, for
> example. In fact, blkdebug and blkverify are already very close to what
> BlockListeners would be.

Yes, and I think considering blkdebug and blkverify help in the design.
They provide the difference between views and listeners: listeners can
be applied to both a protocol and a view, while views can only be
applied to a format.

> > - Protocols -- Protocols tell you where to get the raw bits.
> > - Formats -- Formats transform those raw bits into a block device.
> > - Views -- Views can move from a format to another.  A format can use a
> > default view implementation, or provide its own (e.g. to access
> > different snapshots).
> > - Listeners -- I think a view can have-a listener?
> 
> Where protocols, formats and listeners are-a image (Not the best name,
> I'm open for suggestions. Something like "BDS stack building block"
> would be most accurate...). Or actually not is-a in terms of
> inheritance, but I think it would be the very same thing without any
> subclassing, implemented by a BlockDriver.
> 
> > with the following relationship:
> > 
> > - A format has-a protocol for the raw bits and has-a view for the
> > backing file.
> 
> An image has-a image from which it takes its data (bs->file).

No. A protocol has neither an image below it, nor a backing file.  I
think a view has no backing file either (except as provided by the
format).  I'm not sure that a listener can have a backing file, either;
you could say that blkverify has one, but it could just as well have
three or four, so it's not the same thing.

A format does not do much more than create views, do snapshots, and hold
state that is common to all of its views.

So, let's put aside listeners for a moment, and consider this
alternative hierarchy:

BlockDriver
    Protocol
        FileProtocol
        ...
    View
        QCOW2View
        RawView
        ...
BlockFormat
    QCOW2Format
    RawFormat
    ...

Now we have to figure out how to fit listeners in this picture.

> And it has-a view for the backing file, yes. Both could be a listener.
> 
>> > - A view has-a format, a device has-a view.
>> > - A view can have-a listener?  Or is it formats?
> A view has-a image. This may happen to be a listener, which in turn
> has-a image (could be another listener, a format or a protocol).
> 
> The question is what the semantics is with live snapshots (there are
> probably similar problems, but this is the obvious one). For example we
> could now have:
> 
> mirror -> qcow2 -> blkdebug -> file

Let's be precise here:

mirror -> currentSnapshot -> qcow2 -> blkdebug -> file

- file is a protocol.

- blkdebug is a listener

- qcow2 is a format

- currentSnapshot is a view

- mirror is a listener

The difference between blkdebug/mirror on one side, and currentSnapshot
on the other, is that (as you said) blkdebug/mirror are always stacked
on top of something else. A driver that references currentSnapshot
actually gets mirror.

So we get to the actual hierarchy I'm proposing:

BlockDriver
    BlockSource (see below)
    Protocol (bdrv_file_open)
        FileProtocol
        ...
    View (has-a BlockFormat)
        QCOW2View
        RawView
        ...
    BlockListener (has-a BlockDriver)
        MirrorListener
        BlkDebugListener
BlockFormat (has-a BlockSource)
    QCOW2Format
    RawFormat
    ...

Protocols and views are only internal.  Formats and devices in practice
will only ever see BlockSources.  A BlockSource is a reference a stack of
BlockDrivers, where the base must be a protocol or view and there can
be a number of listeners stacked on top of it.  Listeners can be
added or removed from the stack, and the bottom driver can be swapped
for another (for snapshots).

So, here is how it would look:

  .== BlockSource ==.                   .== BlockSource ===.
  | MirrorListener  |                   | BlkDebugListener |
  | QCOW2View ------+--> QCOW2Format -> | FileProtocol     |
  '================='                   '=================='


> There are two listeners here, mirror and blkdebug. (Things like blkdebug
> are why view has-a listener isn't enough). After creating an external
> snapshot, we expect the graph to look like this (the arrow down is the
> backing file):
> 
> mirror -> qcow2 -> file
>             |
>             +-> qcow2 -> blkdebug -> file

And here:

  .== BlockSource ==.
  | MirrorListener  |                   .== BlockSource ==.
  | QCOW2View ------+--> QCOW2Format -> | FileProtocol    |
  '================='    |              '================='
                         |                                          .== BlockSource ===.
                         |   .== BlockSource ==.                    | BlkDebugListener |
                         '-> | QCOW2View ------+--> QCOW2Format --> | FileProtocol     |
                             '================='                    '=================='

Does it seem sane?

> The question is: Can we assume that any listeners that are on top of the
> first format or protocol (i.e. those that would fit your model) should
> move to the new top-level view? Or would it sometimes make sense to keep
> it at the old one?

I think it depends, but both possibilities should be doable in this model.

Paolo

  reply	other threads:[~2012-02-21 11:37 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-07 10:29 [Qemu-devel] [RFC PATCH] replication agent module Ori Mamluk
2012-02-07 12:12 ` Anthony Liguori
2012-02-07 12:25   ` Dor Laor
2012-02-07 12:30     ` Ori Mamluk
2012-02-07 12:40       ` Anthony Liguori
2012-02-07 14:06         ` Ori Mamluk
2012-02-07 14:40           ` Paolo Bonzini
2012-02-07 14:48             ` Ori Mamluk
2012-02-07 15:47               ` Paolo Bonzini
2012-02-08  6:10                 ` Ori Mamluk
2012-02-08  8:49                   ` Dor Laor
2012-02-08 11:59                     ` Stefan Hajnoczi
2012-02-08  8:55                   ` Kevin Wolf
2012-02-08  9:47                     ` Ori Mamluk
2012-02-08 10:04                       ` Kevin Wolf
2012-02-08 13:28                         ` [Qemu-devel] [RFC] Replication agent design (was [RFC PATCH] replication agent module) Ori Mamluk
2012-02-08 14:59                           ` Stefan Hajnoczi
2012-02-08 14:59                             ` Stefan Hajnoczi
2012-02-19 13:40                             ` Ori Mamluk
2012-02-20 14:32                               ` Paolo Bonzini
2012-02-21  9:03                                 ` [Qemu-devel] BlockDriverState stack and BlockListeners (was: [RFC] Replication agent design) Kevin Wolf
2012-02-21  9:15                                   ` [Qemu-devel] BlockDriverState stack and BlockListeners Paolo Bonzini
2012-02-21  9:49                                     ` Kevin Wolf
2012-02-21 10:09                                       ` Paolo Bonzini
2012-02-21 10:51                                         ` Kevin Wolf
2012-02-21 11:36                                           ` Paolo Bonzini [this message]
2012-02-21 12:22                                             ` Stefan Hajnoczi
2012-02-21 12:57                                               ` Paolo Bonzini
2012-02-21 15:49                                               ` Markus Armbruster
2012-02-21 13:10                                             ` Kevin Wolf
2012-02-21 13:21                                               ` Paolo Bonzini
2012-02-21 15:56                                               ` Markus Armbruster
2012-02-21 16:04                                                 ` Kevin Wolf
2012-02-21 16:19                                                   ` Markus Armbruster
2012-02-21 16:39                                                     ` Kevin Wolf
2012-02-21 17:16                                               ` Stefan Hajnoczi
2012-02-21 10:20                                       ` Ori Mamluk
2012-02-29  8:38                                   ` Ori Mamluk
2012-03-03 11:46                                     ` Stefan Hajnoczi
2012-03-04  5:14                                       ` Ori Mamluk
2012-03-04  8:56                                         ` Paolo Bonzini
2012-03-05 12:04                                         ` Stefan Hajnoczi
2012-02-08 11:02                   ` [Qemu-devel] [RFC PATCH] replication agent module Stefan Hajnoczi
2012-02-08 13:00                     ` [Qemu-devel] [RFC] Replication agent requirements (was [RFC PATCH] replication agent module) Ori Mamluk
2012-02-08 13:30                       ` Anthony Liguori
2012-02-08 12:03                   ` [Qemu-devel] [RFC PATCH] replication agent module Stefan Hajnoczi
2012-02-08 12:46                     ` Paolo Bonzini
2012-02-08 14:39                       ` Stefan Hajnoczi
2012-02-08 14:55                         ` Paolo Bonzini
2012-02-08 15:07                           ` Stefan Hajnoczi
2012-02-07 14:53             ` Kevin Wolf
2012-02-07 15:00             ` Anthony Liguori
2012-02-07 13:34 ` Kevin Wolf
2012-02-07 13:50   ` Stefan Hajnoczi
2012-02-07 13:58     ` Paolo Bonzini
2012-02-07 14:05     ` Paolo Bonzini
2012-02-08 12:17       ` Orit Wasserman
2012-02-07 14:18     ` Ori Mamluk
2012-02-07 14:59     ` Anthony Liguori
2012-02-07 15:20       ` Stefan Hajnoczi
2012-02-07 16:25         ` Anthony Liguori
2012-02-21 16:01       ` Markus Armbruster
2012-02-21 17:31         ` Stefan Hajnoczi
2012-02-07 14:45   ` Ori Mamluk
2012-02-08 12:29     ` Orit Wasserman
2012-02-08 11:45   ` Luiz Capitulino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F4381CE.70604@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dlaor@redhat.com \
    --cc=fsimonce@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=oded@zerto.com \
    --cc=omamluk@zerto.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=tomer@zertodata.com \
    --cc=yairk@zerto.com \
    --cc=zwu.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.