Patchwork virtio-9p: fix QEMU build break

login
register
mail settings
Submitter Aneesh Kumar K.V
Date Oct. 10, 2011, 4:49 p.m.
Message ID <87obxon65g.fsf@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/118789/
State New
Headers show

Comments

Aneesh Kumar K.V - Oct. 10, 2011, 4:49 p.m.
On Mon, 10 Oct 2011 22:05:21 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Mon, 10 Oct 2011 18:30:28 +0800, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
> > qemu build break due to the redefinition of struct file_handle. My qemu.git/HEAD is 8acbc9b21d757a6be4f8492e547b8159703a0547
> > 
> > Below is the log:
> > [root@f15 qemu]# make
> >   CC    qapi-generated/qga-qapi-types.o
> >   LINK  qemu-ga
> >   CC    libhw64/9pfs/virtio-9p-handle.o
> > /home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:31:8: error: redefinition of "struct file_handle"
> > /usr/include/bits/fcntl.h:254:8: note: originally defined here
> > make[1]: *** [9pfs/virtio-9p-handle.o] Error 1
> > make: *** [subdir-libhw64] Error 2
> > 
> > [root@f15 qemu]# rpm -qf /usr/include/bits/fcntl.h
> > glibc-headers-2.13.90-9.x86_64
> > 
> 
> Is this a backported glibc ? On my ubuntu system glibc 2.13 doesn't
> provide struct file_handle. I also checked glib repo at
> http://repo.or.cz/w/glibc.git. The commit introducing struct file_handle
> is 
> 
> $ git describe --contains 158648c0bdda281e252a27c0200dd0ea6f4e0215
> glibc-2.14~200
> 
> 

How about the below patch. This means that handle driver will only work
with latest glibc. Even if i have latest kernel, with an older glibc
handle fs driver backed will be disabled.
Zhiyong Wu - Oct. 11, 2011, 6:09 a.m.
On Tue, Oct 11, 2011 at 12:49 AM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Mon, 10 Oct 2011 22:05:21 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> On Mon, 10 Oct 2011 18:30:28 +0800, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
>> > qemu build break due to the redefinition of struct file_handle. My qemu.git/HEAD is 8acbc9b21d757a6be4f8492e547b8159703a0547
>> >
>> > Below is the log:
>> > [root@f15 qemu]# make
>> >   CC    qapi-generated/qga-qapi-types.o
>> >   LINK  qemu-ga
>> >   CC    libhw64/9pfs/virtio-9p-handle.o
>> > /home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:31:8: error: redefinition of "struct file_handle"
>> > /usr/include/bits/fcntl.h:254:8: note: originally defined here
>> > make[1]: *** [9pfs/virtio-9p-handle.o] Error 1
>> > make: *** [subdir-libhw64] Error 2
>> >
>> > [root@f15 qemu]# rpm -qf /usr/include/bits/fcntl.h
>> > glibc-headers-2.13.90-9.x86_64
>> >
>>
>> Is this a backported glibc ? On my ubuntu system glibc 2.13 doesn't
>> provide struct file_handle. I also checked glib repo at
>> http://repo.or.cz/w/glibc.git. The commit introducing struct file_handle
>> is
>>
>> $ git describe --contains 158648c0bdda281e252a27c0200dd0ea6f4e0215
>> glibc-2.14~200
>>
>>
>
> How about the below patch. This means that handle driver will only work
> with latest glibc. Even if i have latest kernel, with an older glibc
> handle fs driver backed will be disabled.
>
> diff --git a/configure b/configure
> index 24b8df4..0216c53 100755
> --- a/configure
> +++ b/configure
> @@ -2551,6 +2551,18 @@ EOF
>  fi
>
>  ##########################################
> +# check if we have open_by_handle_at
> +
> +open_by_hande_at=no
> +cat > $TMPC << EOF
> +#include <fcntl.h>
> +int main(void) { struct file_handle *fh; open_by_handle_at(0, fh, 0); }
> +EOF
> +if compile_prog "" "" ; then
> +    open_by_handle_at=yes
> +fi
> +
> +##########################################
>  # End of CC checks
>  # After here, no more $cc or $ld runs
>
> @@ -3029,6 +3041,10 @@ if test "$ucontext_coroutine" = "yes" ; then
>   echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
>  fi
>
> +if test "$open_by_handle_at" = "yes" ; then
> +  echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
> +fi
> +
>  # USB host support
>  case "$usb" in
>  linux)
> diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
> index 68e1d9b..bd73d31 100644
> --- a/hw/9pfs/virtio-9p-handle.c
> +++ b/hw/9pfs/virtio-9p-handle.c
> @@ -30,13 +30,24 @@ struct handle_data {
>     int handle_bytes;
>  };
>
> -#if __GLIBC__ <= 2 && __GLIBC_MINOR__ < 14
> +#ifdef CONFIG_OPEN_BY_HANDLE
> +static inline int name_to_handle(int dirfd, const char *name,
> +                                 struct file_handle *fh, int *mnt_id, int flags)
> +{
> +    return name_to_handle_at(dirfd, name, fh, mnt_id, flags);
> +}
> +
> +static inline int open_by_handle(int mountfd, const char *fh, int flags)
> +{
> +    return open_by_handle_at(mountfd, fh, flags);
> +}
> +#else
> +
>  struct file_handle {
> -        unsigned int handle_bytes;
> -        int handle_type;
> -        unsigned char handle[0];
> +    unsigned int handle_bytes;
> +    int handle_type;
> +    unsigned char handle[0];
>  };
> -#endif
>
>  #ifndef AT_EMPTY_PATH
>  #define AT_EMPTY_PATH   0x1000  /* Allow empty relative pathname */
> @@ -45,28 +56,6 @@ struct file_handle {
>  #define O_PATH    010000000
>  #endif
>
> -#ifndef __NR_name_to_handle_at
> -#if defined(__i386__)
> -#define __NR_name_to_handle_at  341
> -#define __NR_open_by_handle_at  342
> -#elif defined(__x86_64__)
> -#define __NR_name_to_handle_at  303
> -#define __NR_open_by_handle_at  304
> -#endif
> -#endif
> -
> -#ifdef __NR_name_to_handle_at
> -static inline int name_to_handle(int dirfd, const char *name,
> -                                 struct file_handle *fh, int *mnt_id, int flags)
> -{
> -    return syscall(__NR_name_to_handle_at, dirfd, name, fh, mnt_id, flags);
> -}
> -
> -static inline int open_by_handle(int mountfd, const char *fh, int flags)
> -{
> -    return syscall(__NR_open_by_handle_at, mountfd, fh, flags);
> -}
> -#else
>  static inline int name_to_handle(int dirfd, const char *name,
>                                  struct file_handle *fh, int *mnt_id, int flags)
>  {
>
It doesn't work for me.

  CC    libhw64/9pfs/virtio-9p-handle.o
/home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c: In function
\u2018open_by_handle\u2019:
/home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:39:5: error:
passing argument 2 of \u2018open_by_handle_at\u2019 from incompatible
pointer type [-Werror]
/usr/include/bits/fcntl.h:335:12: note: expected \u2018struct
file_handle *\u2019 but argument is of type \u2018const char *\u2019
/home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c: At top level:
/home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:30:0: error:
unterminated #else
cc1: all warnings being treated as errors

make[1]: *** [9pfs/virtio-9p-handle.o] Error 1
make: *** [subdir-libhw64] Error 2
David Gibson - Oct. 18, 2011, 3:46 a.m.
On Mon, Oct 10, 2011 at 10:19:31PM +0530, Aneesh Kumar K.V wrote:
> On Mon, 10 Oct 2011 22:05:21 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > On Mon, 10 Oct 2011 18:30:28 +0800, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
> > > qemu build break due to the redefinition of struct file_handle. My qemu.git/HEAD is 8acbc9b21d757a6be4f8492e547b8159703a0547
> > > 
> > > Below is the log:
> > > [root@f15 qemu]# make
> > >   CC    qapi-generated/qga-qapi-types.o
> > >   LINK  qemu-ga
> > >   CC    libhw64/9pfs/virtio-9p-handle.o
> > > /home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:31:8: error: redefinition of "struct file_handle"
> > > /usr/include/bits/fcntl.h:254:8: note: originally defined here
> > > make[1]: *** [9pfs/virtio-9p-handle.o] Error 1
> > > make: *** [subdir-libhw64] Error 2
> > > 
> > > [root@f15 qemu]# rpm -qf /usr/include/bits/fcntl.h
> > > glibc-headers-2.13.90-9.x86_64
> > > 
> > 
> > Is this a backported glibc ? On my ubuntu system glibc 2.13 doesn't
> > provide struct file_handle. I also checked glib repo at
> > http://repo.or.cz/w/glibc.git. The commit introducing struct file_handle
> > is 
> > 
> > $ git describe --contains 158648c0bdda281e252a27c0200dd0ea6f4e0215
> > glibc-2.14~200
> > 
> > 
> 
> How about the below patch. This means that handle driver will only work
> with latest glibc. Even if i have latest kernel, with an older glibc
> handle fs driver backed will be disabled.

This looks like the right approach to me, but at least with my
compiler / libc combinations, the test program isn't quite right:
because the file_handle pointer is never dereferenced, gcc doesn't
complain even if it is undefined.


> diff --git a/configure b/configure
> index 24b8df4..0216c53 100755
> --- a/configure
> +++ b/configure
> @@ -2551,6 +2551,18 @@ EOF
>  fi
>  
>  ##########################################
> +# check if we have open_by_handle_at
> +
> +open_by_hande_at=no
> +cat > $TMPC << EOF
> +#include <fcntl.h>
> +int main(void) { struct file_handle *fh; open_by_handle_at(0, fh, 0); }

Instead, try this:

int main(void) { struct file_handle fh; open_by_handle_at(0, &fh, 0); }


I'd really like to see a patch along these lines merged, so I don't
have to keep hacking it up manually just to get qemu to compile.
Aneesh Kumar K.V - Oct. 18, 2011, 4:59 a.m.
On Tue, 18 Oct 2011 14:46:25 +1100, David Gibson <dwg@au1.ibm.com> wrote:
> On Mon, Oct 10, 2011 at 10:19:31PM +0530, Aneesh Kumar K.V wrote:
> > On Mon, 10 Oct 2011 22:05:21 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > > On Mon, 10 Oct 2011 18:30:28 +0800, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
> > > > qemu build break due to the redefinition of struct file_handle. My qemu.git/HEAD is 8acbc9b21d757a6be4f8492e547b8159703a0547
> > > > 
> > > > Below is the log:
> > > > [root@f15 qemu]# make
> > > >   CC    qapi-generated/qga-qapi-types.o
> > > >   LINK  qemu-ga
> > > >   CC    libhw64/9pfs/virtio-9p-handle.o
> > > > /home/zwu/work/virt/qemu/hw/9pfs/virtio-9p-handle.c:31:8: error: redefinition of "struct file_handle"
> > > > /usr/include/bits/fcntl.h:254:8: note: originally defined here
> > > > make[1]: *** [9pfs/virtio-9p-handle.o] Error 1
> > > > make: *** [subdir-libhw64] Error 2
> > > > 
> > > > [root@f15 qemu]# rpm -qf /usr/include/bits/fcntl.h
> > > > glibc-headers-2.13.90-9.x86_64
> > > > 
> > > 
> > > Is this a backported glibc ? On my ubuntu system glibc 2.13 doesn't
> > > provide struct file_handle. I also checked glib repo at
> > > http://repo.or.cz/w/glibc.git. The commit introducing struct file_handle
> > > is 
> > > 
> > > $ git describe --contains 158648c0bdda281e252a27c0200dd0ea6f4e0215
> > > glibc-2.14~200
> > > 
> > > 
> > 
> > How about the below patch. This means that handle driver will only work
> > with latest glibc. Even if i have latest kernel, with an older glibc
> > handle fs driver backed will be disabled.
> 
> This looks like the right approach to me, but at least with my
> compiler / libc combinations, the test program isn't quite right:
> because the file_handle pointer is never dereferenced, gcc doesn't
> complain even if it is undefined.
> 
> 
> > diff --git a/configure b/configure
> > index 24b8df4..0216c53 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2551,6 +2551,18 @@ EOF
> >  fi
> >  
> >  ##########################################
> > +# check if we have open_by_handle_at
> > +
> > +open_by_hande_at=no
> > +cat > $TMPC << EOF
> > +#include <fcntl.h>
> > +int main(void) { struct file_handle *fh; open_by_handle_at(0, fh, 0); }
> 
> Instead, try this:
> 
> int main(void) { struct file_handle fh; open_by_handle_at(0, &fh, 0); }
> 
> 
> I'd really like to see a patch along these lines merged, so I don't
> have to keep hacking it up manually just to get qemu to compile.

I did send a pull request with the patch 

http://article.gmane.org/gmane.comp.emulators.qemu/120990

I will do a patch on top which will change the configure to what you
suggested above.

-aneesh

Patch

diff --git a/configure b/configure
index 24b8df4..0216c53 100755
--- a/configure
+++ b/configure
@@ -2551,6 +2551,18 @@  EOF
 fi
 
 ##########################################
+# check if we have open_by_handle_at
+
+open_by_hande_at=no
+cat > $TMPC << EOF
+#include <fcntl.h>
+int main(void) { struct file_handle *fh; open_by_handle_at(0, fh, 0); }
+EOF
+if compile_prog "" "" ; then
+    open_by_handle_at=yes
+fi
+
+##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -3029,6 +3041,10 @@  if test "$ucontext_coroutine" = "yes" ; then
   echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
 fi
 
+if test "$open_by_handle_at" = "yes" ; then
+  echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index 68e1d9b..bd73d31 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -30,13 +30,24 @@  struct handle_data {
     int handle_bytes;
 };
 
-#if __GLIBC__ <= 2 && __GLIBC_MINOR__ < 14
+#ifdef CONFIG_OPEN_BY_HANDLE
+static inline int name_to_handle(int dirfd, const char *name,
+                                 struct file_handle *fh, int *mnt_id, int flags)
+{
+    return name_to_handle_at(dirfd, name, fh, mnt_id, flags);
+}
+
+static inline int open_by_handle(int mountfd, const char *fh, int flags)
+{
+    return open_by_handle_at(mountfd, fh, flags);
+}
+#else
+
 struct file_handle {
-        unsigned int handle_bytes;
-        int handle_type;
-        unsigned char handle[0];
+    unsigned int handle_bytes;
+    int handle_type;
+    unsigned char handle[0];
 };
-#endif
 
 #ifndef AT_EMPTY_PATH
 #define AT_EMPTY_PATH   0x1000  /* Allow empty relative pathname */
@@ -45,28 +56,6 @@  struct file_handle {
 #define O_PATH    010000000
 #endif
 
-#ifndef __NR_name_to_handle_at
-#if defined(__i386__)
-#define __NR_name_to_handle_at  341
-#define __NR_open_by_handle_at  342
-#elif defined(__x86_64__)
-#define __NR_name_to_handle_at  303
-#define __NR_open_by_handle_at  304
-#endif
-#endif
-
-#ifdef __NR_name_to_handle_at
-static inline int name_to_handle(int dirfd, const char *name,
-                                 struct file_handle *fh, int *mnt_id, int flags)
-{
-    return syscall(__NR_name_to_handle_at, dirfd, name, fh, mnt_id, flags);
-}
-
-static inline int open_by_handle(int mountfd, const char *fh, int flags)
-{
-    return syscall(__NR_open_by_handle_at, mountfd, fh, flags);
-}
-#else
 static inline int name_to_handle(int dirfd, const char *name,
                                  struct file_handle *fh, int *mnt_id, int flags)
 {