Code review comment for lp:~niedbalski/charms/trusty/cinder/remove-unused-services

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Jorge,

Thanks for the patch! I think this is starting to be a nice work around for the bug. I've made some inline comments which I think need to be addressed.

Perhaps we should first disable the service (cinder service-disable) prior to removing it? Sure its defunct, but it might be nice to also qualify the deletion statement with a disabled qualifier.

Additionally, in >= Liberty, cinder includes the cinder-manage service remove [name], which I think should be the preferred method with this as an option for < liberty.

review: Needs Fixing

« Back to merge proposal