[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 5/5] guestfwd: Ensure guestfwd address is IPv4



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 veillard com  | 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);
 }

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]