[PATCH][TOOLS] libfsimage: portability fixes

[PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

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


Hi Keir!

Attached patch improves portability and adds fixes for NetBSD to libfsimage.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: tools_libfsimage.diff --]
[-- Type: text/x-diff, Size: 4009 bytes --]

diff -r 966c04d42e94 tools/libfsimage/Makefile
--- a/tools/libfsimage/Makefile	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/Makefile	Wed Mar 26 17:12:55 2008 +0100
@@ -2,7 +2,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 include $(XEN_ROOT)/tools/Rules.mk
 
 SUBDIRS-y = common ufs reiserfs iso9660 fat
-SUBDIRS-y += $(shell env CC="$(CC)" ./check-libext2fs)
+SUBDIRS-y += $(shell $(SHELL) env CC="$(CC)" ./check-libext2fs)
 
 .PHONY: all clean install
 all clean install: %: subdirs-%
diff -r 966c04d42e94 tools/libfsimage/Rules.mk
--- a/tools/libfsimage/Rules.mk	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/Rules.mk	Wed Mar 26 17:12:55 2008 +0100
@@ -11,6 +11,7 @@ FSDIR-$(CONFIG_SunOS)-x86_64 = $(PREFIX)
 FSDIR-$(CONFIG_SunOS)-x86_64 = $(PREFIX)/lib/fs/$(FS)/64
 FSDIR-$(CONFIG_SunOS)-x86_32 = $(PREFIX)/lib/fs/$(FS)/
 FSDIR-$(CONFIG_SunOS) = $(FSDIR-$(CONFIG_SunOS)-$(XEN_TARGET_ARCH))
+FSDIR-$(CONFIG_NetBSD) = $(LIBDIR)/fs/$(FS)
 FSDIR = $(FSDIR-y)
 
 FSLIB = fsimage.so
diff -r 966c04d42e94 tools/libfsimage/check-libext2fs
--- a/tools/libfsimage/check-libext2fs	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/check-libext2fs	Wed Mar 26 17:12:55 2008 +0100
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 cat >ext2-test.c <<EOF
 #include <ext2fs/ext2fs.h>
@@ -9,7 +9,9 @@ int main()
 }
 EOF
 
-${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
+if test -z ${CC}; then CC="gcc"; fi
+${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
+
 if [ $? = 0 ]; then
 	echo ext2fs-lib
 else
diff -r 966c04d42e94 tools/libfsimage/common/fsimage_grub.c
--- a/tools/libfsimage/common/fsimage_grub.c	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/common/fsimage_grub.c	Wed Mar 26 17:12:55 2008 +0100
@@ -204,19 +204,47 @@ fsig_devread(fsi_file_t *ffi, unsigned i
 fsig_devread(fsi_file_t *ffi, unsigned int sector, unsigned int offset,
     unsigned int bufsize, char *buf)
 {
-	uint64_t off = ffi->ff_fsi->f_off + ((uint64_t)sector * 512) + offset;
-	ssize_t bytes_read = 0;
+	off_t off;
+	ssize_t ret;
+	int n, r;
+	char tmp[SECTOR_SIZE];
 
-	while (bufsize) {
-		ssize_t ret = pread(ffi->ff_fsi->f_fd, buf + bytes_read,
-		    bufsize, (off_t)off);
-		if (ret == -1)
+	off = ffi->ff_fsi->f_off + ((off_t)sector * SECTOR_SIZE) + offset;
+
+	/*
+	 * Make reads from a raw disk sector-aligned. This is a requirement
+	 * for NetBSD. Split the read up into to three parts to meet this
+	 * requirement.
+	 */
+
+	n = (off & (SECTOR_SIZE - 1));
+	if (n > 0) {
+		r = SECTOR_SIZE - n;
+		if (r > bufsize)
+			r = bufsize;
+		ret = pread(ffi->ff_fsi->f_fd, tmp, SECTOR_SIZE, off - n);
+		if (ret < n + r)
 			return (0);
-		if (ret == 0)
+		memcpy(buf, tmp + n, r);
+		buf += r;
+		bufsize -= r;
+		off += r;
+	}
+
+	n = (bufsize & ~(SECTOR_SIZE - 1));
+	if (n > 0) {
+		ret = pread(ffi->ff_fsi->f_fd, buf, n, off);
+		if (ret < n)
 			return (0);
-
-		bytes_read += ret;
-		bufsize -= ret;
+		buf += n;
+		bufsize -= n;
+		off += n;
+	}
+	if (bufsize > 0) {
+		ret = pread(ffi->ff_fsi->f_fd, tmp, SECTOR_SIZE, off);
+		if (ret < bufsize)
+			return (0);
+		memcpy(buf, tmp, bufsize);
 	}
 
 	return (1);
diff -r 966c04d42e94 tools/libfsimage/common/fsimage_grub.h
--- a/tools/libfsimage/common/fsimage_grub.h	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/common/fsimage_grub.h	Wed Mar 26 17:12:55 2008 +0100
@@ -44,7 +44,7 @@ typedef struct fsig_plugin_ops {
 } fsig_plugin_ops_t;
 
 #define	STAGE1_5
-#define FSYS_BUFLEN 0x8000
+#define FSYS_BUFLEN 0x40000
 #define	SECTOR_BITS 9
 #define	SECTOR_SIZE 0x200
 
diff -r 966c04d42e94 tools/libfsimage/common/fsimage_plugin.c
--- a/tools/libfsimage/common/fsimage_plugin.c	Wed Mar 26 09:12:57 2008 +0000
+++ b/tools/libfsimage/common/fsimage_plugin.c	Wed Mar 26 17:12:55 2008 +0100
@@ -131,7 +131,10 @@ static int load_plugins(void)
 	int err;
 	int ret = -1;
 
-#ifdef __sun__
+#if defined(FSIMAGE_FSDIR)
+	if (fsdir == NULL)
+		fsdir = FSIMAGE_FSDIR;
+#elif defined(__sun__)
 	if (fsdir == NULL)
 		fsdir = "/usr/lib/fs";
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> -SUBDIRS-y += $(shell env CC="$(CC)" ./check-libext2fs)
> +SUBDIRS-y += $(shell $(SHELL) env CC="$(CC)" ./check-libext2fs)

What purpose does this serve ?

> +	/*
> +	 * Make reads from a raw disk sector-aligned. This is a requirement
> +	 * for NetBSD. Split the read up into to three parts to meet this
> +	 * requirement.
> +	 */

Please forgive my ignorance: Does NetBSD offer a different (non-raw)
device which does not have this requirement.  If so perhaps we should
be using it instead - if not, why not ?

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

On Thursday 27 March 2008 11:54:41 Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability 
fixes"):
> > -SUBDIRS-y += $(shell env CC="$(CC)" ./check-libext2fs)
> > +SUBDIRS-y += $(shell $(SHELL) env CC="$(CC)" ./check-libext2fs)
>
> What purpose does this serve ?

Everytime when I submitted a patch where I changed /bin/bash to /bin/sh
John Levon came up with a "Build is broken on Solaris" message.
The fix was always the same: Use $(SHELL) as this is explicitely set for
Solaris.

> > +	/*
> > +	 * Make reads from a raw disk sector-aligned. This is a requirement
> > +	 * for NetBSD. Split the read up into to three parts to meet this
> > +	 * requirement.
> > +	 */
>
> Please forgive my ignorance: Does NetBSD offer a different (non-raw)
> device which does not have this requirement.  If so perhaps we should
> be using it instead - if not, why not ?

The raw device pass requests directly to the underlying device, with
only check/adjustments against the partition bounds. Especially it won't
try to do read/modify/write for write requests, or expand the read if it's
not sector-aligned.

The block device doesn't have this restriction, but allows only ONE open,
therefore it is not usable by pygrub. It also has other side-effects
(as it goes through the buffer cache), it's definitively not useable for the 
NetBSD block device *backend*   or for qemu-dm I/O.

Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> Everytime when I submitted a patch where I changed /bin/bash to /bin/sh
> John Levon came up with a "Build is broken on Solaris" message.
> The fix was always the same: Use $(SHELL) as this is explicitely set for
> Solaris.

People whose /bin/sh is that badly broken deserve to lose IMO - that
is to say, we shouldn't inconvenience other people for their benefit.
We've had this conversation before.

But in any case your syntax was completely wrong and the script has
#!/bin/sh at the top already so I don't see why this is relevant to
you.

> On Thursday 27 March 2008 11:54:41 Ian Jackson wrote:
> > Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability 
> > Please forgive my ignorance: Does NetBSD offer a different (non-raw)
> > device which does not have this requirement.  If so perhaps we should
> > be using it instead - if not, why not ?
> 
> The raw device pass requests directly to the underlying device, with
> only check/adjustments against the partition bounds. Especially it won't
> try to do read/modify/write for write requests, or expand the read if it's
> not sector-aligned.

Yes, but these aren't desirable in this case.

> The block device doesn't have this restriction, but allows only ONE open,
> therefore it is not usable by pygrub. It also has other side-effects
> (as it goes through the buffer cache), it's definitively not useable for the 
> NetBSD block device *backend*   or for qemu-dm I/O.

The problem is that pygrub wants to open the disk multiple times ?
Or is it that xend has already plumbed in the device to qemu-dm or
what have you, and that prevents a second open ?

There doesn't seem to me to be any problem with pygrub's accesses
going through the buffer cache.

I accept that it's not suitable for use as the full block backend, but
perhaps the answer is to pass pygrub an edited version of the device
name, or have pygrub edit it itself.  If we were to use the non-raw
device for pygrub and the raw device for qemu-dm, would things work ?

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[John Levon]

On Thu, Mar 27, 2008 at 03:36:45PM +0000, Ian Jackson wrote:

> Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> > Everytime when I submitted a patch where I changed /bin/bash to /bin/sh
> > John Levon came up with a "Build is broken on Solaris" message.
> > The fix was always the same: Use $(SHELL) as this is explicitely set for
> > Solaris.
> 
> People whose /bin/sh is that badly broken deserve to lose IMO - that

Putting it mildly, I don't think that disabling an important platform
because you have a bee in your bonnet about $(SHELL) is a sensible
implementation choice.

john

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:
> Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: 
portability fixes"):
> > Everytime when I submitted a patch where I changed /bin/bash to /bin/sh
> > John Levon came up with a "Build is broken on Solaris" message.
> > The fix was always the same: Use $(SHELL) as this is explicitely set for
> > Solaris.
>
> People whose /bin/sh is that badly broken deserve to lose IMO - that
> is to say, we shouldn't inconvenience other people for their benefit.
> We've had this conversation before.
>
> But in any case your syntax was completely wrong and the script has
> #!/bin/sh at the top already so I don't see why this is relevant to
> you.
>
> > On Thursday 27 March 2008 11:54:41 Ian Jackson wrote:
> > > Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage:
> > > portability Please forgive my ignorance: Does NetBSD offer a different
> > > (non-raw) device which does not have this requirement.  If so perhaps
> > > we should be using it instead - if not, why not ?
> >
> > The raw device pass requests directly to the underlying device, with
> > only check/adjustments against the partition bounds. Especially it won't
> > try to do read/modify/write for write requests, or expand the read if
> > it's not sector-aligned.
>
> Yes, but these aren't desirable in this case.
>
> > The block device doesn't have this restriction, but allows only ONE open,
> > therefore it is not usable by pygrub. It also has other side-effects
> > (as it goes through the buffer cache), it's definitively not useable for
> > the NetBSD block device *backend*   or for qemu-dm I/O.
>
> The problem is that pygrub wants to open the disk multiple times ?
> Or is it that xend has already plumbed in the device to qemu-dm or
> what have you, and that prevents a second open ?

The block backend opens the block device before pygrub is started.
So, when you have "disk = [ 'phy:/dev/bla,blub,w' ]" in your setup you get
an EBUSY when pygrub tries to open the block device.

> I accept that it's not suitable for use as the full block backend, but
> perhaps the answer is to pass pygrub an edited version of the device
> name, or have pygrub edit it itself.  If we were to use the non-raw
> device for pygrub and the raw device for qemu-dm, would things work ?

I don't think it would work without a lot of work on the backend device,
if it is possible at all. For now, the backend assumes it was given a
block device.

Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:
> > I accept that it's not suitable for use as the full block backend, but
> > perhaps the answer is to pass pygrub an edited version of the device
> > name, or have pygrub edit it itself.  If we were to use the non-raw
> > device for pygrub and the raw device for qemu-dm, would things work ?
> 
> I don't think it would work without a lot of work on the backend device,
> if it is possible at all. For now, the backend assumes it was given a
> block device.

I'm afraid I don't follow this at all.  (What does `block device'
stand in opposition to?)

Is it possible for pygrub to open the non-raw device while the block
backend is using the raw device ?  Or do you mean that the block
backend is already using the non-raw device and that you're having
pygrub use the raw device just so that you are able to use the same
underlying storage object twice simultaneously ?

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

On Friday 28 March 2008 11:42:15 Ian Jackson wrote:
> Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: 
portability fixes"):
> > On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:
> > > I accept that it's not suitable for use as the full block backend, but
> > > perhaps the answer is to pass pygrub an edited version of the device
> > > name, or have pygrub edit it itself.  If we were to use the non-raw
> > > device for pygrub and the raw device for qemu-dm, would things work ?
> >
> > I don't think it would work without a lot of work on the backend device,
> > if it is possible at all. For now, the backend assumes it was given a
> > block device.
>
> I'm afraid I don't follow this at all.  (What does `block device'
> stand in opposition to?)

"block device" is what is given in the guest setup with
"disk = [ 'phy:/dev/wd0a,0x1,w' ]"  (NetBSD syntax)
"disk = [ 'phy:/dev/hda1,hda,w' ]" (Linux syntax)

> Is it possible for pygrub to open the non-raw device while the block
> backend is using the raw device ?  Or do you mean that the block
> backend is already using the non-raw device and that you're having
> pygrub use the raw device just so that you are able to use the same
> underlying storage object twice simultaneously ?

With changeset 17300, pygrub uses a raw device on NetBSD only.
The block backend already opened the block device when pygrub
opens the raw device.

Christoph

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> On Friday 28 March 2008 11:42:15 Ian Jackson wrote:
> > Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: 
> portability fixes"):
> > > On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:
> > > > I accept that it's not suitable for use as the full block backend, but
> > > > perhaps the answer is to pass pygrub an edited version of the device
> > > > name, or have pygrub edit it itself.  If we were to use the non-raw
> > > > device for pygrub and the raw device for qemu-dm, would things work ?
> > >
> > > I don't think it would work without a lot of work on the backend device,
> > > if it is possible at all. For now, the backend assumes it was given a
> > > block device.
> >
> > I'm afraid I don't follow this at all.  (What does `block device'
> > stand in opposition to?)
> 
> "block device" is what is given in the guest setup with
> "disk = [ 'phy:/dev/wd0a,0x1,w' ]"  (NetBSD syntax)
> "disk = [ 'phy:/dev/hda1,hda,w' ]" (Linux syntax)

Yes, I know that.  But you say `the backend assumes it was given a
block device' and I was suggesting giving it the raw block device so
obviously we are talking at cross purposes.

Do you mean that it would somehow be difficult for the block backend
to talk to /dev/rwd0a instead ?  Surely that would be more appropriate
in any case.

> > Is it possible for pygrub to open the non-raw device while the block
> > backend is using the raw device ?  Or do you mean that the block
> > backend is already using the non-raw device and that you're having
> > pygrub use the raw device just so that you are able to use the same
> > underlying storage object twice simultaneously ?
> 
> With changeset 17300, pygrub uses a raw device on NetBSD only.
> The block backend already opened the block device when pygrub
> opens the raw device.

Yes, so you said already.  But you haven't answered my question.

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> -${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> +if test -z ${CC}; then CC="gcc"; fi
> +${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> +

This also seems to serve no purpose as far as I can tell.

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

On Thursday 27 March 2008 12:05:30 Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability 
fixes"):
> > -${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> > +if test -z ${CC}; then CC="gcc"; fi
> > +${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> > +
>
> This also seems to serve no purpose as far as I can tell.

This is for the ancient Solaris /bin/sh.  If John Levon confirms that
this and that the $(SHELL)  mentioned in your other mail 
are NOT needed/required to not break Solaris, you can undo these two changes.

Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][TOOLS] libfsimage: portability fixes

[John Levon]

On Thu, Mar 27, 2008 at 04:16:51PM +0100, Christoph Egger wrote:

> On Thursday 27 March 2008 12:05:30 Ian Jackson wrote:
> > Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability 
> fixes"):
> > > -${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> > > +if test -z ${CC}; then CC="gcc"; fi
> > > +${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> > > +
> >
> > This also seems to serve no purpose as far as I can tell.
> 
> This is for the ancient Solaris /bin/sh.  If John Levon confirms that
> this and that the $(SHELL)  mentioned in your other mail 
> are NOT needed/required to not break Solaris, you can undo these two changes.

It's not needed for the Bourne shell, no.

regards,
john

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Aron Griffis]

Christoph Egger wrote:  [Wed Mar 26 2008, 10:14:33AM EDT]
> diff -r 966c04d42e94 tools/libfsimage/Makefile
> --- a/tools/libfsimage/Makefile	Wed Mar 26 09:12:57 2008 +0000
> +++ b/tools/libfsimage/Makefile	Wed Mar 26 17:12:55 2008 +0100
> @@ -2,7 +2,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  include $(XEN_ROOT)/tools/Rules.mk
>  
>  SUBDIRS-y = common ufs reiserfs iso9660 fat
> -SUBDIRS-y += $(shell env CC="$(CC)" ./check-libext2fs)
> +SUBDIRS-y += $(shell $(SHELL) env CC="$(CC)" ./check-libext2fs)

As Ian asked, what was this intended to do?  It's certainly
wrong:

$ /bin/sh env CC=gcc ./check-libext2fs 
/usr/bin/env: /usr/bin/env: cannot execute binary file

> diff -r 966c04d42e94 tools/libfsimage/check-libext2fs
> --- a/tools/libfsimage/check-libext2fs	Wed Mar 26 09:12:57 2008 +0000
> +++ b/tools/libfsimage/check-libext2fs	Wed Mar 26 17:12:55 2008 +0100
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/bin/sh
>  
>  cat >ext2-test.c <<EOF
>  #include <ext2fs/ext2fs.h>
> @@ -9,7 +9,9 @@ int main()
>  }
>  EOF
>  
> -${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> +if test -z ${CC}; then CC="gcc"; fi
> +${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> +
>  if [ $? = 0 ]; then
>  	echo ext2fs-lib
>  else

This prevents check-libext2fs from being run outside the
Makefile.  It will silently fail compilation if $CC isn't set.
For Bourne shell and cross-platform compatibility, it should be:

${CC-cc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1

Aron

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Ian Jackson]

Aron Griffis writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):
> Christoph Egger wrote:  [Wed Mar 26 2008, 10:14:33AM EDT]
> > -${CC:-gcc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> > +if test -z ${CC}; then CC="gcc"; fi
> > +${CC} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> 
> This prevents check-libext2fs from being run outside the
> Makefile.  It will silently fail compilation if $CC isn't set.
> For Bourne shell and cross-platform compatibility, it should be:

${VARIABLE:-defaultvalue} is in SuSv3 (section 2.6.2, `Use Default
Values') and has been in many previous standards.  Any system whose
/bin/sh can't cope is hopelessly broken.

> ${CC-cc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1

However, in this case, ${VARIABLE-defaultvalue} is an adequate
substitute so we should use that.  With a default of gcc, since
without gcc you're doomed anyway.

Ian.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Aron Griffis]

Ian Jackson wrote:  [Thu Mar 27 2008, 11:13:41AM EDT]
> ${VARIABLE:-defaultvalue} is in SuSv3 (section 2.6.2, `Use Default
> Values') and has been in many previous standards.  Any system whose
> /bin/sh can't cope is hopelessly broken.

Ah, great.

> > ${CC-cc} -o ext2-test ext2-test.c -lext2fs >/dev/null 2>&1
> 
> However, in this case, ${VARIABLE-defaultvalue} is an adequate
> substitute so we should use that.  With a default of gcc, since
> without gcc you're doomed anyway.

Okay, thanks.

Re: [PATCH][TOOLS] libfsimage: portability fixes

[Christoph Egger]

Hi Keir,

changeset 17309 has one hunk too much:

--- a/tools/libfsimage/check-libext2fs	Thu Mar 27 14:43:20 2008 +0000
+++ b/tools/libfsimage/check-libext2fs	Thu Mar 27 15:13:55 2008 +0000
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 cat >ext2-test.c <<EOF
 #include <ext2fs/ext2fs.h>

This causes:

ksh: ./check-libext2fs: No such file or directory

because /bin/bash does not exist on *BSD.
Please apply this:

diff -r ee38b254e98e tools/libfsimage/check-libext2fs
--- a/tools/libfsimage/check-libext2fs  Thu Mar 27 15:13:55 2008 +0000
+++ b/tools/libfsimage/check-libext2fs  Thu Mar 27 18:44:05 2008 +0100
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 cat >ext2-test.c <<EOF
 #include <ext2fs/ext2fs.h>


Christoph