Re: [PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-23 Thread John Snow 



On 12/19/19 1:36 PM, Max Reitz wrote:
> The "migration completed" event may be sent (on the source, to be
> specific) before the migration is actually completed, so the VM runstate
> will still be "finish-migrate" instead of "postmigrate".  So ask the
> users of VM.wait_migration() to specify the final runstate they desire
> and then poll the VM until it has reached that state.  (This should be
> over very quickly, so busy polling is fine.)
> 
> Without this patch, I see intermittent failures in the new iotest 280
> under high system load.  I have not yet seen such failures with other
> iotests that use VM.wait_migration() and query-status afterwards, but
> maybe they just occur even more rarely, or it is because they also wait
> on the destination VM to be running.
> 
> Signed-off-by: Max Reitz 

Reviewed-by: John Snow 

With Kevin's suggestion on the comment touchup or without.

> ---
> v2:
> - Stop breaking 234 and 262 [Kevin]
> ---
>  tests/qemu-iotests/234| 8 
>  tests/qemu-iotests/262| 4 ++--
>  tests/qemu-iotests/280| 2 +-
>  tests/qemu-iotests/iotests.py | 6 +-
>  4 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
> index 34c818c485..59a7f949ec 100755
> --- a/tests/qemu-iotests/234
> +++ b/tests/qemu-iotests/234
> @@ -69,9 +69,9 @@ with iotests.FilePath('img') as img_path, \
>  iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo_a)))
>  with iotests.Timeout(3, 'Migration does not complete'):
>  # Wait for the source first (which includes setup=setup)
> -vm_a.wait_migration()
> +vm_a.wait_migration('postmigrate')
>  # Wait for the destination second (which does not)
> -vm_b.wait_migration()
> +vm_b.wait_migration('running')
>  
>  iotests.log(vm_a.qmp('query-migrate')['return']['status'])
>  iotests.log(vm_b.qmp('query-migrate')['return']['status'])
> @@ -98,9 +98,9 @@ with iotests.FilePath('img') as img_path, \
>  iotests.log(vm_b.qmp('migrate', uri='exec:cat >%s' % (fifo_b)))
>  with iotests.Timeout(3, 'Migration does not complete'):
>  # Wait for the source first (which includes setup=setup)
> -vm_b.wait_migration()
> +vm_b.wait_migration('postmigrate')
>  # Wait for the destination second (which does not)
> -vm_a.wait_migration()
> +vm_a.wait_migration('running')
>  
>  iotests.log(vm_a.qmp('query-migrate')['return']['status'])
>  iotests.log(vm_b.qmp('query-migrate')['return']['status'])
> diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
> index 0963daa806..bbcb5260a6 100755
> --- a/tests/qemu-iotests/262
> +++ b/tests/qemu-iotests/262
> @@ -71,9 +71,9 @@ with iotests.FilePath('img') as img_path, \
>  iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
>  with iotests.Timeout(3, 'Migration does not complete'):
>  # Wait for the source first (which includes setup=setup)
> -vm_a.wait_migration()
> +vm_a.wait_migration('postmigrate')
>  # Wait for the destination second (which does not)
> -vm_b.wait_migration()
> +vm_b.wait_migration('running')
>  
>  iotests.log(vm_a.qmp('query-migrate')['return']['status'])
>  iotests.log(vm_b.qmp('query-migrate')['return']['status'])
> diff --git a/tests/qemu-iotests/280 b/tests/qemu-iotests/280
> index 0b1fa8e1d8..85e9114c5e 100755
> --- a/tests/qemu-iotests/280
> +++ b/tests/qemu-iotests/280
> @@ -45,7 +45,7 @@ with iotests.FilePath('base') as base_path , \
>  vm.qmp_log('migrate', uri='exec:cat > /dev/null')
>  
>  with iotests.Timeout(3, 'Migration does not complete'):
> -vm.wait_migration()
> +vm.wait_migration('postmigrate')
>  
>  iotests.log('\nVM is now stopped:')
>  iotests.log(vm.qmp('query-migrate')['return']['status'])
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 13fd8b5cd2..0b62c42851 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
>  }
>  ]))
>  
> -def wait_migration(self):
> +def wait_migration(self, expect_runstate):
>  while True:
>  event = self.event_wait('MIGRATION')
>  log(event, filters=[filter_qmp_event])
>  if event['data']['status'] == 'completed':
>  break
> +# The event may occur in finish-migrate, so wait for the expected
> +# post-migration runstate
> +while self.qmp('query-status')['return']['status'] != 
> expect_runstate:
> +pass
>  
>  def node_info(self, node_name):
>  nodes = self.qmp('query-named-block-nodes')
> 

-- 
—js

Re: [PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-20 Thread Max Reitz 
On 20.12.19 11:07, Kevin Wolf wrote:
> Am 20.12.2019 um 10:52 hat Max Reitz geschrieben:
>> On 20.12.19 10:32, Kevin Wolf wrote:
>>> Am 19.12.2019 um 19:36 hat Max Reitz geschrieben:
 The "migration completed" event may be sent (on the source, to be
 specific) before the migration is actually completed, so the VM runstate
 will still be "finish-migrate" instead of "postmigrate".  So ask the
 users of VM.wait_migration() to specify the final runstate they desire
 and then poll the VM until it has reached that state.  (This should be
 over very quickly, so busy polling is fine.)

 Without this patch, I see intermittent failures in the new iotest 280
 under high system load.  I have not yet seen such failures with other
 iotests that use VM.wait_migration() and query-status afterwards, but
 maybe they just occur even more rarely, or it is because they also wait
 on the destination VM to be running.

 Signed-off-by: Max Reitz 
>>>
 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
 index 13fd8b5cd2..0b62c42851 100644
 --- a/tests/qemu-iotests/iotests.py
 +++ b/tests/qemu-iotests/iotests.py
 @@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
  }
  ]))
  
 -def wait_migration(self):
 +def wait_migration(self, expect_runstate):
  while True:
  event = self.event_wait('MIGRATION')
  log(event, filters=[filter_qmp_event])
  if event['data']['status'] == 'completed':
  break
 +# The event may occur in finish-migrate, so wait for the expected
 +# post-migration runstate
>>>
>>> That's a bit too specific now that you have expect_runstate.
>>
>> Can you be more specific? :-)
>>
>> If you mean the fact of mentioning “post-migration runstate”, I simply
>> meant that as “the runstate after the migration”.  The specific runstate
>> on the source VM is called “postmigrate”.
> 
> Oh, yes, "postmigrate" is what I had in mind.
> 
>> I wouldn’t mind changing it to “after-migration runstate” or something
>> similar, if that’s what you mean.
> 
> "runstate after migration"?

Sure.

(Now you got me to wonder what permutations are permissible.  “migration
after runstate” isn’t.  “runstate migration after” is just garbage.  So
probebly on RAM and AMR.)

Max



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-20 Thread Kevin Wolf 
Am 20.12.2019 um 10:52 hat Max Reitz geschrieben:
> On 20.12.19 10:32, Kevin Wolf wrote:
> > Am 19.12.2019 um 19:36 hat Max Reitz geschrieben:
> >> The "migration completed" event may be sent (on the source, to be
> >> specific) before the migration is actually completed, so the VM runstate
> >> will still be "finish-migrate" instead of "postmigrate".  So ask the
> >> users of VM.wait_migration() to specify the final runstate they desire
> >> and then poll the VM until it has reached that state.  (This should be
> >> over very quickly, so busy polling is fine.)
> >>
> >> Without this patch, I see intermittent failures in the new iotest 280
> >> under high system load.  I have not yet seen such failures with other
> >> iotests that use VM.wait_migration() and query-status afterwards, but
> >> maybe they just occur even more rarely, or it is because they also wait
> >> on the destination VM to be running.
> >>
> >> Signed-off-by: Max Reitz 
> > 
> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> >> index 13fd8b5cd2..0b62c42851 100644
> >> --- a/tests/qemu-iotests/iotests.py
> >> +++ b/tests/qemu-iotests/iotests.py
> >> @@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
> >>  }
> >>  ]))
> >>  
> >> -def wait_migration(self):
> >> +def wait_migration(self, expect_runstate):
> >>  while True:
> >>  event = self.event_wait('MIGRATION')
> >>  log(event, filters=[filter_qmp_event])
> >>  if event['data']['status'] == 'completed':
> >>  break
> >> +# The event may occur in finish-migrate, so wait for the expected
> >> +# post-migration runstate
> > 
> > That's a bit too specific now that you have expect_runstate.
> 
> Can you be more specific? :-)
> 
> If you mean the fact of mentioning “post-migration runstate”, I simply
> meant that as “the runstate after the migration”.  The specific runstate
> on the source VM is called “postmigrate”.

Oh, yes, "postmigrate" is what I had in mind.

> I wouldn’t mind changing it to “after-migration runstate” or something
> similar, if that’s what you mean.

"runstate after migration"?

Kevin


signature.asc
Description: PGP signature

Re: [PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-20 Thread Max Reitz 
On 20.12.19 10:32, Kevin Wolf wrote:
> Am 19.12.2019 um 19:36 hat Max Reitz geschrieben:
>> The "migration completed" event may be sent (on the source, to be
>> specific) before the migration is actually completed, so the VM runstate
>> will still be "finish-migrate" instead of "postmigrate".  So ask the
>> users of VM.wait_migration() to specify the final runstate they desire
>> and then poll the VM until it has reached that state.  (This should be
>> over very quickly, so busy polling is fine.)
>>
>> Without this patch, I see intermittent failures in the new iotest 280
>> under high system load.  I have not yet seen such failures with other
>> iotests that use VM.wait_migration() and query-status afterwards, but
>> maybe they just occur even more rarely, or it is because they also wait
>> on the destination VM to be running.
>>
>> Signed-off-by: Max Reitz 
> 
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 13fd8b5cd2..0b62c42851 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
>>  }
>>  ]))
>>  
>> -def wait_migration(self):
>> +def wait_migration(self, expect_runstate):
>>  while True:
>>  event = self.event_wait('MIGRATION')
>>  log(event, filters=[filter_qmp_event])
>>  if event['data']['status'] == 'completed':
>>  break
>> +# The event may occur in finish-migrate, so wait for the expected
>> +# post-migration runstate
> 
> That's a bit too specific now that you have expect_runstate.

Can you be more specific? :-)

If you mean the fact of mentioning “post-migration runstate”, I simply
meant that as “the runstate after the migration”.  The specific runstate
on the source VM is called “postmigrate”.

I wouldn’t mind changing it to “after-migration runstate” or something
similar, if that’s what you mean.

Max

>> +while self.qmp('query-status')['return']['status'] != 
>> expect_runstate:
>> +pass
>>  
>>  def node_info(self, node_name):
>>  nodes = self.qmp('query-named-block-nodes')
> 
> Tested-by: Kevin Wolf 
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-20 Thread Kevin Wolf 
Am 19.12.2019 um 19:36 hat Max Reitz geschrieben:
> The "migration completed" event may be sent (on the source, to be
> specific) before the migration is actually completed, so the VM runstate
> will still be "finish-migrate" instead of "postmigrate".  So ask the
> users of VM.wait_migration() to specify the final runstate they desire
> and then poll the VM until it has reached that state.  (This should be
> over very quickly, so busy polling is fine.)
> 
> Without this patch, I see intermittent failures in the new iotest 280
> under high system load.  I have not yet seen such failures with other
> iotests that use VM.wait_migration() and query-status afterwards, but
> maybe they just occur even more rarely, or it is because they also wait
> on the destination VM to be running.
> 
> Signed-off-by: Max Reitz 

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 13fd8b5cd2..0b62c42851 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
>  }
>  ]))
>  
> -def wait_migration(self):
> +def wait_migration(self, expect_runstate):
>  while True:
>  event = self.event_wait('MIGRATION')
>  log(event, filters=[filter_qmp_event])
>  if event['data']['status'] == 'completed':
>  break
> +# The event may occur in finish-migrate, so wait for the expected
> +# post-migration runstate

That's a bit too specific now that you have expect_runstate.

> +while self.qmp('query-status')['return']['status'] != 
> expect_runstate:
> +pass
>  
>  def node_info(self, node_name):
>  nodes = self.qmp('query-named-block-nodes')

Tested-by: Kevin Wolf

[PATCH v2] iotests.py: Let wait_migration wait even more

2019-12-19 Thread Max Reitz 
The "migration completed" event may be sent (on the source, to be
specific) before the migration is actually completed, so the VM runstate
will still be "finish-migrate" instead of "postmigrate".  So ask the
users of VM.wait_migration() to specify the final runstate they desire
and then poll the VM until it has reached that state.  (This should be
over very quickly, so busy polling is fine.)

Without this patch, I see intermittent failures in the new iotest 280
under high system load.  I have not yet seen such failures with other
iotests that use VM.wait_migration() and query-status afterwards, but
maybe they just occur even more rarely, or it is because they also wait
on the destination VM to be running.

Signed-off-by: Max Reitz 
---
v2:
- Stop breaking 234 and 262 [Kevin]
---
 tests/qemu-iotests/234| 8 
 tests/qemu-iotests/262| 4 ++--
 tests/qemu-iotests/280| 2 +-
 tests/qemu-iotests/iotests.py | 6 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
index 34c818c485..59a7f949ec 100755
--- a/tests/qemu-iotests/234
+++ b/tests/qemu-iotests/234
@@ -69,9 +69,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo_a)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_a.wait_migration()
+vm_a.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_b.wait_migration()
+vm_b.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
@@ -98,9 +98,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_b.qmp('migrate', uri='exec:cat >%s' % (fifo_b)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_b.wait_migration()
+vm_b.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_a.wait_migration()
+vm_a.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index 0963daa806..bbcb5260a6 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -71,9 +71,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_a.wait_migration()
+vm_a.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_b.wait_migration()
+vm_b.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
diff --git a/tests/qemu-iotests/280 b/tests/qemu-iotests/280
index 0b1fa8e1d8..85e9114c5e 100755
--- a/tests/qemu-iotests/280
+++ b/tests/qemu-iotests/280
@@ -45,7 +45,7 @@ with iotests.FilePath('base') as base_path , \
 vm.qmp_log('migrate', uri='exec:cat > /dev/null')
 
 with iotests.Timeout(3, 'Migration does not complete'):
-vm.wait_migration()
+vm.wait_migration('postmigrate')
 
 iotests.log('\nVM is now stopped:')
 iotests.log(vm.qmp('query-migrate')['return']['status'])
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 13fd8b5cd2..0b62c42851 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
 }
 ]))
 
-def wait_migration(self):
+def wait_migration(self, expect_runstate):
 while True:
 event = self.event_wait('MIGRATION')
 log(event, filters=[filter_qmp_event])
 if event['data']['status'] == 'completed':
 break
+# The event may occur in finish-migrate, so wait for the expected
+# post-migration runstate
+while self.qmp('query-status')['return']['status'] != expect_runstate:
+pass
 
 def node_info(self, node_name):
 nodes = self.qmp('query-named-block-nodes')
-- 
2.23.0