On Mon, Mar 11, 2013 at 05:13:29PM +0100, 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.refCount and trusted.oldACL respectively.
diff to v2: -basically squashed functionality of 2/4 and 4/4 from previous round
src/security/security_dac.c | 209 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 182 insertions(+), 27 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0b274b7..46cc686 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -25,6 +25,7 @@
#include "security_dac.h" #include "virerror.h" +#include "virfile.h" #include "virutil.h" #include "viralloc.h" #include "virlog.h" @@ -34,6 +35,8 @@
#define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" +#define SECURITY_DAC_XATTR_OLD_ACL "trusted.oldACL" +#define SECURITY_DAC_XATTR_REFCOUNT "trusted.refCount"
I think we want 'libvirt.dac' as a prefix for any xattrs we set from this DAC driver.
@@ -265,11 +406,8 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU }
static int -virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +virSecurityDACChown(const char *path, uid_t uid, gid_t gid) { - VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", - path, (long) uid, (long) gid); - if (chown(path, uid, gid) < 0) { struct stat sb; int chown_errno = errno;
In the traditional chown() codepath, you're never preserving the original uid/gid values in an xattr. It not unknown for admins to mount filesystems with '-o noacl', which blocks use of ACLs, but still allows for xattrs to be used by apps. So we can preserve uid/gid in this case
+ + /* Oops, something went wrong. If ACL or XATTR are not + * supported fall back to chown then. */
- /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); + rc = virSecurityDACChown(path, 0, 0);
Dropping this comment is bogus since you've not fixed the problem it referred to. 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 :|