[PATCH] tests/qtest/ide-test: Remove bad retry_isa test

[PATCH] tests/qtest/ide-test: Remove bad retry_isa test

[Thomas Huth]

The retry_isa test is not doing what it was intended for: The
test_retry_flush() function ignores the machine parameter completely
and thus this test does not get run with the "isapc" machine.
Moreover, in the course of time, the test_retry_flush() has been
changed to depend on PCI-related functions, so this also cannot
be fixed by simply using the machine parameter now. The correct
fix would be to switch the whole test to libqos, but until someone
has time to do this, let's simply drop the retry_isa test for now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/ide-test.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 3f8081e77d..5f9d084052 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -701,7 +701,7 @@ static void test_flush(void)
     free_pci_device(dev);
 }
 
-static void test_retry_flush(const char *machine)
+static void test_pci_retry_flush(void)
 {
     QTestState *qts;
     QPCIDevice *dev;
@@ -790,16 +790,6 @@ static void test_flush_empty_drive(void)
     ide_test_quit(qts);
 }
 
-static void test_pci_retry_flush(void)
-{
-    test_retry_flush("pc");
-}
-
-static void test_isa_retry_flush(void)
-{
-    test_retry_flush("isapc");
-}
-
 typedef struct Read10CDB {
     uint8_t opcode;
     uint8_t flags;
@@ -1050,7 +1040,6 @@ int main(int argc, char **argv)
     qtest_add_func("/ide/flush/nodev", test_flush_nodev);
     qtest_add_func("/ide/flush/empty_drive", test_flush_empty_drive);
     qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
-    qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
 
     qtest_add_func("/ide/cdrom/pio", test_cdrom_pio);
     qtest_add_func("/ide/cdrom/pio_large", test_cdrom_pio_large);
-- 
2.27.0

Re: [PATCH] tests/qtest/ide-test: Remove bad retry_isa test

[John Snow]

On Fri, Jan 21, 2022 at 7:06 AM Thomas Huth <thuth@redhat.com> wrote:
>
> The retry_isa test is not doing what it was intended for: The
> test_retry_flush() function ignores the machine parameter completely
> and thus this test does not get run with the "isapc" machine.
> Moreover, in the course of time, the test_retry_flush() has been
> changed to depend on PCI-related functions, so this also cannot
> be fixed by simply using the machine parameter now. The correct
> fix would be to switch the whole test to libqos, but until someone
> has time to do this, let's simply drop the retry_isa test for now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I assume I had patches to make this make sense at some point, but
they're probably long lost to time by now. Please pardon my dust. 🙇

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  tests/qtest/ide-test.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
> index 3f8081e77d..5f9d084052 100644
> --- a/tests/qtest/ide-test.c
> +++ b/tests/qtest/ide-test.c
> @@ -701,7 +701,7 @@ static void test_flush(void)
>      free_pci_device(dev);
>  }
>
> -static void test_retry_flush(const char *machine)
> +static void test_pci_retry_flush(void)
>  {
>      QTestState *qts;
>      QPCIDevice *dev;
> @@ -790,16 +790,6 @@ static void test_flush_empty_drive(void)
>      ide_test_quit(qts);
>  }
>
> -static void test_pci_retry_flush(void)
> -{
> -    test_retry_flush("pc");
> -}
> -
> -static void test_isa_retry_flush(void)
> -{
> -    test_retry_flush("isapc");
> -}
> -
>  typedef struct Read10CDB {
>      uint8_t opcode;
>      uint8_t flags;
> @@ -1050,7 +1040,6 @@ int main(int argc, char **argv)
>      qtest_add_func("/ide/flush/nodev", test_flush_nodev);
>      qtest_add_func("/ide/flush/empty_drive", test_flush_empty_drive);
>      qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
> -    qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
>
>      qtest_add_func("/ide/cdrom/pio", test_cdrom_pio);
>      qtest_add_func("/ide/cdrom/pio_large", test_cdrom_pio_large);
> --
> 2.27.0
>