Re: [PATCH] tools/virtiofsd: add support for --socket-group
Hi On Wed, Mar 18, 2020 at 12:21 AM Alex Bennée wrote: > > > Dr. David Alan Gilbert writes: > > > * Stefan Hajnoczi (stefa...@redhat.com) wrote: > >> On Mon, Mar 16, 2020 at 10:33:31AM +, Daniel P. Berrangé wrote: > >> > On Sat, Mar 14, 2020 at 02:33:25PM +0100, Marc-André Lureau wrote: > >> > > Hi > >> > > > >> > > On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé > >> > > wrote: > >> > > > > >> > > > On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > >> > > > > If you like running QEMU as a normal user (very common for TCG > >> > > > > runs) > >> > > > > but you have to run virtiofsd as a root user you run into > >> > > > > connection > >> > > > > problems. Adding support for an optional --socket-group allows the > >> > > > > users to keep using the command line. > >> > > > > >> > > > If we're going to support this, then I think we need to put it in > >> > > > the vhost-user.rst specification so we standardize across backends. > >> > > > > >> > > > > >> > > > >> > > Perhaps. Otoh, I wonder if the backend spec should be more limited to > >> > > arguments/introspection that are used by programs. > >> > > > >> > > In this case, I even consider --socket-path to be unnecessary, as a > >> > > management layer can/should provide a preopened & setup fd directly. > >> > > > >> > > What do you think? > >> > > >> > I think there's value in standardization even if it is an option > >> > targetted > >> > at human admins, rather than machine usage. You are right though that > >> > something like libvirt would never use --socket-group, or --socket-path. > >> > Even admins would benefit if all programs followed the same naming for > >> > these. We could document such options as "SHOULD" rather than "MUST" > >> > IOW, we don't mandate --socket-group, but if you're going to provide a > >> > way to control socket group, this option should be used. > >> > >> I agree. It's still useful to have a convention that most vhost-user > >> backend programs follow. > > > > Alex: > > Can you add the doc entry that Stefan and Marc-André are asking > > for; it's probably good they go together. > > Sure - is docs/interop/vhost-user.rst the master spec for vhost-user > daemons? So far, yes. But it might make sense to create a standalone vhost-user-daemons.rst. -- Marc-André Lureau
Re: [PATCH] tools/virtiofsd: add support for --socket-group
Dr. David Alan Gilbert writes: > * Stefan Hajnoczi (stefa...@redhat.com) wrote: >> On Mon, Mar 16, 2020 at 10:33:31AM +, Daniel P. Berrangé wrote: >> > On Sat, Mar 14, 2020 at 02:33:25PM +0100, Marc-André Lureau wrote: >> > > Hi >> > > >> > > On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé >> > > wrote: >> > > > >> > > > On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: >> > > > > If you like running QEMU as a normal user (very common for TCG runs) >> > > > > but you have to run virtiofsd as a root user you run into connection >> > > > > problems. Adding support for an optional --socket-group allows the >> > > > > users to keep using the command line. >> > > > >> > > > If we're going to support this, then I think we need to put it in >> > > > the vhost-user.rst specification so we standardize across backends. >> > > > >> > > > >> > > >> > > Perhaps. Otoh, I wonder if the backend spec should be more limited to >> > > arguments/introspection that are used by programs. >> > > >> > > In this case, I even consider --socket-path to be unnecessary, as a >> > > management layer can/should provide a preopened & setup fd directly. >> > > >> > > What do you think? >> > >> > I think there's value in standardization even if it is an option targetted >> > at human admins, rather than machine usage. You are right though that >> > something like libvirt would never use --socket-group, or --socket-path. >> > Even admins would benefit if all programs followed the same naming for >> > these. We could document such options as "SHOULD" rather than "MUST" >> > IOW, we don't mandate --socket-group, but if you're going to provide a >> > way to control socket group, this option should be used. >> >> I agree. It's still useful to have a convention that most vhost-user >> backend programs follow. > > Alex: > Can you add the doc entry that Stefan and Marc-André are asking > for; it's probably good they go together. Sure - is docs/interop/vhost-user.rst the master spec for vhost-user daemons? > > Dave > >> Stefan -- Alex Bennée
Re: [PATCH] tools/virtiofsd: add support for --socket-group
* Stefan Hajnoczi (stefa...@redhat.com) wrote: > On Mon, Mar 16, 2020 at 10:33:31AM +, Daniel P. Berrangé wrote: > > On Sat, Mar 14, 2020 at 02:33:25PM +0100, Marc-André Lureau wrote: > > > Hi > > > > > > On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé > > > wrote: > > > > > > > > On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > > > > > If you like running QEMU as a normal user (very common for TCG runs) > > > > > but you have to run virtiofsd as a root user you run into connection > > > > > problems. Adding support for an optional --socket-group allows the > > > > > users to keep using the command line. > > > > > > > > If we're going to support this, then I think we need to put it in > > > > the vhost-user.rst specification so we standardize across backends. > > > > > > > > > > > > > > Perhaps. Otoh, I wonder if the backend spec should be more limited to > > > arguments/introspection that are used by programs. > > > > > > In this case, I even consider --socket-path to be unnecessary, as a > > > management layer can/should provide a preopened & setup fd directly. > > > > > > What do you think? > > > > I think there's value in standardization even if it is an option targetted > > at human admins, rather than machine usage. You are right though that > > something like libvirt would never use --socket-group, or --socket-path. > > Even admins would benefit if all programs followed the same naming for > > these. We could document such options as "SHOULD" rather than "MUST" > > IOW, we don't mandate --socket-group, but if you're going to provide a > > way to control socket group, this option should be used. > > I agree. It's still useful to have a convention that most vhost-user > backend programs follow. Alex: Can you add the doc entry that Stefan and Marc-André are asking for; it's probably good they go together. Dave > Stefan -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] tools/virtiofsd: add support for --socket-group
On Mon, Mar 16, 2020 at 10:33:31AM +, Daniel P. Berrangé wrote: > On Sat, Mar 14, 2020 at 02:33:25PM +0100, Marc-André Lureau wrote: > > Hi > > > > On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé > > wrote: > > > > > > On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > > > > If you like running QEMU as a normal user (very common for TCG runs) > > > > but you have to run virtiofsd as a root user you run into connection > > > > problems. Adding support for an optional --socket-group allows the > > > > users to keep using the command line. > > > > > > If we're going to support this, then I think we need to put it in > > > the vhost-user.rst specification so we standardize across backends. > > > > > > > > > > Perhaps. Otoh, I wonder if the backend spec should be more limited to > > arguments/introspection that are used by programs. > > > > In this case, I even consider --socket-path to be unnecessary, as a > > management layer can/should provide a preopened & setup fd directly. > > > > What do you think? > > I think there's value in standardization even if it is an option targetted > at human admins, rather than machine usage. You are right though that > something like libvirt would never use --socket-group, or --socket-path. > Even admins would benefit if all programs followed the same naming for > these. We could document such options as "SHOULD" rather than "MUST" > IOW, we don't mandate --socket-group, but if you're going to provide a > way to control socket group, this option should be used. I agree. It's still useful to have a convention that most vhost-user backend programs follow. Stefan signature.asc Description: PGP signature
Re: [PATCH] tools/virtiofsd: add support for --socket-group
On Sat, Mar 14, 2020 at 02:33:25PM +0100, Marc-André Lureau wrote: > Hi > > On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé > wrote: > > > > On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > > > If you like running QEMU as a normal user (very common for TCG runs) > > > but you have to run virtiofsd as a root user you run into connection > > > problems. Adding support for an optional --socket-group allows the > > > users to keep using the command line. > > > > If we're going to support this, then I think we need to put it in > > the vhost-user.rst specification so we standardize across backends. > > > > > > Perhaps. Otoh, I wonder if the backend spec should be more limited to > arguments/introspection that are used by programs. > > In this case, I even consider --socket-path to be unnecessary, as a > management layer can/should provide a preopened & setup fd directly. > > What do you think? I think there's value in standardization even if it is an option targetted at human admins, rather than machine usage. You are right though that something like libvirt would never use --socket-group, or --socket-path. Even admins would benefit if all programs followed the same naming for these. We could document such options as "SHOULD" rather than "MUST" IOW, we don't mandate --socket-group, but if you're going to provide a way to control socket group, this option should be used. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] tools/virtiofsd: add support for --socket-group
Hi
On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé wrote:
>
> On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote:
> > If you like running QEMU as a normal user (very common for TCG runs)
> > but you have to run virtiofsd as a root user you run into connection
> > problems. Adding support for an optional --socket-group allows the
> > users to keep using the command line.
>
> If we're going to support this, then I think we need to put it in
> the vhost-user.rst specification so we standardize across backends.
>
>
Perhaps. Otoh, I wonder if the backend spec should be more limited to
arguments/introspection that are used by programs.
In this case, I even consider --socket-path to be unnecessary, as a
management layer can/should provide a preopened & setup fd directly.
What do you think?
>
> > Signed-off-by: Alex Bennée
>
>
> >
> > ---
> > v1
> > - tweak documentation and commentary
> > ---
> > docs/tools/virtiofsd.rst| 4
> > tools/virtiofsd/fuse_i.h| 1 +
> > tools/virtiofsd/fuse_lowlevel.c | 6 ++
> > tools/virtiofsd/fuse_virtio.c | 20 ++--
> > 4 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> > index 378594c422a..5a8246b74f8 100644
> > --- a/docs/tools/virtiofsd.rst
> > +++ b/docs/tools/virtiofsd.rst
> > @@ -85,6 +85,10 @@ Options
> >
> >Listen on vhost-user UNIX domain socket at PATH.
> >
> > +.. option:: --socket-group=GROUP
> > +
> > + Set the vhost-user UNIX domain socket gid to GROUP.
> > +
> > .. option:: --fd=FDNUM
> >
> >Accept connections from vhost-user UNIX domain socket file descriptor
> > FDNUM.
> > diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
> > index 1240828208a..492e002181e 100644
> > --- a/tools/virtiofsd/fuse_i.h
> > +++ b/tools/virtiofsd/fuse_i.h
> > @@ -68,6 +68,7 @@ struct fuse_session {
> > size_t bufsize;
> > int error;
> > char *vu_socket_path;
> > +char *vu_socket_group;
> > int vu_listen_fd;
> > int vu_socketfd;
> > struct fv_VuDev *virtio_dev;
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c
> > b/tools/virtiofsd/fuse_lowlevel.c
> > index 2dd36ec03b6..4d1ba2925d1 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
> > LL_OPTION("--debug", debug, 1),
> > LL_OPTION("allow_root", deny_others, 1),
> > LL_OPTION("--socket-path=%s", vu_socket_path, 0),
> > +LL_OPTION("--socket-group=%s", vu_socket_group, 0),
> > LL_OPTION("--fd=%d", vu_listen_fd, 0),
> > LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
> > FUSE_OPT_END
> > @@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct
> > fuse_args *args,
> > "fuse: --socket-path and --fd cannot be given
> > together\n");
> > goto out4;
> > }
> > +if (se->vu_socket_group && !se->vu_socket_path) {
> > +fuse_log(FUSE_LOG_ERR,
> > + "fuse: --socket-group can only be used with
> > --socket-path\n");
> > +goto out4;
> > +}
> >
> > se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() +
> > FUSE_BUFFER_HEADER_SIZE;
> >
> > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> > index 3b6d16a0417..331f9fc65c5 100644
> > --- a/tools/virtiofsd/fuse_virtio.c
> > +++ b/tools/virtiofsd/fuse_virtio.c
> > @@ -31,6 +31,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> >
> > #include "contrib/libvhost-user/libvhost-user.h"
> > @@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct
> > fuse_session *se)
> >
> > /*
> > * Unfortunately bind doesn't let you set the mask on the socket,
> > - * so set umask to 077 and restore it later.
> > + * so set umask appropriately and restore it later.
> > */
> > -old_umask = umask(0077);
> > +if (se->vu_socket_group) {
> > +old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
> > +} else {
> > +old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH
> > | S_IXOTH);
> > +}
> > if (bind(listen_sock, (struct sockaddr *), addr_len) == -1) {
> > fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
> > close(listen_sock);
> > umask(old_umask);
> > return -1;
> > }
> > +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)) {
> > +fuse_log(FUSE_LOG_WARNING,
> > + "vhost socket failed to set group to %s (%d)\n",
> > + se->vu_socket_group, g->gr_gid);
> > +}
> > +}
> > +}
> > umask(old_umask);
> >
> > if (listen(listen_sock, 1) == -1) {
> > --
> > 2.20.1
> >
> >
>
> Regards,
>
Re: [PATCH] tools/virtiofsd: add support for --socket-group
On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > If you like running QEMU as a normal user (very common for TCG runs) > but you have to run virtiofsd as a root user you run into connection > problems. Adding support for an optional --socket-group allows the > users to keep using the command line. > > Signed-off-by: Alex Bennée > > --- > v1 > - tweak documentation and commentary > --- > docs/tools/virtiofsd.rst| 4 > tools/virtiofsd/fuse_i.h| 1 + > tools/virtiofsd/fuse_lowlevel.c | 6 ++ > tools/virtiofsd/fuse_virtio.c | 20 ++-- > 4 files changed, 29 insertions(+), 2 deletions(-) Dan's suggestion sounds like a good idea to me. Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH] tools/virtiofsd: add support for --socket-group
On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote:
> If you like running QEMU as a normal user (very common for TCG runs)
> but you have to run virtiofsd as a root user you run into connection
> problems. Adding support for an optional --socket-group allows the
> users to keep using the command line.
If we're going to support this, then I think we need to put it in
the vhost-user.rst specification so we standardize across backends.
> Signed-off-by: Alex Bennée
>
> ---
> v1
> - tweak documentation and commentary
> ---
> docs/tools/virtiofsd.rst| 4
> tools/virtiofsd/fuse_i.h| 1 +
> tools/virtiofsd/fuse_lowlevel.c | 6 ++
> tools/virtiofsd/fuse_virtio.c | 20 ++--
> 4 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> index 378594c422a..5a8246b74f8 100644
> --- a/docs/tools/virtiofsd.rst
> +++ b/docs/tools/virtiofsd.rst
> @@ -85,6 +85,10 @@ Options
>
>Listen on vhost-user UNIX domain socket at PATH.
>
> +.. option:: --socket-group=GROUP
> +
> + Set the vhost-user UNIX domain socket gid to GROUP.
> +
> .. option:: --fd=FDNUM
>
>Accept connections from vhost-user UNIX domain socket file descriptor
> FDNUM.
> diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
> index 1240828208a..492e002181e 100644
> --- a/tools/virtiofsd/fuse_i.h
> +++ b/tools/virtiofsd/fuse_i.h
> @@ -68,6 +68,7 @@ struct fuse_session {
> size_t bufsize;
> int error;
> char *vu_socket_path;
> +char *vu_socket_group;
> int vu_listen_fd;
> int vu_socketfd;
> struct fv_VuDev *virtio_dev;
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index 2dd36ec03b6..4d1ba2925d1 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
> LL_OPTION("--debug", debug, 1),
> LL_OPTION("allow_root", deny_others, 1),
> LL_OPTION("--socket-path=%s", vu_socket_path, 0),
> +LL_OPTION("--socket-group=%s", vu_socket_group, 0),
> LL_OPTION("--fd=%d", vu_listen_fd, 0),
> LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
> FUSE_OPT_END
> @@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct fuse_args
> *args,
> "fuse: --socket-path and --fd cannot be given together\n");
> goto out4;
> }
> +if (se->vu_socket_group && !se->vu_socket_path) {
> +fuse_log(FUSE_LOG_ERR,
> + "fuse: --socket-group can only be used with
> --socket-path\n");
> +goto out4;
> +}
>
> se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() +
> FUSE_BUFFER_HEADER_SIZE;
>
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index 3b6d16a0417..331f9fc65c5 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -31,6 +31,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
>
> #include "contrib/libvhost-user/libvhost-user.h"
> @@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct fuse_session
> *se)
>
> /*
> * Unfortunately bind doesn't let you set the mask on the socket,
> - * so set umask to 077 and restore it later.
> + * so set umask appropriately and restore it later.
> */
> -old_umask = umask(0077);
> +if (se->vu_socket_group) {
> +old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
> +} else {
> +old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH |
> S_IXOTH);
> +}
> if (bind(listen_sock, (struct sockaddr *), addr_len) == -1) {
> fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
> close(listen_sock);
> umask(old_umask);
> return -1;
> }
> +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)) {
> +fuse_log(FUSE_LOG_WARNING,
> + "vhost socket failed to set group to %s (%d)\n",
> + se->vu_socket_group, g->gr_gid);
> +}
> +}
> +}
> umask(old_umask);
>
> if (listen(listen_sock, 1) == -1) {
> --
> 2.20.1
>
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] tools/virtiofsd: add support for --socket-group
If you like running QEMU as a normal user (very common for TCG runs)
but you have to run virtiofsd as a root user you run into connection
problems. Adding support for an optional --socket-group allows the
users to keep using the command line.
Signed-off-by: Alex Bennée
---
v1
- tweak documentation and commentary
---
docs/tools/virtiofsd.rst| 4
tools/virtiofsd/fuse_i.h| 1 +
tools/virtiofsd/fuse_lowlevel.c | 6 ++
tools/virtiofsd/fuse_virtio.c | 20 ++--
4 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 378594c422a..5a8246b74f8 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -85,6 +85,10 @@ Options
Listen on vhost-user UNIX domain socket at PATH.
+.. option:: --socket-group=GROUP
+
+ Set the vhost-user UNIX domain socket gid to GROUP.
+
.. option:: --fd=FDNUM
Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
index 1240828208a..492e002181e 100644
--- a/tools/virtiofsd/fuse_i.h
+++ b/tools/virtiofsd/fuse_i.h
@@ -68,6 +68,7 @@ struct fuse_session {
size_t bufsize;
int error;
char *vu_socket_path;
+char *vu_socket_group;
int vu_listen_fd;
int vu_socketfd;
struct fv_VuDev *virtio_dev;
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 2dd36ec03b6..4d1ba2925d1 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
LL_OPTION("--debug", debug, 1),
LL_OPTION("allow_root", deny_others, 1),
LL_OPTION("--socket-path=%s", vu_socket_path, 0),
+LL_OPTION("--socket-group=%s", vu_socket_group, 0),
LL_OPTION("--fd=%d", vu_listen_fd, 0),
LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
FUSE_OPT_END
@@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct fuse_args
*args,
"fuse: --socket-path and --fd cannot be given together\n");
goto out4;
}
+if (se->vu_socket_group && !se->vu_socket_path) {
+fuse_log(FUSE_LOG_ERR,
+ "fuse: --socket-group can only be used with --socket-path\n");
+goto out4;
+}
se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() + FUSE_BUFFER_HEADER_SIZE;
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 3b6d16a0417..331f9fc65c5 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -31,6 +31,8 @@
#include
#include
#include
+#include
+#include
#include
#include "contrib/libvhost-user/libvhost-user.h"
@@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct fuse_session
*se)
/*
* Unfortunately bind doesn't let you set the mask on the socket,
- * so set umask to 077 and restore it later.
+ * so set umask appropriately and restore it later.
*/
-old_umask = umask(0077);
+if (se->vu_socket_group) {
+old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
+} else {
+old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH |
S_IXOTH);
+}
if (bind(listen_sock, (struct sockaddr *), addr_len) == -1) {
fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
close(listen_sock);
umask(old_umask);
return -1;
}
+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)) {
+fuse_log(FUSE_LOG_WARNING,
+ "vhost socket failed to set group to %s (%d)\n",
+ se->vu_socket_group, g->gr_gid);
+}
+}
+}
umask(old_umask);
if (listen(listen_sock, 1) == -1) {
--
2.20.1
Re: [RFC PATCH] tools/virtiofsd: add support for --socket-group
On Wed, Mar 04, 2020 at 06:50:25PM +, Alex Bennée wrote: > diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst > index 378594c422a..6d2342f74d4 100644 > --- a/docs/tools/virtiofsd.rst > +++ b/docs/tools/virtiofsd.rst > @@ -85,6 +85,10 @@ Options > >Listen on vhost-user UNIX domain socket at PATH. > > +.. option:: --socket-group=GROUP > + > + Also make vhost-user UNIX domain socket accessible to GROUP. Files only have one gid but the above sentence can be interpreted as "add GROUP" (instead of "set GROUP"). Please drop "Also" to make the meaning clearer. > @@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct fuse_session > *se) > > /* > * Unfortunately bind doesn't let you set the mask on the socket, > - * so set umask to 077 and restore it later. > + * so set umask to appropriately and restore it later. s/ to // signature.asc Description: PGP signature
[RFC PATCH] tools/virtiofsd: add support for --socket-group
If you like running QEMU as a normal user (very common for TCG runs)
but you have to run virtiofsd as a root user you run into connection
problems. Adding support for an optional --socket-group allows the
users to keep using the command line.
Signed-off-by: Alex Bennée
---
docs/tools/virtiofsd.rst| 4
tools/virtiofsd/fuse_i.h| 1 +
tools/virtiofsd/fuse_lowlevel.c | 6 ++
tools/virtiofsd/fuse_virtio.c | 20 ++--
4 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 378594c422a..6d2342f74d4 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -85,6 +85,10 @@ Options
Listen on vhost-user UNIX domain socket at PATH.
+.. option:: --socket-group=GROUP
+
+ Also make vhost-user UNIX domain socket accessible to GROUP.
+
.. option:: --fd=FDNUM
Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
index 1240828208a..492e002181e 100644
--- a/tools/virtiofsd/fuse_i.h
+++ b/tools/virtiofsd/fuse_i.h
@@ -68,6 +68,7 @@ struct fuse_session {
size_t bufsize;
int error;
char *vu_socket_path;
+char *vu_socket_group;
int vu_listen_fd;
int vu_socketfd;
struct fv_VuDev *virtio_dev;
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 2dd36ec03b6..4d1ba2925d1 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
LL_OPTION("--debug", debug, 1),
LL_OPTION("allow_root", deny_others, 1),
LL_OPTION("--socket-path=%s", vu_socket_path, 0),
+LL_OPTION("--socket-group=%s", vu_socket_group, 0),
LL_OPTION("--fd=%d", vu_listen_fd, 0),
LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
FUSE_OPT_END
@@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct fuse_args
*args,
"fuse: --socket-path and --fd cannot be given together\n");
goto out4;
}
+if (se->vu_socket_group && !se->vu_socket_path) {
+fuse_log(FUSE_LOG_ERR,
+ "fuse: --socket-group can only be used with --socket-path\n");
+goto out4;
+}
se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() + FUSE_BUFFER_HEADER_SIZE;
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 3b6d16a0417..13d69525646 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -31,6 +31,8 @@
#include
#include
#include
+#include
+#include
#include
#include "contrib/libvhost-user/libvhost-user.h"
@@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct fuse_session
*se)
/*
* Unfortunately bind doesn't let you set the mask on the socket,
- * so set umask to 077 and restore it later.
+ * so set umask to appropriately and restore it later.
*/
-old_umask = umask(0077);
+if (se->vu_socket_group) {
+old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
+} else {
+old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH |
S_IXOTH);
+}
if (bind(listen_sock, (struct sockaddr *), addr_len) == -1) {
fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
close(listen_sock);
umask(old_umask);
return -1;
}
+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)) {
+fuse_log(FUSE_LOG_WARNING,
+ "vhost socket failed to set group to %s (%d)\n",
+ se->vu_socket_group, g->gr_gid);
+}
+}
+}
umask(old_umask);
if (listen(listen_sock, 1) == -1) {
--
2.20.1
