| Message ID | 162040394890.714971.15502455176528384778.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | virtiofsd: Fix check of chown()'s return value | expand |
On 5/7/21 11:12 AM, Greg Kurz wrote: > Otherwise you always get this warning when using --socket-group=users > > vhost socket failed to set group to users (100) > > While here, print out the error if chown() fails. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > tools/virtiofsd/fuse_virtio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > index 3e13997406bf..638d3ffe2f8a 100644 > --- a/tools/virtiofsd/fuse_virtio.c > +++ b/tools/virtiofsd/fuse_virtio.c > @@ -978,9 +978,9 @@ static int fv_create_listen_socket(struct fuse_session *se) > if (se->vu_socket_group) { > struct group *g = getgrnam(se->vu_socket_group); > if (g) { > - if (!chown(se->vu_socket_path, -1, g->gr_gid)) { > + if (chown(se->vu_socket_path, -1, g->gr_gid) == -1) { > fuse_log(FUSE_LOG_WARNING, > - "vhost socket failed to set group to %s (%d)\n", > + "vhost socket failed to set group to %s (%d): %m\n", Is %m portable? POSIX requires it for syslog, but not for printf (where glibc has it as an extension), but I'm not sure what fuse_log supports. Best might be a manual %s/strerror(errno)
On Fri, 7 May 2021 11:19:07 -0500 Eric Blake <eblake@redhat.com> wrote: > On 5/7/21 11:12 AM, Greg Kurz wrote: > > Otherwise you always get this warning when using --socket-group=users > > > > vhost socket failed to set group to users (100) > > > > While here, print out the error if chown() fails. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > tools/virtiofsd/fuse_virtio.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > > index 3e13997406bf..638d3ffe2f8a 100644 > > --- a/tools/virtiofsd/fuse_virtio.c > > +++ b/tools/virtiofsd/fuse_virtio.c > > @@ -978,9 +978,9 @@ static int fv_create_listen_socket(struct fuse_session *se) > > if (se->vu_socket_group) { > > struct group *g = getgrnam(se->vu_socket_group); > > if (g) { > > - if (!chown(se->vu_socket_path, -1, g->gr_gid)) { > > + if (chown(se->vu_socket_path, -1, g->gr_gid) == -1) { > > fuse_log(FUSE_LOG_WARNING, > > - "vhost socket failed to set group to %s (%d)\n", > > + "vhost socket failed to set group to %s (%d): %m\n", > > Is %m portable? POSIX requires it for syslog, but not for printf (where > glibc has it as an extension), but I'm not sure what fuse_log supports. > Best might be a manual %s/strerror(errno) > virtiofsd is for linux+glibc only and %m is already used a few lines above in the same function.
* Greg Kurz (groug@kaod.org) wrote: > Otherwise you always get this warning when using --socket-group=users > > vhost socket failed to set group to users (100) > > While here, print out the error if chown() fails. > > Signed-off-by: Greg Kurz <groug@kaod.org> probably needs a fixes: but yes. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/fuse_virtio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > index 3e13997406bf..638d3ffe2f8a 100644 > --- a/tools/virtiofsd/fuse_virtio.c > +++ b/tools/virtiofsd/fuse_virtio.c > @@ -978,9 +978,9 @@ static int fv_create_listen_socket(struct fuse_session *se) > if (se->vu_socket_group) { > struct group *g = getgrnam(se->vu_socket_group); > if (g) { > - if (!chown(se->vu_socket_path, -1, g->gr_gid)) { > + if (chown(se->vu_socket_path, -1, g->gr_gid) == -1) { > fuse_log(FUSE_LOG_WARNING, > - "vhost socket failed to set group to %s (%d)\n", > + "vhost socket failed to set group to %s (%d): %m\n", > se->vu_socket_group, g->gr_gid); > } > } > >
On Mon, 10 May 2021 15:16:23 +0100 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Greg Kurz (groug@kaod.org) wrote: > > Otherwise you always get this warning when using --socket-group=users > > > > vhost socket failed to set group to users (100) > > > > While here, print out the error if chown() fails. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > probably needs a fixes: but yes. > Fixes: f6698f2b03b0 ("tools/virtiofsd: add support for --socket-group") > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > --- > > tools/virtiofsd/fuse_virtio.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > > index 3e13997406bf..638d3ffe2f8a 100644 > > --- a/tools/virtiofsd/fuse_virtio.c > > +++ b/tools/virtiofsd/fuse_virtio.c > > @@ -978,9 +978,9 @@ static int fv_create_listen_socket(struct fuse_session *se) > > if (se->vu_socket_group) { > > struct group *g = getgrnam(se->vu_socket_group); > > if (g) { > > - if (!chown(se->vu_socket_path, -1, g->gr_gid)) { > > + if (chown(se->vu_socket_path, -1, g->gr_gid) == -1) { > > fuse_log(FUSE_LOG_WARNING, > > - "vhost socket failed to set group to %s (%d)\n", > > + "vhost socket failed to set group to %s (%d): %m\n", > > se->vu_socket_group, g->gr_gid); > > } > > } > > > >
Le 10/05/2021 à 16:23, Greg Kurz a écrit : > On Mon, 10 May 2021 15:16:23 +0100 > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > >> * Greg Kurz (groug@kaod.org) wrote: >>> Otherwise you always get this warning when using --socket-group=users >>> >>> vhost socket failed to set group to users (100) >>> >>> While here, print out the error if chown() fails. >>> >>> Signed-off-by: Greg Kurz <groug@kaod.org> >> >> probably needs a fixes: but yes. >> > > Fixes: f6698f2b03b0 ("tools/virtiofsd: add support for --socket-group") > Applied to my trivial-patches branch. Thanks, Laurent
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 3e13997406bf..638d3ffe2f8a 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -978,9 +978,9 @@ static int fv_create_listen_socket(struct fuse_session *se) if (se->vu_socket_group) { struct group *g = getgrnam(se->vu_socket_group); if (g) { - if (!chown(se->vu_socket_path, -1, g->gr_gid)) { + if (chown(se->vu_socket_path, -1, g->gr_gid) == -1) { fuse_log(FUSE_LOG_WARNING, - "vhost socket failed to set group to %s (%d)\n", + "vhost socket failed to set group to %s (%d): %m\n", se->vu_socket_group, g->gr_gid); } }
Otherwise you always get this warning when using --socket-group=users vhost socket failed to set group to users (100) While here, print out the error if chown() fails. Signed-off-by: Greg Kurz <groug@kaod.org> --- tools/virtiofsd/fuse_virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)