| Message ID | 1308419348-31934-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote: > Use lstat instead of stat This patch should probably come after 9p2000.u support is added, since now we transparently work 'through' symlinks (because legacy 9p2000 doesn't support them). Once 9p2000.u support is added we could represent symlinks and then a switch to lstat might be due. > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > tools/kvm/virtio/9p.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c > index 209760b..3172c1a 100644 > --- a/tools/kvm/virtio/9p.c > +++ b/tools/kvm/virtio/9p.c > @@ -227,7 +227,7 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg, > struct p9_fid *new_fid = &p9dev->fids[topen->fid]; > struct stat st; > > - if (stat(new_fid->abs_path, &st) < 0) > + if (lstat(new_fid->abs_path, &st) < 0) > return false; > > st2qid(&st, &ropen->qid); > @@ -274,7 +274,7 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg, > fid->fd = open(fid->abs_path, omode2uflags(mode) | O_CREAT, 0777); > } > > - if (stat(fid->abs_path, &st) < 0) > + if (lstat(fid->abs_path, &st) < 0) > return false; > > st2qid(&st, &rcreate->qid); > @@ -308,7 +308,7 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg, > /* Format the new path we're 'walk'ing into */ > sprintf(tmp, "%s/%.*s", fid->path, str->len, (char *)&str->str); > > - if (stat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) > break; > > st2qid(&st, &rwalk->wqids[i]); > @@ -344,7 +344,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg, > for (i = 0; i < VIRTIO_P9_MAX_FID; i++) > p9dev->fids[i].fid = P9_NOFID; > > - if (stat(p9dev->root_dir, &st) < 0) > + if (lstat(p9dev->root_dir, &st) < 0) > return false; > > st2qid(&st, &rattach->qid); > @@ -421,7 +421,7 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg, > while (cur) { > u32 read; > > - stat(rel_to_abs(p9dev, cur->d_name, full_path), &st); > + lstat(rel_to_abs(p9dev, cur->d_name, full_path), &st); > read = virtio_p9_fill_stat(p9dev, cur->d_name, > &st, rstat); > rread->count += read; > @@ -453,7 +453,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg, > struct p9_fid *fid = &p9dev->fids[tstat->fid]; > u32 ret; > > - if (stat(fid->abs_path, &st) < 0) > + if (lstat(fid->abs_path, &st) < 0) > return false; > > ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote: > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote: > > Use lstat instead of stat > > This patch should probably come after 9p2000.u support is added, since > now we transparently work 'through' symlinks (because legacy 9p2000 > doesn't support them). > Once 9p2000.u support is added we could represent symlinks and then a > switch to lstat might be due. > Shouldn't we aim to support 9p2000.L directly rather than supporting all the three versions of protocol ? Linux guest have good support for 9p2000.L Do you think the patch breaks any of the expectation of 9p2000 ?. It would be nice to get the correct file attributes when fetching attributes from the server, irrespective of whether client support symlink or not. -aneesh -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2011-06-19 at 10:47 +0530, Aneesh Kumar K.V wrote: > On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote: > > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote: > > > Use lstat instead of stat > > > > This patch should probably come after 9p2000.u support is added, since > > now we transparently work 'through' symlinks (because legacy 9p2000 > > doesn't support them). > > Once 9p2000.u support is added we could represent symlinks and then a > > switch to lstat might be due. > > > > Shouldn't we aim to support 9p2000.L directly rather than supporting all > the three versions of protocol ? Linux guest have good support for > 9p2000.L I'm perfectly fine with adding support directly for 9p2000.L, I just figured that we'd need to go through the 9p2000.u milestone before getting there instead of getting to 9p2000.L directly. > Do you think the patch breaks any of the expectation of 9p2000 ?. It > would be nice to get the correct file attributes when fetching > attributes from the server, irrespective of whether client support symlink > or not. The idea behind following symlinks was that we could make them transparently work even without protocol support (yes, not exactly what the protocol defines - but it made it much more useful). Removing symlink following also changes more than just the lstat thing, you'd also need O_NOFOLLOW when opening files for example.
On Sun, 19 Jun 2011 10:47:33 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote: > > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote: > > > Use lstat instead of stat > > > > This patch should probably come after 9p2000.u support is added, since > > now we transparently work 'through' symlinks (because legacy 9p2000 > > doesn't support them). > > Once 9p2000.u support is added we could represent symlinks and then a > > switch to lstat might be due. > > > > Shouldn't we aim to support 9p2000.L directly rather than supporting all > the three versions of protocol ? Linux guest have good support for > 9p2000.L > > Do you think the patch breaks any of the expectation of 9p2000 ?. It > would be nice to get the correct file attributes when fetching > attributes from the server, irrespective of whether client support symlink > or not. I sent Message-Id: <1308474599-9832-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> which add error handling to tools/kvm/9p. I guess we don't want server to follow symlink outside export path, hence open with O_NOFOLLOW. -aneesh -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c index 209760b..3172c1a 100644 --- a/tools/kvm/virtio/9p.c +++ b/tools/kvm/virtio/9p.c @@ -227,7 +227,7 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg, struct p9_fid *new_fid = &p9dev->fids[topen->fid]; struct stat st; - if (stat(new_fid->abs_path, &st) < 0) + if (lstat(new_fid->abs_path, &st) < 0) return false; st2qid(&st, &ropen->qid); @@ -274,7 +274,7 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg, fid->fd = open(fid->abs_path, omode2uflags(mode) | O_CREAT, 0777); } - if (stat(fid->abs_path, &st) < 0) + if (lstat(fid->abs_path, &st) < 0) return false; st2qid(&st, &rcreate->qid); @@ -308,7 +308,7 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg, /* Format the new path we're 'walk'ing into */ sprintf(tmp, "%s/%.*s", fid->path, str->len, (char *)&str->str); - if (stat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) break; st2qid(&st, &rwalk->wqids[i]); @@ -344,7 +344,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg, for (i = 0; i < VIRTIO_P9_MAX_FID; i++) p9dev->fids[i].fid = P9_NOFID; - if (stat(p9dev->root_dir, &st) < 0) + if (lstat(p9dev->root_dir, &st) < 0) return false; st2qid(&st, &rattach->qid); @@ -421,7 +421,7 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg, while (cur) { u32 read; - stat(rel_to_abs(p9dev, cur->d_name, full_path), &st); + lstat(rel_to_abs(p9dev, cur->d_name, full_path), &st); read = virtio_p9_fill_stat(p9dev, cur->d_name, &st, rstat); rread->count += read; @@ -453,7 +453,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg, struct p9_fid *fid = &p9dev->fids[tstat->fid]; u32 ret; - if (stat(fid->abs_path, &st) < 0) + if (lstat(fid->abs_path, &st) < 0) return false; ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
Use lstat instead of stat Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> --- tools/kvm/virtio/9p.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)