-- >8 --
Greg,

Usually, I use 9p as rootfs and run qemu in one of these way. First,
using initrd with built-in 9p modules:

$ sudo /path/to/qemu-system-x86_64 \
    -machine accel=kvm -m 1G -nographic -vga none -serial mon:stdio \
    -kernel $PWD/rootfs/boot/vmlinuz-3.16.0-4-amd64 \
    -initrd $PWD/rootfs/boot/initrd.img-3.16.0-4-amd64 \
    -append "root=rootfs rw rootfstype=9p rootflags=trans=virtio,version=9p2000.u console=ttyS0" \
    -fsdev local,id=fsdev0,path=$PWD/rootfs,security_model=passthrough \
    -device virtio-9p-pci,fsdev=fsdev0,mount_tag=rootfs

Second, with compiled into the kernel 9p support and without initrd (but
this way quite tricky):

$ sudo /path/to/qemu-system-x86_64 \
    -machine accel=kvm -m 1G -nographic -vga none -serial mon:stdio \
    -kernel /path/to/bzImage \
    -append "root=/dev/root rw rootfstype=9p rootflags=trans=virtio,version=9p2000.u console=ttyS0" \
    -fsdev local,id=fsdev0,path=$PWD/rootfs,security_model=passthrough \
    -device virtio-9p-pci,fsdev=fsdev0,mount_tag=/dev/root

This patch mostly uses your commit message, because it is much more
informative than I could ever suggest.

--
Best regards
Jan Dakinevich
-- >8 --
If the client is using 9p2000.u, the following occurs:

$ cd ${virtfs_shared_dir}
$ mkdir -p a/b/c
$ ls a/b
ls: cannot access 'a/b/a': No such file or directory
ls: cannot access 'a/b/b': No such file or directory
a  b  c

instead of the expected:

$ ls a/b
c

This is a regression introduced by commit f57f5878578a;
local_name_to_path() now resolves ".." and "." in paths,
and v9fs_do_readdir_with_stat()->stat_to_v9stat() then
copies the basename of the resulting path to the response.
With the example above, this means that "." and ".." are
turned into "b" and "a" respectively...

Actually, the name we need to pass is the d_name field of
the dirent. Meanwhile, name argument of stat_to_v9stat is
preserved, since it used to symbolic links resolving.

To satisfy stat_to_v9stat(), v9fs_stat() takes a care of the
logic from old stat_to_v9stat() for determining basename of
the given path.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
---
 hw/9pfs/9p.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 4d4ed85..6cb9dfc 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -804,11 +804,11 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf)
 }
 
 static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
+                                       const char *basename,
                                        const struct stat *stbuf,
                                        V9fsStat *v9stat)
 {
     int err;
-    const char *str;
 
     memset(v9stat, 0, sizeof(*v9stat));
 
@@ -842,14 +842,7 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
                 "HARDLINKCOUNT", (unsigned long)stbuf->st_nlink);
     }
 
-    str = strrchr(name->data, '/');
-    if (str) {
-        str += 1;
-    } else {
-        str = name->data;
-    }
-
-    v9fs_string_sprintf(&v9stat->name, "%s", str);
+    v9fs_string_sprintf(&v9stat->name, "%s", basename);
 
     v9stat->size = 61 +
         v9fs_string_size(&v9stat->name) +
@@ -1058,6 +1051,7 @@ static void coroutine_fn v9fs_stat(void *opaque)
     struct stat stbuf;
     V9fsFidState *fidp;
     V9fsPDU *pdu = opaque;
+    char *basename;
 
     err = pdu_unmarshal(pdu, offset, "d", &fid);
     if (err < 0) {
@@ -1074,7 +1068,9 @@ static void coroutine_fn v9fs_stat(void *opaque)
     if (err < 0) {
         goto out;
     }
-    err = stat_to_v9stat(pdu, &fidp->path, &stbuf, &v9stat);
+    basename = g_path_get_basename(fidp->path.data);
+    err = stat_to_v9stat(pdu, &fidp->path, basename, &stbuf, &v9stat);
+    g_free(basename);
     if (err < 0) {
         goto out;
     }
@@ -1750,7 +1746,7 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
         if (err < 0) {
             break;
         }
-        err = stat_to_v9stat(pdu, &path, &stbuf, &v9stat);
+        err = stat_to_v9stat(pdu, &path, dent->d_name, &stbuf, &v9stat);
         if (err < 0) {
             break;
         }
-- 
2.10.1

On Tue, 19 Sep 2017 16:28:58 +0200
Jan Dakinevich <jan.dakinevich@gmail.com> wrote:

> -- >8 --  
> Greg,
> 
> Usually, I use 9p as rootfs and run qemu in one of these way. First,
> using initrd with built-in 9p modules:
> 
> $ sudo /path/to/qemu-system-x86_64 \
>     -machine accel=kvm -m 1G -nographic -vga none -serial mon:stdio \
>     -kernel $PWD/rootfs/boot/vmlinuz-3.16.0-4-amd64 \
>     -initrd $PWD/rootfs/boot/initrd.img-3.16.0-4-amd64 \
>     -append "root=rootfs rw rootfstype=9p rootflags=trans=virtio,version=9p2000.u console=ttyS0" \
>     -fsdev local,id=fsdev0,path=$PWD/rootfs,security_model=passthrough \
>     -device virtio-9p-pci,fsdev=fsdev0,mount_tag=rootfs
> 
> Second, with compiled into the kernel 9p support and without initrd (but
> this way quite tricky):
> 
> $ sudo /path/to/qemu-system-x86_64 \
>     -machine accel=kvm -m 1G -nographic -vga none -serial mon:stdio \
>     -kernel /path/to/bzImage \
>     -append "root=/dev/root rw rootfstype=9p rootflags=trans=virtio,version=9p2000.u console=ttyS0" \
>     -fsdev local,id=fsdev0,path=$PWD/rootfs,security_model=passthrough \
>     -device virtio-9p-pci,fsdev=fsdev0,mount_tag=/dev/root
> 

Thanks. I'll give a try.

> This patch mostly uses your commit message, because it is much more
> informative than I could ever suggest.
> 

No problem.

> --
> Best regards
> Jan Dakinevich
> -- >8 -- 

BTW, 'git am' takes all the mail body down to the '---' to build the commit
message, and assumes the patch starts at the first 'diff -'. Anything in
between is silently dropped.

This means that the commit for this patch had to be manually edited to
remove the '-- 8< --' ... '-- >8 --' part.

When you have something to add in the mail that shouldn't go into the
commit message, just put it between the '---' and the patch and it
will be ignored by 'git am' as explained above.

> If the client is using 9p2000.u, the following occurs:
> 
> $ cd ${virtfs_shared_dir}
> $ mkdir -p a/b/c
> $ ls a/b
> ls: cannot access 'a/b/a': No such file or directory
> ls: cannot access 'a/b/b': No such file or directory
> a  b  c
> 
> instead of the expected:
> 
> $ ls a/b
> c
> 
> This is a regression introduced by commit f57f5878578a;
> local_name_to_path() now resolves ".." and "." in paths,
> and v9fs_do_readdir_with_stat()->stat_to_v9stat() then
> copies the basename of the resulting path to the response.
> With the example above, this means that "." and ".." are
> turned into "b" and "a" respectively...
> 
> Actually, the name we need to pass is the d_name field of
> the dirent. Meanwhile, name argument of stat_to_v9stat is
> preserved, since it used to symbolic links resolving.
> 
> To satisfy stat_to_v9stat(), v9fs_stat() takes a care of the
> logic from old stat_to_v9stat() for determining basename of
> the given path.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Even if we agreed on the list that this was the way to go, I'm not
the author, so it shouldn't contain my Signed-off-by yet... It will
have to when I repost it for the pull request though

Anyway, thanks for finding the issue and taking time to fix it.

I hope to merge this and the other patches soon.

Cheers,

--
Greg

> Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
> ---
>  hw/9pfs/9p.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 4d4ed85..6cb9dfc 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -804,11 +804,11 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf)
>  }
>  
>  static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
> +                                       const char *basename,
>                                         const struct stat *stbuf,
>                                         V9fsStat *v9stat)
>  {
>      int err;
> -    const char *str;
>  
>      memset(v9stat, 0, sizeof(*v9stat));
>  
> @@ -842,14 +842,7 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
>                  "HARDLINKCOUNT", (unsigned long)stbuf->st_nlink);
>      }
>  
> -    str = strrchr(name->data, '/');
> -    if (str) {
> -        str += 1;
> -    } else {
> -        str = name->data;
> -    }
> -
> -    v9fs_string_sprintf(&v9stat->name, "%s", str);
> +    v9fs_string_sprintf(&v9stat->name, "%s", basename);
>  
>      v9stat->size = 61 +
>          v9fs_string_size(&v9stat->name) +
> @@ -1058,6 +1051,7 @@ static void coroutine_fn v9fs_stat(void *opaque)
>      struct stat stbuf;
>      V9fsFidState *fidp;
>      V9fsPDU *pdu = opaque;
> +    char *basename;
>  
>      err = pdu_unmarshal(pdu, offset, "d", &fid);
>      if (err < 0) {
> @@ -1074,7 +1068,9 @@ static void coroutine_fn v9fs_stat(void *opaque)
>      if (err < 0) {
>          goto out;
>      }
> -    err = stat_to_v9stat(pdu, &fidp->path, &stbuf, &v9stat);
> +    basename = g_path_get_basename(fidp->path.data);
> +    err = stat_to_v9stat(pdu, &fidp->path, basename, &stbuf, &v9stat);
> +    g_free(basename);
>      if (err < 0) {
>          goto out;
>      }
> @@ -1750,7 +1746,7 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          if (err < 0) {
>              break;
>          }
> -        err = stat_to_v9stat(pdu, &path, &stbuf, &v9stat);
> +        err = stat_to_v9stat(pdu, &path, dent->d_name, &stbuf, &v9stat);
>          if (err < 0) {
>              break;
>          }



-- 
Gregory Kurz                                     kurzgreg@fr.ibm.com
                                                 gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/LTC                      http://www.ibm.com
Tel 33-5-6218-1607

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.