Hi David,
On 3/25/20 4:15 PM, Dr. David Alan Gilbert wrote:
* Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
On 3/25/20 3:10 PM, Oksana Voshchana wrote:
Hi Philippe
Thanks for the review
I have some comments
On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé
<phi...@redhat.com <mailto:phi...@redhat.com>> wrote:
Hi Oksana,
v2 was
https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so
this is v3. Please increment the version in the patch subject.
You could also send a simple "ping" to the specific patch, instead of
resending it.
On 3/25/20 12:31 PM, Oksana Vohchana wrote:
> The exec migration test isn't run a whole test scenario.
> This patch fixes it
>
> Signed-off-by: Oksana Vohchana <ovosh...@redhat.com
<mailto:ovosh...@redhat.com>>
v1 of this patch has already received reviewers tags
(https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html),
please collect them and keep them when you resend the same patch:
I have reposted patch without this fix because this change isn't related
to the series:
https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html
Is it make sense to keep this fix as a separate patch?
Fixes: 2e768cb682bf
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com
<mailto:phi...@redhat.com>>
Tested-by: Wainer dos Santos Moschetta <waine...@redhat.com
<mailto:waine...@redhat.com>>
> ---
> tests/acceptance/migration.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/acceptance/migration.py
b/tests/acceptance/migration.py
> index a8367ca023..0365289cda 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -70,8 +70,8 @@ class Migration(Test):
>
> @skipUnless(find_command('nc', default=False), "'nc'
command not found")
> def test_migration_with_exec(self):
> - """
> - The test works for both netcat-traditional and
netcat-openbsd packages
> - """
> + """The test works for both netcat-traditional and
netcat-openbsd packages."""
Btw why are you changing the comment style?
I got failure in PEP257
OK, next time please add comment in the patch description too.
> free_port = self._get_free_port()
> dest_uri = 'exec:nc -l localhost %u' % free_port
> + src_uri = 'exec:nc localhost %u' % free_port
> + self.do_migrate(dest_uri, src_uri)
>
Alex, if there is no Python testing pullreq, can you take this patch
via
your testing tree?
Cc'ing David since it is migration related.
I tend to leave the tests/acceptance to someone other than the migration
tree; so feel free to take them via testing or trivial given the size.
Acceptance tests are in the same tests/acceptance directory for no
particular reason. We thought they would share some common code but this
code is in the python/qemu/ directory.
Similarly to C Qtests stored in the tests/qtest, acceptance are in
tests/acceptance. tests/qtest have various maintainers, when a
maintainer has interest in a qtest (subsystem covered by the test), the
test is added to the subsystem maintained section. I'd rather see the
same with acceptance tests. For example with Oksana's test, I can review
that it correctly uses the acceptance testing framework, and it doesn't
break the rest of the tests, but I have no idea if it properly tests the
migration features it aims to.
Back to this particular patch, if Eduardo/Cleber ack I can send a
pullreq for rc2.
Dave
Thanks,
Phil.
Thanks
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
