On Wed, Mar 06, 2013 at 03:05:56PM +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. > --- > src/security/security_dac.c | 32 +++++++++++++++++++++++++++----- > 1 file changed, 27 insertions(+), 5 deletions(-) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 76a1dc6..8805a5b 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -329,12 +329,12 @@ virSecurityDACGetRememberedLabel(const char *path, > goto cleanup; > ret = refCount; > } else { > - if (virFileGetAttr(path, SECURITY_DAC_XATTR_OLD_OWNER, &oldOwner) < 0 || > - !oldOwner) > - return ret; > + if (!virFileGetAttr(path, SECURITY_DAC_XATTR_OLD_OWNER, &oldOwner) && > + oldOwner) { > > - if (parseIds(oldOwner, user, group) < 0) > - goto cleanup; > + if (parseIds(oldOwner, user, group) < 0) > + goto cleanup; > + } > > virFileRemoveAttr(path, SECURITY_DAC_XATTR_REFCOUNT); > virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_OWNER); > @@ -384,6 +384,9 @@ virSecurityDACSetOwnership(const char *path, > gid_t gid, > bool remember) > { > + virErrorPtr err; > + int rv; > + > VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", > path, (long) uid, (long) gid); > > @@ -391,6 +394,25 @@ virSecurityDACSetOwnership(const char *path, > virSecurityDACRememberLabel(path) < 0) > return -1; > > + if (remember) { > + if ((rv = virFileSetACL(path, uid, S_IRUSR | S_IWUSR)) == 0) { > + /* No chown is necessary, so remove oldOwner xattr. */ > + virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_OWNER); Ewww, just re-arrange this code, so that you don't set the xattr in the first place when you use ACLs. > + } > + } else { > + rv = virFileRemoveACL(path, uid); And this should just be done directly in the restore method 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