On Wed, Nov 04, 2009 at 07:10:41PM +0000, Daniel P. Berrange wrote:
> On Wed, Nov 04, 2009 at 04:22:02PM +0000, Matthew Booth wrote:
> > * src/conf/domain_conf.c: Throw an error if guestfwd address isn't IPv4
> > ---
> > src/conf/domain_conf.c | 7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 94bce1e..ec2a1bc 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -1482,6 +1482,13 @@ virDomainChrDefParseXML(virConnectPtr conn,
> > goto error;
> > }
> >
> > + if(def->target.addr->stor.ss_family != AF_INET) {
>
> ^^^^ whitespace issue
Okay, I fixed a number of those in that module too
> > + virDomainReportError(conn, VIR_ERR_NO_SUPPORT, "%s",
> > + _("guestfwd channel only supports "
> > + "IPv4 addresses"));
> > + goto error;
> > + }
> > +
> > if(portStr == NULL) {
> > virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN, "%s",
> > _("guestfwd channel does "
> > --
>
> NO_SUPPORT isn't really the best error code to use here - that's intended
> for public API calls which aren't implemented by a driver.
>
> Our reporting of XML configurations which aren't valid is pretty rubbish
> as we don't have a standard error code - people have been making it up as
> we go along.
>
> I think we should add a new error code:
>
> VIR_ERR_CONFIG_UNSUPPORTED
>
> whcih we can then standardize on for this kind of thing
yup, makes sense, we have VIR_ERR_XML_ERROR for pure XML configuration
errors where the constructs are just invalid (I fixed a few in that module,
it was using VIR_ERR_INVALID_DOMAIN :-) but that one is for valid but
unsupported configurations.
I ended up with the following patch which I pushed
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index fa5cac4..4c28501 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -170,6 +170,7 @@ typedef enum {
VIR_WAR_NO_SECRET, /* failed to start secret storage */
VIR_ERR_INVALID_SECRET, /* invalid secret */
VIR_ERR_NO_SECRET, /* secret not found */
+ VIR_ERR_CONFIG_UNSUPPORTED, /* unsupported configuration construct */
} virErrorNumber;
/**
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 94bce1e..90bd0e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1365,8 +1365,8 @@ virDomainChrDefParseXML(virConnectPtr conn,
nodeName = (const char *) node->name;
if ((def->targetType = virDomainChrTargetTypeFromString(nodeName)) < 0) {
/* channel is handled below */
- if(STRNEQ(nodeName, "channel")) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ if (STRNEQ(nodeName, "channel")) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("unknown target type for character device: %s"),
nodeName);
return NULL;
@@ -1421,10 +1421,10 @@ virDomainChrDefParseXML(virConnectPtr conn,
protocol = virXMLPropString(cur, "type");
} else if (xmlStrEqual(cur->name, BAD_CAST "target")) {
/* If target type isn't set yet, expect it to be set here */
- if(def->targetType == VIR_DOMAIN_CHR_TARGET_TYPE_NULL) {
+ if (def->targetType == VIR_DOMAIN_CHR_TARGET_TYPE_NULL) {
targetType = virXMLPropString(cur, "type");
- if(targetType == NULL) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN, "%s",
+ if (targetType == NULL) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR, "%s",
_("character device target does "
"not define a type"));
goto error;
@@ -1432,7 +1432,7 @@ virDomainChrDefParseXML(virConnectPtr conn,
if ((def->targetType =
virDomainChrTargetTypeFromString(targetType)) < 0)
{
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("unknown target type for "
"character device: %s"),
targetType);
@@ -1446,14 +1446,14 @@ virDomainChrDefParseXML(virConnectPtr conn,
case VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL:
case VIR_DOMAIN_CHR_TARGET_TYPE_CONSOLE:
portStr = virXMLPropString(cur, "port");
- if(portStr == NULL) {
+ if (portStr == NULL) {
/* Not required. It will be assigned automatically
* later */
break;
}
- if(virStrToLong_ui(portStr, NULL, 10, &port) < 0) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ if (virStrToLong_ui(portStr, NULL, 10, &port) < 0) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("Invalid port number: %s"),
portStr);
goto error;
@@ -1464,32 +1464,39 @@ virDomainChrDefParseXML(virConnectPtr conn,
addrStr = virXMLPropString(cur, "address");
portStr = virXMLPropString(cur, "port");
- if(addrStr == NULL) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN, "%s",
+ if (addrStr == NULL) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR, "%s",
_("guestfwd channel does not "
"define a target address"));
goto error;
}
- if(VIR_ALLOC(def->target.addr) < 0) {
+ if (VIR_ALLOC(def->target.addr) < 0) {
virReportOOMError(conn);
goto error;
}
- if(virSocketParseAddr(addrStr, def->target.addr, 0) < 0)
+ if (virSocketParseAddr(addrStr, def->target.addr, 0) < 0)
{
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("%s is not a valid address"),
addrStr);
goto error;
}
- if(portStr == NULL) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN, "%s",
+ if (def->target.addr->stor.ss_family != AF_INET) {
+ virDomainReportError(conn, VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("guestfwd channel only supports "
+ "IPv4 addresses"));
+ goto error;
+ }
+
+ if (portStr == NULL) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR, "%s",
_("guestfwd channel does "
"not define a target port"));
goto error;
}
- if(virStrToLong_ui(portStr, NULL, 10, &port) < 0) {
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ if (virStrToLong_ui(portStr, NULL, 10, &port) < 0) {
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("Invalid port number: %s"),
portStr);
goto error;
@@ -1498,7 +1505,7 @@ virDomainChrDefParseXML(virConnectPtr conn,
break;
default:
- virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
+ virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("unexpected target type type %u"),
def->targetType);
}
@@ -2732,7 +2739,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
}
if (!(flags & VIR_DOMAIN_XML_INACTIVE))
- if((virXPathLong(conn, "string(./@id)", ctxt, &id)) < 0)
+ if ((virXPathLong(conn, "string(./@id)", ctxt, &id)) < 0)
id = -1;
def->id = (int)id;
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 10f979c..c8e8623 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -1089,6 +1089,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _("Secret not found: %s");
break;
+ case VIR_ERR_CONFIG_UNSUPPORTED:
+ if (info == NULL)
+ errmsg = _("unsupported configuration");
+ else
+ errmsg = _("unsupported configuration: %s");
+ break;
}
return (errmsg);
}
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list