On 03/05/2013 02:28 AM, Peter Krempa wrote: > On 03/04/13 23:49, Eric Blake wrote: >> There is some controversy[1] on the qemu list on whether qemu should >> have ever allowed arbitrary file name passthrough, or whether it >> should be restricted to JUST /dev/random and /dev/hwrng. It is >> always easier to add support for additional filenames than it is >> to remove support for something once released, so this patch >> restricts libvirt 1.0.3 (where the virtio-random backend was first >> supported) to just the two uncontroversial names, letting us defer >> to a later date any decision on whether supporting arbitrary files >> makes sense. Additionally, since qemu 1.4 does NOT support >> /dev/fdset/nnn fd passthrough for the backend, limiting to just >> two known names means that we don't get tempted to try fd >> passthrough where it won't work. >> >> [1]https://lists.gnu.org/archive/html/qemu-devel/2013-03/threads.html#00023 >> >> >> * src/conf/domain_conf.c (virDomainRNGDefParseXML): Only allow >> /dev/random and /dev/hwrng. >> * docs/schemas/domaincommon.rng: Flag invalid files. >> * docs/formatdomain.html.in (elementsRng): Document this. >> * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args: >> Update test to match. >> * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml: >> Likewise. >> --- >> >> This needs to be acked before libvirt 1.0.3; otherwise we are >> stuck supporting arbitrary name passthrough. > > Okay, this is fair enough for now. > >> >> docs/formatdomain.html.in | 3 ++- >> docs/schemas/domaincommon.rng | 5 ++++- >> src/conf/domain_conf.c | 7 +++++++ >> tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +- >> tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml | 2 +- >> 5 files changed, 15 insertions(+), 4 deletions(-) >> > ... >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 995cf0c..9c96cf1 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -7423,6 +7423,13 @@ virDomainRNGDefParseXML(const xmlNodePtr node, >> switch ((enum virDomainRNGBackend) def->backend) { >> case VIR_DOMAIN_RNG_BACKEND_RANDOM: >> def->source.file = virXPathString("string(./backend)", ctxt); >> + if (STRNEQ(def->source.file, "/dev/random") && >> + STRNEQ(def->source.file, "/dev/hwrng")) { >> + virReportError(VIR_ERR_XML_ERROR, >> + _("file '%s' is not a supported random >> source"), >> + def->source.file); >> + goto error; > > This is yet another part of the stuff that will need to be moved out > of the parser when I finish the XML parsing callbacks. Is it? I think if something is described in the RNG, it can be validated directly in the parser. I think the only time that we should need to move the validation out of the parser is in cases where it isn't possible to describe in the RNG (i.e. in order to determine validity, you must a) know information specific to the hypervisor or host environment, or b) know the settings of other elements of the domain beyond the current object being parsed.) In this case, the value of def->source.file is being limited dependent on def->backend (or in xpath nomenclature "./devices/rng/backend" is being restricted based on the value of "./devices/rng/backend/@model". There is no dependency on hypervisor, host, or any piece of the domain definition outside this one element. So even if this wasn't a (possibly) temporary restriction, I think it's fine where it is. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list