9:49 a.m.
This patch changes the implementations lots of functions which parse
XML documents, so that if the XML document is not well-formed then we
get detailed error messages.
The general form of the change is:
static void
catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
{
// a callback which calls virDomainReportError
}
virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
virCapsPtr caps,
const char *xmlStr)
{
xmlParserCtxtPtr pctxt;
pctxt = xmlNewParserCtxt ();
pctxt->sax->error = catchXMLError;
xml = xmlCtxtReadDoc (pctxt, //...) etc.
There are some unavoidable shortcomings:
(1) There is no place to stash user pointers during the callback (the
suggestively named pctxt->userData field is already used for something
else), so we cannot pass the virConnectPtr to the error function. As
a result, virterror will store the error in a global variable, and
callers will probably not be able to access it.
(2) The XML parser routinely produces multiple error messages, and
virterror throws away all but the last one.
The errors do, however, get printed to stderr.
You can test this by using 'virsh define', 'virsh net-define', 'virsh
pool-define', etc. on not-well-formed XML files.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
9:57 a.m.
On Wed, Jul 30, 2008 at 03:49:17PM +0100, Richard W.M. Jones wrote:
This patch changes the implementations lots of functions which parse
XML documents, so that if the XML document is not well-formed then we
get detailed error messages.
The general form of the change is:
static void
catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
{
// a callback which calls virDomainReportError
}
virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
virCapsPtr caps,
const char *xmlStr)
{
xmlParserCtxtPtr pctxt;
pctxt = xmlNewParserCtxt ();
pctxt->sax->error = catchXMLError;
xml = xmlCtxtReadDoc (pctxt, //...) etc.
There are some unavoidable shortcomings:
(1) There is no place to stash user pointers during the callback (the
suggestively named pctxt->userData field is already used for something
else), so we cannot pass the virConnectPtr to the error function. As
a result, virterror will store the error in a global variable, and
callers will probably not be able to access it.
Yes, that makes it rather limited & not really any benefit as is.
We can get around this though with some cleverness of thread locals.
Either we can annotate a global variable with __thread, or use the
pthread_once_t/pthread_key_t to store it.
So before starting the parse, initialize these to 'empty' or some other
defined state. Do the parse, and the error handler can store the
problems. Once finished we can read the details out (& clear any
state to avoid mem leak) and put them into our virConnectPtr error
object. Since you're using thread locals this is all nice & safe from
race conditions with multiple parses taking place in parallel.
(2) The XML parser routinely produces multiple error messages, and
virterror throws away all but the last one.
It'd probably be more use to only report the first one, since the latter
problems may well be caused by earlier problems.
The errors do, however, get printed to stderr.
You won't see those except with the 'test' or 'xen' drivers run
locally. Anything going via the daemon will be output on the
daemon's stderr (/dev/null) and not on the client.
Daniel
--
|: Red Hat, Engineering, London -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 :|
10:34 a.m.
On Wed, Jul 30, 2008 at 03:57:57PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 30, 2008 at 03:49:17PM +0100, Richard W.M. Jones wrote:
> This patch changes the implementations lots of functions which parse
> XML documents, so that if the XML document is not well-formed then we
> get detailed error messages.
>
> The general form of the change is:
>
> static void
> catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
> {
> // a callback which calls virDomainReportError
> }
>
> virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
> virCapsPtr caps,
> const char *xmlStr)
> {
> xmlParserCtxtPtr pctxt;
>
> pctxt = xmlNewParserCtxt ();
> pctxt->sax->error = catchXMLError;
>
> xml = xmlCtxtReadDoc (pctxt, //...) etc.
>
> There are some unavoidable shortcomings:
>
> (1) There is no place to stash user pointers during the callback (the
> suggestively named pctxt->userData field is already used for something
> else), so we cannot pass the virConnectPtr to the error function. As
> a result, virterror will store the error in a global variable, and
> callers will probably not be able to access it.
void *_private; /* For user data, libxml won't touch it */
just set _private in the parser context, you should get the parser context
back in the SAX handler, then you can get to your _private data. that
should work and avoid the mess of global variables and thread stuff...
> (2) The XML parser routinely produces multiple error messages,
and
> virterror throws away all but the last one.
It'd probably be more use to only report the first one, since the latter
problems may well be caused by earlier problems.
Likely but not always correct, maybe the solution is to build a big
string concatenating all the errors and passing it back
You won't see those except with the 'test' or
'xen' drivers run
locally. Anything going via the daemon will be output on the
daemon's stderr (/dev/null) and not on the client.
yeah, unfortunately we have to assume a model where each errors can't
be reported individually,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
5959
days inactive
5959
days old
2 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones