Overall, this looks good. Your alternate implementation misses LVM support, and leaves an unused file behind in the instance directory. Other than that, it's acceptable for a late-stage change, IMO.
Best Regards, Solly ----- Original Message ----- > From: "Michael Still" <mi...@stillhq.com> > To: "OpenStack Development Mailing List" <openstack-dev@lists.openstack.org> > Sent: Monday, September 22, 2014 5:07:58 AM > Subject: [openstack-dev] Reverting recent refactorings of RBD support for > config drives > > Hi. > > Today I encountered bug 1369627 [1] as I trolled the status of release > critical bugs, which appears to be fall out from the decision to > implement adding support for config drives stored in RBD. While I have > no problem with that being at thing we do, I'm concerned by the way it > was implemented -- the image caching code for libvirt was being used > to "cache" the config drive, and then upload it to ceph as a side > effect of the image caching mechanism. > > I'd prefer we don't to it that way, and given its introduced as > security bug, I have proposed the following reverts: > > - https://review.openstack.org/#/c/123070/ > - https://review.openstack.org/#/c/123071/ > - https://review.openstack.org/#/c/123072/ > > Now, because I want to move us forward, I've also proposed an > alternate implementation which achieves the same thing without using > the caching code: > > - https://review.openstack.org/#/c/123073/ > > The new implementation only supports RBD, but that's mostly because > its the only image storage backend in the libvirt driver where it > makes immediate sense to do this sort of thing. I think this code > could do with a refactor, but I was attempting to produce the minimum > functional implementation given where we are in the release cycle. > > Persuant to our revert policy [2], I am asking cores to take a look at > these patches as soon as possible. > > Thanks, > Michael > > 1: https://bugs.launchpad.net/nova/+bug/1369627 > 2: > https://github.com/openstack/nova/blob/master/doc/source/devref/policies.rst > > -- > Rackspace Australia > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev