[libvirt] [PATCHv2 1/2] virDomainSaveXML: Reject domains which name contain '/'

[Eric Blake]

On 02/07/2013 06:27 AM, 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 '..'
> ---
>  src/conf/domain_conf.c | 7 +++++++
>  1 file changed, 7 insertions(+)

I like the end goal, but am not sure the approach was right.  That is,
adding the checks to domain_conf.c feels too strong.  I can envision the
set of allowed vs. restricted names being different for different
hypervisors - just because the qemu driver creates a .log file based on
the domain name doesn't mean that other drivers will do the same (I
don't have enough experience with ESX to say for certain, but as far as
I know, libvirt doesn't create any additional files based on domain
names for esx:// connections and therefore does not need to restrict
'/'; or ESX may have its own set of even tighter rules on what forms a
valid name).  I think the approach in commit 790f912b4 of sticking the
restriction in qemu_driver.c is better; the parser should accept
everything, and then the driver should decide whether it was valid.

Also, I see no reason to split this into two patches; adding the
restrictions in the snapshot case was split into two patches only
because of a late-breaking review.  And I'm not sure that
VIR_ERR_XML_DETAIL was the best choice of error message in the earlier
patches that you copied from, although fixing it to something nicer
would be a separate cleanup.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130207/9d44c489/attachment-0001.sig>