Re: [libvirt] [PATCH 3/3] security_dac: Favour ACLs over chown()

On 05.09.2013 16:14, Daniel P. Berrange wrote: > On Wed, Aug 28, 2013 at 12:27:40PM +0200, Michal Privoznik wrote: >> On filesystems supporting ACLs we don't need to do a chown but we >> can just set ACLs to gain access for qemu. However, since we are >> setting these on too low level, where we don't know if disk is >> just a read only or read write, we set read write access >> unconditionally. >> >> >From implementation POV, a reference counter is introduced, so ACL is >> restored only on the last restore attempt in order to not cut off other >> domains. And since a file may had an ACL for a user already set, we need >> to keep this as well. Both these, the reference counter and original ACL >> are stored as extended attributes named trusted.libvirt.dac.refCount and >> trusted.libvirt.dac.oldACL respectively. >> >> However, some filesystems doesn't support ACLs, XATTRs, or both. So the >> code is made to favour ACLs among with tracking the reference count. If >> this fails, we fall back to chown() with best effort to remember the >> original owner of file. >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/security/security_dac.c | 297 +++++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 268 insertions(+), 29 deletions(-) >> >> diff --git a/src/security/security_dac.c b/src/security/security_dac.c >> index 8cbb083..913b68e 100644 >> --- a/src/security/security_dac.c >> +++ b/src/security/security_dac.c >> @@ -37,6 +37,9 @@ >> >> #define VIR_FROM_THIS VIR_FROM_SECURITY >> #define SECURITY_DAC_NAME "dac" >> +#define SECURITY_DAC_XATTR_OLD_ACL "trusted.libvirt.dac.oldACL" >> +#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.libvirt.dac.oldOwner" >> +#define SECURITY_DAC_XATTR_REFCOUNT "trusted.libvirt.dac.refCount" > > Use of "trusted." is non-portable, per the long mail thread last > time this was posted[1]. The discussion there was never resolved in > any way, so NACK to this patch as is. > > Daniel > > [1] https://www.redhat.com/archives/libvir-list/2013-March/msg01610.html > Okay then, consider this squashed in: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 47af4e8..acc80c4 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -37,9 +37,9 @@ #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" -#define SECURITY_DAC_XATTR_OLD_ACL "trusted.libvirt.dac.oldACL" -#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.libvirt.dac.oldOwner" -#define SECURITY_DAC_XATTR_REFCOUNT "trusted.libvirt.dac.refCount" +#define SECURITY_DAC_XATTR_OLD_ACL "user.libvirt.dac.oldACL" +#define SECURITY_DAC_XATTR_OLD_OWNER "user.libvirt.dac.oldOwner" +#define SECURITY_DAC_XATTR_REFCOUNT "user.libvirt.dac.refCount"