Re: [libvirt] [PATCH] Fix messages using VIR_ERR_XML_ERROR

On 05/12/2011 11:47 PM, Cole Robinson wrote: > This error code has existed since the dawn of time, yet the messages it > generates are almost universally busted. Here's a small sampling: > > src/conf/domain_conf.c:4889 : XML description for missing root element is not well formed or invalid > src/conf/domain_conf.c:4951 : XML description for unknown device type is not well formed or invalid > src/conf/domain_conf.c:5460 : XML description for maximum vcpus must be an integer is not well formed or invalid > src/conf/domain_conf.c:5468 : XML description for invalid maxvcpus %(count)lu is not well formed or invalid > > Fix up the error code to instead be > > XML error: <msg> > > Adjust the few locations that we using the original correctly (or shouldn't > have been using the error code at all). > > Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; > --- > src/test/test_driver.c | 21 ++++++++++++++------- > src/util/virterror.c | 4 ++-- > src/xen/xm_internal.c | 5 +++-- > 3 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > index 119b027..a9b306e 100644 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -811,7 +811,8 @@ static int testOpenFromFile(virConnectPtr conn, > if (ret == 0) { > nodeInfo->nodes = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node cpu numa nodes")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu nodes value")); > goto error; > } > > @@ -819,7 +820,8 @@ static int testOpenFromFile(virConnectPtr conn, > if (ret == 0) { > nodeInfo->sockets = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node cpu sockets")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu sockets value")); > goto error; > } > > @@ -827,7 +829,8 @@ static int testOpenFromFile(virConnectPtr conn, > if (ret == 0) { > nodeInfo->cores = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node cpu cores")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu cores value")); > goto error; > } > > @@ -835,7 +838,8 @@ static int testOpenFromFile(virConnectPtr conn, > if (ret == 0) { > nodeInfo->threads = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node cpu threads")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu threads value")); > goto error; > } > > @@ -846,14 +850,16 @@ static int testOpenFromFile(virConnectPtr conn, > nodeInfo->cpus = l; > } > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node active cpu")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu active value")); > goto error; > } > ret = virXPathLong("string(/node/cpu/mhz[1])", ctxt, &l); > if (ret == 0) { > nodeInfo->mhz = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node cpu mhz")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node cpu mhz value")); > goto error; > } > > @@ -872,7 +878,8 @@ static int testOpenFromFile(virConnectPtr conn, > if (ret == 0) { > nodeInfo->memory = l; > } else if (ret == -2) { > - testError(VIR_ERR_XML_ERROR, "%s", _("node memory")); > + testError(VIR_ERR_XML_ERROR, "%s", > + _("invalid node memory value")); > goto error; > } > > diff --git a/src/util/virterror.c b/src/util/virterror.c > index fbb4a45..881a7dc 100644 > --- a/src/util/virterror.c > +++ b/src/util/virterror.c > @@ -925,9 +925,9 @@ virErrorMsg(virErrorNumber error, const char *info) > break; > case VIR_ERR_XML_ERROR: > if (info == NULL) > - errmsg = _("XML description not well formed or invalid"); > + errmsg = _("XML description is not well formed or invalid"); > else > - errmsg = _("XML description for %s is not well formed or invalid"); > + errmsg = _("XML error: %s"); > break; > case VIR_ERR_DOM_EXIST: > if (info == NULL) > diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c > index 69e14c3..d0035c9 100644 > --- a/src/xen/xm_internal.c > +++ b/src/xen/xm_internal.c > @@ -1515,8 +1515,9 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, > break; > } > default: > - xenXMError(VIR_ERR_XML_ERROR, > - "%s", _("unknown device")); > + xenXMError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("device type '%s' cannot be detached"), > + virDomainDeviceTypeToString(dev->type)); > goto cleanup; > } > Well, I've started to work on this weeks ago, but left it for more important stuff. I've created new error code, to distinguish XML errors (not well formed XML), bad values (e.g. string given when uint is expected), and unsupported values. But you got a good point.