On Mon, Aug 24, 2015 at 10:32:03AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 17, 2015 at 12:16:48PM -0700, Martin Kletzander wrote:We are automatically generating some socket paths for domains, but all those paths end up in a directory that's the same for multiple domains. The problem is that multiple domains can each run with different seclabels (users, selinux contexts, etc.). The idea here is to create a per-domain directory labelled in a way that each domain can access its own unix sockets. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886 Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 +++--- src/qemu/qemu_process.c | 57 +++++++++++++++++++++- .../qemuxml2argv-channel-virtio-unix.args | 7 +-- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5a4fdf09a2b..abc57d762075 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8051,7 +8051,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket && virAsprintf(&graphics->data.vnc.socket, - "%s/%s.vnc", cfg->libDir, def->name) == -1) + "%s/domain-%s/vnc.sock", cfg->libDir, def->name) == -1) goto error; virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 84e5fa530cba..0a9ed6babb4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1307,16 +1307,12 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto cleanup; } - if (dev->data.chr->target.name) { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s", - cfg->channelTargetDir, - def->name, dev->data.chr->target.name) < 0) - goto cleanup; - } else { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s", - cfg->channelTargetDir, def->name) < 0) - goto cleanup; - } + if (virAsprintf(&dev->data.chr->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name, + dev->data.chr->target.name ? dev->data.chr->target.name + : "unknown.sock") < 0) + goto cleanup;This worries me a little - IIUC we could end up with multiple devices using the same "unknown.sock" file path. If I'm reading correctly, the original code had this problem too. Would we be justified in raising an error in this scenario ? ACK anyway, since it appears the problem already existed.
To be honest, this worried me a *lot* actually. But since it already existed and I spent way too much trying to fix this as cleanly as possible, I rather posted it as is thinking the porblem might be dealt with later on, although I'm not quite sure it's worth the code change. Thanks for the review.
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 :|
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list