On Fri, May 03, 2013 at 10:30:19AM -0600, Eric Blake wrote:
> On 05/02/2013 06:03 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> >
> > Currently the parsing of XML is pushed down into the various
> > migration helper APIs. This makes it difficult to insert the
> > correct access control checks, since one helper API services
> > many public APIs. Pull the parsing of XML up to the top level
> > of the QEMU driver APIs
> > ---
> > src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++++++++++++++++++++++---
> > src/qemu/qemu_migration.c | 35 +++++-------------
> > src/qemu/qemu_migration.h | 6 ++--
> > 3 files changed, 99 insertions(+), 34 deletions(-)
>
> ACK.
>
> >
> > + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> > + goto cleanup;
> > +
> > + if (!(def = virDomainDefParseString(dom_xml, caps, driver->xmlopt,
> > + QEMU_EXPECTED_VIRT_TYPES,
> > + VIR_DOMAIN_XML_INACTIVE)))
> > + goto cleanup;
> > +
> > + if (dname) {
> > + VIR_FREE(def->name);
> > + if (!(def->name = strdup(dname))) {
> > + virReportOOMError();
> > + goto cleanup;
> > + }
> > + }
>
> This section is repeated a bit; is it worth further factoring it into a
> helper function to be called from each site?
In doing the refactoring I'm tending to prefer clarity of code even
at the cost of some duplicated code, since it makes it easier to
visibly see that the ACL check are being done at the correct time
or place. Removing the special cases is also letting me do some
automated analysis of ACL rules
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list