On Mon, Apr 28, 2008 at 01:44:32PM -0400, Cole Robinson wrote:
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
Here is the updated patch. Took a bit more work to take into account the multiple models, building and parsing the xen soundhw string, checking for duplicates, etc.
The current version uses the format:
<sound model='m1'/> <sound model='m2'/>
To enable support for models m1 and m2. The code will fail if you attempt to define an xml config which specifies a model that isn't in the whitelist (currently composed of 'sb16, 'es1370', and 'pcspk'). Unknown values from a xend sexpr or config file will be silently ignored.
One question: should the value 'all' be recognized from a xen domain and translated into a <sound> tag for every item in the whitelist? 'all' is an accepted value for a xen domain, since it just passes the string to qemu. This isn't in the code but I only thought of it now.
Sigh, that's a good question. I think I'd probably turn it into a list of 3 <sound> tags, so that if they load the XML back in, it'll get changed to the canonical explicit format.
+static int qemudParseSoundXML(virConnectPtr conn, + struct qemud_vm_sound_def *sound, + xmlNodePtr node) { + + xmlChar *model = NULL; + model = xmlGetProp(node, BAD_CAST "model"); + + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing sound model")); + goto error; + } + if ((sound->model = qemudSoundModelFromString(conn, (char *) model)) < 0) + goto error; + + if (model) + xmlFree(model); + return 0; + + error: + if (model) + xmlFree(model); + return -1; +}
IIRC, xmlFree already copes with NULL, the 'if()' bits are not required. You can check with 'make syntax-check' and see if it complains about these lines. Aside from this, it looks reasonable - I'd like to see a couple of XML configs added to the test cases for xen and QEMU to validate the parsing and formatting of XML. Regards, Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|