diff mbox

[3/8] migration: add new capability test-only

Message ID 1444198846-5383-4-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Oct. 7, 2015, 6:20 a.m. UTC
From: Igor Redko <redkoi@virtuozzo.com>

This patch declares a new migration capability that allows
to distinguish between true migration and the test.

An alternative is using a custom URI but in this case the
following problems should be solved:
1/ QEMUFile abstraction hides the real transport type
2/ We must be ensured that VM will not be stopped due to
“successful migration”

Possible solutions:
1/ Using global variable analogous to
s->enabled_capabilities[MIGRATION_CAPABILITY_TEST_ONLY] that
will be initialized according to the migration type at the
beginning of the migration process
2/ Additional hook, presence of implementation of which allows
to decide whether the migration is true

Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
Reviewed-by: Anna Melekhova <annam@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 include/migration/migration.h | 2 ++
 migration/migration.c         | 9 +++++++++
 qapi-schema.json              | 5 ++++-
 qmp-commands.hx               | 1 +
 4 files changed, 16 insertions(+), 1 deletion(-)

Comments

Eric Blake Oct. 7, 2015, 3:05 p.m. UTC | #1
On 10/07/2015 12:20 AM, Denis V. Lunev wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
> 
> This patch declares a new migration capability that allows
> to distinguish between true migration and the test.
> 
> An alternative is using a custom URI but in this case the
> following problems should be solved:
> 1/ QEMUFile abstraction hides the real transport type
> 2/ We must be ensured that VM will not be stopped due to
> “successful migration”
> 
> Possible solutions:
> 1/ Using global variable analogous to
> s->enabled_capabilities[MIGRATION_CAPABILITY_TEST_ONLY] that
> will be initialized according to the migration type at the
> beginning of the migration process
> 2/ Additional hook, presence of implementation of which allows
> to decide whether the migration is true
> 

Do we want to name the capability 'x-test-only', since this series is
experimental, to make it obvious that production code should not rely on it?


> +++ b/qapi-schema.json
> @@ -540,11 +540,14 @@
>  # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>  #          to speed up convergence of RAM migration. (since 1.6)
>  #
> +# @test-only: If enabled, QEMU instead of migrating will estimate migration
> +#          time basing on given downtime and current dirty page rate

If we do decide to add this to mainline, it would need 'since 2.5' (or
whatever version) documentation.
Denis V. Lunev Oct. 8, 2015, 2:54 p.m. UTC | #2
On 10/07/2015 06:05 PM, Eric Blake wrote:
> On 10/07/2015 12:20 AM, Denis V. Lunev wrote:
>> From: Igor Redko <redkoi@virtuozzo.com>
>>
>> This patch declares a new migration capability that allows
>> to distinguish between true migration and the test.
>>
>> An alternative is using a custom URI but in this case the
>> following problems should be solved:
>> 1/ QEMUFile abstraction hides the real transport type
>> 2/ We must be ensured that VM will not be stopped due to
>> “successful migration”
>>
>> Possible solutions:
>> 1/ Using global variable analogous to
>> s->enabled_capabilities[MIGRATION_CAPABILITY_TEST_ONLY] that
>> will be initialized according to the migration type at the
>> beginning of the migration process
>> 2/ Additional hook, presence of implementation of which allows
>> to decide whether the migration is true
>>
> Do we want to name the capability 'x-test-only', since this series is
> experimental, to make it obvious that production code should not rely on it?
>
actually the answer is 'no'. This feature is designated for production.
May be the name is just wrong or description is bad :) Yep, I know this
for sure...

OK. The idea of this feature is to estimate efforts required to
perform migration and downtime during that migration.
This estimate is necessary to decide whether we are able
to automatically move the VM from one node (loaded) to
different node with less load.

Should we rename 'test' to 'estimate' or something like that
to make the situation clearer?

On the other hand this 'estimate' state would be very useful
for testing that is why I have proposed this code to John.
At least we were able to catch recently fixed hang on migration
cancel (in pipeline already).

I am completely open to any idea.


>> +++ b/qapi-schema.json
>> @@ -540,11 +540,14 @@
>>   # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>>   #          to speed up convergence of RAM migration. (since 1.6)
>>   #
>> +# @test-only: If enabled, QEMU instead of migrating will estimate migration
>> +#          time basing on given downtime and current dirty page rate
> If we do decide to add this to mainline, it would need 'since 2.5' (or
> whatever version) documentation.
>
Sure!
Dr. David Alan Gilbert Oct. 9, 2015, 3:19 p.m. UTC | #3
* Denis V. Lunev (den@openvz.org) wrote:
> On 10/07/2015 06:05 PM, Eric Blake wrote:
> >On 10/07/2015 12:20 AM, Denis V. Lunev wrote:
> >>From: Igor Redko <redkoi@virtuozzo.com>
> >>
> >>This patch declares a new migration capability that allows
> >>to distinguish between true migration and the test.
> >>
> >>An alternative is using a custom URI but in this case the
> >>following problems should be solved:
> >>1/ QEMUFile abstraction hides the real transport type
> >>2/ We must be ensured that VM will not be stopped due to
> >>“successful migration”
> >>
> >>Possible solutions:
> >>1/ Using global variable analogous to
> >>s->enabled_capabilities[MIGRATION_CAPABILITY_TEST_ONLY] that
> >>will be initialized according to the migration type at the
> >>beginning of the migration process
> >>2/ Additional hook, presence of implementation of which allows
> >>to decide whether the migration is true
> >>
> >Do we want to name the capability 'x-test-only', since this series is
> >experimental, to make it obvious that production code should not rely on it?
> >
> actually the answer is 'no'. This feature is designated for production.
> May be the name is just wrong or description is bad :) Yep, I know this
> for sure...
> 
> OK. The idea of this feature is to estimate efforts required to
> perform migration and downtime during that migration.
> This estimate is necessary to decide whether we are able
> to automatically move the VM from one node (loaded) to
> different node with less load.

Yes, this is the use we had also been considering.

> Should we rename 'test' to 'estimate' or something like that
> to make the situation clearer?
> 
> On the other hand this 'estimate' state would be very useful
> for testing that is why I have proposed this code to John.
> At least we were able to catch recently fixed hang on migration
> cancel (in pipeline already).
> 
> I am completely open to any idea.

I can see how 'test' might confuse with other test things; but
I'm not too fussy about the name.  I was thinking 'null' if it
was a transport name.

Dave

> 
> 
> >>+++ b/qapi-schema.json
> >>@@ -540,11 +540,14 @@
> >>  # @auto-converge: If enabled, QEMU will automatically throttle down the guest
> >>  #          to speed up convergence of RAM migration. (since 1.6)
> >>  #
> >>+# @test-only: If enabled, QEMU instead of migrating will estimate migration
> >>+#          time basing on given downtime and current dirty page rate
> >If we do decide to add this to mainline, it would need 'since 2.5' (or
> >whatever version) documentation.
> >
> Sure!
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/include/migration/migration.h b/include/migration/migration.h
index deb0d21..8611750 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -179,6 +179,8 @@  int migrate_compress_threads(void);
 int migrate_decompress_threads(void);
 bool migrate_use_events(void);
 
+bool migrate_is_test(void);
+
 void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
diff --git a/migration/migration.c b/migration/migration.c
index b710a2d..e0cad54 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -954,6 +954,15 @@  int migrate_use_xbzrle(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_XBZRLE];
 }
 
+bool migrate_is_test(void)
+{
+    MigrationState *s;
+
+    s = migrate_get_current();
+
+    return s->enabled_capabilities[MIGRATION_CAPABILITY_TEST_ONLY];
+}
+
 int64_t migrate_xbzrle_cache_size(void)
 {
     MigrationState *s;
diff --git a/qapi-schema.json b/qapi-schema.json
index 8b0520c..38bf199 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -540,11 +540,14 @@ 
 # @auto-converge: If enabled, QEMU will automatically throttle down the guest
 #          to speed up convergence of RAM migration. (since 1.6)
 #
+# @test-only: If enabled, QEMU instead of migrating will estimate migration
+#          time basing on given downtime and current dirty page rate
+#
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
   'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
-           'compress', 'events'] }
+           'compress', 'events', 'test-only'] }
 
 ##
 # @MigrationCapabilityStatus
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d2ba800..741d088 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3456,6 +3456,7 @@  Enable/Disable migration capabilities
 - "auto-converge": throttle down guest to help convergence of migration
 - "zero-blocks": compress zero blocks during block migration
 - "events": generate events for each migration state change
+- "test-only": don't send any data, instead estimate dirty page rate
 
 Arguments: