Re: [libvirt] [PATCH 0/3] conf: reject '/' in object names

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