On Tue, Mar 08, 2011 at 10:13:44PM -0700, Eric Blake wrote:
Device names can be manipulated, so it is better to also log the major/minor device number corresponding to the cgroup ACL changes that libvirt made. This required some refactoring of the relatively new qemu cgroup audit code.
Also, qemuSetupChardevCgroup was only auditing on failure, not success.
+/* Return rdev=nn:mm in hex for block and character devices, rdev=? + * for other file types or stat failure, or NULL on allocation + * failure. */ +#if defined major && defined minor +static char * +qemuAuditGetRdev(const char *path) +{ + char *ret; + struct stat sb; + + if (stat(path, &sb) == 0 && + (S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode))) { + int maj = major(sb.st_rdev); + int min = minor(sb.st_rdev); + virAsprintf(&ret, "rdev=%02X:%02X", maj, min); + } else { + ret = strdup("rdev=?"); + } + return ret; +} +#else +static char * +qemuAuditGetRdev(const char *path ATTRIBUTE_UNUSED) +{ + return strdup("rdev=?"); +} +#endif
Rather than have the two strdup("rdev=?") calls, I reckon it would be better to just return NULL. Then the caller can just check for NULL itself & fallback to a static "rdev=?". In fact, perhaps this should just do virAsprintf(&ret, "%02X:%02X", maj, min); And...
+void +qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup, + const char *reason, const char *path, int rc) +{ + char *detail; + char *rdev; + char *extra; + + /* Nothing to audit for regular files. */ + if (rc > 0) + return; + + if (!(detail = virAuditEncode("path", path)) || + !(rdev = qemuAuditGetRdev(path)) || + virAsprintf(&extra, "path path=%s %s", path, rdev) < 0) {
...here do virAsprintf(&extra, "path path=%s rdev=%s", path, VIR_AUDIT_STR(rdev)) < 0) { ACK, to the rest of the patch though. 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 :|