On Donnerstag, 21. April 2022 10:26:11 CEST Greg Kurz wrote: > On Tue, 19 Apr 2022 13:43:30 +0200 > > Christian Schoenebeck <qemu_...@crudebyte.com> wrote: > > When mapped POSIX ACL is used, we are ignoring errors when trying > > to remove a POSIX ACL xattr that does not exist. On Linux hosts we > > would get ENODATA in such cases, on macOS hosts however we get > > ENOATTR instead, so ignore ENOATTR errors as well. > > > > This patch fixes e.g. a command on Linux guest like: > > cp --preserve=mode old new > > > > Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com> > > --- > > > > hw/9pfs/9p-posix-acl.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/hw/9pfs/9p-posix-acl.c b/hw/9pfs/9p-posix-acl.c > > index eadae270dd..2bf155f941 100644 > > --- a/hw/9pfs/9p-posix-acl.c > > +++ b/hw/9pfs/9p-posix-acl.c > > @@ -65,7 +65,13 @@ static int mp_pacl_removexattr(FsContext *ctx, > > > > int ret; > > > > ret = local_removexattr_nofollow(ctx, path, MAP_ACL_ACCESS); > > > > - if (ret == -1 && errno == ENODATA) { > > + if (ret == -1 && > > + (errno == ENODATA > > +#ifdef ENOATTR > > + || errno == ENOATTR > > +#endif > > + ) > > We already have this in <qemu/xattr.h> which is included by > 9p-posix-acl.c : > > /* > * Modern distributions (e.g. Fedora 15), have no libattr.so, place attr.h > * in /usr/include/sys, and don't have ENOATTR. > */ > > > #ifdef CONFIG_LIBATTR > # include <attr/xattr.h> > #else > # if !defined(ENOATTR) > # define ENOATTR ENODATA > # endif > # include <sys/xattr.h> > #endif > > I guess this patch could just s/ENODATA/ENOATTR/ to avoid the > extra ifdefery.
Not viable, because macOS does have both ENODATA==96 and ENOATTR==93. On Linux the two macros were historically defined to the same numeric values, that's why it worked there. Maybe I should define a separate macro like: #if ... # define P9_ENOATTR ENOATTR #else # define P9_ENOATTR ENODATA #end ? Actually good that you pointed me at this, because I just realized there is a 2nd place in 9p-posix-acl.c which would require this as well. For some reason the 2nd place just did not trigger while I was testing it on macOS. Best regards, Christian Schoenebeck