Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-27 Thread Dr. David Alan Gilbert
* Balamuruhan S (bal...@linux.vnet.ibm.com) wrote:
> On Fri, Apr 20, 2018 at 06:57:21PM +0100, Dr. David Alan Gilbert (git) wrote:

> > +- Busses and devices should be able to explicitly specify addresses when
> > +  instantiated, and management tools should use those.  For example,
> > +  when hot adding USB devices it's important to specify the ports
> > +  and addresses, since implicit ordering based on the command line order
> > +  may be different on the destination.  This can result in the
> > +  device state being loaded into the wrong device.
> 
> Would you like to add a note about taking care of migrating drc states incase
> of hot adding devices, that could ensure hotunplug device safely after
> migration ?

That's something Power specific as I understand, but I don't know any
details of it.  What would you say as a general warning ?

> > +When we migrate a device, we save/load the state as a series
> > +of fields.  Some times, due to bugs or new functionality, we need to
> > +change the state to store more/different information.  Changing the 
> > migration
> > +state saved for a device can break migration comppatibility unless
> > +care is taken to use the appropriate techniques.  In general QEMU tries
> > +to maintain forward migration compaitibility (i.e. migrating from
> > +QEMU n->n+1) and there are users who benefit from backwards compatibility
> 
> typo - %s/backwards/backward

Interesting, I'd always said 'backwards' and there's only one
place I said 'backward', but I guess 'backward' is more consistent with
'forward' so I'll remove all the 's's

> > +Firmware
> > +
> > +
> > +Migration migrates the copies of RAM and ROM, and thus when running
> > +on the destination it includes the firmware from the source. Even after
> > +resetting a VM, the old firmware is used.   Only once QEMU has been 
> > restarted
> 
> typo with 2 spaces

I seem to have 3 there; I use 2 in most places; so made it consistent.

Thanks,

Dave

> Only after QEMU has been restarted the new firmware will be used.
> 
> -- Bala
> 
> > +is the new firmware in use.
> > +
> > +- Changes in firmware size can cause changes in the required RAMBlock size
> > +  to hold the firmware and thus migration can fail.  In practice it's best
> > +  to pad firmware images to convenient powers of 2 with plenty of space
> > +  for growth.
> > +
> > +- Care should be taken with device emulation code so that newer
> > +  emulation code can work with older firmware to allow forward migration.
> > +
> > +- Care should be taken with newer firmware so that backwards migration
> > +  to older systems with older device emulation code will work.
> > +
> > +In some cases it may be best to tie specific firmware versions to specific
> > +versioned machine types to cut down on the combinations that will need
> > +support.  This is also useful when newer versions of firmware outgrow
> > +the padding.
> > +
> > -- 
> > 2.17.0
> > 
> > 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-27 Thread Dr. David Alan Gilbert
* Peter Xu (pet...@redhat.com) wrote:
> On Fri, Apr 20, 2018 at 06:57:21PM +0100, Dr. David Alan Gilbert (git) wrote:
> 
> [...]
> 
> >  Saving the state of one device
> >  ==
> >  
> > -The state of a device is saved using intermediate buffers.  There are
> > -some helper functions to assist this saving.
> > -
> > -There is a new concept that we have to explain here: device state
> > -version.  When we migrate a device, we save/load the state as a series
> > -of fields.  Some times, due to bugs or new functionality, we need to
> > -change the state to store more/different information.  We use the
> > -version to identify each time that we do a change.  Each version is
> > -associated with a series of fields saved.  The `save_state` always saves
> > -the state as the newer version.  But `load_state` sometimes is able to
> > -load state from an older version.
> > -
> > -Legacy way
> > ---
> > -
> > -This way is going to disappear as soon as all current users are ported to 
> > VMSTATE.
> > -
> > -Each device has to register two functions, one to save the state and
> > -another to load the state back.
> > -
> > -.. code:: c
> > -
> > -  int register_savevm(DeviceState *dev,
> > -  const char *idstr,
> > -  int instance_id,
> > -  int version_id,
> > -  SaveStateHandler *save_state,
> > -  LoadStateHandler *load_state,
> > -  void *opaque);
> > -
> > -  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
> > -  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
> > -
> > -The important functions for the device state format are the `save_state`
> > -and `load_state`.  Notice that `load_state` receives a version_id
> > -parameter to know what state format is receiving.  `save_state` doesn't
> > -have a version_id parameter because it always uses the latest version.
> > +For most devices, the state is saved in a single call to the migration
> > +infrastrucutre; these are *non-iterative* devices.  The data for these
> > +devices is sent at the end of precopy migration, when the CPUs are paused.
> 
> > +Where the data associated with the device is very large (e.g. RAM or large 
> > tables)
> > +see the iterative device section below.
> 
> I would prefer:
> 
>   There are also devices called *iterative* devices, which contain a
>   huge amount of data as device states (e.g., RAM or large tables).
>   Please refer to the iterative device section below for more information.

OK, I've gone with:
There are also *iterative* devices, which contain a very large amount of
data (e.g. RAM or large tables).  See the iterative device section below.

> > +
> > +General advice for device developers
> > +
> > +
> > +- The migration state saved should reflect the device being modelled rather
> > +  than the way your implementation works.   That way if you change the 
> > implementation
> > +  later the migration stream will stay compatible.  That model may include
> > +  internal state that's not directly visible in a register.
> > +
> > +- When saving a migration stream the device code may walk and check
> > +  the state of the device.  These checks might fail in various ways (e.g.
> > +  discovering internal state is corrupt or that the guest has done 
> > something bad).
> > +  Consider carefully before asserting/aborting at this point, since the
> > +  normal response from users is that *migration broke their VM* since it 
> > had
> > +  apparently been running fine until then.
> 
> Maybe we can give some further suggestions?
> 
>   Instead of aborting the whole VM, we can dump some error messages
>   when necessary for further diagnose. Or, we can set the device into
>   error state (or even disable the device) where capable so that at
>   least other part of the VM won't be affected, then the guest OS can
>   decide what to do next with that.

I've put:
  In these error cases, the device
  should log a message indicating the cause of error, and should consider
  putting the device into an error state, allowing the rest of the VM to
  continue execution.


> > +
> > +- The migration might happen at an inconvenient point,
> > +  e.g. right in the middle of the guest reprogramming the device, during
> > +  guest reboot or shutdown or while the device is waiting for external IO.
> > +  It's strongly preferred that migrations do not fail in this situation,
> > +  since in the cloud environment migrations might happen automatically to
> > +  VMs that the administrator doesn't directly control.
> > +
> > +- If you do need to fail a migration, ensure that sufficient information
> > +  is logged to identify what went wrong.
> > +
> > +- The destination should treat an incoming migration stream as hostile
> > +  (which we do to varying degrees in the existing code).  Check that 
> > offsets
> > +  into buffers and the 

Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-25 Thread Peter Xu
On Fri, Apr 20, 2018 at 06:57:21PM +0100, Dr. David Alan Gilbert (git) wrote:

[...]

>  Saving the state of one device
>  ==
>  
> -The state of a device is saved using intermediate buffers.  There are
> -some helper functions to assist this saving.
> -
> -There is a new concept that we have to explain here: device state
> -version.  When we migrate a device, we save/load the state as a series
> -of fields.  Some times, due to bugs or new functionality, we need to
> -change the state to store more/different information.  We use the
> -version to identify each time that we do a change.  Each version is
> -associated with a series of fields saved.  The `save_state` always saves
> -the state as the newer version.  But `load_state` sometimes is able to
> -load state from an older version.
> -
> -Legacy way
> ---
> -
> -This way is going to disappear as soon as all current users are ported to 
> VMSTATE.
> -
> -Each device has to register two functions, one to save the state and
> -another to load the state back.
> -
> -.. code:: c
> -
> -  int register_savevm(DeviceState *dev,
> -  const char *idstr,
> -  int instance_id,
> -  int version_id,
> -  SaveStateHandler *save_state,
> -  LoadStateHandler *load_state,
> -  void *opaque);
> -
> -  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
> -  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
> -
> -The important functions for the device state format are the `save_state`
> -and `load_state`.  Notice that `load_state` receives a version_id
> -parameter to know what state format is receiving.  `save_state` doesn't
> -have a version_id parameter because it always uses the latest version.
> +For most devices, the state is saved in a single call to the migration
> +infrastrucutre; these are *non-iterative* devices.  The data for these
> +devices is sent at the end of precopy migration, when the CPUs are paused.

> +Where the data associated with the device is very large (e.g. RAM or large 
> tables)
> +see the iterative device section below.

I would prefer:

  There are also devices called *iterative* devices, which contain a
  huge amount of data as device states (e.g., RAM or large tables).
  Please refer to the iterative device section below for more information.

> +
> +General advice for device developers
> +
> +
> +- The migration state saved should reflect the device being modelled rather
> +  than the way your implementation works.   That way if you change the 
> implementation
> +  later the migration stream will stay compatible.  That model may include
> +  internal state that's not directly visible in a register.
> +
> +- When saving a migration stream the device code may walk and check
> +  the state of the device.  These checks might fail in various ways (e.g.
> +  discovering internal state is corrupt or that the guest has done something 
> bad).
> +  Consider carefully before asserting/aborting at this point, since the
> +  normal response from users is that *migration broke their VM* since it had
> +  apparently been running fine until then.

Maybe we can give some further suggestions?

  Instead of aborting the whole VM, we can dump some error messages
  when necessary for further diagnose. Or, we can set the device into
  error state (or even disable the device) where capable so that at
  least other part of the VM won't be affected, then the guest OS can
  decide what to do next with that.

> +
> +- The migration might happen at an inconvenient point,
> +  e.g. right in the middle of the guest reprogramming the device, during
> +  guest reboot or shutdown or while the device is waiting for external IO.
> +  It's strongly preferred that migrations do not fail in this situation,
> +  since in the cloud environment migrations might happen automatically to
> +  VMs that the administrator doesn't directly control.
> +
> +- If you do need to fail a migration, ensure that sufficient information
> +  is logged to identify what went wrong.
> +
> +- The destination should treat an incoming migration stream as hostile
> +  (which we do to varying degrees in the existing code).  Check that offsets
> +  into buffers and the like can't cause overruns.  Fail the incoming 
> migration
> +  in the case of a corrupted stream like this.
> +
> +- Take care with internal device state or behaviour that might become
> +  migration version dependent.  For example, the order of PCI capabilities
> +  is required to stay constant across migration.  Another example would
> +  be that a special case handled by subsections (see below) might become
> +  much more common if a default behaviour is changed.
> +
> +- Migrations timing out or being failed by higher levels of management,
> +  or failures of the destination host are not unusual, and care should

Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-25 Thread Balamuruhan S
On Fri, Apr 20, 2018 at 06:57:21PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Update the migration docs:
> 
> Among other changes:
>   * Added a general list of advice for device authors
>   * Reordered the section on conditional state (subsections etc)
> into the order we prefer.
>   * Add a note about firmware
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  docs/devel/migration.rst | 524 +++
>  1 file changed, 370 insertions(+), 154 deletions(-)
> 
> diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
> index e32b087f6e..5e2b2a2760 100644
> --- a/docs/devel/migration.rst
> +++ b/docs/devel/migration.rst
> @@ -28,11 +28,11 @@ the guest to be stopped.  Typically the time that the 
> guest is
>  unresponsive during live migration is the low hundred of milliseconds
>  (notice that this depends on a lot of things).
> 
> -Types of migration
> -==
> +Transports
> +==
> 
> -Now that we have talked about live migration, there are several ways
> -to do migration:
> +The migration stream is normally just a byte stream that can be passed
> +over any transport.
> 
>  - tcp migration: do the migration using tcp sockets
>  - unix migration: do the migration using unix sockets
> @@ -40,16 +40,16 @@ to do migration:
>  - fd migration: do the migration using an file descriptor that is
>passed to QEMU.  QEMU doesn't care how this file descriptor is opened.
> 
> -All these four migration protocols use the same infrastructure to
> +In addition support is included for migration using RDMA migration which
> +transports the page data using ``RDMA``, where the hardware takes care of
> +transporting the pages, and the load on the CPU is much lower.  While the
> +internals of RDMA migration are a bit different, this isn't really visible
> +outside the RAM migration code.
> +
> +All these migration protocols use the same infrastructure to
>  save/restore state devices.  This infrastructure is shared with the
>  savevm/loadvm functionality.
> 
> -State Live Migration
> -
> -
> -This is used for RAM and block devices.  It is not yet ported to vmstate.
> -
> -
>  Common infrastructure
>  =
> 
> @@ -57,60 +57,72 @@ The files, sockets or fd's that carry the migration 
> stream are abstracted by
>  the  ``QEMUFile`` type (see `migration/qemu-file.h`).  In most cases this
>  is connected to a subtype of ``QIOChannel`` (see `io/`).
> 
> +
>  Saving the state of one device
>  ==
> 
> -The state of a device is saved using intermediate buffers.  There are
> -some helper functions to assist this saving.
> -
> -There is a new concept that we have to explain here: device state
> -version.  When we migrate a device, we save/load the state as a series
> -of fields.  Some times, due to bugs or new functionality, we need to
> -change the state to store more/different information.  We use the
> -version to identify each time that we do a change.  Each version is
> -associated with a series of fields saved.  The `save_state` always saves
> -the state as the newer version.  But `load_state` sometimes is able to
> -load state from an older version.
> -
> -Legacy way
> ---
> -
> -This way is going to disappear as soon as all current users are ported to 
> VMSTATE.
> -
> -Each device has to register two functions, one to save the state and
> -another to load the state back.
> -
> -.. code:: c
> -
> -  int register_savevm(DeviceState *dev,
> -  const char *idstr,
> -  int instance_id,
> -  int version_id,
> -  SaveStateHandler *save_state,
> -  LoadStateHandler *load_state,
> -  void *opaque);
> -
> -  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
> -  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
> -
> -The important functions for the device state format are the `save_state`
> -and `load_state`.  Notice that `load_state` receives a version_id
> -parameter to know what state format is receiving.  `save_state` doesn't
> -have a version_id parameter because it always uses the latest version.
> +For most devices, the state is saved in a single call to the migration
> +infrastrucutre; these are *non-iterative* devices.  The data for these
> +devices is sent at the end of precopy migration, when the CPUs are paused.
> +Where the data associated with the device is very large (e.g. RAM or large 
> tables)
> +see the iterative device section below.
> +
> +General advice for device developers
> +
> +
> +- The migration state saved should reflect the device being modelled rather
> +  than the way your implementation works.   That way if you change the 
> implementation
> +  later the migration stream will stay compatible.  That model 

Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-23 Thread Dr. David Alan Gilbert
* Eric Blake (ebl...@redhat.com) wrote:
> On 04/20/2018 12:57 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Update the migration docs:
> > 
> > Among other changes:
> >   * Added a general list of advice for device authors
> >   * Reordered the section on conditional state (subsections etc)
> > into the order we prefer.
> >   * Add a note about firmware
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  docs/devel/migration.rst | 524 +++
> >  1 file changed, 370 insertions(+), 154 deletions(-)
> 
> > @@ -40,16 +40,16 @@ to do migration:
> >  - fd migration: do the migration using an file descriptor that is
> >passed to QEMU.  QEMU doesn't care how this file descriptor is opened.
> >  
> > -All these four migration protocols use the same infrastructure to
> > +In addition support is included for migration using RDMA migration which
> 
> The double migration in this phrase sounds redundant.  I think you can:
> 
> s/In addition/In addition,/
> s/RDMA migration which/RDMA, which/

Done

> > +transports the page data using ``RDMA``, where the hardware takes care of
> > +transporting the pages, and the load on the CPU is much lower.  While the
> > +internals of RDMA migration are a bit different, this isn't really visible
> > +outside the RAM migration code.
> > +
> 
> > +For most devices, the state is saved in a single call to the migration
> > +infrastrucutre; these are *non-iterative* devices.  The data for these
> 
> s/infrastrucutre/infrastructure/

Done

> > +devices is sent at the end of precopy migration, when the CPUs are paused.
> > +Where the data associated with the device is very large (e.g. RAM or large 
> > tables)
> > +see the iterative device section below.
> 
> > +
> > +- Migrations timing out or being failed by higher levels of management,
> > +  or failures of the destination host are not unusual, and care should
> > +  be taken to ensure that the source VM can be restarted up until the point
> > +  when the destination starts runing.  Valid examples include the 
> > management
> 
> s/runing/running/

Done

> > +  layer reverting the migration even though the QEMU level of migration has
> > +  succeeded.  For this reason, the state on the source VM should not be
> > +  destroyed during the migration process in normal use.
> > +
> > +- Busses and devices should be able to explicitly specify addresses when
> 
> s/Busses/Buses/ ? (both spellings are common, but busses can also be
> confused with a synonym of kisses)

Done (best not to ask...)

> > +When we migrate a device, we save/load the state as a series
> > +of fields.  Some times, due to bugs or new functionality, we need to
> > +change the state to store more/different information.  Changing the 
> > migration
> > +state saved for a device can break migration comppatibility unless
> 
> s/comppatibility/compatibility/

Done
> > +care is taken to use the appropriate techniques.  In general QEMU tries
> > +to maintain forward migration compaitibility (i.e. migrating from
> 
> s/compaitibility/compatibility/

Done
> 
> > +QEMU n->n+1) and there are users who benefit from backwards compatibility
> > +as well.
> 
> (fun - 3 spellings among 3 uses in 2 sentences - at least the last one
> was right)
> 
> 
> > +Note that the contents of the sections for iterative migration tend
> > +to be open-coded by the devices;  care should be taken in parsing
> 
> Why the double space?

No particular reason; gone.

> 
> > +
> > +The ``device data`` in each section consists of the data produced
> > +by the code described above.  For non-iterative devices they have a single
> > +section; iterative devices have an initial and last section and a set
> > +of parts inbetween.
> 
> s/inbetween/in between/

Done

> > +Note that there is very little checking by the common code of the integrity
> > +of the ``device data`` contents, that's upto the devices themselves.
> 
> s/upto/up to/

Done

> > +Only a unidirectional stream is required for normal migration, however a
> > +``return path`` can be created when bidirecitonal communication is desired.
> 
> s/bidirecitonal/bidirectional/

Done

Thanks

Dave

> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH] migration: update docs

2018-04-20 Thread Eric Blake
On 04/20/2018 12:57 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Update the migration docs:
> 
> Among other changes:
>   * Added a general list of advice for device authors
>   * Reordered the section on conditional state (subsections etc)
> into the order we prefer.
>   * Add a note about firmware
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  docs/devel/migration.rst | 524 +++
>  1 file changed, 370 insertions(+), 154 deletions(-)

> @@ -40,16 +40,16 @@ to do migration:
>  - fd migration: do the migration using an file descriptor that is
>passed to QEMU.  QEMU doesn't care how this file descriptor is opened.
>  
> -All these four migration protocols use the same infrastructure to
> +In addition support is included for migration using RDMA migration which

The double migration in this phrase sounds redundant.  I think you can:

s/In addition/In addition,/
s/RDMA migration which/RDMA, which/

> +transports the page data using ``RDMA``, where the hardware takes care of
> +transporting the pages, and the load on the CPU is much lower.  While the
> +internals of RDMA migration are a bit different, this isn't really visible
> +outside the RAM migration code.
> +

> +For most devices, the state is saved in a single call to the migration
> +infrastrucutre; these are *non-iterative* devices.  The data for these

s/infrastrucutre/infrastructure/

> +devices is sent at the end of precopy migration, when the CPUs are paused.
> +Where the data associated with the device is very large (e.g. RAM or large 
> tables)
> +see the iterative device section below.

> +
> +- Migrations timing out or being failed by higher levels of management,
> +  or failures of the destination host are not unusual, and care should
> +  be taken to ensure that the source VM can be restarted up until the point
> +  when the destination starts runing.  Valid examples include the management

s/runing/running/

> +  layer reverting the migration even though the QEMU level of migration has
> +  succeeded.  For this reason, the state on the source VM should not be
> +  destroyed during the migration process in normal use.
> +
> +- Busses and devices should be able to explicitly specify addresses when

s/Busses/Buses/ ? (both spellings are common, but busses can also be
confused with a synonym of kisses)

> +When we migrate a device, we save/load the state as a series
> +of fields.  Some times, due to bugs or new functionality, we need to
> +change the state to store more/different information.  Changing the migration
> +state saved for a device can break migration comppatibility unless

s/comppatibility/compatibility/

> +care is taken to use the appropriate techniques.  In general QEMU tries
> +to maintain forward migration compaitibility (i.e. migrating from

s/compaitibility/compatibility/

> +QEMU n->n+1) and there are users who benefit from backwards compatibility
> +as well.

(fun - 3 spellings among 3 uses in 2 sentences - at least the last one
was right)


> +Note that the contents of the sections for iterative migration tend
> +to be open-coded by the devices;  care should be taken in parsing

Why the double space?


> +
> +The ``device data`` in each section consists of the data produced
> +by the code described above.  For non-iterative devices they have a single
> +section; iterative devices have an initial and last section and a set
> +of parts inbetween.

s/inbetween/in between/

> +Note that there is very little checking by the common code of the integrity
> +of the ``device data`` contents, that's upto the devices themselves.

s/upto/up to/

> +Only a unidirectional stream is required for normal migration, however a
> +``return path`` can be created when bidirecitonal communication is desired.

s/bidirecitonal/bidirectional/

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] migration: update docs

2018-04-20 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

Update the migration docs:

Among other changes:
  * Added a general list of advice for device authors
  * Reordered the section on conditional state (subsections etc)
into the order we prefer.
  * Add a note about firmware

Signed-off-by: Dr. David Alan Gilbert 
---
 docs/devel/migration.rst | 524 +++
 1 file changed, 370 insertions(+), 154 deletions(-)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index e32b087f6e..5e2b2a2760 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -28,11 +28,11 @@ the guest to be stopped.  Typically the time that the guest 
is
 unresponsive during live migration is the low hundred of milliseconds
 (notice that this depends on a lot of things).
 
-Types of migration
-==
+Transports
+==
 
-Now that we have talked about live migration, there are several ways
-to do migration:
+The migration stream is normally just a byte stream that can be passed
+over any transport.
 
 - tcp migration: do the migration using tcp sockets
 - unix migration: do the migration using unix sockets
@@ -40,16 +40,16 @@ to do migration:
 - fd migration: do the migration using an file descriptor that is
   passed to QEMU.  QEMU doesn't care how this file descriptor is opened.
 
-All these four migration protocols use the same infrastructure to
+In addition support is included for migration using RDMA migration which
+transports the page data using ``RDMA``, where the hardware takes care of
+transporting the pages, and the load on the CPU is much lower.  While the
+internals of RDMA migration are a bit different, this isn't really visible
+outside the RAM migration code.
+
+All these migration protocols use the same infrastructure to
 save/restore state devices.  This infrastructure is shared with the
 savevm/loadvm functionality.
 
-State Live Migration
-
-
-This is used for RAM and block devices.  It is not yet ported to vmstate.
-
-
 Common infrastructure
 =
 
@@ -57,60 +57,72 @@ The files, sockets or fd's that carry the migration stream 
are abstracted by
 the  ``QEMUFile`` type (see `migration/qemu-file.h`).  In most cases this
 is connected to a subtype of ``QIOChannel`` (see `io/`).
 
+
 Saving the state of one device
 ==
 
-The state of a device is saved using intermediate buffers.  There are
-some helper functions to assist this saving.
-
-There is a new concept that we have to explain here: device state
-version.  When we migrate a device, we save/load the state as a series
-of fields.  Some times, due to bugs or new functionality, we need to
-change the state to store more/different information.  We use the
-version to identify each time that we do a change.  Each version is
-associated with a series of fields saved.  The `save_state` always saves
-the state as the newer version.  But `load_state` sometimes is able to
-load state from an older version.
-
-Legacy way
---
-
-This way is going to disappear as soon as all current users are ported to 
VMSTATE.
-
-Each device has to register two functions, one to save the state and
-another to load the state back.
-
-.. code:: c
-
-  int register_savevm(DeviceState *dev,
-  const char *idstr,
-  int instance_id,
-  int version_id,
-  SaveStateHandler *save_state,
-  LoadStateHandler *load_state,
-  void *opaque);
-
-  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
-  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
-
-The important functions for the device state format are the `save_state`
-and `load_state`.  Notice that `load_state` receives a version_id
-parameter to know what state format is receiving.  `save_state` doesn't
-have a version_id parameter because it always uses the latest version.
+For most devices, the state is saved in a single call to the migration
+infrastrucutre; these are *non-iterative* devices.  The data for these
+devices is sent at the end of precopy migration, when the CPUs are paused.
+Where the data associated with the device is very large (e.g. RAM or large 
tables)
+see the iterative device section below.
+
+General advice for device developers
+
+
+- The migration state saved should reflect the device being modelled rather
+  than the way your implementation works.   That way if you change the 
implementation
+  later the migration stream will stay compatible.  That model may include
+  internal state that's not directly visible in a register.
+
+- When saving a migration stream the device code may walk and check
+  the state of the device.  These checks might fail in various ways (e.g.
+  discovering internal state is corrupt or that the guest has done something 
bad).
+  

Re: [Qemu-devel] [PATCH] migration: Update docs to discourage version bumps

2017-02-10 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)"  writes:
> 
> > From: "Dr. David Alan Gilbert" 
> >
> > Version bumps break backwards migration; update the docs
> > to explain to people that's bad and how to avoid it.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  docs/migration.txt | 42 ++
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/docs/migration.txt b/docs/migration.txt
> > index b462ead..f375f4b 100644
> > --- a/docs/migration.txt
> > +++ b/docs/migration.txt
> > @@ -161,6 +161,11 @@ include/hw/hw.h.
> >  
> >  === More about versions ===
> >  
> > +Version numbers are intended for major incompatible changes to the
> > +migration of a device, and using them breaks backwards-migration
> > +compatibility; in general most changes can be made by adding Subsections
> > +(see below) or _TEST macros (see below) which won't break compatibility.
> > +
> >  You can see that there are several version fields:
> >  
> >  - version_id: the maximum version_id supported by VMState for that device.
> > @@ -175,6 +180,9 @@ version_id.  And the function load_state_old() (if 
> > present) is able to
> >  load state from minimum_version_id_old to minimum_version_id.  This
> >  function is deprecated and will be removed when no more users are left.
> >  
> > +The VMState with the 'version_id' value will always be generated and thus
> > +can't be loaded by any older QEMU.
> > +
> 
> Suggest active voice: "Saving state will always create ...".
> 
> >  ===  Massaging functions ===
> >  
> >  Sometimes, it is not enough to be able to save the state directly
> > @@ -292,6 +300,40 @@ save/send this state when we are in the middle of a 
> > pio operation
> >  not enabled, the values on that fields are garbage and don't need to
> >  be sent.
> >  
> > +Using a condition function that checks a 'property' to determine whether
> > +to send a subsection allows backwards migration compatibility.
> > +For example;
> > +   a) Add a new property using DEFINE_PROP_BOOL - e.g. support-foo and
> > +  default it to true.
> > +   b) Add an entry to the HW_COMPAT_ for the previous version
> > +  that sets the property to false.
> > +   c) Add a static bool  support_foo function
> 
> Let's add "... that tests the property"
> 
> > +   d) Add a subsection with a .needed set to the support_foo function
> > +   e) (potentially) Add a pre_load that sets up a default value for 'foo'
> > +  to be used if the subsection isn't loaded.
> > +
> > +Now that subsection will not be generated when using an older
> > +machine type and the migration stream will be accepted by older
> > +QEMU versions.
> 
> Suppressing a subsection for older machine types is obviously fine when
> the device state transmitted in the subsection is unused with these
> machine types.  This isn't usually the case, however.
> 
> If it's used, then it resets to a default state on migration.  Not
> visible when it already is in the default state.  When it isn't,
> migration has a side effect on the device, which can range from from
> benign to disastrous.  Trade-off: ability to migrate vs. side effect.  I
> think we should point it out explicitly.
> 
> When the side effect is too serious to accept, but non-default state is
> sufficiently rare, we can choose to still enable backward migration in
> default state, by having .needed() return "is in non-default state".
> Improves "can't migrate backwards" to "occasionally can't migrate
> backwards".  I think this technique should be mentioned as well.  I know
> you dislike the "random" failures it brings; feel free to add dire
> warnings.

OK, I've added an extra paragraph for that in the v2 I just posted.

Dave

> > +
> > += Not sending existing elements =
> > +
> > +Sometimes members of the VMState are no longer needed;
> > +  removing them will break migration compatibility
> > +  making them version dependent and bumping the version will break 
> > backwards
> > +   migration compatibility.
> > +
> > +The best way is to:
> > +  a) Add a new property/compatibility/function in the same way for 
> > subsections
> > +above.
> > +  b) replace the VMSTATE macro with the _TEST version of the macro, e.g.:
> > + VMSTATE_UINT32(foo, barstruct)
> > +   becomes
> > + VMSTATE_UINT32_TEST(foo, barstruct, pre_version_baz)
> > +
> > +  Sometime in the future when we no longer care about the ancient
> > +versions these can be killed off.
> > +
> >  = Return path =
> >  
> >  In most migration scenarios there is only a single data path that runs
> 
> Lovely improvement, thanks!
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH] migration: Update docs to discourage version bumps

2017-02-10 Thread Markus Armbruster
"Dr. David Alan Gilbert (git)"  writes:

> From: "Dr. David Alan Gilbert" 
>
> Version bumps break backwards migration; update the docs
> to explain to people that's bad and how to avoid it.
>
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  docs/migration.txt | 42 ++
>  1 file changed, 42 insertions(+)
>
> diff --git a/docs/migration.txt b/docs/migration.txt
> index b462ead..f375f4b 100644
> --- a/docs/migration.txt
> +++ b/docs/migration.txt
> @@ -161,6 +161,11 @@ include/hw/hw.h.
>  
>  === More about versions ===
>  
> +Version numbers are intended for major incompatible changes to the
> +migration of a device, and using them breaks backwards-migration
> +compatibility; in general most changes can be made by adding Subsections
> +(see below) or _TEST macros (see below) which won't break compatibility.
> +
>  You can see that there are several version fields:
>  
>  - version_id: the maximum version_id supported by VMState for that device.
> @@ -175,6 +180,9 @@ version_id.  And the function load_state_old() (if 
> present) is able to
>  load state from minimum_version_id_old to minimum_version_id.  This
>  function is deprecated and will be removed when no more users are left.
>  
> +The VMState with the 'version_id' value will always be generated and thus
> +can't be loaded by any older QEMU.
> +

Suggest active voice: "Saving state will always create ...".

>  ===  Massaging functions ===
>  
>  Sometimes, it is not enough to be able to save the state directly
> @@ -292,6 +300,40 @@ save/send this state when we are in the middle of a pio 
> operation
>  not enabled, the values on that fields are garbage and don't need to
>  be sent.
>  
> +Using a condition function that checks a 'property' to determine whether
> +to send a subsection allows backwards migration compatibility.
> +For example;
> +   a) Add a new property using DEFINE_PROP_BOOL - e.g. support-foo and
> +  default it to true.
> +   b) Add an entry to the HW_COMPAT_ for the previous version
> +  that sets the property to false.
> +   c) Add a static bool  support_foo function

Let's add "... that tests the property"

> +   d) Add a subsection with a .needed set to the support_foo function
> +   e) (potentially) Add a pre_load that sets up a default value for 'foo'
> +  to be used if the subsection isn't loaded.
> +
> +Now that subsection will not be generated when using an older
> +machine type and the migration stream will be accepted by older
> +QEMU versions.

Suppressing a subsection for older machine types is obviously fine when
the device state transmitted in the subsection is unused with these
machine types.  This isn't usually the case, however.

If it's used, then it resets to a default state on migration.  Not
visible when it already is in the default state.  When it isn't,
migration has a side effect on the device, which can range from from
benign to disastrous.  Trade-off: ability to migrate vs. side effect.  I
think we should point it out explicitly.

When the side effect is too serious to accept, but non-default state is
sufficiently rare, we can choose to still enable backward migration in
default state, by having .needed() return "is in non-default state".
Improves "can't migrate backwards" to "occasionally can't migrate
backwards".  I think this technique should be mentioned as well.  I know
you dislike the "random" failures it brings; feel free to add dire
warnings.

> +
> += Not sending existing elements =
> +
> +Sometimes members of the VMState are no longer needed;
> +  removing them will break migration compatibility
> +  making them version dependent and bumping the version will break backwards
> +   migration compatibility.
> +
> +The best way is to:
> +  a) Add a new property/compatibility/function in the same way for 
> subsections
> +above.
> +  b) replace the VMSTATE macro with the _TEST version of the macro, e.g.:
> + VMSTATE_UINT32(foo, barstruct)
> +   becomes
> + VMSTATE_UINT32_TEST(foo, barstruct, pre_version_baz)
> +
> +  Sometime in the future when we no longer care about the ancient
> +versions these can be killed off.
> +
>  = Return path =
>  
>  In most migration scenarios there is only a single data path that runs

Lovely improvement, thanks!



[Qemu-devel] [PATCH] migration: Update docs to discourage version bumps

2017-02-09 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

Version bumps break backwards migration; update the docs
to explain to people that's bad and how to avoid it.

Signed-off-by: Dr. David Alan Gilbert 
---
 docs/migration.txt | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/docs/migration.txt b/docs/migration.txt
index b462ead..f375f4b 100644
--- a/docs/migration.txt
+++ b/docs/migration.txt
@@ -161,6 +161,11 @@ include/hw/hw.h.
 
 === More about versions ===
 
+Version numbers are intended for major incompatible changes to the
+migration of a device, and using them breaks backwards-migration
+compatibility; in general most changes can be made by adding Subsections
+(see below) or _TEST macros (see below) which won't break compatibility.
+
 You can see that there are several version fields:
 
 - version_id: the maximum version_id supported by VMState for that device.
@@ -175,6 +180,9 @@ version_id.  And the function load_state_old() (if present) 
is able to
 load state from minimum_version_id_old to minimum_version_id.  This
 function is deprecated and will be removed when no more users are left.
 
+The VMState with the 'version_id' value will always be generated and thus
+can't be loaded by any older QEMU.
+
 ===  Massaging functions ===
 
 Sometimes, it is not enough to be able to save the state directly
@@ -292,6 +300,40 @@ save/send this state when we are in the middle of a pio 
operation
 not enabled, the values on that fields are garbage and don't need to
 be sent.
 
+Using a condition function that checks a 'property' to determine whether
+to send a subsection allows backwards migration compatibility.
+For example;
+   a) Add a new property using DEFINE_PROP_BOOL - e.g. support-foo and
+  default it to true.
+   b) Add an entry to the HW_COMPAT_ for the previous version
+  that sets the property to false.
+   c) Add a static bool  support_foo function
+   d) Add a subsection with a .needed set to the support_foo function
+   e) (potentially) Add a pre_load that sets up a default value for 'foo'
+  to be used if the subsection isn't loaded.
+
+Now that subsection will not be generated when using an older
+machine type and the migration stream will be accepted by older
+QEMU versions.
+
+= Not sending existing elements =
+
+Sometimes members of the VMState are no longer needed;
+  removing them will break migration compatibility
+  making them version dependent and bumping the version will break backwards
+   migration compatibility.
+
+The best way is to:
+  a) Add a new property/compatibility/function in the same way for subsections
+above.
+  b) replace the VMSTATE macro with the _TEST version of the macro, e.g.:
+ VMSTATE_UINT32(foo, barstruct)
+   becomes
+ VMSTATE_UINT32_TEST(foo, barstruct, pre_version_baz)
+
+  Sometime in the future when we no longer care about the ancient
+versions these can be killed off.
+
 = Return path =
 
 In most migration scenarios there is only a single data path that runs
-- 
2.9.3