Hi Martin,
On Fri, Apr 01, 2016 at 01:11:21PM +0200, Martin Kletzander wrote:
On Thu, Mar 31, 2016 at 05:00:09PM +0200, Guido Günther wrote:
>The directory name changed in a89f05ba8df095875f5ec8a9065a585af63a010b.
>---
>src/security/virt-aa-helper.c | 2 ++
>1 file changed, 2 insertions(+)
>
>diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
>index a2d7226..0ded671 100644
>--- a/src/security/virt-aa-helper.c
>+++ b/src/security/virt-aa-helper.c
>@@ -1366,6 +1366,8 @@ main(int argc, char **argv)
> LOCALSTATEDIR, ctl->def->name);
> virBufferAsprintf(&buf, "
\"%s/lib/libvirt/qemu/domain-%s/monitor.sock\" rw,\n",
> LOCALSTATEDIR, ctl->def->name);
>+ virBufferAsprintf(&buf, "
\"%s/lib/libvirt/qemu/domain-*-%.*s/monitor.sock\" rw,\n",
Shouldn't this be domain-%d-... with the %d being ctl->def->id? Or is
it not known at this point? Then I think it should allow only numbers
between the dashes. If that's possible.
It is if we change virt-aa-helper slightly to not only parse
VIR_DOMAIN_DEF_PARSE_INACTIVE:
From 13a93c59af04785317d3e33b4f5c308cf4c9c3de Mon Sep 17 00:00:00 2001
Message-Id:
<13a93c59af04785317d3e33b4f5c308cf4c9c3de.1459516663.git.agx(a)sigxcpu.org>
From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx(a)sigxcpu.org>
Date: Thu, 31 Mar 2016 15:44:59 +0200
Subject: [PATCH] apparmor: QEMU monitor socket moved
To: libvir-list(a)redhat.com
The directory name changed in a89f05ba8df095875f5ec8a9065a585af63a010b.
This unbreaks launching QEMU/KVM VMs with apparmor enabled. It also adds
the directory for the qemu guest-agent socket which is not known when
parsing the domain XML.
---
src/security/virt-aa-helper.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a2d7226..50d2a08 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -762,8 +762,8 @@ get_definition(vahControl * ctl, const char *xmlStr)
}
ctl->def = virDomainDefParseString(xmlStr,
- ctl->caps, ctl->xmlopt,
- VIR_DOMAIN_DEF_PARSE_INACTIVE);
+ ctl->caps, ctl->xmlopt, 0);
+
if (ctl->def == NULL) {
vah_error(ctl, 0, _("could not parse XML"));
goto exit;
@@ -1366,6 +1366,10 @@ main(int argc, char **argv)
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf, "
\"%s/lib/libvirt/qemu/domain-%s/monitor.sock\" rw,\n",
LOCALSTATEDIR, ctl->def->name);
+ virBufferAsprintf(&buf, "
\"%s/lib/libvirt/qemu/domain-%d-%.*s/*\" rw,\n",
+ LOCALSTATEDIR, ctl->def->id, 20,
ctl->def->name);
+ virBufferAsprintf(&buf, "
\"%s/lib/libvirt/qemu/channel/target/domain-%d-%.*s/*\" rw,\n",
+ LOCALSTATEDIR, ctl->def->id, 20,
ctl->def->name);
virBufferAsprintf(&buf, "
\"%s/run/libvirt/**/%s.pid\" rwk,\n",
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\"
rwk,\n",
--
2.8.0.rc3
Another question, though: shouldn't there be also vnc.sock in
case that
is enabled? Basically we create this (and the
qemu/channel/target/domain-...) directory just for that particular
domain, so it should have access to the whole directory. Also the
channel/target one, I believe. Or did I miss something?
I added the channel/target one as well since the socket path does not
seem to be available when we parse the XML. Since the directories are
domain specific I went for allowing access to the whole dir.
While the above works as expected on the _first_ start of a domain it
fails on the second start since (at least in 1.3.3~rc1) we don't change
the domain id in the directory names on subsequent starts:
First start:
# virsh list
Id Name State
----------------------------------------------------
1 jessie running
# find /var/lib/libvirt/ -name 'domain-*-jessie'
/var/lib/libvirt/qemu/channel/target/domain-1-jessie
/var/lib/libvirt/qemu/domain-1-jessie
Second start:
# virsh list
Id Name State
----------------------------------------------------
2 jessie running
# find /var/lib/libvirt/ -name 'domain-*-jessie'
/var/lib/libvirt/qemu/channel/target/domain-1-jessie
/var/lib/libvirt/qemu/domain-1-jessie
Shouldn't that be:
/var/lib/libvirt/qemu/channel/target/domain-2-jessie
/var/lib/libvirt/qemu/domain-2-jessie
Is this on purpose or rather a bug in the code that generates the socket
paths?
Cheers,
-- Guido