All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester
@ 2016-09-26 20:17 Thomas Huth
  2016-09-27  4:17 ` David Gibson
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2016-09-26 20:17 UTC (permalink / raw)
  To: qemu-devel, Jason Wang, Samuel Thibault
  Cc: Jan Kiszka, Michael S Tsirkin, Victor Kaplansky, David Gibson,
	Alexander Graf, qemu-ppc

The firmware of the pseries machine, SLOF, is able to load files via
IPv6 networking, too. So to test both, network bootloading on ppc64
and IPv6 (via Slirp) , let's add some PXE tests for this environment,
too. Since we can not use the normal x86 boot sector for network boot
loading, we use a simple Forth script on ppc64 instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include |  1 +
 tests/boot-sector.c    |  9 +++++++++
 tests/pxe-test.c       | 22 +++++++++++++++-------
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d8101b3..18bc698 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -270,6 +270,7 @@ check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
 check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
 check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
 check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
+check-qtest-ppc64-y += tests/pxe-test$(EXESUF)
 
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 
diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index 3ffe298..e3193c0 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -77,6 +77,15 @@ int boot_sector_init(const char *fname)
         fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
         return 1;
     }
+
+    /* For Open Firmware based system, we can use a Forth script instead */
+    if (strcmp(qtest_get_arch(), "ppc64") == 0) {
+        memset(boot_sector, ' ', sizeof boot_sector);
+        sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
+                LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
+                HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
+    }
+
     fwrite(boot_sector, 1, sizeof boot_sector, f);
     fclose(f);
     return 0;
diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index b2cc355..0bdb7a1 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -21,14 +21,14 @@
 
 static const char *disk = "tests/pxe-test-disk.raw";
 
-static void test_pxe_one(const char *params)
+static void test_pxe_one(const char *params, bool ipv6)
 {
     char *args;
 
-    args = g_strdup_printf("-machine accel=tcg "
-                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s "
-                           "%s ",
-                           disk, params);
+    args = g_strdup_printf("-machine accel=tcg -boot order=n "
+                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,"
+                           "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on",
+                           ipv6 ? "on" : "off", params);
 
     qtest_start(args);
     boot_sector_test();
@@ -38,12 +38,17 @@ static void test_pxe_one(const char *params)
 
 static void test_pxe_e1000(void)
 {
-    test_pxe_one("-device e1000,netdev=" NETNAME);
+    test_pxe_one("-device e1000,netdev=" NETNAME, false);
 }
 
 static void test_pxe_virtio_pci(void)
 {
-    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME);
+    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME, false);
+}
+
+static void test_pxe_spapr_vlan(void)
+{
+    test_pxe_one("-vga none -device spapr-vlan,netdev=" NETNAME, true);
 }
 
 int main(int argc, char *argv[])
@@ -60,6 +65,9 @@ int main(int argc, char *argv[])
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         qtest_add_func("pxe/e1000", test_pxe_e1000);
         qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
+    } else if (strcmp(arch, "ppc64") == 0) {
+        qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
+        qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester
  2016-09-26 20:17 [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester Thomas Huth
@ 2016-09-27  4:17 ` David Gibson
  2016-09-27  7:17   ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2016-09-27  4:17 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Jason Wang, Samuel Thibault, Jan Kiszka,
	Michael S Tsirkin, Victor Kaplansky, Alexander Graf, qemu-ppc

[-- Attachment #1: Type: text/plain, Size: 4929 bytes --]

On Mon, Sep 26, 2016 at 10:17:46PM +0200, Thomas Huth wrote:
> The firmware of the pseries machine, SLOF, is able to load files via
> IPv6 networking, too. So to test both, network bootloading on ppc64
> and IPv6 (via Slirp) , let's add some PXE tests for this environment,
> too. Since we can not use the normal x86 boot sector for network boot
> loading, we use a simple Forth script on ppc64 instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I certainly approve of testing IPv6 more, a couple of queries about
the details though:

> ---
>  tests/Makefile.include |  1 +
>  tests/boot-sector.c    |  9 +++++++++
>  tests/pxe-test.c       | 22 +++++++++++++++-------
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index d8101b3..18bc698 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -270,6 +270,7 @@ check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
>  check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>  check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
>  check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
> +check-qtest-ppc64-y += tests/pxe-test$(EXESUF)
>  
>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>  
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index 3ffe298..e3193c0 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -77,6 +77,15 @@ int boot_sector_init(const char *fname)
>          fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
>          return 1;
>      }
> +
> +    /* For Open Firmware based system, we can use a Forth script instead */
> +    if (strcmp(qtest_get_arch(), "ppc64") == 0) {

As always, I'm uneasy about using arch based tests for what's really a
machine type property.  Still, as a test case, I guess we can fix that
when and if someone actually tries to run it for a ppc machine that's
not spapr (or an x86 machine that's not pc, theoretically speaking).

> +        memset(boot_sector, ' ', sizeof boot_sector);
> +        sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
> +                LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
> +                HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
> +    }
> +
>      fwrite(boot_sector, 1, sizeof boot_sector, f);
>      fclose(f);
>      return 0;
> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index b2cc355..0bdb7a1 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -21,14 +21,14 @@
>  
>  static const char *disk = "tests/pxe-test-disk.raw";
>  
> -static void test_pxe_one(const char *params)
> +static void test_pxe_one(const char *params, bool ipv6)

Is it wise to keep the "PXE" name.  OF style netbooting isn't really
PXE in the sense of the Intel PXE spec, although it overlaps in the
underlying protocols used.


>  {
>      char *args;
>  
> -    args = g_strdup_printf("-machine accel=tcg "
> -                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s "
> -                           "%s ",
> -                           disk, params);
> +    args = g_strdup_printf("-machine accel=tcg -boot order=n "
> +                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,"
> +                           "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on",
> +                           ipv6 ? "on" : "off", params);
>  
>      qtest_start(args);
>      boot_sector_test();
> @@ -38,12 +38,17 @@ static void test_pxe_one(const char *params)
>  
>  static void test_pxe_e1000(void)
>  {
> -    test_pxe_one("-device e1000,netdev=" NETNAME);
> +    test_pxe_one("-device e1000,netdev=" NETNAME, false);
>  }
>  
>  static void test_pxe_virtio_pci(void)
>  {
> -    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME);
> +    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME, false);
> +}
> +
> +static void test_pxe_spapr_vlan(void)
> +{
> +    test_pxe_one("-vga none -device spapr-vlan,netdev=" NETNAME, true);

Shouldn't we test both IPv4 *and* IPv6 for spapr - AFAICT this is only
testing v6.

>  }
>  
>  int main(int argc, char *argv[])
> @@ -60,6 +65,9 @@ int main(int argc, char *argv[])
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          qtest_add_func("pxe/e1000", test_pxe_e1000);
>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
> +        qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester
  2016-09-27  4:17 ` David Gibson
@ 2016-09-27  7:17   ` Thomas Huth
  2016-09-28  1:59     ` David Gibson
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2016-09-27  7:17 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-devel, Jason Wang, Samuel Thibault, Jan Kiszka,
	Michael S Tsirkin, Victor Kaplansky, Alexander Graf, qemu-ppc

[-- Attachment #1: Type: text/plain, Size: 6134 bytes --]

On 27.09.2016 06:17, David Gibson wrote:
> On Mon, Sep 26, 2016 at 10:17:46PM +0200, Thomas Huth wrote:
>> The firmware of the pseries machine, SLOF, is able to load files via
>> IPv6 networking, too. So to test both, network bootloading on ppc64
>> and IPv6 (via Slirp) , let's add some PXE tests for this environment,
>> too. Since we can not use the normal x86 boot sector for network boot
>> loading, we use a simple Forth script on ppc64 instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> I certainly approve of testing IPv6 more, a couple of queries about
> the details though:
> 
>> ---
>>  tests/Makefile.include |  1 +
>>  tests/boot-sector.c    |  9 +++++++++
>>  tests/pxe-test.c       | 22 +++++++++++++++-------
>>  3 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index d8101b3..18bc698 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -270,6 +270,7 @@ check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/pxe-test$(EXESUF)
>>  
>>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>  
>> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
>> index 3ffe298..e3193c0 100644
>> --- a/tests/boot-sector.c
>> +++ b/tests/boot-sector.c
>> @@ -77,6 +77,15 @@ int boot_sector_init(const char *fname)
>>          fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
>>          return 1;
>>      }
>> +
>> +    /* For Open Firmware based system, we can use a Forth script instead */
>> +    if (strcmp(qtest_get_arch(), "ppc64") == 0) {
> 
> As always, I'm uneasy about using arch based tests for what's really a
> machine type property.  Still, as a test case, I guess we can fix that
> when and if someone actually tries to run it for a ppc machine that's
> not spapr (or an x86 machine that's not pc, theoretically speaking).

As long as we don't have a fancy qtest_get_machine() function, I think
this is the best we can do right now. And since this code has to be
touched anyway when another machine type should be used to run the
boot_sector_init() function, I think it's OK to postpone this to this
later point in time.

>> +        memset(boot_sector, ' ', sizeof boot_sector);
>> +        sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
>> +                LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
>> +                HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
>> +    }
>> +
>>      fwrite(boot_sector, 1, sizeof boot_sector, f);
>>      fclose(f);
>>      return 0;
>> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
>> index b2cc355..0bdb7a1 100644
>> --- a/tests/pxe-test.c
>> +++ b/tests/pxe-test.c
>> @@ -21,14 +21,14 @@
>>  
>>  static const char *disk = "tests/pxe-test-disk.raw";
>>  
>> -static void test_pxe_one(const char *params)
>> +static void test_pxe_one(const char *params, bool ipv6)
> 
> Is it wise to keep the "PXE" name.  OF style netbooting isn't really
> PXE in the sense of the Intel PXE spec, although it overlaps in the
> underlying protocols used.

Strictly speaking, you're right. But the overlap from the networking
protocol point of view is 95%, I'd guess, basically you can say that:

 PXE = TFTP + DHCP + some few DHCP extensions

... and PXE also defines a x86 API which of course does not apply for ppc.

So in my experience, most people simply talk / know about PXE, but
rather mean network booting via DHCP + TFTP. So I'm fine with keeping
the pxe wording here, but if you like, I can also add another patch to
get rid of this (but then the whole file should also be renamed, I
guess? ... is this worth the effort here?)

>>  {
>>      char *args;
>>  
>> -    args = g_strdup_printf("-machine accel=tcg "
>> -                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s "
>> -                           "%s ",
>> -                           disk, params);
>> +    args = g_strdup_printf("-machine accel=tcg -boot order=n "
>> +                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,"
>> +                           "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on",
>> +                           ipv6 ? "on" : "off", params);
>>  
>>      qtest_start(args);
>>      boot_sector_test();
>> @@ -38,12 +38,17 @@ static void test_pxe_one(const char *params)
>>  
>>  static void test_pxe_e1000(void)
>>  {
>> -    test_pxe_one("-device e1000,netdev=" NETNAME);
>> +    test_pxe_one("-device e1000,netdev=" NETNAME, false);
>>  }
>>  
>>  static void test_pxe_virtio_pci(void)
>>  {
>> -    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME);
>> +    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME, false);
>> +}
>> +
>> +static void test_pxe_spapr_vlan(void)
>> +{
>> +    test_pxe_one("-vga none -device spapr-vlan,netdev=" NETNAME, true);
> 
> Shouldn't we test both IPv4 *and* IPv6 for spapr - AFAICT this is only
> testing v6.

The IPv4 part of SLOF is already exercised via the test_pxe_virtio_pci
test, so I don't think we'd gain a lot more of test coverage by running
the spapr-vlan test additionally with IPv4. And since this test is
rather slow (it takes a couple of seconds), I think it's better to only
test IPv6 with spapr-vlan.

>>  }
>>  
>>  int main(int argc, char *argv[])
>> @@ -60,6 +65,9 @@ int main(int argc, char *argv[])
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          qtest_add_func("pxe/e1000", test_pxe_e1000);
>>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
>> +    } else if (strcmp(arch, "ppc64") == 0) {
>> +        qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
>> +        qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
>>      }
>>      ret = g_test_run();
>>      boot_sector_cleanup(disk);

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester
  2016-09-27  7:17   ` Thomas Huth
@ 2016-09-28  1:59     ` David Gibson
  2016-09-28  6:38       ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2016-09-28  1:59 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Jason Wang, Samuel Thibault, Jan Kiszka,
	Michael S Tsirkin, Victor Kaplansky, Alexander Graf, qemu-ppc

[-- Attachment #1: Type: text/plain, Size: 7115 bytes --]

On Tue, Sep 27, 2016 at 09:17:19AM +0200, Thomas Huth wrote:
> On 27.09.2016 06:17, David Gibson wrote:
> > On Mon, Sep 26, 2016 at 10:17:46PM +0200, Thomas Huth wrote:
> >> The firmware of the pseries machine, SLOF, is able to load files via
> >> IPv6 networking, too. So to test both, network bootloading on ppc64
> >> and IPv6 (via Slirp) , let's add some PXE tests for this environment,
> >> too. Since we can not use the normal x86 boot sector for network boot
> >> loading, we use a simple Forth script on ppc64 instead.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> > 
> > I certainly approve of testing IPv6 more, a couple of queries about
> > the details though:
> > 
> >> ---
> >>  tests/Makefile.include |  1 +
> >>  tests/boot-sector.c    |  9 +++++++++
> >>  tests/pxe-test.c       | 22 +++++++++++++++-------
> >>  3 files changed, 25 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/tests/Makefile.include b/tests/Makefile.include
> >> index d8101b3..18bc698 100644
> >> --- a/tests/Makefile.include
> >> +++ b/tests/Makefile.include
> >> @@ -270,6 +270,7 @@ check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
> >>  check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
> >>  check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
> >>  check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
> >> +check-qtest-ppc64-y += tests/pxe-test$(EXESUF)
> >>  
> >>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
> >>  
> >> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> >> index 3ffe298..e3193c0 100644
> >> --- a/tests/boot-sector.c
> >> +++ b/tests/boot-sector.c
> >> @@ -77,6 +77,15 @@ int boot_sector_init(const char *fname)
> >>          fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
> >>          return 1;
> >>      }
> >> +
> >> +    /* For Open Firmware based system, we can use a Forth script instead */
> >> +    if (strcmp(qtest_get_arch(), "ppc64") == 0) {
> > 
> > As always, I'm uneasy about using arch based tests for what's really a
> > machine type property.  Still, as a test case, I guess we can fix that
> > when and if someone actually tries to run it for a ppc machine that's
> > not spapr (or an x86 machine that's not pc, theoretically speaking).
> 
> As long as we don't have a fancy qtest_get_machine() function, I think
> this is the best we can do right now. And since this code has to be
> touched anyway when another machine type should be used to run the
> boot_sector_init() function, I think it's OK to postpone this to this
> later point in time.

I concur.

> >> +        memset(boot_sector, ' ', sizeof boot_sector);
> >> +        sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
> >> +                LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
> >> +                HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
> >> +    }
> >> +
> >>      fwrite(boot_sector, 1, sizeof boot_sector, f);
> >>      fclose(f);
> >>      return 0;
> >> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> >> index b2cc355..0bdb7a1 100644
> >> --- a/tests/pxe-test.c
> >> +++ b/tests/pxe-test.c
> >> @@ -21,14 +21,14 @@
> >>  
> >>  static const char *disk = "tests/pxe-test-disk.raw";
> >>  
> >> -static void test_pxe_one(const char *params)
> >> +static void test_pxe_one(const char *params, bool ipv6)
> > 
> > Is it wise to keep the "PXE" name.  OF style netbooting isn't really
> > PXE in the sense of the Intel PXE spec, although it overlaps in the
> > underlying protocols used.
> 
> Strictly speaking, you're right. But the overlap from the networking
> protocol point of view is 95%, I'd guess, basically you can say that:
> 
>  PXE = TFTP + DHCP + some few DHCP extensions

(aside on subtle English usage at [0] if you're interested)

> ... and PXE also defines a x86 API which of course does not apply for ppc.
> 
> So in my experience, most people simply talk / know about PXE, but
> rather mean network booting via DHCP + TFTP. So I'm fine with keeping
> the pxe wording here, but if you like, I can also add another patch to
> get rid of this (but then the whole file should also be renamed, I
> guess? ... is this worth the effort here?)

Hm.. you convinced me.  Let's just leave the name as is.

> 
> >>  {
> >>      char *args;
> >>  
> >> -    args = g_strdup_printf("-machine accel=tcg "
> >> -                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s "
> >> -                           "%s ",
> >> -                           disk, params);
> >> +    args = g_strdup_printf("-machine accel=tcg -boot order=n "
> >> +                           "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,"
> >> +                           "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on",
> >> +                           ipv6 ? "on" : "off", params);
> >>  
> >>      qtest_start(args);
> >>      boot_sector_test();
> >> @@ -38,12 +38,17 @@ static void test_pxe_one(const char *params)
> >>  
> >>  static void test_pxe_e1000(void)
> >>  {
> >> -    test_pxe_one("-device e1000,netdev=" NETNAME);
> >> +    test_pxe_one("-device e1000,netdev=" NETNAME, false);
> >>  }
> >>  
> >>  static void test_pxe_virtio_pci(void)
> >>  {
> >> -    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME);
> >> +    test_pxe_one("-device virtio-net-pci,netdev=" NETNAME, false);
> >> +}
> >> +
> >> +static void test_pxe_spapr_vlan(void)
> >> +{
> >> +    test_pxe_one("-vga none -device spapr-vlan,netdev=" NETNAME, true);
> > 
> > Shouldn't we test both IPv4 *and* IPv6 for spapr - AFAICT this is only
> > testing v6.
> 
> The IPv4 part of SLOF is already exercised via the test_pxe_virtio_pci
> test, so I don't think we'd gain a lot more of test coverage by running
> the spapr-vlan test additionally with IPv4. And since this test is
> rather slow (it takes a couple of seconds), I think it's better to only
> test IPv6 with spapr-vlan.

Fair enough.  Ok, I'll go back and apply this patch as is.

> 
> >>  }
> >>  
> >>  int main(int argc, char *argv[])
> >> @@ -60,6 +65,9 @@ int main(int argc, char *argv[])
> >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >>          qtest_add_func("pxe/e1000", test_pxe_e1000);
> >>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
> >> +    } else if (strcmp(arch, "ppc64") == 0) {
> >> +        qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
> >> +        qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
> >>      }
> >>      ret = g_test_run();
> >>      boot_sector_cleanup(disk);
> 
>  Thomas
> 
> 

[0] A native speaker would probably say "a few" DHCP extensions here.
"some few", oddly enough, reads as very slight sarcasm implying that
there are actually quite a lot of extensions, or at least more than
you'd expect.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester
  2016-09-28  1:59     ` David Gibson
@ 2016-09-28  6:38       ` Thomas Huth
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2016-09-28  6:38 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-devel, Jason Wang, Samuel Thibault, Jan Kiszka,
	Michael S Tsirkin, Victor Kaplansky, Alexander Graf, qemu-ppc

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

On 28.09.2016 03:59, David Gibson wrote:
> On Tue, Sep 27, 2016 at 09:17:19AM +0200, Thomas Huth wrote:
>> On 27.09.2016 06:17, David Gibson wrote:
>>> On Mon, Sep 26, 2016 at 10:17:46PM +0200, Thomas Huth wrote:
[...]
>>>> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
>>>> index b2cc355..0bdb7a1 100644
>>>> --- a/tests/pxe-test.c
>>>> +++ b/tests/pxe-test.c
>>>> @@ -21,14 +21,14 @@
>>>>  
>>>>  static const char *disk = "tests/pxe-test-disk.raw";
>>>>  
>>>> -static void test_pxe_one(const char *params)
>>>> +static void test_pxe_one(const char *params, bool ipv6)
>>>
>>> Is it wise to keep the "PXE" name.  OF style netbooting isn't really
>>> PXE in the sense of the Intel PXE spec, although it overlaps in the
>>> underlying protocols used.
>>
>> Strictly speaking, you're right. But the overlap from the networking
>> protocol point of view is 95%, I'd guess, basically you can say that:
>>
>>  PXE = TFTP + DHCP + some few DHCP extensions
> 
> (aside on subtle English usage at [0] if you're interested)
[...]
> [0] A native speaker would probably say "a few" DHCP extensions here.
> "some few", oddly enough, reads as very slight sarcasm implying that
> there are actually quite a lot of extensions, or at least more than
> you'd expect.

Oh, good to know, that's the things that you miss as a non-native
speaker ... so I actually really meant "a few" here (though some of the
extensions are IMHO rather strange).

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-09-28  6:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 20:17 [Qemu-devel] [PATCH] tests: Test IPv6 and ppc64 in the PXE tester Thomas Huth
2016-09-27  4:17 ` David Gibson
2016-09-27  7:17   ` Thomas Huth
2016-09-28  1:59     ` David Gibson
2016-09-28  6:38       ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.