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

Re: [libvirt] [PATCH 1/6] Allow network model to contain "-"



On 12/08/2011 05:38 PM, Eric Blake wrote:
>>          int i;
>>          for (i = 0 ; i < strlen(model) ; i++) {

Hmm - an O(n) function call on an O(n) loop, for a quadratic action (of
course, it's in the noise, since the user's model name is likely short).
 But we can do better with a more efficient search for bogus bytes
(strspn is O(n), if implemented well in libc).

>> -            int char_ok = c_isalnum(model[i]) || model[i] == '_';
>> -            if (!char_ok) {
>> +            if (!c_isalnum(model[i]) && model[i] != '_' && model[i] != '-') {
> 
> I'm not sure if we need to tweak our RNG grammar to also allow this in
> the XML validation.  I'll check that out tomorrow, when I get around to
> applying this one and reviewing the rest of the series.

It turns out the XML didn't do any validation at all.  Here's what I
came up with - tightening the RNG and relaxing the domain_conf code, so
that they now match.  Since the concept is the same as yours, I went
ahead and pushed it, but I claimed authorship on this one, since I
practically rewrote it.

diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng
index b8fbcf9..27ec1da 100644
--- i/docs/schemas/domaincommon.rng
+++ w/docs/schemas/domaincommon.rng
@@ -1276,7 +1276,11 @@
       </optional>
       <optional>
         <element name="model">
-          <attribute name="type"/>
+          <attribute name="type">
+            <data type="string">
+              <param name='pattern'>[a-zA-Z0-9\-_]+</param>
+            </data>
+          </attribute>
           <empty/>
         </element>
       </optional>
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 5de33b9..f3ec777 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -3385,6 +3385,9 @@ error:
     return ret;
 }

+#define NET_MODEL_CHARS \
+    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-"
+
 /* Parse the XML definition for a network interface
  * @param node XML nodeset to parse for net definition
  * @return 0 on success, -1 on failure
@@ -3699,15 +3702,13 @@ virDomainNetDefParseXML(virCapsPtr caps,
      * reasonable, not that it is a supported NIC type.  FWIW kvm
      * supports these types as of April 2008:
      * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
+     * QEMU PPC64 supports spapr-vlan
      */
     if (model != NULL) {
-        int i;
-        for (i = 0 ; i < strlen(model) ; i++) {
-            if (!c_isalnum(model[i]) && model[i] != '_' && model[i] !=
'-') {
-                virDomainReportError(VIR_ERR_INVALID_ARG, "%s",
-                                     _("Model name contains invalid
characters"));
-                goto error;
-            }
+        if (strspn(model, NET_MODEL_CHARS) < strlen(model)) {
+            virDomainReportError(VIR_ERR_INVALID_ARG, "%s",
+                                 _("Model name contains invalid
characters"));
+            goto error;
         }
         def->model = model;
         model = NULL;


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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