<div dir="ltr"><div>My (negative) feedback is on the review - I'm really not sure that this matches what I understood the vision of privsep to be at all.<br><br>- If this is the vision / the new vision then I think it is majorly flawed. <br><br>- If it is skipping the vision in the name of expediency of implementation, then I think it has gone too far in that direction and we've better off holding off one more cycle and putting it in next cycle instead with a touch more purity of vision.<br><br></div>Apologies since you've clearly put work into it, and I should have provided such feedback earlier.<br><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 24 February 2016 at 14:59, Michał Dulko <span dir="ltr"><<a href="mailto:michal.dulko@intel.com" target="_blank">michal.dulko@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 02/24/2016 04:51 AM, Angus Lees wrote:<br>
> Re: <a href="https://review.openstack.org/#/c/277224" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/277224</a><br>
><br>
> Most of the various required changes have flushed out by now, and this<br>
> change now passes the dsvm-full integration tests(*).<br>
><br>
> (*) well, the experimental job anyway.  It still relies on a<br>
> merged-but-not-yet-released change in oslo.privsep so gate + 3rd party<br>
> won't pass until that happens.<br>
><br>
> What?<br>
> This change replaces os-brick's use of rootwrap with a quick+dirty<br>
> privsep-based drop-in replacement.  Privsep doesn't actually provide<br>
> much security isolation when used in this way, but it *does* now run<br>
> commands with CAP_SYS_ADMIN (still uid=0/gid=0) rather than full root<br>
> superpowers.  The big win from a practical point of view is that it<br>
> also means os-brick's rootwrap filters file is essentially deleted and<br>
> no longer has to be manually merged with downstream projects.<br>
><br>
> Code changes required in nova/cinder:<br>
> There is one change each to nova+cinder to add the relevant<br>
> privsep-helper command to rootwrap filters, and a devstack change to<br>
> add a nova.conf/cinder.conf setting.  That's it - this is otherwise a<br>
> backwards/forwards compatible change for nova+cinder.<br>
><br>
> Deployment changes required in nova/cinder:<br>
> A new "privsep_rootwrap.helper_command" needs to be defined in<br>
> nova/cinder.conf (default is something sensible using sudo), and<br>
> rootwrap filters or sudoers updated depending on the exact command<br>
> chosen.  Be aware that any commands will now be run with CAP_SYS_ADMIN<br>
> (only), and if that's insufficient for your hardware/drivers it can be<br>
> tweaked with other oslo_config options.<br>
><br>
> Risks:<br>
> The end-result is still just running the same commands as before, via<br>
> a different path - so there's not a lot of adventurousness here.  The<br>
> big behavioural change is CAP_SYS_ADMIN, and (as highlighted above)<br>
> it's conceivable that the driver for some exotic os-brick/cinder<br>
> hardware out there wants something more than that.<br>
><br>
> Work remaining:<br>
> - global-requirements change needed (for os-brick) once the latest<br>
> oslo.privsep release is made<br>
> - cinder/nova/devstack changes need to be merged<br>
> - after the above, the os-brick gate integration jobs will be able to<br>
> pass, and it can be merged<br>
> - If we want to *force* the new version of os-brick, we then need an<br>
> appropriate global-requirements os-brick bump<br>
> - Documentation, release notes, etc<br>
><br>
> I'll continue chewing through those remaining work items, but<br>
> essentially this is now in your combined hands to prioritise for<br>
> mitaka as you deem appropriate.<br>
><br>
>  - Gus<br>
><br>
<br>
</div></div>It seems too me that risks are higher than advantages. Moreover final<br>
release for libraries like os.brick should happen in just 2 days and I<br>
don't believe we have time to get every part of the job merged given how<br>
long TODO list is.<br>
<br>
Just my $0.02.<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div>-- <br>Duncan Thomas</div></div></div>
</div>