[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virDomainSaveXML: Reject domains which name contain '/'

On 05.02.2013 16:47, Daniel P. Berrange wrote:
> On Tue, Feb 05, 2013 at 04:45:18PM +0100, Michal Privoznik wrote:
>> On 05.02.2013 12:32, Christophe Fergeau wrote:
>>> Similarly to 790f912b4 which rejects snapshots names containing,
>>> this commit changes virDomainSaveXML to reject domains with a '/'
>>> in their name. The domain name is used as a filename, so this
>>> leads to unexpected results when used in combination with '..'
>>> ---
>>> If someone is using domains with '/' in their name, this will cause a
>>> regression for them as libvirt will no longer be able to re-write the
>>> domain XML and will just fail. However, if someone is (ab)using this,
>>> the domain must end up in an already existing directory as libvirt will
>>> not create it so this is hopefully unconvenient enough that noone is
>>> doing that.
>>> If we only want to error out on new domain creation, this may be doable
>>> by introducing a virDomainSaveXMLFlags, but will be more invasive.
>>> Christophe
>> The more problems with naming XML files I see the more I think we should
>> not tie it to domain names. Basically there's no need to save domain
>> named XYZ to XYZ.xml. Yes, it's helpful when the names match, however if
>> domain name contains forbidden characters (e.g. '/'), we can use UUID
>> for instance for the name of XML file. To keep track which domain is
>> associated witch which file we can store that kind of information in
>> virDomainObj.
> I think you understate the benefits here. UUID based filenames are just
> absolutely *ing horrific for an admin to see and work work and I am very
> glad libvirt does not use such a naming scheme.
> Daniel

But we discourage users to touch /etc/libvirt/qemu/*.xml files anyway.
Moreover, we advise to define domain by not copying the files but using
libvirt APIs.

But then again - I am not telling we should switch to my approach for
good for all domains. We should use it only for those names, where admin
is stupid enough to name a domain using shell escape sequences or with
'/' contained, etc. These admins don't want to take look under
/etc/libvirt/ anyway.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]