[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor |
Date: |
Tue, 05 Jun 2012 11:25:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 |
Am 05.06.2012 03:00, schrieb Michael Roth:
> This is an import of Anthony's qidl compiler, with some changes squashed
> in to add support for doing the visitor generation via QEMU's qapi code
> generators rather than directly.
>
> Documentation has been imported as well, as is also viewable at:
>
> https://github.com/aliguori/qidl/blob/master/qc.md
>
> This will be used to add annotations to device structs to aid in
> generating visitors that can be used to serialize/unserialize them.
>
> Signed-off-by: Michael Roth <address@hidden>
> ---
> scripts/qc.md | 331 ++++++++++++++++++++++++++++++++++++++
> scripts/qc.py | 494
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 825 insertions(+), 0 deletions(-)
> create mode 100644 scripts/qc.md
> create mode 100755 scripts/qc.py
>
> diff --git a/scripts/qc.md b/scripts/qc.md
> new file mode 100644
> index 0000000..4cf4b21
> --- /dev/null
> +++ b/scripts/qc.md
I think docs/ would be a better place than scripts/
> +Getting Started
> +---------------
> +
> +The first step is to move your device struct definition to a header file.
> This
> +header file should only contain the struct definition and any preprocessor
> +declarations you need to define the structure. This header file will act as
> +the source for the QC IDL compiler.
> +
> +Do not include any function declarations in this header file as QC does not
> +understand function declarations.
Couldn't we use a header file (or even source file) that has some magic
markers for the IDL compiler? Like:
... random stuff ...
/* QIDL START */
struct Foo {
...
};
/* QIDL END */
... random stuff ...
Adding a new header file for each device really doesn't look like a
desirable thing, and this way it could be avoided.
> +Determining What State Gets Saved
> +---------------------------------
> +
> +By default, QC saves every field in a structure it sees. This provides
> maximum
> +correctness by default. However, device structures generally contain state
> +that reflects state that is in someway duplicated or not guest visible. This
> +more often that not reflects design implementation details.
> +
> +Since design implementation details change over time, saving this state makes
> +compatibility hard to maintain since it would effectively lock down a
> device's
> +implementation.
> +
> +QC allows a device author to suppress certain fields from being saved
> although
> +there are very strict rules about when this is allowed and what needs to be
> done
> +to ensure that this does not impact correctness.
> +
> +There are three cases where state can be suppressed: when it is
> **immutable**,
> +**derived**, or **broken**. In addition, QC can decide at run time whether
> to
> +suppress a field by assigning it a **default** value.
What about fields that have a known state or whose state doesn't matter?
For example, when migrating a disk, we know that no I/O requests are in
flight because they are flushed before sending the device state.
The fields describing the in-flight request are not immutable, because
the do change a lot during runtime. They are not really derived either
because they don't depend on other fields and they don't need a
post-load function. Broken implies that there is a bug, but there isn't.
It's just that their value after migration simply doesn't matter.
> +It can be subtle whether a field is truly immutable. A good example is a
> +*QEMUTimer*. Timer's will usually have their timeout modified with a call to
> +*qemu_mod_timer()* even though they are only assigned in the device
> +initialization function.
> +
> +If the timer is always modified with a fixed value that is not dependent on
> +guest state, then the timer is immutable since it's unaffected by the state
> of
> +the guest.
> +
> +On the other hand, if the timer is modified based on guest state (such as a
> +guest programmed time out), then the timer carries state. It may be
> necessary
> +to save/restore the timer or mark it as **_derived** and work with it
> +accordingly.
Another instance of the above problem: What if a timer value is changed
dynamically for example as an optimisation, but for correctness it's not
really important what it is?
And even with a fixed timeout, as long as it's guest visible, can you
avoid transferring it, strictly speaking? It carries information about
when the next timeout occurs.
> +### Default Values
> +
> +In many cases, a field that gets marked broken was not originally saved
> because
> +in the vast majority of the time, the field does not contain a meaningful
> value.
> +
> +In the case of our *thr* example, the field usually does not have a
> meaningful
> +value.
> +
> +Instead of always saving the field, QC has another mechanism that allows the
> +field to be saved only when it has a meaningful value. This is done using
> the
> +**_default()** marker. The default marker tells QC that if the field
> currently
> +has a specific value, do not save the value as part of serialization.
> +
> +When loading a field, QC will assign the default value to the field before it
> +tries to load the field. If the field cannot be loaded, QC will ignore the
> +error and rely on the default value.
> +
> +Using default values, we can fix broken fields while also minimizing the
> cases
> +where we break live migration compatibility. The **_default()** marker can
> be
> +used in conjunction with the **_broken** marker. We can extend our example
> as
> +follows:
> +
> + typedef struct SerialDevice {
> + SysBusDevice parent;
> +
> +
> + uint8_t thr _default(0); // transmit holding register
> + uint8_t lsr; // line status register
> + uint8_t ier; // interrupt enable register
> +
> + int _derived int_pending; // whether we have a pending queued
> interrupt
> + CharDriverState _immutable *chr;
> + } SerialDevice;
> +
> +The following guidelines should be followed when using a default marker:
> +
> + 1. Is the field set to the default value both during device initialization
> and
> + whenever the field is no longer in use?
> +
> + 2. If the non-default value is expected to occur often, then consider using
> the
> + **_broken** marker along with the default marker and using a flag day to
> + remove the **_broken** marker.
> +
> + 3. In general, setting default values as the value during device
> initialization
> + is a good idea even if the field was never broken. This gives us maximum
> + flexibility in the long term.
> +
> + 4. Never change a default value without renaming a field. The default
> value is
> + part of the device's ABI.
> +
> +The first guideline is particularly important. In the case of QEMU's real
> +*SerialDevice*, it would be necessary to add code to set the *thr* register
> to
> +zero after the byte has been successfully transmitted. Otherwise, it is
> +unlikely that it would ever contain the default value.
Does this use subsections internally? How would we convert existing
subsections that have a specific name and may contain more than one
field? We also have is_needed functions that are more complex than just
comparing to a default value.
So I guess my real question is what the concept for maintaining
migration compatibility is.
Kevin
- Re: [Qemu-devel] [PATCH 02/17] qidl: add qc definitions, (continued)
Re: [Qemu-devel] [PATCH 02/17] qidl: add qc definitions, Paolo Bonzini, 2012/06/05
[Qemu-devel] [PATCH 04/17] qapi: QmpOutputVisitor, implement array handling, Michael Roth, 2012/06/04
[Qemu-devel] [PATCH 03/17] qapi: add visitor interfaces for arrays, Michael Roth, 2012/06/04
[Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Michael Roth, 2012/06/04
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Anthony Liguori, 2012/06/04
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Michael Roth, 2012/06/05
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Paolo Bonzini, 2012/06/05
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Anthony Liguori, 2012/06/05
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Paolo Bonzini, 2012/06/06
- Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Anthony Liguori, 2012/06/06
Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Kevin Wolf, 2012/06/06
Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor, Peter Maydell, 2012/06/05