Messages in this thread | | | Date | Wed, 25 Jul 2018 05:32:00 +0200 | From | Dominique Martinet <> | Subject | Re: [PATCH] fs/9p/xattr.c: catch the error of p9_client_clunk when setting xattr failed |
| |
piaojun wrote on Wed, Jul 25, 2018: > In my testing, v9fs_fid_xattr_set will return successfully even if the > backend ext4 filesystem has no space to store xattr key-value. That will > cause inconsistent behavior between front end and back end. The reason is > that lsetxattr will be triggered by p9_client_clunk, and unfortunately we > did not catch the error. This patch will catch the error to notify upper > caller. > > p9_client_clunk (in 9p) > p9_client_rpc(clnt, P9_TCLUNK, "d", fid->fid); > v9fs_clunk (in qemu) > put_fid > free_fid > v9fs_xattr_fid_clunk > v9fs_co_lsetxattr > s->ops->lsetxattr > ext4_xattr_user_set (in host ext4 filesystem) > > Signed-off-by: Jun Piao <piaojun@huawei.com>
Good catch!
LGTM on its own but see comment below for further error checking. Please let me know if you want to do this in a v2 or submit a separate patch altogether, I can pick this one up as it is.
> --- > fs/9p/xattr.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c > index f329eee..352abc3 100644 > --- a/fs/9p/xattr.c > +++ b/fs/9p/xattr.c > @@ -105,7 +105,7 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const char *name, > { > struct kvec kvec = {.iov_base = (void *)value, .iov_len = value_len}; > struct iov_iter from; > - int retval; > + int retval, err; > > iov_iter_kvec(&from, WRITE | ITER_KVEC, &kvec, 1, value_len); > > @@ -126,7 +126,9 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const char *name, > retval); > else > p9_client_write(fid, 0, &from, &retval);
I'm not sure what to do about this but it's also possible for p9_client_write to not write the full length without setting and error.
We should probably compare the return value of p9_client_write and value_len to detect short writes and return an error in this case.
> - p9_client_clunk(fid); > + err = p9_client_clunk(fid); > + if (!retval && err) > + retval = err; > return retval; > } > > --
| |