libfsimage: portability fixes

Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability

tools/libfsimage/Makefile include $(XEN_ROOT)/tools/Rules.mk · line 5

Quoted 2 lines

tools/libfsimage/Makefile +1 -1

-SUBDIRS-y += $(shell env CC="$(CC)" ./check-libext2fs)
+SUBDIRS-y += $(shell $(SHELL) env CC="$(CC)" ./check-libext2fs)

What purpose does this serve ?

tools/libfsimage/common/fsimage_grub.c fsig_devread(fsi_file_t *ffi, unsigned i · line 218

Quoted 5 lines

tools/libfsimage/common/fsimage_grub.c +5 -0

+	/*
+	 * 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 ?

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.

Quoted 4 lines

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.

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 Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage: portability fixes"):

Quoted 4 lines

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

Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage:

Quoted 4 lines

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.

Quoted 10 lines · On Thursday 27 March 2008 11:54:41 Ian Jackson wrote:

Quoted 4 lines

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.

Quoted 4 lines

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 ?

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 ?

Quoted 4 lines

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 Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage:

Quoted 6 lines · On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:

Quoted 4 lines

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 ?

Quoted 3 lines

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 ?

Quoted 1 line

Christoph Egger writes ("Re: [Xen-devel] [PATCH][TOOLS] libfsimage:

portability fixes"):

Quoted 10 lines

Quoted 9 lines · On Thursday 27 March 2008 16:36:45 Ian Jackson wrote:

Quoted 4 lines

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.

Quoted 2 lines

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)

Quoted 5 lines

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 Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability

tools/libfsimage/check-libext2fs int main() · line 14

Quoted 4 lines

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

tools/libfsimage/check-libext2fs +3 -0

+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.

Quoted 1 line · On Thursday 27 March 2008 12:05:30 Ian Jackson wrote:

Christoph Egger writes ("[Xen-devel] [PATCH][TOOLS] libfsimage: portability

fixes"):

Quoted 6 lines

tools/libfsimage/check-libext2fs int main() · line 14

Quoted 4 lines

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

tools/libfsimage/check-libext2fs +3 -0

+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.

diff -r 966c04d42e94 tools/libfsimage/check-libext2fs

tools/libfsimage/check-libext2fs +1 -1

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 cat >ext2-test.c <<EOF
 #include <ext2fs/ext2fs.h>

Christoph Egger wrote: [Wed Mar 26 2008, 10:14:33AM EDT]

Quoted 3 lines

-${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:

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

${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.

Quoted 1 line

${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.