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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list