1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs

Pino Toscano posted 1 patch 5 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180801130807.16944-1-ptoscano@redhat.com
Test syntax-check passed
tests/viriscsitest.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
[libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
Posted by Pino Toscano 5 years, 8 months ago 
viriscsitest tries to ensure the interface IQN used is a specific one,
checking later on that it is the same all during the whole test.  Since
the IQN generation involves random bytes, viriscsitest got a fake
virRandomBytes from the virrandommock helper library, setting static
values.  virRandomBits(), called by virStorageBackendCreateIfaceIQN(),
always requests 8 random bytes, chopping off the ones not requested by
the caller -- this meant that on big endian machines it would chop bytes
from the wrong size of the data buffer, and thus not returning the
expected numbers.

As a fix, do not rely on the mock virRandomBytes, but provide an own
version of it: this version will fill the values in the expected order,
depending on the endianness of the system.  This way, the result of
virStorageBackendCreateIfaceIQN() will be what the test actually
expects.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 tests/viriscsitest.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c
index 4fd7ff6e19..d43c0115d3 100644
--- a/tests/viriscsitest.c
+++ b/tests/viriscsitest.c
@@ -21,6 +21,7 @@
 #include <config.h>
 
 #include "testutils.h"
+#include "virrandom.h"
 
 #ifdef WIN32
 int
@@ -36,6 +37,32 @@ main(void)
 
 # define VIR_FROM_THIS VIR_FROM_NONE
 
+bool isLittleEndian(void);
+bool isLittleEndian(void)
+{
+    static int cache = -1;
+    if (cache < 0) {
+        int val = 1;
+        char *ptr = (char *)&val;
+        cache = *ptr ? 1 : 0;
+    }
+    return cache > 0;
+}
+
+
+int
+virRandomBytes(unsigned char *buf,
+               size_t buflen)
+{
+    size_t i;
+    const bool isLE = isLittleEndian();
+
+    for (i = 0; i < buflen; i++)
+        buf[i] = isLE ? i : buflen - i - 1;
+
+    return 0;
+}
+
 static const char *iscsiadmSessionOutput =
     "tcp: [1] 10.20.30.40:3260,1 iqn.2004-06.example:example1:iscsi.test\n"
     "tcp: [2] 10.20.30.41:3260,1 iqn.2005-05.example:example1:iscsi.hello\n"
@@ -368,6 +395,5 @@ mymain(void)
     return EXIT_SUCCESS;
 }
 
-VIR_TEST_MAIN_PRELOAD(mymain,
-                      abs_builddir "/.libs/virrandommock.so")
+VIR_TEST_MAIN(mymain)
 #endif /* WIN32 */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
Posted by Bjoern Walk 5 years, 8 months ago 
Pino Toscano <ptoscano@redhat.com> [2018-08-01, 03:08PM +0200]:
> viriscsitest tries to ensure the interface IQN used is a specific one,
> checking later on that it is the same all during the whole test.  Since
> the IQN generation involves random bytes, viriscsitest got a fake
> virRandomBytes from the virrandommock helper library, setting static
> values.  virRandomBits(), called by virStorageBackendCreateIfaceIQN(),
> always requests 8 random bytes, chopping off the ones not requested by
> the caller -- this meant that on big endian machines it would chop bytes
> from the wrong size of the data buffer, and thus not returning the
> expected numbers.
> 
> As a fix, do not rely on the mock virRandomBytes, but provide an own
> version of it: this version will fill the values in the expected order,
> depending on the endianness of the system.  This way, the result of
> virStorageBackendCreateIfaceIQN() will be what the test actually
> expects.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>

Tested-by: Bjoern Walk <bwalk@linux.ibm.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
Posted by Daniel P. Berrangé 5 years, 8 months ago 
On Wed, Aug 01, 2018 at 03:08:07PM +0200, Pino Toscano wrote:
> viriscsitest tries to ensure the interface IQN used is a specific one,
> checking later on that it is the same all during the whole test.  Since
> the IQN generation involves random bytes, viriscsitest got a fake
> virRandomBytes from the virrandommock helper library, setting static
> values.  virRandomBits(), called by virStorageBackendCreateIfaceIQN(),
> always requests 8 random bytes, chopping off the ones not requested by
> the caller -- this meant that on big endian machines it would chop bytes
> from the wrong size of the data buffer, and thus not returning the
> expected numbers.
> 
> As a fix, do not rely on the mock virRandomBytes, but provide an own
> version of it: this version will fill the values in the expected order,
> depending on the endianness of the system.  This way, the result of
> virStorageBackendCreateIfaceIQN() will be what the test actually
> expects.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>

Mailman is having deliver problems so you might not have received
the patch that was sent earlier for this problem:

https://www.redhat.com/archives/libvir-list/2018-August/msg00039.html

I prefer that approach since it avoids the endianness problems for
all future usage too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
Posted by Pino Toscano 5 years, 8 months ago 
On Wednesday, 1 August 2018 15:24:05 CEST Daniel P. Berrangé wrote:
> On Wed, Aug 01, 2018 at 03:08:07PM +0200, Pino Toscano wrote:
> > viriscsitest tries to ensure the interface IQN used is a specific one,
> > checking later on that it is the same all during the whole test.  Since
> > the IQN generation involves random bytes, viriscsitest got a fake
> > virRandomBytes from the virrandommock helper library, setting static
> > values.  virRandomBits(), called by virStorageBackendCreateIfaceIQN(),
> > always requests 8 random bytes, chopping off the ones not requested by
> > the caller -- this meant that on big endian machines it would chop bytes
> > from the wrong size of the data buffer, and thus not returning the
> > expected numbers.
> > 
> > As a fix, do not rely on the mock virRandomBytes, but provide an own
> > version of it: this version will fill the values in the expected order,
> > depending on the endianness of the system.  This way, the result of
> > virStorageBackendCreateIfaceIQN() will be what the test actually
> > expects.
> > 
> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> 
> Mailman is having deliver problems so you might not have received
> the patch that was sent earlier for this problem:
> 
> https://www.redhat.com/archives/libvir-list/2018-August/msg00039.html
> 
> I prefer that approach since it avoids the endianness problems for
> all future usage too.

I replied to that patch, as IMHO there is no "endianness" for random
data, and thus virRandomBytes() ought to not do any manipulation.

-- 
Pino Toscano--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.