* [PATCH][TOOLS] libfsimage: portability fixes
@ 2008-03-26 14:14 Christoph Egger
2008-03-27 10:54 ` Ian Jackson
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Christoph Egger @ 2008-03-26 14:14 UTC (permalink / raw)
To: xen-devel
[-- 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-26 14:14 [PATCH][TOOLS] libfsimage: portability fixes Christoph Egger
@ 2008-03-27 10:54 ` Ian Jackson
2008-03-27 15:13 ` Christoph Egger
2008-03-27 11:05 ` Ian Jackson
2008-03-27 15:05 ` Aron Griffis
2 siblings, 1 reply; 16+ messages in thread
From: Ian Jackson @ 2008-03-27 10:54 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-26 14:14 [PATCH][TOOLS] libfsimage: portability fixes Christoph Egger
2008-03-27 10:54 ` Ian Jackson
@ 2008-03-27 11:05 ` Ian Jackson
2008-03-27 15:16 ` Christoph Egger
2008-03-27 15:05 ` Aron Griffis
2 siblings, 1 reply; 16+ messages in thread
From: Ian Jackson @ 2008-03-27 11:05 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-26 14:14 [PATCH][TOOLS] libfsimage: portability fixes Christoph Egger
2008-03-27 10:54 ` Ian Jackson
2008-03-27 11:05 ` Ian Jackson
@ 2008-03-27 15:05 ` Aron Griffis
2008-03-27 15:13 ` Ian Jackson
2 siblings, 1 reply; 16+ messages in thread
From: Aron Griffis @ 2008-03-27 15:05 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 10:54 ` Ian Jackson
@ 2008-03-27 15:13 ` Christoph Egger
2008-03-27 15:36 ` Ian Jackson
0 siblings, 1 reply; 16+ messages in thread
From: Christoph Egger @ 2008-03-27 15:13 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:05 ` Aron Griffis
@ 2008-03-27 15:13 ` Ian Jackson
2008-03-27 15:56 ` Aron Griffis
0 siblings, 1 reply; 16+ messages in thread
From: Ian Jackson @ 2008-03-27 15:13 UTC (permalink / raw)
To: Aron Griffis; +Cc: Christoph Egger, xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 11:05 ` Ian Jackson
@ 2008-03-27 15:16 ` Christoph Egger
2008-03-27 16:26 ` John Levon
0 siblings, 1 reply; 16+ messages in thread
From: Christoph Egger @ 2008-03-27 15:16 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:13 ` Christoph Egger
@ 2008-03-27 15:36 ` Ian Jackson
2008-03-27 16:23 ` John Levon
2008-03-28 10:08 ` Christoph Egger
0 siblings, 2 replies; 16+ messages in thread
From: Ian Jackson @ 2008-03-27 15:36 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
@ 2008-03-27 15:36 Christoph Egger
0 siblings, 0 replies; 16+ messages in thread
From: Christoph Egger @ 2008-03-27 15:36 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:13 ` Ian Jackson
@ 2008-03-27 15:56 ` Aron Griffis
0 siblings, 0 replies; 16+ messages in thread
From: Aron Griffis @ 2008-03-27 15:56 UTC (permalink / raw)
To: Ian Jackson; +Cc: Christoph Egger, xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:36 ` Ian Jackson
@ 2008-03-27 16:23 ` John Levon
2008-03-28 10:08 ` Christoph Egger
1 sibling, 0 replies; 16+ messages in thread
From: John Levon @ 2008-03-27 16:23 UTC (permalink / raw)
To: Ian Jackson; +Cc: Christoph Egger, xen-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:16 ` Christoph Egger
@ 2008-03-27 16:26 ` John Levon
0 siblings, 0 replies; 16+ messages in thread
From: John Levon @ 2008-03-27 16:26 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel, Ian Jackson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-27 15:36 ` Ian Jackson
2008-03-27 16:23 ` John Levon
@ 2008-03-28 10:08 ` Christoph Egger
2008-03-28 10:42 ` Ian Jackson
1 sibling, 1 reply; 16+ messages in thread
From: Christoph Egger @ 2008-03-28 10:08 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-28 10:08 ` Christoph Egger
@ 2008-03-28 10:42 ` Ian Jackson
2008-03-28 12:40 ` Christoph Egger
0 siblings, 1 reply; 16+ messages in thread
From: Ian Jackson @ 2008-03-28 10:42 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel, 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-28 10:42 ` Ian Jackson
@ 2008-03-28 12:40 ` Christoph Egger
2008-03-28 14:19 ` Ian Jackson
0 siblings, 1 reply; 16+ messages in thread
From: Christoph Egger @ 2008-03-28 12:40 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][TOOLS] libfsimage: portability fixes
2008-03-28 12:40 ` Christoph Egger
@ 2008-03-28 14:19 ` Ian Jackson
0 siblings, 0 replies; 16+ messages in thread
From: Ian Jackson @ 2008-03-28 14:19 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2008-03-28 14:19 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-26 14:14 [PATCH][TOOLS] libfsimage: portability fixes Christoph Egger
2008-03-27 10:54 ` Ian Jackson
2008-03-27 15:13 ` Christoph Egger
2008-03-27 15:36 ` Ian Jackson
2008-03-27 16:23 ` John Levon
2008-03-28 10:08 ` Christoph Egger
2008-03-28 10:42 ` Ian Jackson
2008-03-28 12:40 ` Christoph Egger
2008-03-28 14:19 ` Ian Jackson
2008-03-27 11:05 ` Ian Jackson
2008-03-27 15:16 ` Christoph Egger
2008-03-27 16:26 ` John Levon
2008-03-27 15:05 ` Aron Griffis
2008-03-27 15:13 ` Ian Jackson
2008-03-27 15:56 ` Aron Griffis
-- strict thread matches above, loose matches on Subject: below --
2008-03-27 15:36 Christoph Egger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.