| Message ID | 87mxdl2h0c.fsf@skywalker.in.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 01, 2011 at 03:03:23PM +0530, Aneesh Kumar K.V wrote: > +#ifndef CONFIG_UTIMENSAT > + /* > + * We support handle fs driver only if all related > + * syscalls are provided by host. > + */ Perhaps a ./configure check should be added to see whether the handle syscalls are supported instead of using CONFIG_UTIMENSAT. Stefan
On Mon, 3 Oct 2011 08:37:52 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Sat, Oct 01, 2011 at 03:03:23PM +0530, Aneesh Kumar K.V wrote: > > +#ifndef CONFIG_UTIMENSAT > > + /* > > + * We support handle fs driver only if all related > > + * syscalls are provided by host. > > + */ > > Perhaps a ./configure check should be added to see whether the handle > syscalls are supported instead of using CONFIG_UTIMENSAT. > We already do check for handle syscall. Since glibc doesn't have the this syscall yet, I added the check in virtio-9p-handle.c as below #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); } #else static inline int name_to_handle(int dirfd, const char *name, struct file_handle *fh, int *mnt_id, int flags) { errno = ENOSYS; return -1; } -aneesh
On Mon, Oct 03, 2011 at 04:10:50PM +0530, Aneesh Kumar K.V wrote: > On Mon, 3 Oct 2011 08:37:52 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Sat, Oct 01, 2011 at 03:03:23PM +0530, Aneesh Kumar K.V wrote: > > > +#ifndef CONFIG_UTIMENSAT > > > + /* > > > + * We support handle fs driver only if all related > > > + * syscalls are provided by host. > > > + */ > > > > Perhaps a ./configure check should be added to see whether the handle > > syscalls are supported instead of using CONFIG_UTIMENSAT. > > > > We already do check for handle syscall. Since glibc doesn't have the > this syscall yet, I added the check in virtio-9p-handle.c as below CONFIG_UTIMENSAT is defined when the host has glibc >= 2.6. Handle syscalls are available on Linux 2.6.39 but not exposed by glibc. Therefore CONFIG_UTIMENSAT has nothing to do with handle syscalls and does not mean handle syscalls are supported. I would drop that hunk of the patch or test for the actual handle syscalls in ./configure. Stefan
On Tue, 4 Oct 2011 08:18:07 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Oct 03, 2011 at 04:10:50PM +0530, Aneesh Kumar K.V wrote: > > On Mon, 3 Oct 2011 08:37:52 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > > On Sat, Oct 01, 2011 at 03:03:23PM +0530, Aneesh Kumar K.V wrote: > > > > +#ifndef CONFIG_UTIMENSAT > > > > + /* > > > > + * We support handle fs driver only if all related > > > > + * syscalls are provided by host. > > > > + */ > > > > > > Perhaps a ./configure check should be added to see whether the handle > > > syscalls are supported instead of using CONFIG_UTIMENSAT. > > > > > > > We already do check for handle syscall. Since glibc doesn't have the > > this syscall yet, I added the check in virtio-9p-handle.c as below > > CONFIG_UTIMENSAT is defined when the host has glibc >= 2.6. > > Handle syscalls are available on Linux 2.6.39 but not exposed by glibc. > > Therefore CONFIG_UTIMENSAT has nothing to do with handle syscalls and > does not mean handle syscalls are supported. I would drop that hunk of > the patch or test for the actual handle syscalls in ./configure. Here is what i am trying to achieve with the patch. For handle based fs driver to work we need to have 1) support for handle syscall 2) support for fd based syscalls like futimens, fstatat, readlinkat, fchmod, fchownat, openat etc. Now handle syscalls are recently added to kernel and glibc doesn't have support for that. So what we did is to add handle syscall in virtio-9p-handle.c via syscall(2). Now if the syscall is not supported by the host kernel we will get ENOSYS. I only added support for i386 and x86_64, because most the syscall number varies with different archs. For other archs the wrapper returns ENOSYS. So instead of checking for handle syscalls in configure script we did the above to make sure it work without failure in most of the case. Once we have glibc support for handle syscall the above changes should be dropped in favor of configure script test. Now for the fd based syscall dependency, we didn't initially had any check for that because my expectation was most glibc should have support for them. But RHEL 5 build failure indicate that futimens is not supported. We were already checking for futimens in configure so i added changes to make sure if futimens is not supported handle_utimensat returns error. (That was not added as a run time check, but rather a compile error fix). Now should we allow handle based fs driver if futimens is not supported. I was suggesting we should not; hence the check in init to return error when we don't support futimens. The later part of init routine do check whether handle syscalls are supported and disable handle fs driver if they are not. -aneesh
On Tue, Oct 04, 2011 at 02:18:20PM +0530, Aneesh Kumar K.V wrote: > On Tue, 4 Oct 2011 08:18:07 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Mon, Oct 03, 2011 at 04:10:50PM +0530, Aneesh Kumar K.V wrote: > > > On Mon, 3 Oct 2011 08:37:52 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > > > On Sat, Oct 01, 2011 at 03:03:23PM +0530, Aneesh Kumar K.V wrote: > > > > > +#ifndef CONFIG_UTIMENSAT > > > > > + /* > > > > > + * We support handle fs driver only if all related > > > > > + * syscalls are provided by host. > > > > > + */ > > > > > > > > Perhaps a ./configure check should be added to see whether the handle > > > > syscalls are supported instead of using CONFIG_UTIMENSAT. > > > > > > > > > > We already do check for handle syscall. Since glibc doesn't have the > > > this syscall yet, I added the check in virtio-9p-handle.c as below > > > > CONFIG_UTIMENSAT is defined when the host has glibc >= 2.6. > > > > Handle syscalls are available on Linux 2.6.39 but not exposed by glibc. > > > > Therefore CONFIG_UTIMENSAT has nothing to do with handle syscalls and > > does not mean handle syscalls are supported. I would drop that hunk of > > the patch or test for the actual handle syscalls in ./configure. > > Here is what i am trying to achieve with the patch. For handle based fs > driver to work we need to have > > 1) support for handle syscall > 2) support for fd based syscalls like futimens, fstatat, readlinkat, > fchmod, fchownat, openat etc. > > Now handle syscalls are recently added to kernel and glibc doesn't have > support for that. So what we did is to add handle syscall in > virtio-9p-handle.c via syscall(2). Now if the syscall is not supported > by the host kernel we will get ENOSYS. I only added support for i386 and > x86_64, because most the syscall number varies with different archs. For > other archs the wrapper returns ENOSYS. So instead of checking for > handle syscalls in configure script we did the above to make sure it > work without failure in most of the case. Once we have glibc support for > handle syscall the above changes should be dropped in favor of > configure script test. > > Now for the fd based syscall dependency, we didn't initially had any > check for that because my expectation was most glibc should > have support for them. But RHEL 5 build failure indicate that futimens > is not supported. We were already checking for futimens in configure so > i added changes to make sure if futimens is not supported > handle_utimensat returns error. (That was not added as a run time > check, but rather a compile error fix). Now should we allow handle based fs > driver if futimens is not supported. I was suggesting we should not; > hence the check in init to return error when we don't support > futimens. The later part of init routine do check whether handle > syscalls are supported and disable handle fs driver if they are not. Okay, then the comment should be: /* * We support handle fs driver only if futimens is provided by the host */ The scenario where it might be possible to hit the CONFIG_UTIMENSAT is with a modern kernel paired with an old userspace. The handle syscall which we call directly would succeed but the futimens(2) would not be available. On a sane system the handle syscall fails because the kernel doesn't support it (and futimens isn't there either). Stefan
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c index 860b0e3..9e9ceb3 100644 --- a/hw/9pfs/virtio-9p-handle.c +++ b/hw/9pfs/virtio-9p-handle.c @@ -386,12 +386,17 @@ static int handle_utimensat(FsContext *ctx, V9fsPath *fs_path, int fd, ret; struct handle_data *data = (struct handle_data *)ctx->private; +#ifdef CONFIG_UTIMENSAT fd = open_by_handle(data->mountfd, fs_path->data, O_NONBLOCK); if (fd < 0) { return fd; } ret = futimens(fd, buf); close(fd); +#else + ret = -1; + errno = ENOSYS; +#endif return ret; } @@ -591,8 +596,16 @@ static int handle_init(FsContext *ctx) int ret, mnt_id; struct statfs stbuf; struct file_handle fh; - struct handle_data *data = g_malloc(sizeof(struct handle_data)); + struct handle_data *data; +#ifndef CONFIG_UTIMENSAT + /* + * We support handle fs driver only if all related + * syscalls are provided by host. + */ + return -1; +#endif + data = g_malloc(sizeof(struct handle_data)); data->mountfd = open(ctx->fs_root, O_DIRECTORY); if (data->mountfd < 0) { ret = data->mountfd;