[Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState

Peter Xu posted 6 patches 8 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1496980142-8986-1-git-send-email-peterx@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker failed
Test s390x passed
There is a newer version of this series
hw/core/machine.c             |  6 +--
hw/i386/pc_piix.c             |  3 --
hw/ppc/spapr.c                |  3 --
hw/xen/xen-common.c           | 12 ++++--
include/hw/boards.h           |  3 ++
include/hw/compat.h           | 12 ++++++
include/migration/migration.h | 36 +++++++++++++++--
include/sysemu/sysemu.h       |  1 -
migration/migration.c         | 92 +++++++++++++++++++++++++++++--------------
migration/savevm.c            | 28 ++++---------
vl.c                          |  9 ++++-
11 files changed, 136 insertions(+), 69 deletions(-)
[Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Peter Xu 8 years, 4 months ago
v2
- (I didn't add Juan's r-b since I touched the patches)
- remove once parameter in migrate_get_current() since not needed
- add one more patch to export register_compat_prop(), then use it in
  the following patches in xen_init().

I picked this topic out as suggested by Juan. Also I did what Juan has
suggested in previous discussions that I moved lots of global
parameters into MigrationState, and let them be properties. Then we
can use HW_COMPAT_* and "-global migration.xxx=xxx" formular.

Currently register_compat_prop() is exported to be used by xen_init().

If this can be merged and okay, we can move on to convert more things
into properties for migration.

Please review. Thanks.

Peter Xu (6):
  machine: export register_compat_prop()
  migration: let MigrationState be a qdev
  migration: move global_state.optional out
  migration: move only_migratable to MigrationState
  migration: move skip_configuration out
  migration: move skip_section_footers

 hw/core/machine.c             |  6 +--
 hw/i386/pc_piix.c             |  3 --
 hw/ppc/spapr.c                |  3 --
 hw/xen/xen-common.c           | 12 ++++--
 include/hw/boards.h           |  3 ++
 include/hw/compat.h           | 12 ++++++
 include/migration/migration.h | 36 +++++++++++++++--
 include/sysemu/sysemu.h       |  1 -
 migration/migration.c         | 92 +++++++++++++++++++++++++++++--------------
 migration/savevm.c            | 28 ++++---------
 vl.c                          |  9 ++++-
 11 files changed, 136 insertions(+), 69 deletions(-)

-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Juan Quintela 8 years, 4 months ago
Peter Xu <peterx@redhat.com> wrote:
> v2
> - (I didn't add Juan's r-b since I touched the patches)
> - remove once parameter in migrate_get_current() since not needed
> - add one more patch to export register_compat_prop(), then use it in
>   the following patches in xen_init().
>
> I picked this topic out as suggested by Juan. Also I did what Juan has
> suggested in previous discussions that I moved lots of global
> parameters into MigrationState, and let them be properties. Then we
> can use HW_COMPAT_* and "-global migration.xxx=xxx" formular.
>
> Currently register_compat_prop() is exported to be used by xen_init().
>
> If this can be merged and okay, we can move on to convert more things
> into properties for migration.
>
> Please review. Thanks.

Hi

Nice work, thanks.

One question, once this is accepted, it would be easy to add to this the
migration_capabilities and migration_paramenters, right?

Later, Juan.

Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Peter Xu 8 years, 4 months ago
On Fri, Jun 09, 2017 at 09:48:32AM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > v2
> > - (I didn't add Juan's r-b since I touched the patches)
> > - remove once parameter in migrate_get_current() since not needed
> > - add one more patch to export register_compat_prop(), then use it in
> >   the following patches in xen_init().
> >
> > I picked this topic out as suggested by Juan. Also I did what Juan has
> > suggested in previous discussions that I moved lots of global
> > parameters into MigrationState, and let them be properties. Then we
> > can use HW_COMPAT_* and "-global migration.xxx=xxx" formular.
> >
> > Currently register_compat_prop() is exported to be used by xen_init().
> >
> > If this can be merged and okay, we can move on to convert more things
> > into properties for migration.
> >
> > Please review. Thanks.
> 
> Hi
> 
> Nice work, thanks.
> 
> One question, once this is accepted, it would be easy to add to this the
> migration_capabilities and migration_paramenters, right?

Yes. If this series would be merged, I'll work on them soon. Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Markus Armbruster 8 years, 4 months ago
Test compile gripes:

    hw/xen/xen-common.c: In function ‘xen_init’:
    hw/xen/xen-common.c:147:5: warning: implicit declaration of function ‘register_compat_prop’ [-Wimplicit-function-declaration]
         register_compat_prop("migration", "store-global-state", "off");
         ^~~~~~~~~~~~~~~~~~~~
    hw/xen/xen-common.c:147:5: warning: nested extern declaration of ‘register_compat_prop’ [-Wnested-externs]

You might want to install Xen development packages to catch such issues
earlier.

Test run:

    $ qemu-system-x86_64 -device migration,help
    migration.skip-section-footer=bool
    migration.store-global-state=bool
    migration.only-migratable=bool
    migration.skip-configuration=bool

What would adding this device do?

Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Juan Quintela 8 years, 4 months ago
Markus Armbruster <armbru@redhat.com> wrote:
> Test compile gripes:
>
>     hw/xen/xen-common.c: In function ‘xen_init’:
>     hw/xen/xen-common.c:147:5: warning: implicit declaration of function ‘register_compat_prop’ [-Wimplicit-function-declaration]
>          register_compat_prop("migration", "store-global-state", "off");
>          ^~~~~~~~~~~~~~~~~~~~
>     hw/xen/xen-common.c:147:5: warning: nested extern declaration of ‘register_compat_prop’ [-Wnested-externs]
>
> You might want to install Xen development packages to catch such issues
> earlier.
>
> Test run:
>
>     $ qemu-system-x86_64 -device migration,help
>     migration.skip-section-footer=bool
>     migration.store-global-state=bool
>     migration.only-migratable=bool
>     migration.skip-configuration=bool
>
> What would adding this device do?

Parameters, capabilities, options for migration.

This was what we discussed on irc.  We want to have a way to control
migration features that depend on versions.

So we disable new features for old machine types (normal thing that we
do).  But right now, creating such a new feature requires creating a
couple of functions to set/clear the feature, adding includes on the
destination side, etc.

This was supposed to be a global property (or a qemu_opt, or whatever).
Just something that could be enabled/disabled easily on machine types,
and if possible independently of machine types.  This (for instance)
would allow that store-global-state is disabled by defaulte for
machine-2.9 (I forgot the exact name for the machine type), but I can
enable it by hand.  That is very handy for testing.

So, I think that the question is, how/where can we set that kind of
properties?

At least for me, being able to *also* set migration
capabilities/parameters with this mechanism on the command line would be
very nice, for instance

   -global migration.xbzrle on

or

   -global migration.max-speed 2G

I don't care about what is the exact syntax, or where we hang them,
i.e. a new device, a new list, somewhere that already exist.  That is
what we ask for advice.

Thanks for the review and the suggestions.

Later, Juan.

Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Peter Xu 8 years, 4 months ago
On Fri, Jun 09, 2017 at 04:02:37PM +0200, Markus Armbruster wrote:
> Test compile gripes:
> 
>     hw/xen/xen-common.c: In function ‘xen_init’:
>     hw/xen/xen-common.c:147:5: warning: implicit declaration of function ‘register_compat_prop’ [-Wimplicit-function-declaration]
>          register_compat_prop("migration", "store-global-state", "off");
>          ^~~~~~~~~~~~~~~~~~~~
>     hw/xen/xen-common.c:147:5: warning: nested extern declaration of ‘register_compat_prop’ [-Wnested-externs]
> 
> You might want to install Xen development packages to catch such issues
> earlier.

Sorry I have missed to check that. I'll definitely verify this in the
following up posts.

> 
> Test run:
> 
>     $ qemu-system-x86_64 -device migration,help
>     migration.skip-section-footer=bool
>     migration.store-global-state=bool
>     migration.only-migratable=bool
>     migration.skip-configuration=bool
> 
> What would adding this device do?

Hmm, the same answer in the other thread - I may need the compat bits
and "-global", so I am using QDev for migration, not only QObject.

Haven't investigate how difficult would it be to move these features
from QDev into QObject.  For now, do you think user_creatable=false an
workable solution for this? We'll still see this help for sure, but at
least we won't be able to create the device using "-device migration",
also we won't see the migration entry in "-device help".

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 0/6] migration: objectify MigrationState
Posted by Markus Armbruster 8 years, 4 months ago
Peter Xu <peterx@redhat.com> writes:

> On Fri, Jun 09, 2017 at 04:02:37PM +0200, Markus Armbruster wrote:
>> Test compile gripes:
>> 
>>     hw/xen/xen-common.c: In function ‘xen_init’:
>>     hw/xen/xen-common.c:147:5: warning: implicit declaration of function ‘register_compat_prop’ [-Wimplicit-function-declaration]
>>          register_compat_prop("migration", "store-global-state", "off");
>>          ^~~~~~~~~~~~~~~~~~~~
>>     hw/xen/xen-common.c:147:5: warning: nested extern declaration of ‘register_compat_prop’ [-Wnested-externs]
>> 
>> You might want to install Xen development packages to catch such issues
>> earlier.
>
> Sorry I have missed to check that. I'll definitely verify this in the
> following up posts.
>
>> 
>> Test run:
>> 
>>     $ qemu-system-x86_64 -device migration,help
>>     migration.skip-section-footer=bool
>>     migration.store-global-state=bool
>>     migration.only-migratable=bool
>>     migration.skip-configuration=bool
>> 
>> What would adding this device do?
>
> Hmm, the same answer in the other thread - I may need the compat bits
> and "-global", so I am using QDev for migration, not only QObject.
>
> Haven't investigate how difficult would it be to move these features
> from QDev into QObject.  For now, do you think user_creatable=false an
> workable solution for this? We'll still see this help for sure, but at
> least we won't be able to create the device using "-device migration",
> also we won't see the migration entry in "-device help".

Hacks that aren't exposed in external interfaces can be tolerable.  In
particular when we have concrete ideas on how to get rid of them later
on.

Don't worry about visibility in "-device migration,help".