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
- Cole