[libvirt] [PATCH] Portability fixes for directory storage backend

# HG changeset patch # User john.levon@sun.com # Date 1229399267 28800 # Node ID 53a18080ea210168c044d7ade8dd8db0881d5c85 # Parent cd2fb266824178c4d63296b587eeedb2360f9f1c Portability fixes for directory storage backend Signed-off-by: John Levon <john.levon@sun.com> diff --git a/configure.in b/configure.in --- a/configure.in +++ b/configure.in @@ -75,7 +75,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid]) dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h]) +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h endian.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -31,10 +31,14 @@ #include <errno.h> #include <fcntl.h> #include <unistd.h> +#include <string.h> + +#ifdef HAVE_ENDIAN_H #include <endian.h> -#include <byteswap.h> -#include <mntent.h> -#include <string.h> +#else +#define __LITTLE_ENDIAN 1234 +#define __BIG_ENDIAN 4321 +#endif #include <libxml/parser.h> #include <libxml/tree.h> @@ -240,6 +244,9 @@ static int virStorageBackendProbeFile(vi } #if WITH_STORAGE_FS + +#include <mntent.h> + struct _virNetfsDiscoverState { const char *host; virStoragePoolSourceList list;

john.levon@sun.com wrote: ...
diff --git a/configure.in b/configure.in --- a/configure.in +++ b/configure.in @@ -75,7 +75,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid])
dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h]) +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h endian.h])
dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -31,10 +31,14 @@ #include <errno.h> #include <fcntl.h> #include <unistd.h> +#include <string.h> + +#ifdef HAVE_ENDIAN_H #include <endian.h> -#include <byteswap.h> -#include <mntent.h> -#include <string.h> +#else +#define __LITTLE_ENDIAN 1234 +#define __BIG_ENDIAN 4321 +#endif
#include <libxml/parser.h> #include <libxml/tree.h> @@ -240,6 +244,9 @@ static int virStorageBackendProbeFile(vi }
#if WITH_STORAGE_FS + +#include <mntent.h>
Adding the inclusions of <string.h> and <mntent.h> look fine, but rather than testing for endian.h, it seems we can do away with it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN constants aren't really required. Rather, all you need is an indication (dare I say boolean?) of whether a fileTypeInfo entry is big- or little-endian. However, even though the current code treats this field as effectively boolean (testing a single value and then "else" for the rest), we can leave it as a wider enum. I propose this:
From 65074e8d636a2eae10ceacf2619a6be77f45ee2a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 17 Dec 2008 13:54:04 +0100 Subject: [PATCH] portability: don't include <endian.h>
* src/storage_backend_fs.c: Don't include <endian.h>: not needed. (LV_BIG_ENDIAN, LV_LITTLE_ENDIAN): Define. Use those instead of __BIG_ENDIAN and __LITTLE_ENDIAN. --- src/storage_backend_fs.c | 29 ++++++++++++++++------------- 1 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 91131e5..8bc77fc 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -31,7 +31,6 @@ #include <errno.h> #include <fcntl.h> #include <unistd.h> -#include <endian.h> #include <byteswap.h> #include <mntent.h> #include <string.h> @@ -47,6 +46,10 @@ #include "memory.h" #include "xml.h" +enum lv_endian { + LV_LITTLE_ENDIAN = 1, /* 1234 */ + LV_BIG_ENDIAN /* 4321 */ +}; /* Either 'magic' or 'extension' *must* be provided */ struct FileTypeInfo { @@ -54,7 +57,7 @@ struct FileTypeInfo { const char *magic; /* Optional string of file magic * to check at head of file */ const char *extension; /* Optional file extension to check */ - int endian; /* Endianness of file format */ + enum lv_endian endian; /* Endianness of file format */ int versionOffset; /* Byte offset from start of file * where we find version number, * -1 to skip version test */ @@ -69,16 +72,16 @@ const struct FileTypeInfo const fileTypeInfo[] = { /* Bochs */ /* XXX Untested { VIR_STORAGE_VOL_BOCHS, "Bochs Virtual HD Image", NULL, - __LITTLE_ENDIAN, 64, 0x20000, + LV_LITTLE_ENDIAN, 64, 0x20000, 32+16+16+4+4+4+4+4, 8, 1 },*/ /* CLoop */ /* XXX Untested { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, - __LITTLE_ENDIAN, -1, 0, + LV_LITTLE_ENDIAN, -1, 0, -1, 0, 0 }, */ /* Cow */ { VIR_STORAGE_VOL_FILE_COW, "OOOM", NULL, - __BIG_ENDIAN, 4, 2, + LV_BIG_ENDIAN, 4, 2, 4+4+1024+4, 8, 1 }, /* DMG */ /* XXX QEMU says there's no magic for dmg, but we should check... */ @@ -92,31 +95,31 @@ const struct FileTypeInfo const fileTypeInfo[] = { /* Parallels */ /* XXX Untested { VIR_STORAGE_VOL_FILE_PARALLELS, "WithoutFreeSpace", NULL, - __LITTLE_ENDIAN, 16, 2, + LV_LITTLE_ENDIAN, 16, 2, 16+4+4+4+4, 4, 512 }, */ /* QCow */ { VIR_STORAGE_VOL_FILE_QCOW, "QFI", NULL, - __BIG_ENDIAN, 4, 1, + LV_BIG_ENDIAN, 4, 1, 4+4+8+4+4, 8, 1 }, /* QCow 2 */ { VIR_STORAGE_VOL_FILE_QCOW2, "QFI", NULL, - __BIG_ENDIAN, 4, 2, + LV_BIG_ENDIAN, 4, 2, 4+4+8+4+4, 8, 1 }, /* VMDK 3 */ /* XXX Untested { VIR_STORAGE_VOL_FILE_VMDK, "COWD", NULL, - __LITTLE_ENDIAN, 4, 1, + LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 4, 512 }, */ /* VMDK 4 */ { VIR_STORAGE_VOL_FILE_VMDK, "KDMV", NULL, - __LITTLE_ENDIAN, 4, 1, + LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 8, 512 }, /* Connectix / VirtualPC */ /* XXX Untested { VIR_STORAGE_VOL_FILE_VPC, "conectix", NULL, - __BIG_ENDIAN, -1, 0, + LV_BIG_ENDIAN, -1, 0, -1, 0, 0}, */ }; @@ -173,7 +176,7 @@ static int virStorageBackendProbeFile(virConnectPtr conn, if (fileTypeInfo[i].versionNumber != -1) { int version; - if (fileTypeInfo[i].endian == __LITTLE_ENDIAN) { + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { version = (head[fileTypeInfo[i].versionOffset+3] << 24) | (head[fileTypeInfo[i].versionOffset+2] << 16) | (head[fileTypeInfo[i].versionOffset+1] << 8) | @@ -190,7 +193,7 @@ static int virStorageBackendProbeFile(virConnectPtr conn, /* Optionally extract capacity from file */ if (fileTypeInfo[i].sizeOffset != -1) { - if (fileTypeInfo[i].endian == __LITTLE_ENDIAN) { + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { def->capacity = ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | -- 1.6.1.rc2.316.geb2f0

On Wed, Dec 17, 2008 at 01:55:59PM +0100, Jim Meyering wrote:
Adding the inclusions of <string.h> and <mntent.h> look fine,
They were just moved.
but rather than testing for endian.h, it seems we can do away with it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN constants aren't really required. Rather, all you need is
Fine by me, if you prefer it this way. regards john

John Levon <john.levon@Sun.COM> wrote:
On Wed, Dec 17, 2008 at 01:55:59PM +0100, Jim Meyering wrote:
Adding the inclusions of <string.h> and <mntent.h> look fine,
They were just moved.
but rather than testing for endian.h, it seems we can do away with it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN constants aren't really required. Rather, all you need is
Fine by me, if you prefer it this way.
Of course. Avoiding an #ifdef like that is worth an iteration. I've committed the patch I proposed, with one change: also remove the unnecessary inclusion of <byteswap.h>.

On Tue, Dec 16, 2008 at 06:28:44PM -0800, john.levon@sun.com wrote:
# HG changeset patch # User john.levon@sun.com # Date 1229399267 28800 # Node ID 53a18080ea210168c044d7ade8dd8db0881d5c85 # Parent cd2fb266824178c4d63296b587eeedb2360f9f1c Portability fixes for directory storage backend
Signed-off-by: John Levon <john.levon@sun.com>
ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange
-
Jim Meyering
-
John Levon
-
john.levon@sun.com