>>> On 9/2/2014 at 04:54 PM, in message <20140902085434.GA21282@xxxxxxxxxx>, "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: > On Tue, Sep 02, 2014 at 03:40:42PM +0800, Chunyan Liu wrote: > > To use virtio-serial device, unix socket created for chardev with > > default umask(022) has insufficient permissions. > > > > e.g. start kvm guest with: > > -device virtio-serial \ > > -chardev socket,path=/tmp/foo,server,nowait,id=foo \ > > -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 > > > > Check permissions for the socket file that has been created in the host > > to enable communication through virtual serial ports in the guest: > > #ls -l /tmp/somefile.sock > > srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock > > > > Other users in the qemu group (like real user, test engines, etc) cannot > > write to this socket. > > > > Problem reported here: > > https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 > > https://bugzilla.novell.com/show_bug.cgi?id=888166 > > > > This patch tries to add a 'umask' option to 'chardev', so that user > > can have chance to indicate a umask overwritting the default one (default > > is 022), then create unix sockets with expected permissions. > > > > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx> > > --- > > This is patch for qemu. > > > > qemu-char.c | 3 +++ > > qemu-options.hx | 9 +++++++-- > > util/qemu-sockets.c | 12 +++++++++++- > > 3 files changed, 21 insertions(+), 3 deletions(-) > > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > > index 5d38395..facf2c6 100644 > > --- a/util/qemu-sockets.c > > +++ b/util/qemu-sockets.c > > @@ -680,7 +680,8 @@ int unix_listen_opts(QemuOpts *opts, Error **errp) > > { > > struct sockaddr_un un; > > const char *path = qemu_opt_get(opts, "path"); > > - int sock, fd; > > + int newmask = qemu_opt_get_number(opts, "umask", 0); > > + int sock, fd, oldmask; > > > > sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); > > if (sock < 0) { > > @@ -708,10 +709,19 @@ int unix_listen_opts(QemuOpts *opts, Error **errp) > > } > > > > unlink(un.sun_path); > > + if (newmask) { > > + oldmask = umask(newmask); > > + } > > if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { > > + if (newmask) { > > + umask(oldmask); > > + } > > error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED); > > goto err; > > } > > + if (newmask) { > > + umask(oldmask); > > + } > > Setting umask() is not thread-safe as it affects the entire process. While > this is OK for chardevs which are cold-plugged at startup, once QEMU is > running it is not OK to alter umask during hotplug of devices. > > Wouldn't it be simpler for libvirt to simply set the umask to 002 when it > first launches QEMU, avoiding the need for trying todo this per device. I think that's OK. Only one thing: I'm not sure if permissions of any other file created in qemu will be changed due to this change, and if that is unexpected or not. > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ > :| > |: http://libvirt.org -o- http://virt-manager.org > :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ > :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc > :| > > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list