Re: [libvirt] [PATCH 1/8] Add access control filtering of domain objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list domain objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 91 +++++++++++++++++++++++++++-------------
src/conf/domain_conf.h | 17 ++++++--
src/libxl/libxl_driver.c | 15 ++++---
src/lxc/lxc_driver.c | 15 ++++---
src/openvz/openvz_driver.c | 7 ++--
src/parallels/parallels_driver.c | 14 ++++---
src/qemu/qemu_driver.c | 24 ++++++-----
src/rpc/gendispatch.pl | 42 ++++++++++++-------
src/test/test_driver.c | 13 +++---
src/uml/uml_driver.c | 15 ++++---
src/vmware/vmware_driver.c | 12 +++---
11 files changed, 172 insertions(+), 93 deletions(-)
Big, but that's to be expected given our track record of multiple
listing APIs.
int
virDomainObjListNumOfDomains(virDomainObjListPtr doms,
- int active)
+ bool active,
Nice conversion of int->bool on something that was already used as a bool.
@@ -12654,8 +12658,7 @@ static int
qemuDomainSnapshotListNames(virDomainPtr domain, char **names,
if (virDomainSnapshotListNamesEnsureACL(domain->conn, vm->def) < 0)
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
flags);
A bit of spurious reformatting here; you touch the same code later in
the series, so you may want to rebase to avoid the churn, although I
won't insist.
@@ -12732,8 +12735,7 @@
qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
flags);
and again
cleanup:
if (vm)
@@ -12790,8 +12792,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
- flags);
+ n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
flags);
Here too. :)
@@ -1775,18 +1788,18 @@ elsif ($mode eq "client") {
push @argvars, $arg;
}
- if ($action eq "Check") {
- print "/* Returns: -1 on error, 0 on denied, 1 on allowed
*/\n";
- } else {
- print "/* Returns: -1 on error (denied==error), 0 on allowed
*/\n";
- }
- print "int $apiname(" . join(", ", @argdecls) .
")\n";
+ print "/* Returns: $fail on error/denied, $pass on allowed
*/\n";
+ print "$ret $apiname(" . join(", ", @argdecls) .
")\n";
This is a semantic change for Check functions, but I can agree with it
(in the security world, telling a person that they were denied due to
security as opposed to some other reason is sometimes in itself a
security hole due to the information leak). But is it worth splitting
into a separate patch, since it is an independent improvement used by
all of the rest of the series, and not something fundamental to listing
domains?
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)