On Tue, Jul 02, 2013 at 01:17:44PM -0600, Eric Blake wrote:
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Ensure that all APIs which list domain objects filter them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange@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. :)
Opps, left over from code I backed out.
@@ -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?
Yeah should be moved to a dedicated patch.
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 :|