[Qemu-devel] [PATCH] virtio-9p-handle: define AT_EMPTY_PATH if needed

[Qemu-devel] [PATCH] virtio-9p-handle: define AT_EMPTY_PATH if needed

[Serge Hallyn]

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

[Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Stefano Stabellini]

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

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

Re: [Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Stefan Weil]

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.

Re: [Qemu-devel] [PATCH] virtio-9p-handle: define AT_EMPTY_PATH if needed

[Aneesh Kumar K.V]

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

Re: [Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Serge Hallyn]

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

Re: [Qemu-devel] [PATCH] virtio-9p-handle: define AT_EMPTY_PATH if needed

[Serge Hallyn]

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
> 

Re: [Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Stefan Weil]

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.

Re: [Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Serge Hallyn]

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

Re: [Qemu-devel] [BUG QEMU 1.1] virtio-9p-handle does not compile

[Aneesh Kumar K.V]

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