[Qemu-devel] [PATCH] ide-test: Fix endianness problems

[Qemu-devel] [PATCH] ide-test: Fix endianness problems

[Kevin Wolf]

The test case passes on big endian hosts now (tested on ppc64)

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/ide-test.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index bdc1da7..365e995 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -252,7 +252,10 @@ static void test_bmdma_simple_rw(void)
     uintptr_t guest_buf = guest_alloc(guest_malloc, len);
 
     PrdtEntry prdt[] = {
-        { .addr = guest_buf, .size = len | PRDT_EOT },
+        {
+            .addr = cpu_to_le32(guest_buf),
+            .size = cpu_to_le32(len | PRDT_EOT),
+        },
     };
 
     buf = g_malloc(len);
@@ -304,7 +307,10 @@ static void test_bmdma_short_prdt(void)
     uint8_t status;
 
     PrdtEntry prdt[] = {
-        { .addr = 0, .size = 0x10 | PRDT_EOT },
+        {
+            .addr = 0,
+            .size = cpu_to_le32(0x10 | PRDT_EOT),
+        },
     };
 
     /* Normal request */
@@ -325,7 +331,10 @@ static void test_bmdma_long_prdt(void)
     uint8_t status;
 
     PrdtEntry prdt[] = {
-        { .addr = 0, .size = 0x1000 | PRDT_EOT },
+        {
+            .addr = 0,
+            .size = cpu_to_le32(0x1000 | PRDT_EOT),
+        },
     };
 
     /* Normal request */
@@ -355,6 +364,17 @@ static void test_bmdma_teardown(void)
     ide_test_quit();
 }
 
+static void string_cpu_to_be16(uint16_t *s, size_t bytes)
+{
+    g_assert((bytes & 1) == 0);
+    bytes /= 2;
+
+    while (bytes--) {
+        *s = cpu_to_be16(*s);
+        s++;
+    }
+}
+
 static void test_identify(void)
 {
     uint8_t data;
@@ -389,10 +409,12 @@ static void test_identify(void)
     assert_bit_clear(data, BSY | DF | ERR | DRQ);
 
     /* Check serial number/version in the buffer */
-    ret = memcmp(&buf[10], "ettsidks            ", 20);
+    string_cpu_to_be16(&buf[10], 20);
+    ret = memcmp(&buf[10], "testdisk            ", 20);
     g_assert(ret == 0);
 
-    ret = memcmp(&buf[23], "evsroi n", 8);
+    string_cpu_to_be16(&buf[23], 8);
+    ret = memcmp(&buf[23], "version ", 8);
     g_assert(ret == 0);
 
     /* Write cache enabled bit */
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5?] ide-test: Fix endianness problems

[Paolo Bonzini]

Il 15/05/2013 15:00, Kevin Wolf ha scritto:
> The test case passes on big endian hosts now (tested on ppc64)
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

For 1.5?  Do we need an extra -rc?

Paolo

Re: [Qemu-devel] [PATCH for-1.5?] ide-test: Fix endianness problems

[Kevin Wolf]

Am 15.05.2013 um 15:15 hat Paolo Bonzini geschrieben:
> Il 15/05/2013 15:00, Kevin Wolf ha scritto:
> > The test case passes on big endian hosts now (tested on ppc64)
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> 
> For 1.5?  Do we need an extra -rc?

An extra -rc for a test case fix on big endian hosts is probably too
much... But if it can still be applied for the release, sure, why not.

Kevin

Re: [Qemu-devel] [PATCH for-1.5?] ide-test: Fix endianness problems

[Paolo Bonzini]

Il 15/05/2013 15:24, Kevin Wolf ha scritto:
> Am 15.05.2013 um 15:15 hat Paolo Bonzini geschrieben:
>> Il 15/05/2013 15:00, Kevin Wolf ha scritto:
>>> The test case passes on big endian hosts now (tested on ppc64)
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>
>> For 1.5?  Do we need an extra -rc?
> 
> An extra -rc for a test case fix on big endian hosts is probably too
> much... But if it can still be applied for the release, sure, why not.

Well, we have Mac OS X not building, BSDs not supporting the GTK+
release, and SLIRP broken on Windows.  At least the first two have
patches on the list, the last is bisected but no patch yet.

Paolo

Re: [Qemu-devel] [PATCH for-1.5?] ide-test: Fix endianness problems

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 15/05/2013 15:24, Kevin Wolf ha scritto:
>> Am 15.05.2013 um 15:15 hat Paolo Bonzini geschrieben:
>>> Il 15/05/2013 15:00, Kevin Wolf ha scritto:
>>>> The test case passes on big endian hosts now (tested on ppc64)
>>>>
>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>>
>>> For 1.5?  Do we need an extra -rc?
>> 
>> An extra -rc for a test case fix on big endian hosts is probably too
>> much... But if it can still be applied for the release, sure, why not.
>
> Well, we have Mac OS X not building,

Peter's patch fixes it and will be in -rc2

> BSDs not supporting the GTK+

We don't have a formal support statement, but I don't think it's
controversial to say that BSD hosts are a secondary platform from a host
point of view.

A build issue in an optional feature on a secondary platform is not
something I'd consider a release blocker.

> release, and SLIRP broken on Windows.

This is a tough one since it's the default networking backend.  That
said, it likely has been broken for a while (months) and no one
noticed.  That makes me think that fixing in stable (particularly if we
scheduled a stable release for two weeks after 1.5.0) is reasonable.

Regards,

Anthony Liguori

>  At least the first two have
> patches on the list, the last is bisected but no patch yet.
>
> Paolo

Re: [Qemu-devel] [PATCH for-1.5?] ide-test: Fix endianness problems

[mdroth]

On Wed, May 15, 2013 at 10:37:43AM -0500, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 15/05/2013 15:24, Kevin Wolf ha scritto:
> >> Am 15.05.2013 um 15:15 hat Paolo Bonzini geschrieben:
> >>> Il 15/05/2013 15:00, Kevin Wolf ha scritto:
> >>>> The test case passes on big endian hosts now (tested on ppc64)
> >>>>
> >>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >>>
> >>> For 1.5?  Do we need an extra -rc?
> >> 
> >> An extra -rc for a test case fix on big endian hosts is probably too
> >> much... But if it can still be applied for the release, sure, why not.
> >
> > Well, we have Mac OS X not building,
> 
> Peter's patch fixes it and will be in -rc2
> 
> > BSDs not supporting the GTK+
> 
> We don't have a formal support statement, but I don't think it's
> controversial to say that BSD hosts are a secondary platform from a host
> point of view.
> 
> A build issue in an optional feature on a secondary platform is not
> something I'd consider a release blocker.
> 
> > release, and SLIRP broken on Windows.
> 
> This is a tough one since it's the default networking backend.  That
> said, it likely has been broken for a while (months) and no one
> noticed.  That makes me think that fixing in stable (particularly if we
> scheduled a stable release for two weeks after 1.5.0) is reasonable.

This is a bit tight. Freeze for 1.4.2 is the same day as 1.5.0, so we'd
be testing 2 stable releases at the same time.

So if we do this, I think it we should restrict it to fixing the specific
issues we're considering as potential 1.5.0 release blockers so we can
focus on those in testing rather than opening it up for general fixes.

> 
> Regards,
> 
> Anthony Liguori
> 
> >  At least the first two have
> > patches on the list, the last is bisected but no patch yet.
> >
> > Paolo
> 

Re: [Qemu-devel] [PATCH] ide-test: Fix endianness problems

[Stefan Hajnoczi]

On Wed, May 15, 2013 at 03:00:39PM +0200, Kevin Wolf wrote:
> @@ -355,6 +364,17 @@ static void test_bmdma_teardown(void)
>      ide_test_quit();
>  }
>  
> +static void string_cpu_to_be16(uint16_t *s, size_t bytes)
> +{
> +    g_assert((bytes & 1) == 0);
> +    bytes /= 2;
> +
> +    while (bytes--) {
> +        *s = cpu_to_be16(*s);
> +        s++;
> +    }
> +}
> +
>  static void test_identify(void)
>  {
>      uint8_t data;
> @@ -389,10 +409,12 @@ static void test_identify(void)
>      assert_bit_clear(data, BSY | DF | ERR | DRQ);
>  
>      /* Check serial number/version in the buffer */
> -    ret = memcmp(&buf[10], "ettsidks            ", 20);
> +    string_cpu_to_be16(&buf[10], 20);
> +    ret = memcmp(&buf[10], "testdisk            ", 20);
>      g_assert(ret == 0);
>  
> -    ret = memcmp(&buf[23], "evsroi n", 8);
> +    string_cpu_to_be16(&buf[23], 8);
> +    ret = memcmp(&buf[23], "version ", 8);

It would have been simpler to specify string_cpu_to_be16() length in
"elements" instead of bytes.  Then you can drop the assertion and
conversion.  Not a problem though.

Anthony: Please take this patch without a pull request.  I think me
sending pull requests for a single late-rc fix doesn't add value.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH] ide-test: Fix endianness problems

[Kevin Wolf]

Am 16.05.2013 um 11:04 hat Stefan Hajnoczi geschrieben:
> Anthony: Please take this patch without a pull request.  I think me
> sending pull requests for a single late-rc fix doesn't add value.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

It's already merged.

Kevin

Re: [Qemu-devel] [PATCH] ide-test: Fix endianness problems

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori