Re: [Qemu-devel] [RFC PATCH 00/16] visitor+BER migration format

[Michael S. Tsirkin]

On Tue, Mar 25, 2014 at 08:17:11PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> This is a work in progress cut of my visitor+BER format migration
> world; there's lots to do but it's starting to get there.
> I'd appreciate if anyone who has more experience with either the
> visitor code, or ASN.1/BER in general has a look to see if I'm
> doing anything particularly odd.
> 
> I've cherry picked bits of Michael Roth's visitor set from 2011 and
> Stefan Berger's BER code from this time last year, but not necessarily
> used things in the same way.

Nice! Will take a look at patches later, just responding to cover
letter now:

> The good:
>   It can perform a migration using a BER format file that's described
>    and verifiable against a schema.
> 
>   It can also perform a migration using the old binary format.
> 
>   A lot of the nasty detail of the old format is hidden in the compatibility
>   visitor (e.g. the subsection end detection).
> 
> The ugly:
>    1) The encoding for zero pages is bloated, it can be simplified a 
>       lot (see previous discussion with Michael from a week or two back
>       that I've not yet implemented)
>    2) There are a few shims that get passed visitors but need files
>       or the other way that should be removable. The shim to get from
>       a qemu file back to a visitor is patch 3 and marked as 'tmp' and
>       the main uses are in piix4, pci.c, spapr_vscsi.c.
>    3) There are places where the visitor interface is too tied to
>       the old file format, but most of those can be removed with effort.
>    4) I've made extensions to the Visitor type, many of these are very
>       migration specific; and use things like get_next_type and the list
>       visitors in quite a different way from the current uses. 
>       Should I really have a separate type?
>       (see patch 1)
>    5) At the moment you select BER output format by setting an environment
>       variable ( export QEMUMIGFORMAT=BER ) , I need to put more thought
>       in to the right way to do this, there are some harder questions like
>       what happens to devices that are still using pre-vmstate encodings
>       (that are currently sent as blobs) when they eventually convert over
>       and thus how to keep compatibility with earlier BER output versions
>       where they were blobs.

A simple way is the way we always changed migration
format - trying this to the machine type.
Old machine type gets you binary, newer one BER with blobs,
then we can add more flags gradually.

With BER we'll actually be able to do smarter things in the future,
such as send out multiple formats and have destination select
the one it understands, but this isn't mandatory as a first step.

>    6) At the moment for blobs (i.e. pre-vmstate) the whole blob gets loaded
>       into memory and then processed; that's not necessary and can be done
>       with a QEMUFile shim.
>    7) I need to sort out my Error** handling (probably in a lot of places)

That's mostly broken in migration as it is.

>    8) The visitors should be able to share much more code.
> 
> The not-yet-done:
>    a) XBZRLE
>    b) block-migration
>    c) SPAPR (it's an iterative migration like RAM and block)
>    d) RDMA (I'm guessing I've broken it, but haven't tried)
>    e) Floats are currently sent as blobs.

So at least a,b and c  need to be done.
Not sure about RDMA - it was never supported and no one
seems to work on it.
Maybe, somehow, it can use the old format when enabled?


> What I've done:
>   There are separate output visitors (binary compatible, debug and BER) and
> they're selected by setting QEMUMIGFORMAT to one of 'debug', 'BER' or just
> leaving it unset.
> 
>   The Input visitor is selected automatically from the first 4 bytes of the
> file.

Not really necessary if you use machine type.

> 
>   In general most types are BER sequences of indefinite length, with some
> types that I've allocated an Application specific type tag to.  There is a 
> hook
> to give any VMState it's own type tag (which I've shown using one example
> for cpu-common).  Integers and strings are standard 'universal' types.
> 
>   Objects with .get/.put methods or register_savevm are saved as an 'octet 
> string'.
> There are a few places where a device registered with .get/.put calls back 
> into
> vmstate_save_state for part of their state (typically for devices built on 
> PCI)
> and when they do that, even when in BER mode, those components get stored 
> inside
> the octet string in the old format.
> 
>   I've used the 'asn1c' tool to validate the schema (which is
> in docs/specs/migration.schema) and also to verify streams that I've produced
> match the schema.
> 
>   I've tested it with virt-test (hacked to have different source/dest qemu's)
> and tried bin-bin, ber-ber, and pre-visitor-qemu -> this and
> this -> pre-visitor-qemu just on the standard migration.default.tcp migration
> test, but I've not tried a large combination of configurations.
> 
>   My fix for qemu_peek_buffer that's on the list is needed to stabilise
> the binary format input visitor.
> 
>   I'll keep chipping away at that list, and would expect to pop another
> version out in a month or so.
> 
> Dave
> 
> Dr. David Alan Gilbert (16):
>   Visitor: Add methods for migration format use
>   QEMUSizedBuffer/QEMUFile
>   qemu-file: Add set/get tmp_visitor
>   Header/constant/types fixes for visitors
>   Visitor: Binary compatible output visitor
>   Visitor: Debug output visitor
>   Visitor: Binary compatible input visitor
>   Visitor: Output path
>   Visitor: Load path
>   Visitor: Common types to use visitors
>   Choose output visitor based on env variable
>   BER Visitor: Create output visitor
>   BER Visitor: Create input visitor
>   Start some BER format docs
>   ASN.1 schema for new migration format
>   Wire in BER visitors

One nifty feature of Stefan's patches was that a
set of unit tests for parser was included.
Worth reusing?

>  arch_init.c                                    |  231 +++--
>  block-migration.c                              |   13 +-
>  docs/migration.txt                             |   34 +-
>  docs/specs/migration.schema                    |  113 +++
>  exec.c                                         |    2 +
>  hw/acpi/piix4.c                                |    5 +-
>  hw/pci/pci.c                                   |    5 +-
>  hw/ppc/spapr.c                                 |    9 +-
>  hw/scsi/spapr_vscsi.c                          |    6 +-
>  include/migration/migration.h                  |   17 +
>  include/migration/qemu-file.h                  |   31 +
>  include/migration/vmstate.h                    |   28 +-
>  include/qapi/ber.h                             |  108 +++
>  include/qapi/qemu-file-ber-input-visitor.h     |   26 +
>  include/qapi/qemu-file-ber-output-visitor.h    |   26 +
>  include/qapi/qemu-file-binary-input-visitor.h  |   27 +
>  include/qapi/qemu-file-binary-output-visitor.h |   26 +
>  include/qapi/qemu-file-debug-output-visitor.h  |   26 +
>  include/qapi/visitor-impl.h                    |   23 +
>  include/qapi/visitor.h                         |   51 ++
>  include/qemu/typedefs.h                        |    4 +-
>  qapi/Makefile.objs                             |    4 +-
>  qapi/qapi-visit-core.c                         |   80 ++
>  qapi/qemu-file-ber-input-visitor.c             | 1163 
> ++++++++++++++++++++++++
>  qapi/qemu-file-ber-output-visitor.c            |  916 +++++++++++++++++++
>  qapi/qemu-file-binary-input-visitor.c          |  688 ++++++++++++++
>  qapi/qemu-file-binary-output-visitor.c         |  564 ++++++++++++
>  qapi/qemu-file-debug-output-visitor.c          |  471 ++++++++++
>  qemu-file.c                                    |  423 +++++++++
>  savevm.c                                       |  412 +++++++--
>  vmstate.c                                      |  619 ++++++-------
>  31 files changed, 5593 insertions(+), 558 deletions(-)
>  create mode 100644 docs/specs/migration.schema
>  create mode 100644 include/qapi/ber.h
>  create mode 100644 include/qapi/qemu-file-ber-input-visitor.h
>  create mode 100644 include/qapi/qemu-file-ber-output-visitor.h
>  create mode 100644 include/qapi/qemu-file-binary-input-visitor.h
>  create mode 100644 include/qapi/qemu-file-binary-output-visitor.h
>  create mode 100644 include/qapi/qemu-file-debug-output-visitor.h
>  create mode 100644 qapi/qemu-file-ber-input-visitor.c
>  create mode 100644 qapi/qemu-file-ber-output-visitor.c
>  create mode 100644 qapi/qemu-file-binary-input-visitor.c
>  create mode 100644 qapi/qemu-file-binary-output-visitor.c
>  create mode 100644 qapi/qemu-file-debug-output-visitor.c
> 
> -- 
> 1.8.5.3