11:13 p.m.
This supercedes the unreviewed v1:
https://www.redhat.com/archives/libvir-list/2011-March/msg00257.html
The more I kept working on this, the more things I found that needed
fixing. As it is, it's too late for me tonight, so patch 8 is
currently untested, and patch 9/8 is needed to audit the hotplug
devices used in 'virsh attach-interface'. But earlier patches are
in good shape, so I want to start the review.
Perhaps this series should be re-arranged a bit; let me know if you
want the final version to see patches in any different order (although
due to some of my renames, it will take me longer to do rebasing
that shuffles patch order).
Eric Blake (8):
audit: tweak audit messages to match conventions
audit: split cgroup audit types to allow more information
audit: also audit cgroup controller path
audit: audit use of /dev/vhost-net
audit: rename remaining qemu audit functions
cgroup: allow fine-tuning of device ACL permissions
audit: also audit cgroup ACL permissions
qemu: support vhost in attach-interface
src/libvirt_private.syms | 1 +
src/lxc/lxc_controller.c | 9 +-
src/qemu/qemu_audit.c | 263 ++++++++++++++++++++++++++++++++++-----------
src/qemu/qemu_audit.h | 83 +++++++++------
src/qemu/qemu_cgroup.c | 57 +++++-----
src/qemu/qemu_command.c | 10 +-
src/qemu/qemu_command.h | 5 +
src/qemu/qemu_driver.c | 44 ++++----
src/qemu/qemu_hotplug.c | 94 ++++++++++++-----
src/qemu/qemu_migration.c | 14 ++--
src/qemu/qemu_process.c | 6 +-
src/util/cgroup.c | 63 ++++++++----
src/util/cgroup.h | 31 +++++-
13 files changed, 466 insertions(+), 214 deletions(-)
--
1.7.4
11:13 p.m.
New subject: [libvirt] [PATCHv2 1/8] audit: tweak audit messages to match conventions
* src/qemu/qemu_audit.c (qemuDomainHostdevAudit): Avoid use of
"type", which has a pre-defined meaning.
(qemuDomainCgroupAudit): Likewise, as well as "item".
---
v2: no real change
src/qemu/qemu_audit.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index b1948c8..0f954c0 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -159,7 +159,7 @@ qemuDomainHostdevAudit(virDomainObjPtr vm,
}
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=dev reason=%s %s uuid=%s type=%s %s",
+ "resrc=dev reason=%s %s uuid=%s bus=%s %s",
reason, vmname, uuidstr,
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type),
device);
@@ -200,14 +200,14 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm,
return;
}
if (name &&
- !(detail = virAuditEncode(STREQ(item, "path") ? "path" :
"type",
+ !(detail = virAuditEncode(STREQ(item, "path") ? "path" :
"category",
name))) {
VIR_WARN0("OOM while encoding audit message");
goto cleanup;
}
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s item=%s%s%s",
+ "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
reason, vmname, uuidstr,
item, detail ? " " : "", detail ? detail :
"");
--
1.7.4
9:02 a.m.
New subject: [libvirt] [PATCHv2 1/8] audit: tweak audit messages to match conventions
On Tue, Mar 08, 2011 at 10:13:43PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.c (qemuDomainHostdevAudit): Avoid use of
"type", which has a pre-defined meaning.
(qemuDomainCgroupAudit): Likewise, as well as "item".
---
v2: no real change
src/qemu/qemu_audit.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
ACK
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 :|
11:13 p.m.
New subject: [libvirt] [PATCHv2 2/8] audit: split cgroup audit types to allow more information
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.
* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): Delete.
(qemuAuditCgroup, qemuAuditCgroupMajor, qemuAuditCgroupPath): New
prototypes.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Rename...
(qemuAuditCgroup): ...and drop a parameter.
(qemuAuditCgroupMajor, qemuAuditCgroupPath): New functions, to
allow listing device major/minor in audit.
(qemuAuditGetRdev): New helper function.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust callers.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupHostUsbDeviceCgroup, qemuSetupCgroup)
(qemuTeardownDiskPathDeny): Likewise.
(qemuSetupChardevCgroup): Likewise, fixing missing audit.
---
v2: fix a logic bug I noticed, choose nicer function names
src/qemu/qemu_audit.c | 126 ++++++++++++++++++++++++++++++++++++++++--------
src/qemu/qemu_audit.h | 22 ++++++--
src/qemu/qemu_cgroup.c | 29 ++++-------
src/qemu/qemu_driver.c | 10 +---
4 files changed, 135 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 0f954c0..56b0b74 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -23,12 +23,43 @@
#include <config.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
#include "qemu_audit.h"
#include "virtaudit.h"
#include "uuid.h"
#include "logging.h"
#include "memory.h"
+/* 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
+
void qemuDomainDiskAudit(virDomainObjPtr vm,
virDomainDiskDefPtr oldDef,
virDomainDiskDefPtr newDef,
@@ -106,7 +137,7 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
* qemuDomainHostdevAudit:
* @vm: domain making a change in pass-through host device
* @hostdev: device being attached or removed
- * @reason: one of "start, "attach", or "detach"
+ * @reason: one of "start", "attach", or "detach"
* @success: true if the device passthrough operation succeeded
*
* Log an audit message about an attempted device passthrough change.
@@ -172,51 +203,104 @@ cleanup:
/**
- * qemuDomainCgroupAudit:
+ * qemuAuditCgroup:
* @vm: domain making the cgroups ACL change
* @cgroup: cgroup that manages the devices
* @reason: either "allow" or "deny"
- * @item: one of "all", "path", or "major"
- * @name: NULL for @item of "all", device path for @item of "path",
and
- * string describing major device type for @item of "major"
+ * @extra: additional details, in the form "all",
+ * "major category=xyz maj=nn", or "path path=xyz dev=nn:mm" (the
+ * latter two are generated by qemuAuditCgroupMajor and
+ * qemuAuditCgroupPath).
* @success: true if the cgroup operation succeeded
*
* Log an audit message about an attempted cgroup device ACL change.
*/
-void qemuDomainCgroupAudit(virDomainObjPtr vm,
- virCgroupPtr cgroup ATTRIBUTE_UNUSED,
- const char *reason,
- const char *item,
- const char *name,
- bool success)
+void
+qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+ const char *reason, const char *extra, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
- char *detail = NULL;
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name))) {
VIR_WARN0("OOM while encoding audit message");
return;
}
- if (name &&
- !(detail = virAuditEncode(STREQ(item, "path") ? "path" :
"category",
- name))) {
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=cgroup reason=%s %s uuid=%s class=%s",
+ reason, vmname, uuidstr, extra);
+
+ VIR_FREE(vmname);
+}
+
+/**
+ * qemuAuditCgroupMajor:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @maj: the major number of the device category
+ * @name: a textual name for that device category, alphabetic only
+ * @success: true if the cgroup operation succeeded
+ *
+ * Log an audit message about an attempted cgroup device ACL change.
+ */
+void
+qemuAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup,
+ const char *reason, int maj, const char *name,
+ bool success)
+{
+ char *extra;
+
+ if (virAsprintf(&extra, "major category=%s maj=%02X", name, maj) <
0) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+
+ qemuAuditCgroup(vm, cgroup, reason, extra, success);
+
+ VIR_FREE(extra);
+}
+
+/**
+ * qemuAuditCgroupPath:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @path: the device being adjusted
+ * @rc: > 0 if not a device, 0 if success, < 0 if failure
+ *
+ * Log an audit message about an attempted cgroup device ACL change to
+ * a specific device.
+ */
+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) {
VIR_WARN0("OOM while encoding audit message");
goto cleanup;
}
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
- reason, vmname, uuidstr,
- item, detail ? " " : "", detail ? detail :
"");
+ qemuAuditCgroup(vm, cgroup, reason, extra, rc == 0);
cleanup:
- VIR_FREE(vmname);
+ VIR_FREE(extra);
VIR_FREE(detail);
+ VIR_FREE(rdev);
}
-
/**
* qemuDomainResourceAudit:
* @vm: domain making an integer resource change
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 247edde..53855e2 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -43,12 +43,22 @@ void qemuDomainHostdevAudit(virDomainObjPtr vm,
virDomainHostdevDefPtr def,
const char *reason,
bool success);
-void qemuDomainCgroupAudit(virDomainObjPtr vm,
- virCgroupPtr group,
- const char *reason,
- const char *item,
- const char *name,
- bool success);
+void qemuAuditCgroup(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *extra,
+ bool success);
+void qemuAuditCgroupMajor(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ int maj,
+ const char *name,
+ bool success);
+void qemuAuditCgroupPath(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *path,
+ int rc);
void qemuDomainMemoryAudit(virDomainObjPtr vm,
unsigned long long oldmem,
unsigned long long newmem,
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e71d3fa..ebf9ad5 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -67,9 +67,7 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
rc = virCgroupAllowDevicePath(data->cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -110,9 +108,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
rc = virCgroupDenyDevicePath(data->cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "deny",
"path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -155,9 +151,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk",
dev->source.data.file.path);
rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path);
- if (rc < 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path",
- dev->source.data.file.path, rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
+ dev->source.data.file.path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -178,9 +173,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path '%s' for USB device", path);
rc = virCgroupAllowDevicePath(data->cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -216,7 +209,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
qemuCgroupData data = { vm, cgroup };
rc = virCgroupDenyAllDevices(cgroup);
- qemuDomainCgroupAudit(vm, cgroup, "deny", "all", NULL, rc ==
0);
+ qemuAuditCgroup(vm, cgroup, "deny", "all", rc == 0);
if (rc != 0) {
if (rc == -EPERM) {
VIR_WARN0("Group devices ACL is not accessible, disabling
whitelisting");
@@ -234,7 +227,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"pty", rc == 0);
+ qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
+ "pty", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/pts/ devices"));
@@ -247,8 +241,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"sound",
- rc == 0);
+ qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
+ "sound", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/snd/ devices"));
@@ -259,8 +253,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup,
deviceACL[i]);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path",
- deviceACL[i], rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
if (rc < 0 &&
rc != -ENOENT) {
virReportSystemError(-rc,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f7cbad..0f7b5ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1964,8 +1964,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
goto endjob;
}
rc = virCgroupAllowDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path", path,
rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -2015,8 +2014,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path,
rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
path, vm->def->name, rc);
@@ -2048,9 +2046,7 @@ endjob:
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path",
path,
- rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
path, vm->def->name, rc);
--
1.7.4
9:07 a.m.
New subject: [libvirt] [PATCHv2 2/8] audit: split cgroup audit types to allow more information
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 :|
10:01 a.m.
New subject: [libvirt] [PATCHv2 2/8] audit: split cgroup audit types to allow more information
On 03/09/2011 08:07 AM, Daniel P. Berrange wrote:
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...
> + 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) {
Yeah, that's nicer. Will do.
ACK, to the rest of the patch though.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [PATCHv2 3/8] audit: also audit cgroup controller path
Although the cgroup device ACL controller path can be worked out
by researching the code, it is more efficient to include that
information directly in the audit message.
* src/util/cgroup.h (virCgroupPathOfController): New prototype.
* src/util/cgroup.c (virCgroupPathOfController): Export.
* src/libvirt_private.syms: Likewise.
* src/qemu/qemu_audit.c (qemuAuditCgroup): Use it.
---
v2: rebase onto other changes
src/libvirt_private.syms | 1 +
src/qemu/qemu_audit.c | 19 ++++++++++++++++---
src/util/cgroup.c | 8 ++++----
src/util/cgroup.h | 5 +++++
4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index efcf3c5..c0da78e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -79,6 +79,7 @@ virCgroupKill;
virCgroupKillRecursive;
virCgroupKillPainfully;
virCgroupMounted;
+virCgroupPathOfController;
virCgroupRemove;
virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 56b0b74..08eb431 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -216,11 +216,13 @@ cleanup:
* Log an audit message about an attempted cgroup device ACL change.
*/
void
-qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup,
const char *reason, const char *extra, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
+ char *controller = NULL;
+ char *detail;
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name))) {
@@ -228,11 +230,22 @@ qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup
ATTRIBUTE_UNUSED,
return;
}
+ virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_DEVICES,
+ NULL, &controller);
+
+ if (!(detail = virAuditEncode("cgroup", VIR_AUDIT_STR(controller)))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s",
- reason, vmname, uuidstr, extra);
+ "resrc=cgroup reason=%s %s uuid=%s %s class=%s",
+ reason, vmname, uuidstr, detail, extra);
+cleanup:
VIR_FREE(vmname);
+ VIR_FREE(controller);
+ VIR_FREE(detail);
}
/**
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 8551acd..46358ab 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -254,10 +254,10 @@ static int virCgroupDetect(virCgroupPtr group)
#endif
-static int virCgroupPathOfController(virCgroupPtr group,
- int controller,
- const char *key,
- char **path)
+int virCgroupPathOfController(virCgroupPtr group,
+ int controller,
+ const char *key,
+ char **path)
{
if (controller == -1) {
int i;
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index d468cb3..b3c5f27 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -40,6 +40,11 @@ int virCgroupForDomain(virCgroupPtr driver,
virCgroupPtr *group,
int create);
+int virCgroupPathOfController(virCgroupPtr group,
+ int controller,
+ const char *key,
+ char **path);
+
int virCgroupAddTask(virCgroupPtr group, pid_t pid);
int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
--
1.7.4
9:10 a.m.
New subject: [libvirt] [PATCHv2 3/8] audit: also audit cgroup controller path
On Tue, Mar 08, 2011 at 10:13:45PM -0700, Eric Blake wrote:
Although the cgroup device ACL controller path can be worked out
by researching the code, it is more efficient to include that
information directly in the audit message.
* src/util/cgroup.h (virCgroupPathOfController): New prototype.
* src/util/cgroup.c (virCgroupPathOfController): Export.
* src/libvirt_private.syms: Likewise.
* src/qemu/qemu_audit.c (qemuAuditCgroup): Use it.
---
v2: rebase onto other changes
src/libvirt_private.syms | 1 +
src/qemu/qemu_audit.c | 19 ++++++++++++++++---
src/util/cgroup.c | 8 ++++----
src/util/cgroup.h | 5 +++++
4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index efcf3c5..c0da78e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -79,6 +79,7 @@ virCgroupKill;
virCgroupKillRecursive;
virCgroupKillPainfully;
virCgroupMounted;
+virCgroupPathOfController;
virCgroupRemove;
virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 56b0b74..08eb431 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -216,11 +216,13 @@ cleanup:
* Log an audit message about an attempted cgroup device ACL change.
*/
void
-qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup,
const char *reason, const char *extra, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
+ char *controller = NULL;
+ char *detail;
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name))) {
@@ -228,11 +230,22 @@ qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup
ATTRIBUTE_UNUSED,
return;
}
+ virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_DEVICES,
+ NULL, &controller);
+
+ if (!(detail = virAuditEncode("cgroup", VIR_AUDIT_STR(controller)))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s",
- reason, vmname, uuidstr, extra);
+ "resrc=cgroup reason=%s %s uuid=%s %s class=%s",
+ reason, vmname, uuidstr, detail, extra);
I think perhaps we should make a better effort to output the
audit event if creating 'detail' fails. eg remove the goto cleanup
and do 'detail ? detail : "cgroup=?"' here
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 :|
10:59 a.m.
New subject: [libvirt] [PATCHv2 3/8] audit: also audit cgroup controller path
On 03/09/2011 08:10 AM, Daniel P. Berrange wrote:
> + virCgroupPathOfController(cgroup,
VIR_CGROUP_CONTROLLER_DEVICES,
> + NULL, &controller);
> +
> + if (!(detail = virAuditEncode("cgroup", VIR_AUDIT_STR(controller))))
{
> + VIR_WARN0("OOM while encoding audit message");
> + goto cleanup;
> + }
> +
> VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
> - "resrc=cgroup reason=%s %s uuid=%s class=%s",
> - reason, vmname, uuidstr, extra);
> + "resrc=cgroup reason=%s %s uuid=%s %s class=%s",
> + reason, vmname, uuidstr, detail, extra);
I think perhaps we should make a better effort to output the
audit event if creating 'detail' fails. eg remove the goto cleanup
and do 'detail ? detail : "cgroup=?"' here
Sure. The VIR_AUDIT call is then likely to fail for the same OOM
reason, but we might as well try it rather than giving up right away,
since the cgroup mount point is not the central detail of the audit message.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [PATCHv2 4/8] audit: audit use of /dev/vhost-net
Passing the vhost net device fd to qemu is worth an audit point,
since it is a kernel-managed device.
This patch points out that qemu still can't hot-plug and hot-unplug
vhost-net interfaces.
* src/qemu/qemu_audit.h (qemuAuditNetVhost): New prototype.
* src/qemu/qemu_audit.c (qemuAuditNetVhost): New function.
* src/qemu/qemu_command.c (qemuOpenVhostNet): Add audit point and
new parameter.
(qemuBuildCommandLine): Adjust caller.
---
v2: new patch; still missing an audit point for where /dev/net/tun
is opened, and the name should probably be qemuAuditNetDevice
(since it is feasible to open just /dev/net/tun and not /dev/vhost-net
when the xml asks for that). Perhaps should be shuffled to live
after patch 8/8.
src/qemu/qemu_audit.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_audit.h | 5 +++++
src/qemu/qemu_command.c | 8 +++++---
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 08eb431..1965a20 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -132,6 +132,46 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
VIR_FREE(vmname);
}
+/**
+ * qemuAuditNetVhost:
+ * @vm: domain receiving a vhost-net device
+ * @def: details of network device being attached or removed
+ * @device: device being attached
+ * @reason: one of "start", "attach", or "detach"
+ * @success: true if the device passthrough operation succeeded
+ *
+ * Log an audit message about an attempted device passthrough change.
+ */
+void
+qemuAuditNetVhost(virDomainDefPtr vmDef,
+ virDomainNetDefPtr netDef, const char *device,
+ const char *reason, bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char macstr[VIR_MAC_STRING_BUFLEN];
+ char *vmname;
+ char *devname;
+ char *rdev;
+
+ virUUIDFormat(vmDef->uuid, uuidstr);
+ virFormatMacAddr(netDef->mac, macstr);
+ if (!(vmname = virAuditEncode("vm", vmDef->name)) ||
+ !(devname = virAuditEncode("path", device)) ||
+ !(rdev = qemuAuditGetRdev(device))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=net reason=%s %s uuid=%s net='%s' %s %s",
+ reason, vmname, uuidstr,
+ macstr, devname, rdev);
+
+cleanup:
+ VIR_FREE(vmname);
+ VIR_FREE(devname);
+ VIR_FREE(rdev);
+}
/**
* qemuDomainHostdevAudit:
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 53855e2..9f08362 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -39,6 +39,11 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
virDomainNetDefPtr newDef,
const char *reason,
bool success);
+void qemuAuditNetVhost(virDomainDefPtr vmDef,
+ virDomainNetDefPtr netDef,
+ const char *device,
+ const char *reason,
+ bool success);
void qemuDomainHostdevAudit(virDomainObjPtr vm,
virDomainHostdevDefPtr def,
const char *reason,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 198a4e2..d5f5a70 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -35,6 +35,7 @@
#include "uuid.h"
#include "c-ctype.h"
#include "domain_nwfilter.h"
+#include "qemu_audit.h"
#include <sys/utsname.h>
#include <sys/stat.h>
@@ -304,11 +305,11 @@ cleanup:
static int
-qemuOpenVhostNet(virDomainNetDefPtr net,
+qemuOpenVhostNet(virDomainDefPtr def,
+ virDomainNetDefPtr net,
virBitmapPtr qemuCaps,
int *vhostfd)
{
-
*vhostfd = -1; /* assume we won't use vhost */
/* If the config says explicitly to not use vhost, return now */
@@ -343,6 +344,7 @@ qemuOpenVhostNet(virDomainNetDefPtr net,
}
*vhostfd = open("/dev/vhost-net", O_RDWR);
+ qemuAuditNetVhost(def, net, "/dev/vhost-net", "start", *vhostfd
>= 0);
/* If the config says explicitly to use vhost and we couldn't open it,
* report an error.
@@ -3495,7 +3497,7 @@ qemuBuildCommandLine(virConnectPtr conn,
network device */
int vhostfd;
- if (qemuOpenVhostNet(net, qemuCaps, &vhostfd) < 0)
+ if (qemuOpenVhostNet(def, net, qemuCaps, &vhostfd) < 0)
goto error;
if (vhostfd >= 0) {
virCommandTransferFD(cmd, vhostfd);
--
1.7.4
9:19 a.m.
New subject: [libvirt] [PATCHv2 4/8] audit: audit use of /dev/vhost-net
On Tue, Mar 08, 2011 at 10:13:46PM -0700, Eric Blake wrote:
Passing the vhost net device fd to qemu is worth an audit point,
since it is a kernel-managed device.
This patch points out that qemu still can't hot-plug and hot-unplug
vhost-net interfaces.
* src/qemu/qemu_audit.h (qemuAuditNetVhost): New prototype.
* src/qemu/qemu_audit.c (qemuAuditNetVhost): New function.
* src/qemu/qemu_command.c (qemuOpenVhostNet): Add audit point and
new parameter.
(qemuBuildCommandLine): Adjust caller.
---
v2: new patch; still missing an audit point for where /dev/net/tun
is opened, and the name should probably be qemuAuditNetDevice
(since it is feasible to open just /dev/net/tun and not /dev/vhost-net
when the xml asks for that). Perhaps should be shuffled to live
after patch 8/8.
There are several devices for networking
- With type=bridge or type=network, /dev/net/tun is opened to
get a FD for a tap device instance
- With type=direct, /dev/tap%d is opened to get an FD for
a macvtap device instance
In both cases, /dev/vhost-net can *also* be opened. The tun/macvtap
device provides the connectivity to the host NIC. The vhost-net
device provides kernel acceleration for the QEMU NIC backend
So I think we need to be auditing all of these devices, if we're
going todo any of them
+void
+qemuAuditNetVhost(virDomainDefPtr vmDef,
+ virDomainNetDefPtr netDef, const char *device,
+ const char *reason, bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char macstr[VIR_MAC_STRING_BUFLEN];
+ char *vmname;
+ char *devname;
+ char *rdev;
+
+ virUUIDFormat(vmDef->uuid, uuidstr);
+ virFormatMacAddr(netDef->mac, macstr);
+ if (!(vmname = virAuditEncode("vm", vmDef->name)) ||
+ !(devname = virAuditEncode("path", device)) ||
+ !(rdev = qemuAuditGetRdev(device))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=net reason=%s %s uuid=%s net='%s' %s %s",
+ reason, vmname, uuidstr,
+ macstr, devname, rdev);
A similar thought here about rdev as per the earlier patch
in the series
--
|: 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 :|
2:50 p.m.
New subject: [libvirt] [PATCHv2 4/8] audit: audit use of /dev/vhost-net
On 03/09/2011 08:19 AM, Daniel P. Berrange wrote:
There are several devices for networking
- With type=bridge or type=network, /dev/net/tun is opened to
get a FD for a tap device instance
- With type=direct, /dev/tap%d is opened to get an FD for
a macvtap device instance
In both cases, /dev/vhost-net can *also* be opened. The tun/macvtap
device provides the connectivity to the host NIC. The vhost-net
device provides kernel acceleration for the QEMU NIC backend
So I think we need to be auditing all of these devices, if we're
going todo any of them
Agreed, and did that in v3:
https://www.redhat.com/archives/libvir-list/2011-March/msg00416.html
> + if (!(vmname = virAuditEncode("vm",
vmDef->name)) ||
> + !(devname = virAuditEncode("path", device)) ||
> + !(rdev = qemuAuditGetRdev(device))) {
> + VIR_WARN0("OOM while encoding audit message");
> + goto cleanup;
> + }
> +
> + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
> + "resrc=net reason=%s %s uuid=%s net='%s' %s %s",
> + reason, vmname, uuidstr,
> + macstr, devname, rdev);
A similar thought here about rdev as per the earlier patch
in the series
Yep, that got fixed while rebasing on the earlier part of the series.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [PATCHv2 5/8] audit: rename remaining qemu audit functions
* src/qemu/qemu_audit.h: The pattern qemuDomainXXXAudit is
inconsistent; prefer qemuAuditXXX instead.
* src/qemu/qemu_audit.c: Reflect the renames.
* src/qemu/qemu_driver.c: Likewise.
* src/qemu/qemu_hotplug.c: Likewise.
* src/qemu/qemu_process.c: Likewise.
* src/qemu/qemu_migration.c: Likewise.
---
v2: new patch
src/qemu/qemu_audit.c | 84 +++++++++++++++++++++------------------------
src/qemu/qemu_audit.h | 58 ++++++++++++++++--------------
src/qemu/qemu_driver.c | 25 ++++++-------
src/qemu/qemu_hotplug.c | 40 +++++++++++-----------
src/qemu/qemu_migration.c | 14 ++++----
src/qemu/qemu_process.c | 6 ++--
6 files changed, 112 insertions(+), 115 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 1965a20..705cab7 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -60,11 +60,10 @@ qemuAuditGetRdev(const char *path ATTRIBUTE_UNUSED)
}
#endif
-void qemuDomainDiskAudit(virDomainObjPtr vm,
- virDomainDiskDefPtr oldDef,
- virDomainDiskDefPtr newDef,
- const char *reason,
- bool success)
+void
+qemuAuditDisk(virDomainObjPtr vm,
+ virDomainDiskDefPtr oldDef, virDomainDiskDefPtr newDef,
+ const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
@@ -102,11 +101,10 @@ cleanup:
}
-void qemuDomainNetAudit(virDomainObjPtr vm,
- virDomainNetDefPtr oldDef,
- virDomainNetDefPtr newDef,
- const char *reason,
- bool success)
+void
+qemuAuditNet(virDomainObjPtr vm,
+ virDomainNetDefPtr oldDef, virDomainNetDefPtr newDef,
+ const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char newMacstr[VIR_MAC_STRING_BUFLEN];
@@ -174,7 +172,7 @@ cleanup:
}
/**
- * qemuDomainHostdevAudit:
+ * qemuAuditHostdev:
* @vm: domain making a change in pass-through host device
* @hostdev: device being attached or removed
* @reason: one of "start", "attach", or "detach"
@@ -183,10 +181,8 @@ cleanup:
* Log an audit message about an attempted device passthrough change.
*/
void
-qemuDomainHostdevAudit(virDomainObjPtr vm,
- virDomainHostdevDefPtr hostdev,
- const char *reason,
- bool success)
+qemuAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev,
+ const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
@@ -355,7 +351,7 @@ cleanup:
}
/**
- * qemuDomainResourceAudit:
+ * qemuAuditResource:
* @vm: domain making an integer resource change
* @resource: name of the resource: "mem" or "vcpu"
* @oldval: the old value of the resource
@@ -366,12 +362,9 @@ cleanup:
* Log an audit message about an attempted resource change.
*/
static void
-qemuDomainResourceAudit(virDomainObjPtr vm,
- const char *resource,
- unsigned long long oldval,
- unsigned long long newval,
- const char *reason,
- bool success)
+qemuAuditResource(virDomainObjPtr vm, const char *resource,
+ unsigned long long oldval, unsigned long long newval,
+ const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
@@ -391,26 +384,24 @@ qemuDomainResourceAudit(virDomainObjPtr vm,
}
void
-qemuDomainMemoryAudit(virDomainObjPtr vm,
- unsigned long long oldmem, unsigned long long newmem,
- const char *reason, bool success)
+qemuAuditMemory(virDomainObjPtr vm,
+ unsigned long long oldmem, unsigned long long newmem,
+ const char *reason, bool success)
{
- return qemuDomainResourceAudit(vm, "mem", oldmem, newmem, reason,
success);
+ return qemuAuditResource(vm, "mem", oldmem, newmem, reason, success);
}
void
-qemuDomainVcpuAudit(virDomainObjPtr vm,
- unsigned int oldvcpu, unsigned int newvcpu,
- const char *reason, bool success)
+qemuAuditVcpu(virDomainObjPtr vm,
+ unsigned int oldvcpu, unsigned int newvcpu,
+ const char *reason, bool success)
{
- return qemuDomainResourceAudit(vm, "vcpu", oldvcpu, newvcpu, reason,
- success);
+ return qemuAuditResource(vm, "vcpu", oldvcpu, newvcpu, reason, success);
}
-static void qemuDomainLifecycleAudit(virDomainObjPtr vm,
- const char *op,
- const char *reason,
- bool success)
+static void
+qemuAuditLifecycle(virDomainObjPtr vm, const char *op,
+ const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
@@ -429,39 +420,42 @@ static void qemuDomainLifecycleAudit(virDomainObjPtr vm,
}
-void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success)
+void
+qemuAuditDomainStart(virDomainObjPtr vm, const char *reason, bool success)
{
int i;
for (i = 0 ; i < vm->def->ndisks ; i++) {
virDomainDiskDefPtr disk = vm->def->disks[i];
if (disk->src) /* Skips CDROM without media initially inserted */
- qemuDomainDiskAudit(vm, NULL, disk, "start", true);
+ qemuAuditDisk(vm, NULL, disk, "start", true);
}
for (i = 0 ; i < vm->def->nnets ; i++) {
virDomainNetDefPtr net = vm->def->nets[i];
- qemuDomainNetAudit(vm, NULL, net, "start", true);
+ qemuAuditNet(vm, NULL, net, "start", true);
}
for (i = 0 ; i < vm->def->nhostdevs ; i++) {
virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
- qemuDomainHostdevAudit(vm, hostdev, "start", true);
+ qemuAuditHostdev(vm, hostdev, "start", true);
}
- qemuDomainMemoryAudit(vm, 0, vm->def->mem.cur_balloon, "start",
true);
- qemuDomainVcpuAudit(vm, 0, vm->def->vcpus, "start", true);
+ qemuAuditMemory(vm, 0, vm->def->mem.cur_balloon, "start", true);
+ qemuAuditVcpu(vm, 0, vm->def->vcpus, "start", true);
- qemuDomainLifecycleAudit(vm, "start", reason, success);
+ qemuAuditLifecycle(vm, "start", reason, success);
}
-void qemuDomainStopAudit(virDomainObjPtr vm, const char *reason)
+void
+qemuAuditDomainStop(virDomainObjPtr vm, const char *reason)
{
- qemuDomainLifecycleAudit(vm, "stop", reason, true);
+ qemuAuditLifecycle(vm, "stop", reason, true);
}
-void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success)
+void
+qemuAuditSecurityLabel(virDomainObjPtr vm, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 9f08362..84874a2 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -27,27 +27,30 @@
# include "domain_conf.h"
# include "cgroup.h"
-void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success);
-void qemuDomainStopAudit(virDomainObjPtr vm, const char *reason);
-void qemuDomainDiskAudit(virDomainObjPtr vm,
- virDomainDiskDefPtr oldDef,
- virDomainDiskDefPtr newDef,
- const char *reason,
- bool success);
-void qemuDomainNetAudit(virDomainObjPtr vm,
- virDomainNetDefPtr oldDef,
- virDomainNetDefPtr newDef,
- const char *reason,
- bool success);
+void qemuAuditDomainStart(virDomainObjPtr vm,
+ const char *reason,
+ bool success);
+void qemuAuditDomainStop(virDomainObjPtr vm,
+ const char *reason);
+void qemuAuditDisk(virDomainObjPtr vm,
+ virDomainDiskDefPtr oldDef,
+ virDomainDiskDefPtr newDef,
+ const char *reason,
+ bool success);
+void qemuAuditNet(virDomainObjPtr vm,
+ virDomainNetDefPtr oldDef,
+ virDomainNetDefPtr newDef,
+ const char *reason,
+ bool success);
void qemuAuditNetVhost(virDomainDefPtr vmDef,
virDomainNetDefPtr netDef,
const char *device,
const char *reason,
bool success);
-void qemuDomainHostdevAudit(virDomainObjPtr vm,
- virDomainHostdevDefPtr def,
- const char *reason,
- bool success);
+void qemuAuditHostdev(virDomainObjPtr vm,
+ virDomainHostdevDefPtr def,
+ const char *reason,
+ bool success);
void qemuAuditCgroup(virDomainObjPtr vm,
virCgroupPtr group,
const char *reason,
@@ -64,16 +67,17 @@ void qemuAuditCgroupPath(virDomainObjPtr vm,
const char *reason,
const char *path,
int rc);
-void qemuDomainMemoryAudit(virDomainObjPtr vm,
- unsigned long long oldmem,
- unsigned long long newmem,
- const char *reason,
- bool success);
-void qemuDomainVcpuAudit(virDomainObjPtr vm,
- unsigned int oldvcpu,
- unsigned int newvcpu,
- const char *reason,
- bool success);
-void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success);
+void qemuAuditMemory(virDomainObjPtr vm,
+ unsigned long long oldmem,
+ unsigned long long newmem,
+ const char *reason,
+ bool success);
+void qemuAuditVcpu(virDomainObjPtr vm,
+ unsigned int oldvcpu,
+ unsigned int newvcpu,
+ const char *reason,
+ bool success);
+void qemuAuditSecurityLabel(virDomainObjPtr vm,
+ bool success);
#endif /* __QEMU_AUDIT_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f7b5ec..1981cdf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1266,7 +1266,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char
*xml,
if (qemuProcessStart(conn, driver, vm, NULL,
(flags & VIR_DOMAIN_START_PAUSED) != 0,
-1, NULL, VIR_VM_OP_CREATE) < 0) {
- qemuDomainStartAudit(vm, "booted", false);
+ qemuAuditDomainStart(vm, "booted", false);
if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains,
vm);
@@ -1277,7 +1277,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char
*xml,
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_BOOTED);
- qemuDomainStartAudit(vm, "booted", true);
+ qemuAuditDomainStart(vm, "booted", true);
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
if (dom) dom->id = vm->def->id;
@@ -1492,7 +1492,7 @@ static int qemudDomainDestroy(virDomainPtr dom) {
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
- qemuDomainStopAudit(vm, "destroyed");
+ qemuAuditDomainStop(vm, "destroyed");
if (!vm->persistent) {
if (qemuDomainObjEndJob(vm) > 0)
@@ -1604,8 +1604,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long
newmem) {
qemuDomainObjEnterMonitor(vm);
r = qemuMonitorSetBalloon(priv->mon, newmem);
qemuDomainObjExitMonitor(vm);
- qemuDomainMemoryAudit(vm, vm->def->mem.cur_balloon, newmem,
"update",
- r == 1);
+ qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r ==
1);
if (r < 0)
goto endjob;
@@ -2024,7 +2023,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
/* Shut it down */
qemuProcessStop(driver, vm, 0);
- qemuDomainStopAudit(vm, "saved");
+ qemuAuditDomainStop(vm, "saved");
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SAVED);
@@ -2420,7 +2419,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
endjob:
if ((ret == 0) && (flags & VIR_DUMP_CRASH)) {
qemuProcessStop(driver, vm, 0);
- qemuDomainStopAudit(vm, "crashed");
+ qemuAuditDomainStop(vm, "crashed");
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
@@ -2552,7 +2551,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int
nvcpus)
cleanup:
qemuDomainObjExitMonitor(vm);
- qemuDomainVcpuAudit(vm, oldvcpus, nvcpus, "update", rc == 1);
+ qemuAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1);
return ret;
unsupported:
@@ -3330,14 +3329,14 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
*read_pid = -1;
if (ret < 0) {
- qemuDomainStartAudit(vm, "restored", false);
+ qemuAuditDomainStart(vm, "restored", false);
goto out;
}
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_RESTORED);
- qemuDomainStartAudit(vm, "restored", true);
+ qemuAuditDomainStart(vm, "restored", true);
if (event)
qemuDomainEventQueue(driver, event);
@@ -3675,7 +3674,7 @@ static int qemudDomainObjStart(virConnectPtr conn,
ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, -1, NULL,
VIR_VM_OP_CREATE);
- qemuDomainStartAudit(vm, "booted", ret >= 0);
+ qemuAuditDomainStart(vm, "booted", ret >= 0);
if (ret >= 0) {
virDomainEventPtr event =
virDomainEventNewFromObj(vm,
@@ -6358,7 +6357,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
false, -1, NULL, VIR_VM_OP_CREATE);
- qemuDomainStartAudit(vm, "from-snapshot", rc >= 0);
+ qemuAuditDomainStart(vm, "from-snapshot", rc >= 0);
if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0)
goto endjob;
if (rc < 0)
@@ -6391,7 +6390,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (virDomainObjIsActive(vm)) {
qemuProcessStop(driver, vm, 0);
- qemuDomainStopAudit(vm, "from-snapshot");
+ qemuAuditDomainStop(vm, "from-snapshot");
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9991031..7895062 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -108,7 +108,7 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, origdisk, disk, "update", ret >= 0);
+ qemuAuditDisk(vm, origdisk, disk, "update", ret >= 0);
if (ret < 0)
goto error;
@@ -203,7 +203,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, NULL, disk, "attach", ret >= 0);
+ qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -435,7 +435,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, NULL, disk, "attach", ret >= 0);
+ qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -518,7 +518,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, NULL, disk, "attach", ret >= 0);
+ qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -653,13 +653,13 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorAddNetdev(priv->mon, netstr) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, NULL, net, "attach", false);
+ qemuAuditNet(vm, NULL, net, "attach", false);
goto try_tapfd_close;
}
} else {
if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, NULL, net, "attach", false);
+ qemuAuditNet(vm, NULL, net, "attach", false);
goto try_tapfd_close;
}
}
@@ -685,14 +685,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, NULL, net, "attach", false);
+ qemuAuditNet(vm, NULL, net, "attach", false);
goto try_remove;
}
} else {
if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
&guestAddr) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, NULL, net, "attach", false);
+ qemuAuditNet(vm, NULL, net, "attach", false);
goto try_remove;
}
net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
@@ -700,7 +700,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, NULL, net, "attach", true);
+ qemuAuditNet(vm, NULL, net, "attach", true);
ret = 0;
@@ -842,7 +842,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
memcpy(&hostdev->info.addr.pci, &guestAddr, sizeof(guestAddr));
}
- qemuDomainHostdevAudit(vm, hostdev, "attach", ret == 0);
+ qemuAuditHostdev(vm, hostdev, "attach", ret == 0);
if (ret < 0)
goto error;
@@ -919,7 +919,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver,
hostdev->source.subsys.u.usb.bus,
hostdev->source.subsys.u.usb.device);
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainHostdevAudit(vm, hostdev, "attach", ret == 0);
+ qemuAuditHostdev(vm, hostdev, "attach", ret == 0);
if (ret < 0)
goto error;
@@ -1194,7 +1194,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
+ qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0)
@@ -1277,7 +1277,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver,
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
+ qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
virDomainDiskRemove(vm->def, i);
@@ -1486,14 +1486,14 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(vm);
- qemuDomainNetAudit(vm, detach, NULL, "detach", false);
+ qemuAuditNet(vm, detach, NULL, "detach", false);
goto cleanup;
}
} else {
if (qemuMonitorRemovePCIDevice(priv->mon,
&detach->info.addr.pci) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, detach, NULL, "detach", false);
+ qemuAuditNet(vm, detach, NULL, "detach", false);
goto cleanup;
}
}
@@ -1502,19 +1502,19 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, detach, NULL, "detach", false);
+ qemuAuditNet(vm, detach, NULL, "detach", false);
goto cleanup;
}
} else {
if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, detach, NULL, "detach", false);
+ qemuAuditNet(vm, detach, NULL, "detach", false);
goto cleanup;
}
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainNetAudit(vm, detach, NULL, "detach", true);
+ qemuAuditNet(vm, detach, NULL, "detach", true);
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0)
@@ -1615,7 +1615,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver,
ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainHostdevAudit(vm, detach, "detach", ret == 0);
+ qemuAuditHostdev(vm, detach, "detach", ret == 0);
if (ret < 0)
return -1;
@@ -1714,7 +1714,7 @@ int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorDelDevice(priv->mon, detach->info.alias);
qemuDomainObjExitMonitorWithDriver(driver, vm);
- qemuDomainHostdevAudit(vm, detach, "detach", ret == 0);
+ qemuAuditHostdev(vm, detach, "detach", ret == 0);
if (ret < 0)
return -1;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 822cb18..8ce6e17 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -324,7 +324,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
-1, NULL, VIR_VM_OP_MIGRATE_IN_START);
VIR_FREE(migrateFrom);
if (internalret < 0) {
- qemuDomainStartAudit(vm, "migrated", false);
+ qemuAuditDomainStart(vm, "migrated", false);
/* Note that we don't set an error here because qemuProcessStart
* should have already done that.
*/
@@ -338,7 +338,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
if (virFDStreamConnectUNIX(st,
unixfile,
false) < 0) {
- qemuDomainStartAudit(vm, "migrated", false);
+ qemuAuditDomainStart(vm, "migrated", false);
qemuProcessStop(driver, vm, 0);
if (!vm->persistent) {
if (qemuDomainObjEndJob(vm) > 0)
@@ -351,7 +351,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
goto endjob;
}
- qemuDomainStartAudit(vm, "migrated", true);
+ qemuAuditDomainStart(vm, "migrated", true);
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
@@ -532,7 +532,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver,
snprintf (migrateFrom, sizeof (migrateFrom), "tcp:0.0.0.0:%d", this_port);
if (qemuProcessStart(dconn, driver, vm, migrateFrom, true,
-1, NULL, VIR_VM_OP_MIGRATE_IN_START) < 0) {
- qemuDomainStartAudit(vm, "migrated", false);
+ qemuAuditDomainStart(vm, "migrated", false);
/* Note that we don't set an error here because qemuProcessStart
* should have already done that.
*/
@@ -544,7 +544,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver,
goto endjob;
}
- qemuDomainStartAudit(vm, "migrated", true);
+ qemuAuditDomainStart(vm, "migrated", true);
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_MIGRATED);
@@ -1089,7 +1089,7 @@ int qemuMigrationPerform(struct qemud_driver *driver,
/* Clean up the source domain. */
qemuProcessStop(driver, vm, 1);
- qemuDomainStopAudit(vm, "migrated");
+ qemuAuditDomainStop(vm, "migrated");
resume = 0;
event = virDomainEventNewFromObj(vm,
@@ -1268,7 +1268,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
}
} else {
qemuProcessStop(driver, vm, 1);
- qemuDomainStopAudit(vm, "failed");
+ qemuAuditDomainStop(vm, "failed");
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_FAILED);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ee1b4c4..d3b0691 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -131,7 +131,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
qemuProcessStop(driver, vm, 0);
- qemuDomainStopAudit(vm, hasError ? "failed" : "shutdown");
+ qemuAuditDomainStop(vm, hasError ? "failed" : "shutdown");
if (!vm->persistent)
virDomainRemoveInactive(&driver->domains, vm);
@@ -1935,10 +1935,10 @@ int qemuProcessStart(virConnectPtr conn,
then generate a security label for isolation */
VIR_DEBUG0("Generating domain security label (if required)");
if (virSecurityManagerGenLabel(driver->securityManager, vm) < 0) {
- qemuDomainSecurityLabelAudit(vm, false);
+ qemuAuditSecurityLabel(vm, false);
goto cleanup;
}
- qemuDomainSecurityLabelAudit(vm, true);
+ qemuAuditSecurityLabel(vm, true);
VIR_DEBUG0("Generating setting domain security labels (if required)");
if (virSecurityManagerSetAllLabel(driver->securityManager,
--
1.7.4
9:19 a.m.
New subject: [libvirt] [PATCHv2 5/8] audit: rename remaining qemu audit functions
On Tue, Mar 08, 2011 at 10:13:47PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.h: The pattern qemuDomainXXXAudit is
inconsistent; prefer qemuAuditXXX instead.
* src/qemu/qemu_audit.c: Reflect the renames.
* src/qemu/qemu_driver.c: Likewise.
* src/qemu/qemu_hotplug.c: Likewise.
* src/qemu/qemu_process.c: Likewise.
* src/qemu/qemu_migration.c: Likewise.
---
v2: new patch
src/qemu/qemu_audit.c | 84 +++++++++++++++++++++------------------------
src/qemu/qemu_audit.h | 58 ++++++++++++++++--------------
src/qemu/qemu_driver.c | 25 ++++++-------
src/qemu/qemu_hotplug.c | 40 +++++++++++-----------
src/qemu/qemu_migration.c | 14 ++++----
src/qemu/qemu_process.c | 6 ++--
6 files changed, 112 insertions(+), 115 deletions(-)
ACK
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 :|
11:13 p.m.
New subject: [libvirt] [PATCHv2 6/8] cgroup: allow fine-tuning of device ACL permissions
Adding audit points showed that we were granting too much privilege
to qemu; it should not need any mknod rights to recreate any
devices other than ptys. On the other hand, lxc should have all
device privileges. The solution is adding a flag parameter.
This also lets us restrict write access to read-only disks.
* src/util/cgroup.h (virCgroup*Device*): Adjust prototypes.
* src/util/cgroup.c (virCgroupAllowDevice)
(virCgroupAllowDeviceMajor, virCgroupAllowDevicePath)
(virCgroupDenyDevice, virCgroupDenyDeviceMajor)
(virCgroupDenyDevicePath): Add parameter.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update clients.
* src/lxc/lxc_controller.c (lxcSetContainerResources): Likewise.
* src/qemu/qemu_cgroup.c: Likewise.
(qemuSetupDiskPathAllo): Also, honor read-only disks.
---
v2: new patch
src/lxc/lxc_controller.c | 9 +++++--
src/qemu/qemu_cgroup.c | 27 +++++++++++++---------
src/qemu/qemu_driver.c | 9 +++++--
src/util/cgroup.c | 55 +++++++++++++++++++++++++++++++++------------
src/util/cgroup.h | 26 ++++++++++++++++-----
5 files changed, 88 insertions(+), 38 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index d2b113c..296b302 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1,5 +1,6 @@
/*
- * Copyright (C) 2010 Red Hat, Inc. Copyright IBM Corp. 2008
+ * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright IBM Corp. 2008
*
* lxc_controller.c: linux container process controller
*
@@ -168,7 +169,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
rc = virCgroupAllowDevice(cgroup,
dev->type,
dev->major,
- dev->minor);
+ dev->minor,
+ VIR_CGROUP_DEVICE_RWM);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %c:%d:%d for domain
%s"),
@@ -177,7 +179,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
}
}
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY,
+ VIR_CGROUP_DEVICE_RWM);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow PYT devices for domain %s"),
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ebf9ad5..83063a9 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -56,7 +56,7 @@ int qemuCgroupControllerActive(struct qemud_driver *driver,
}
static int
-qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
const char *path,
size_t depth ATTRIBUTE_UNUSED,
void *opaque)
@@ -65,8 +65,9 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int rc;
VIR_DEBUG("Process path %s for disk", path);
- /* XXX RO vs RW */
- rc = virCgroupAllowDevicePath(data->cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path,
+ (disk->readonly ? VIR_CGROUP_DEVICE_READ
+ : VIR_CGROUP_DEVICE_RW));
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
@@ -106,8 +107,8 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int rc;
VIR_DEBUG("Process path %s for disk", path);
- /* XXX RO vs RW */
- rc = virCgroupDenyDevicePath(data->cgroup, path);
+ rc = virCgroupDenyDevicePath(data->cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
@@ -150,7 +151,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk",
dev->source.data.file.path);
- rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path);
+ rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
dev->source.data.file.path, rc);
if (rc < 0) {
@@ -172,7 +174,8 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
int rc;
VIR_DEBUG("Process path '%s' for USB device", path);
- rc = virCgroupAllowDevicePath(data->cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
@@ -226,7 +229,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
goto cleanup;
}
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
"pty", rc == 0);
if (rc != 0) {
@@ -240,7 +244,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC
&&
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
"sound", rc == 0);
if (rc != 0) {
@@ -251,8 +256,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
for (i = 0; deviceACL[i] != NULL ; i++) {
- rc = virCgroupAllowDevicePath(cgroup,
- deviceACL[i]);
+ rc = virCgroupAllowDevicePath(cgroup, deviceACL[i],
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
if (rc < 0 &&
rc != -ENOENT) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1981cdf..7b4edc5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1962,7 +1962,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
vm->def->name);
goto endjob;
}
- rc = virCgroupAllowDevicePath(cgroup, path);
+ rc = virCgroupAllowDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
@@ -2012,7 +2013,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
VIR_WARN("failed to restore save state label on %s", path);
if (cgroup != NULL) {
- rc = virCgroupDenyDevicePath(cgroup, path);
+ rc = virCgroupDenyDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
@@ -2044,7 +2046,8 @@ endjob:
}
if (cgroup != NULL) {
- rc = virCgroupDenyDevicePath(cgroup, path);
+ rc = virCgroupDenyDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 46358ab..9986e53 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -1081,7 +1081,7 @@ int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long
*kb)
/**
* virCgroupDenyAllDevices:
*
- * @group: The cgroup to deny devices for
+ * @group: The cgroup to deny all permissions, for all devices
*
* Returns: 0 on success
*/
@@ -1100,15 +1100,20 @@ int virCgroupDenyAllDevices(virCgroupPtr group)
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device
* @minor: The minor number of the device
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Returns: 0 on success
*/
-int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor)
+int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+ if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "")
== -1) {
rc = -ENOMEM;
goto out;
}
@@ -1129,15 +1134,20 @@ out:
* @group: The cgroup to allow an entire device major type for
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Returns: 0 on success
*/
-int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
+int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) {
+ if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "")
== -1) {
rc = -ENOMEM;
goto out;
}
@@ -1157,6 +1167,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int
major)
*
* @group: The cgroup to allow the device for
* @path: the device to allow
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Queries the type of device and its major/minor number, and
* adds that to the cgroup ACL
@@ -1165,7 +1176,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int
major)
* negative errno value on failure
*/
#if defined(major) && defined(minor)
-int virCgroupAllowDevicePath(virCgroupPtr group, const char *path)
+int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms)
{
struct stat sb;
@@ -1178,11 +1189,13 @@ int virCgroupAllowDevicePath(virCgroupPtr group, const char
*path)
return virCgroupAllowDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
- minor(sb.st_rdev));
+ minor(sb.st_rdev),
+ perms);
}
#else
int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
- const char *path ATTRIBUTE_UNUSED)
+ const char *path ATTRIBUTE_UNUSED,
+ int perms ATTRIBUTE_UNUSED)
{
return -ENOSYS;
}
@@ -1196,15 +1209,20 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device
* @minor: The minor number of the device
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
*
* Returns: 0 on success
*/
-int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor)
+int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+ if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "")
== -1) {
rc = -ENOMEM;
goto out;
}
@@ -1225,15 +1243,20 @@ out:
* @group: The cgroup to deny an entire device major type for
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
*
* Returns: 0 on success
*/
-int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major)
+int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) {
+ if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "")
== -1) {
rc = -ENOMEM;
goto out;
}
@@ -1249,7 +1272,7 @@ int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int
major)
}
#if defined(major) && defined(minor)
-int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
+int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms)
{
struct stat sb;
@@ -1262,11 +1285,13 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
return virCgroupDenyDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
- minor(sb.st_rdev));
+ minor(sb.st_rdev),
+ perms);
}
#else
int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
- const char *path ATTRIBUTE_UNUSED)
+ const char *path ATTRIBUTE_UNUSED,
+ int perms ATTRIBUTE_UNUSED)
{
return -ENOSYS;
}
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index b3c5f27..16ffb46 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -60,27 +60,41 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long
*kb);
int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb);
int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb);
+enum {
+ VIR_CGROUP_DEVICE_READ = 1,
+ VIR_CGROUP_DEVICE_WRITE = 2,
+ VIR_CGROUP_DEVICE_MKNOD = 4,
+ VIR_CGROUP_DEVICE_RW = VIR_CGROUP_DEVICE_READ | VIR_CGROUP_DEVICE_WRITE,
+ VIR_CGROUP_DEVICE_RWM = VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD,
+};
+
int virCgroupDenyAllDevices(virCgroupPtr group);
int virCgroupAllowDevice(virCgroupPtr group,
char type,
int major,
- int minor);
+ int minor,
+ int perms);
int virCgroupAllowDeviceMajor(virCgroupPtr group,
char type,
- int major);
+ int major,
+ int perms);
int virCgroupAllowDevicePath(virCgroupPtr group,
- const char *path);
+ const char *path,
+ int perms);
int virCgroupDenyDevice(virCgroupPtr group,
char type,
int major,
- int minor);
+ int minor,
+ int perms);
int virCgroupDenyDeviceMajor(virCgroupPtr group,
char type,
- int major);
+ int major,
+ int perms);
int virCgroupDenyDevicePath(virCgroupPtr group,
- const char *path);
+ const char *path,
+ int perms);
int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);
--
1.7.4
9:24 a.m.
New subject: [libvirt] [PATCHv2 6/8] cgroup: allow fine-tuning of device ACL permissions
On Tue, Mar 08, 2011 at 10:13:48PM -0700, Eric Blake wrote:
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ebf9ad5..83063a9 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -226,7 +229,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
goto cleanup;
}
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
"pty", rc == 0);
if (rc != 0) {
I'm not entirely sure that we actually need RWM here, rather than
just RW. QEMU doesn't do any mknod in /dev/pts/XXX. The entries
in that filesystem just magically appear from the kernel when
you open /dev/ptmx.
@@ -240,7 +244,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC
&&
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL))))
{
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
"sound", rc == 0);
if (rc != 0) {
Almost certain we don't need RWM for sound devices, just RW
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 :|
11:46 a.m.
New subject: [libvirt] [PATCHv2 6/8] cgroup: allow fine-tuning of device ACL permissions
On 03/09/2011 08:24 AM, Daniel P. Berrange wrote:
On Tue, Mar 08, 2011 at 10:13:48PM -0700, Eric Blake wrote:
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index ebf9ad5..83063a9 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -226,7 +229,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
> goto cleanup;
> }
>
> - rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
> + rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
> + VIR_CGROUP_DEVICE_RWM);
> qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
> "pty", rc == 0);
> if (rc != 0) {
I'm not entirely sure that we actually need RWM here, rather than
just RW. QEMU doesn't do any mknod in /dev/pts/XXX. The entries
in that filesystem just magically appear from the kernel when
you open /dev/ptmx.
I tested with just RW instead of RWM, and was still able to do 'virsh
console' with no change in behavior, so I went with this change.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [PATCHv2 7/8] audit: also audit cgroup ACL permissions
* src/qemu/qemu_audit.h (qemuAuditCgroupMajor)
(qemuAuditCgroupPath): Add parameter.
* src/qemu/qemu_audit.c (qemuAuditCgroupMajor)
(qemuAuditCgroupPath): Add 'acl=rwm' to cgroup audit entries.
* src/qemu/qemu_cgroup.c: Update clients.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Likewise.
---
v2: new patch; perhaps patch should be floated before patch 2, and
then this patch squashed into patch 2, so that I'm only touching
qemuAuditCgroupPath once?
src/qemu/qemu_audit.c | 12 ++++++++----
src/qemu/qemu_audit.h | 2 ++
src/qemu/qemu_cgroup.c | 15 ++++++++-------
src/qemu/qemu_driver.c | 6 +++---
4 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 705cab7..ea4f22b 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -291,6 +291,7 @@ cleanup:
* @reason: either "allow" or "deny"
* @maj: the major number of the device category
* @name: a textual name for that device category, alphabetic only
+ * @perms: string containing "r", "w", and/or "m" as
appropriate
* @success: true if the cgroup operation succeeded
*
* Log an audit message about an attempted cgroup device ACL change.
@@ -298,11 +299,12 @@ cleanup:
void
qemuAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup,
const char *reason, int maj, const char *name,
- bool success)
+ const char *perms, bool success)
{
char *extra;
- if (virAsprintf(&extra, "major category=%s maj=%02X", name, maj) <
0) {
+ if (virAsprintf(&extra, "major category=%s maj=%02X acl=%s",
+ name, maj, perms) < 0) {
VIR_WARN0("OOM while encoding audit message");
return;
}
@@ -318,6 +320,7 @@ qemuAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup,
* @cgroup: cgroup that manages the devices
* @reason: either "allow" or "deny"
* @path: the device being adjusted
+ * @perms: string containing "r", "w", and/or "m" as
appropriate
* @rc: > 0 if not a device, 0 if success, < 0 if failure
*
* Log an audit message about an attempted cgroup device ACL change to
@@ -325,7 +328,8 @@ qemuAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup,
*/
void
qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup,
- const char *reason, const char *path, int rc)
+ const char *reason, const char *path, const char *perms,
+ int rc)
{
char *detail;
char *rdev;
@@ -337,7 +341,7 @@ qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup,
if (!(detail = virAuditEncode("path", path)) ||
!(rdev = qemuAuditGetRdev(path)) ||
- virAsprintf(&extra, "path path=%s %s", path, rdev) < 0) {
+ virAsprintf(&extra, "path path=%s %s acl=%s", path, rdev, perms)
< 0) {
VIR_WARN0("OOM while encoding audit message");
goto cleanup;
}
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 84874a2..a9300d3 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -61,11 +61,13 @@ void qemuAuditCgroupMajor(virDomainObjPtr vm,
const char *reason,
int maj,
const char *name,
+ const char *perms,
bool success);
void qemuAuditCgroupPath(virDomainObjPtr vm,
virCgroupPtr group,
const char *reason,
const char *path,
+ const char *perms,
int rc);
void qemuAuditMemory(virDomainObjPtr vm,
unsigned long long oldmem,
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 83063a9..f65445c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -68,7 +68,8 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
rc = virCgroupAllowDevicePath(data->cgroup, path,
(disk->readonly ? VIR_CGROUP_DEVICE_READ
: VIR_CGROUP_DEVICE_RW));
- qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path,
+ disk->readonly ? "r" : "rw", rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -109,7 +110,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
rc = virCgroupDenyDevicePath(data->cgroup, path,
VIR_CGROUP_DEVICE_RWM);
- qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path,
"rwm", rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -154,7 +155,7 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
- dev->source.data.file.path, rc);
+ dev->source.data.file.path, "rw", rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -176,7 +177,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path '%s' for USB device", path);
rc = virCgroupAllowDevicePath(data->cgroup, path,
VIR_CGROUP_DEVICE_RW);
- qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path,
"rw", rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -232,7 +233,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
- "pty", rc == 0);
+ "pty", "rwm", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/pts/ devices"));
@@ -247,7 +248,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR,
VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
- "sound", rc == 0);
+ "sound", "rwm", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/snd/ devices"));
@@ -258,7 +259,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup, deviceACL[i],
VIR_CGROUP_DEVICE_RW);
- qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
+ qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i],
"rw", rc);
if (rc < 0 &&
rc != -ENOENT) {
virReportSystemError(-rc,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b4edc5..ca2b61d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1964,7 +1964,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
}
rc = virCgroupAllowDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RW);
- qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
+ qemuAuditCgroupPath(vm, cgroup, "allow", path, "rw", rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -2015,7 +2015,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RWM);
- qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, "rwm", rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
path, vm->def->name, rc);
@@ -2048,7 +2048,7 @@ endjob:
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RWM);
- qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, "rwm",
rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
path, vm->def->name, rc);
--
1.7.4
9:26 a.m.
New subject: [libvirt] [PATCHv2 7/8] audit: also audit cgroup ACL permissions
On Tue, Mar 08, 2011 at 10:13:49PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.h (qemuAuditCgroupMajor)
(qemuAuditCgroupPath): Add parameter.
* src/qemu/qemu_audit.c (qemuAuditCgroupMajor)
(qemuAuditCgroupPath): Add 'acl=rwm' to cgroup audit entries.
* src/qemu/qemu_cgroup.c: Update clients.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Likewise.
---
v2: new patch; perhaps patch should be floated before patch 2, and
then this patch squashed into patch 2, so that I'm only touching
qemuAuditCgroupPath once?
I don't think it hugely matters.
src/qemu/qemu_audit.c | 12 ++++++++----
src/qemu/qemu_audit.h | 2 ++
src/qemu/qemu_cgroup.c | 15 ++++++++-------
src/qemu/qemu_driver.c | 6 +++---
4 files changed, 21 insertions(+), 14 deletions(-)
ACK, unless it needs some changes based on my two comments to
the previous patch about certain RWM vs RW usage.
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 :|
12:51 p.m.
New subject: [libvirt] [PATCHv2 7/8] audit: also audit cgroup ACL permissions
On 03/09/2011 08:26 AM, Daniel P. Berrange wrote:
On Tue, Mar 08, 2011 at 10:13:49PM -0700, Eric Blake wrote:
> * src/qemu/qemu_audit.h (qemuAuditCgroupMajor)
> (qemuAuditCgroupPath): Add parameter.
> * src/qemu/qemu_audit.c (qemuAuditCgroupMajor)
> (qemuAuditCgroupPath): Add 'acl=rwm' to cgroup audit entries.
> * src/qemu/qemu_cgroup.c: Update clients.
> * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Likewise.
> ---
>
> v2: new patch; perhaps patch should be floated before patch 2, and
> then this patch squashed into patch 2, so that I'm only touching
> qemuAuditCgroupPath once?
I don't think it hugely matters.
> src/qemu/qemu_audit.c | 12 ++++++++----
> src/qemu/qemu_audit.h | 2 ++
> src/qemu/qemu_cgroup.c | 15 ++++++++-------
> src/qemu/qemu_driver.c | 6 +++---
> 4 files changed, 21 insertions(+), 14 deletions(-)
ACK, unless it needs some changes based on my two comments to
the previous patch about certain RWM vs RW usage.
It needed a tweak ("rw" vs. "rwm"). I've applied patches 1-3 and
5-7,
and will be doing a PATCHv3 for patch 4, 8, and other followups for
auditing network devices after I finish more testing.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [RFC: PATCHv2 8/8] qemu: support vhost in attach-interface
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Honor vhost
designations, similar to qemu_command code paths.
* src/qemu/qemu_command.h (qemuOpenVhostNet): New prototype.
* src/qemu/qemu_command.c (qemuOpenVhostNet): Export.
---
v2: new patch; as yet untested, and needs patch 9/8 to do
audit hooks for the opened devices
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 5 ++++
src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d5f5a70..af36866 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -304,7 +304,7 @@ cleanup:
}
-static int
+int
qemuOpenVhostNet(virDomainDefPtr def,
virDomainNetDefPtr net,
virBitmapPtr qemuCaps,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1902472..4c0713c 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -129,6 +129,11 @@ int qemuPhysIfaceConnect(virConnectPtr conn,
const unsigned char *vmuuid,
enum virVMOperationType vmop);
+int qemuOpenVhostNet(virDomainDefPtr def,
+ virDomainNetDefPtr net,
+ virBitmapPtr qemuCaps,
+ int *vhostfd);
+
int qemudCanonicalizeMachine(struct qemud_driver *driver,
virDomainDefPtr def);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7895062..9fbd004 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -552,6 +552,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
qemuDomainObjPrivatePtr priv = vm->privateData;
char *tapfd_name = NULL;
int tapfd = -1;
+ char *vhostfd_name = NULL;
+ int vhostfd = -1;
char *nicstr = NULL;
char *netstr = NULL;
int ret = -1;
@@ -592,6 +594,24 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
return -1;
}
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ net->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ /* Attempt to use vhost-net mode for these types of
+ network device */
+ if (qemuOpenVhostNet(vm->def, net, qemuCaps, &vhostfd) < 0)
+ goto cleanup;
+
+ if (vhostfd >= 0 &&
+ priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network device type '%s' cannot be attached:
"
+ "qemu is not using a unix socket monitor"),
+ virDomainNetTypeToString(net->type));
+ goto cleanup;
+ }
+ }
+
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0)
goto no_memory;
@@ -636,15 +656,32 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
}
}
- /* FIXME - need to support vhost-net here (5th arg) */
+ if (vhostfd != -1) {
+ if (virAsprintf(&vhostfd_name, "vhostfd-%s", net->info.alias)
< 0)
+ goto no_memory;
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ if (qemuMonitorSendFileHandle(priv->mon, vhostfd_name, vhostfd) < 0) {
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+ goto try_tapfd_close;
+ }
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("guest unexpectedly quit"));
+ goto cleanup;
+ }
+ }
+
if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (!(netstr = qemuBuildHostNetStr(net, ',',
- -1, tapfd_name, 0)))
+ -1, tapfd_name, vhostfd_name)))
goto try_tapfd_close;
} else {
if (!(netstr = qemuBuildHostNetStr(net, ' ',
- vlan, tapfd_name, 0)))
+ vlan, tapfd_name, vhostfd_name)))
goto try_tapfd_close;
}
@@ -666,6 +703,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
qemuDomainObjExitMonitorWithDriver(driver, vm);
VIR_FORCE_CLOSE(tapfd);
+ VIR_FORCE_CLOSE(vhostfd);
if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -720,6 +758,8 @@ cleanup:
VIR_FREE(netstr);
VIR_FREE(tapfd_name);
VIR_FORCE_CLOSE(tapfd);
+ VIR_FREE(vhostfd_name);
+ VIR_FORCE_CLOSE(vhostfd);
return ret;
@@ -759,10 +799,14 @@ try_tapfd_close:
if (!virDomainObjIsActive(vm))
goto cleanup;
- if (tapfd_name) {
+ if (tapfd_name || vhostfd_name) {
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0)
+ if (tapfd_name &&
+ qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0)
VIR_WARN("Failed to close tapfd with '%s'", tapfd_name);
+ if (vhostfd_name &&
+ qemuMonitorCloseFileHandle(priv->mon, vhostfd_name) < 0)
+ VIR_WARN("Failed to close vhostfd with '%s'",
vhostfd_name);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
--
1.7.4
9:28 a.m.
New subject: [libvirt] [RFC: PATCHv2 8/8] qemu: support vhost in attach-interface
On Tue, Mar 08, 2011 at 10:13:50PM -0700, Eric Blake wrote:
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Honor vhost
designations, similar to qemu_command code paths.
* src/qemu/qemu_command.h (qemuOpenVhostNet): New prototype.
* src/qemu/qemu_command.c (qemuOpenVhostNet): Export.
---
v2: new patch; as yet untested, and needs patch 9/8 to do
audit hooks for the opened devices
I think it looks basically sane, but yeah needs to be tested
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 :|
5008
days inactive
5008
days old
21 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake