On 04/26/2016 04:26 PM, Laine Stump wrote:
On 04/26/2016 03:21 PM, Cole Robinson wrote:
An embedded '/' in object names doesn't really work for any of our stateful drivers, so let's explicitly reject it.
https://bugzilla.redhat.com/show_bug.cgi?id=639923 https://bugzilla.redhat.com/show_bug.cgi?id=787604
Was there any discussion I may have missed about why we shouldn't escape characters like this in the name rather than just forbidding them?
No discussion besides some old comments in those bugs. But how do you escape '/' for a unix filename? It's not really possible... we'd probably need some kind of URI escape sequence to make it work, which just opens up another world of pain making sure that the filename we put on disk doesn't collide with any legitimately named VM. Crazyness ensues
This doesn't prevent us doing that in the future if we want though, and it does provide a useful error message where there was previous semi-mystery, so ACK to the series (but do we really need the feature flag for domains? Seems like a bit of overkill.)
It's possible that existing VMs in non-stateful drivers already have a '/' in the name, and we shouldn't accidentally reject them. Googling indicates that vmware allows / in .vmx names at least. It may not matter in practice but I wanted to be conservative. Thanks for the review, I'll wait a couple days to see if anyone else wants to comment before pushing - Cole