| Message ID | 20120224195143.GA16353@vostro.hallyn.com |
|---|---|
| State | New |
| Headers | show |
Hi Anthony, currently QEMU 1.1 doesn't compile virtfs correctly on Ubuntu: qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_update_file_cred’: qemu/hw/9pfs/virtio-9p-handle.c:66:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) qemu/hw/9pfs/virtio-9p-handle.c:66:58: note: each undeclared identifier is reported only once for each function it appears in qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_lstat’: qemu/hw/9pfs/virtio-9p-handle.c:87:34: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_symlink’: qemu/hw/9pfs/virtio-9p-handle.c:314:62: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_link’: qemu/hw/9pfs/virtio-9p-handle.c:337:45: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_chown’: qemu/hw/9pfs/virtio-9p-handle.c:373:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) a patch was sent on the 24th of Feb to fix the issue (also see below): http://marc.info/?l=qemu-devel&m=133011313912147 Even though it is not particularly pretty, in the absence of better alternatives it should probably be applied. Cheers, Stefano On Fri, 24 Feb 2012, Serge Hallyn wrote: > If AT_EMPTY_PATH is not in one of the included files, go ahead and > define it. qemu won't compile on ubuntu for me without this. > > (Note - alternatively we could #include <linux/fcntl.h> to pick > up the definitions there) > > Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> > --- > hw/9pfs/virtio-9p-handle.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c > index f96d17a..e403a84 100644 > --- a/hw/9pfs/virtio-9p-handle.c > +++ b/hw/9pfs/virtio-9p-handle.c > @@ -39,6 +39,15 @@ > #ifndef BTRFS_SUPER_MAGIC > #define BTRFS_SUPER_MAGIC 0x9123683E > #endif > +#ifndef AT_REMOVEDIR > +#define AT_REMOVEDIR 0x200 > +#endif > +#ifndef AT_EMPTY_PATH > +#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ > +#endif > +#ifndef O_PATH > +#define O_PATH 010000000 > +#endif > > struct handle_data { > int mountfd; > -- > 1.7.9 > >
Am 06.06.2012 12:50, schrieb Stefano Stabellini: > Hi Anthony, > currently QEMU 1.1 doesn't compile virtfs correctly on Ubuntu: > > qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_update_file_cred’: > qemu/hw/9pfs/virtio-9p-handle.c:66:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > qemu/hw/9pfs/virtio-9p-handle.c:66:58: note: each undeclared identifier is reported only once for each function it appears in > qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_lstat’: > qemu/hw/9pfs/virtio-9p-handle.c:87:34: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_symlink’: > qemu/hw/9pfs/virtio-9p-handle.c:314:62: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_link’: > qemu/hw/9pfs/virtio-9p-handle.c:337:45: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_chown’: > qemu/hw/9pfs/virtio-9p-handle.c:373:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > > > a patch was sent on the 24th of Feb to fix the issue (also see below): > > http://marc.info/?l=qemu-devel&m=133011313912147 > > Even though it is not particularly pretty, in the absence of better > alternatives it should probably be applied. > > Cheers, > > Stefano > > > > > > On Fri, 24 Feb 2012, Serge Hallyn wrote: >> If AT_EMPTY_PATH is not in one of the included files, go ahead and >> define it. qemu won't compile on ubuntu for me without this. >> >> (Note - alternatively we could #include<linux/fcntl.h> to pick >> up the definitions there) >> >> Signed-off-by: Serge Hallyn<serge.hallyn@canonical.com> >> --- >> hw/9pfs/virtio-9p-handle.c | 9 +++++++++ >> 1 files changed, 9 insertions(+), 0 deletions(-) >> >> diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c >> index f96d17a..e403a84 100644 >> --- a/hw/9pfs/virtio-9p-handle.c >> +++ b/hw/9pfs/virtio-9p-handle.c >> @@ -39,6 +39,15 @@ >> #ifndef BTRFS_SUPER_MAGIC >> #define BTRFS_SUPER_MAGIC 0x9123683E >> #endif >> +#ifndef AT_REMOVEDIR >> +#define AT_REMOVEDIR 0x200 >> +#endif >> +#ifndef AT_EMPTY_PATH >> +#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ >> +#endif >> +#ifndef O_PATH >> +#define O_PATH 010000000 >> +#endif >> >> struct handle_data { >> int mountfd; >> -- >> 1.7.9 The patch will fix the compiler error messages, but will the resulting code work? Maybe it has runtime dependencies (Linux kernel?) which should be checked at runtime. Would an enhanced test in configure be a better solution? It could disable VirtFS automatically if the definitions are missing. On Ubuntu Lenny, there is no definition for AT_EMPTY_PATH, not even in linux/fcntl.h. Regards, Stefan W.
Serge Hallyn <serge.hallyn@canonical.com> writes: > If AT_EMPTY_PATH is not in one of the included files, go ahead and > define it. qemu won't compile on ubuntu for me without this. > > (Note - alternatively we could #include <linux/fcntl.h> to pick > up the definitions there) Then why don't we do that ? I do get the below errors In file included from /usr/include/x86_64-linux-gnu/asm/fcntl.h:1:0, from /usr/include/linux/fcntl.h:4, from /home/opensource/sources/qemu/qemu-upstream/hw/9pfs/virtio-9p-handle.c:28: /usr/include/asm-generic/fcntl.h:127:8: error: redefinition of ‘struct f_owner_ex’ /usr/include/x86_64-linux-gnu/bits/fcntl.h:204:8: note: originally defined here In file included from /usr/include/x86_64-linux-gnu/asm/fcntl.h:1:0, from /usr/include/linux/fcntl.h:4, Are they do to ubuntu multi-arch changes ? > > Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> > --- > hw/9pfs/virtio-9p-handle.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c > index f96d17a..e403a84 100644 > --- a/hw/9pfs/virtio-9p-handle.c > +++ b/hw/9pfs/virtio-9p-handle.c > @@ -39,6 +39,15 @@ > #ifndef BTRFS_SUPER_MAGIC > #define BTRFS_SUPER_MAGIC 0x9123683E > #endif > +#ifndef AT_REMOVEDIR > +#define AT_REMOVEDIR 0x200 > +#endif > +#ifndef AT_EMPTY_PATH > +#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ > +#endif > +#ifndef O_PATH > +#define O_PATH 010000000 > +#endif > > struct handle_data { > int mountfd; > -- > 1.7.9 -aneesh
Quoting Stefan Weil (sw@weilnetz.de): > Am 06.06.2012 12:50, schrieb Stefano Stabellini: > >Hi Anthony, > >currently QEMU 1.1 doesn't compile virtfs correctly on Ubuntu: > > > >qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_update_file_cred’: > >qemu/hw/9pfs/virtio-9p-handle.c:66:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > >qemu/hw/9pfs/virtio-9p-handle.c:66:58: note: each undeclared identifier is reported only once for each function it appears in > >qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_lstat’: > >qemu/hw/9pfs/virtio-9p-handle.c:87:34: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > >qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_symlink’: > >qemu/hw/9pfs/virtio-9p-handle.c:314:62: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > >qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_link’: > >qemu/hw/9pfs/virtio-9p-handle.c:337:45: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > >qemu/hw/9pfs/virtio-9p-handle.c: In function ‘handle_chown’: > >qemu/hw/9pfs/virtio-9p-handle.c:373:58: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function) > > > > > >a patch was sent on the 24th of Feb to fix the issue (also see below): > > > >http://marc.info/?l=qemu-devel&m=133011313912147 > > > >Even though it is not particularly pretty, in the absence of better > >alternatives it should probably be applied. > > > >Cheers, > > > >Stefano > > > > > > > > > > > >On Fri, 24 Feb 2012, Serge Hallyn wrote: > >>If AT_EMPTY_PATH is not in one of the included files, go ahead and > >>define it. qemu won't compile on ubuntu for me without this. > >> > >>(Note - alternatively we could #include<linux/fcntl.h> to pick > >>up the definitions there) > >> > >>Signed-off-by: Serge Hallyn<serge.hallyn@canonical.com> > >>--- > >> hw/9pfs/virtio-9p-handle.c | 9 +++++++++ > >> 1 files changed, 9 insertions(+), 0 deletions(-) > >> > >>diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c > >>index f96d17a..e403a84 100644 > >>--- a/hw/9pfs/virtio-9p-handle.c > >>+++ b/hw/9pfs/virtio-9p-handle.c > >>@@ -39,6 +39,15 @@ > >> #ifndef BTRFS_SUPER_MAGIC > >> #define BTRFS_SUPER_MAGIC 0x9123683E > >> #endif > >>+#ifndef AT_REMOVEDIR > >>+#define AT_REMOVEDIR 0x200 > >>+#endif > >>+#ifndef AT_EMPTY_PATH > >>+#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ > >>+#endif > >>+#ifndef O_PATH > >>+#define O_PATH 010000000 > >>+#endif > >> > >> struct handle_data { > >> int mountfd; > >>-- > >>1.7.9 > > > The patch will fix the compiler error messages, but will the > resulting code work? Maybe it has runtime dependencies > (Linux kernel?) which should be checked at runtime. > > Would an enhanced test in configure be a better solution? > It could disable VirtFS automatically if the definitions > are missing. > > On Ubuntu Lenny, there is no definition for AT_EMPTY_PATH, (Debian Lenny I presume) True, the flag is newer than I thought. How could the configure option be enhanced? Could we simply grep for it in /usr/include/linux/fcntl.h, set an option saying it exists, then still do the #ifndef in hw/9pfs/virtio-9p-handle.c? (The problem is that the glibc and linux headers are currently not compatible... a separate problem, yes, but one I can't currently overcome AFAIK) > not even in linux/fcntl.h. > > Regards, > Stefan W. >
Quoting Aneesh Kumar K.V (aneesh.kumar@linux.vnet.ibm.com): > Serge Hallyn <serge.hallyn@canonical.com> writes: > > > If AT_EMPTY_PATH is not in one of the included files, go ahead and > > define it. qemu won't compile on ubuntu for me without this. > > > > (Note - alternatively we could #include <linux/fcntl.h> to pick > > up the definitions there) > > Then why don't we do that ? I do get the below errors > > In file included from /usr/include/x86_64-linux-gnu/asm/fcntl.h:1:0, > from /usr/include/linux/fcntl.h:4, > from /home/opensource/sources/qemu/qemu-upstream/hw/9pfs/virtio-9p-handle.c:28: > /usr/include/asm-generic/fcntl.h:127:8: error: redefinition of ‘struct f_owner_ex’ > /usr/include/x86_64-linux-gnu/bits/fcntl.h:204:8: note: originally defined here > In file included from /usr/include/x86_64-linux-gnu/asm/fcntl.h:1:0, > from /usr/include/linux/fcntl.h:4, > > Are they do to ubuntu multi-arch changes ? I don't think so. I got these awhile ago as well. The libc and linux headers simply don't seem to be compatible. > > > > Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> > > --- > > hw/9pfs/virtio-9p-handle.c | 9 +++++++++ > > 1 files changed, 9 insertions(+), 0 deletions(-) > > > > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c > > index f96d17a..e403a84 100644 > > --- a/hw/9pfs/virtio-9p-handle.c > > +++ b/hw/9pfs/virtio-9p-handle.c > > @@ -39,6 +39,15 @@ > > #ifndef BTRFS_SUPER_MAGIC > > #define BTRFS_SUPER_MAGIC 0x9123683E > > #endif > > +#ifndef AT_REMOVEDIR > > +#define AT_REMOVEDIR 0x200 > > +#endif > > +#ifndef AT_EMPTY_PATH > > +#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ > > +#endif > > +#ifndef O_PATH > > +#define O_PATH 010000000 > > +#endif > > > > struct handle_data { > > int mountfd; > > -- > > 1.7.9 > > -aneesh >
Am 06.06.2012 19:49, schrieb Serge Hallyn: > Quoting Stefan Weil (sw@weilnetz.de): >> The patch will fix the compiler error messages, but will the >> resulting code work? Maybe it has runtime dependencies >> (Linux kernel?) which should be checked at runtime. >> >> Would an enhanced test in configure be a better solution? >> It could disable VirtFS automatically if the definitions >> are missing. >> >> On Ubuntu Lenny, there is no definition for AT_EMPTY_PATH, > (Debian Lenny I presume) Sorry, my mistake. I meant Ubuntu Lucid. > True, the flag is newer than I thought. How could the configure option > be enhanced? Could we simply grep for it in /usr/include/linux/fcntl.h, > set an option saying it exists, then still do the #ifndef in > hw/9pfs/virtio-9p-handle.c? > > (The problem is that the glibc and linux headers are currently not > compatible... a separate problem, yes, but one I can't currently > overcome AFAIK) I'd use virtio-9p-handle only if AT_EMPTY_PATH is defined without requiring a hack like using linux/fcntl.h. configure can check that. Should I send a patch which does this check? Regards, Stefan W.
----- Original message ----- > Am 06.06.2012 19:49, schrieb Serge Hallyn: > > Quoting Stefan Weil (sw@weilnetz.de): > > > The patch will fix the compiler error messages, but will the > > > resulting code work? Maybe it has runtime dependencies > > > (Linux kernel?) which should be checked at runtime. > > > > > > Would an enhanced test in configure be a better solution? > > > It could disable VirtFS automatically if the definitions > > > are missing. > > > > > > On Ubuntu Lenny, there is no definition for AT_EMPTY_PATH, > > (Debian Lenny I presume) > > Sorry, my mistake. I meant Ubuntu Lucid. > > > True, the flag is newer than I thought. How could the configure option > > be enhanced? Could we simply grep for it in > > /usr/include/linux/fcntl.h, set an option saying it exists, then still > > do the #ifndef in hw/9pfs/virtio-9p-handle.c? > > > > (The problem is that the glibc and linux headers are currently not > > compatible... a separate problem, yes, but one I can't currently > > overcome AFAIK) > > I'd use virtio-9p-handle only if AT_EMPTY_PATH is defined without > requiring a hack like using linux/fcntl.h. > > configure can check that. Should I send a patch which does this check? That'd be great, please do - thanks! -Serge
Stefan Weil <sw@weilnetz.de> writes: > Am 06.06.2012 12:50, schrieb Stefano Stabellini: >> Hi Anthony, >> currently QEMU 1.1 doesn't compile virtfs correctly on Ubuntu: >> ..... > > The patch will fix the compiler error messages, but will the > resulting code work? Maybe it has runtime dependencies > (Linux kernel?) which should be checked at runtime. > > Would an enhanced test in configure be a better solution? > It could disable VirtFS automatically if the definitions > are missing. > > On Ubuntu Lenny, there is no definition for AT_EMPTY_PATH, > not even in linux/fcntl.h. We enable virtio-9p-handle only if open_by_handle_at is supported by glibc. And if open_by_handle_at is supported by glibc AT_EMPTY_PATH should also be there, because they all went in linux kernel for the same feature set. So if glibc didn't pick AT_EMPTY_PATH definition and took open_by_handle_at feature, it should be fixed in glibc. BTW I am still not clear why including linux/fcntl.h gives those compile errors. IMHO fixing those errors and including linux/fcntl.h in virtio-9p-handle.c would be a better fix than redefining these constants. -aneesh
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c index f96d17a..e403a84 100644 --- a/hw/9pfs/virtio-9p-handle.c +++ b/hw/9pfs/virtio-9p-handle.c @@ -39,6 +39,15 @@ #ifndef BTRFS_SUPER_MAGIC #define BTRFS_SUPER_MAGIC 0x9123683E #endif +#ifndef AT_REMOVEDIR +#define AT_REMOVEDIR 0x200 +#endif +#ifndef AT_EMPTY_PATH +#define AT_EMPTY_PATH 0x1000 /* Allow empty relative pathname */ +#endif +#ifndef O_PATH +#define O_PATH 010000000 +#endif struct handle_data { int mountfd;
If AT_EMPTY_PATH is not in one of the included files, go ahead and define it. qemu won't compile on ubuntu for me without this. (Note - alternatively we could #include <linux/fcntl.h> to pick up the definitions there) Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> --- hw/9pfs/virtio-9p-handle.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)