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

Re: [libvirt] [PATCH 3/5 v3] Adding the element pf to network xml.



On 01/11/2012 11:08 AM, Eric Blake wrote:
> On 01/10/2012 05:41 PM, Eric Blake wrote:
>>>>          /* all of these modes can use a pool of physical interfaces */
>>>>          nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes);
>>>> -        if (nForwardIfs < 0)
>>>> +        if (nForwardIfs <= 0) {
>>>> +            virNetworkReportError(VIR_ERR_XML_ERROR,
>>>> +                                  _("No interface pool given, checking for SRIOV pf")); 
>>>> +            nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);
>>>> +            
>>>> +            if (nForwardPfs <= 0) {
>>>> +                virNetworkReportError(VIR_ERR_XML_ERROR,
>>>> +                                      _("No interface pool or SRIOV physical device given"));
>>>
>>> This has to be a check for '< 0', not '<= 0', or else you get LOTS of
>>> 'make check' failures.
>>
>> Also, your patch touched the rng, but failed to add documentation for
>> the new XML, nor tests that prove we can parse it.  And in adding those
>> tests, I found that your rng additions don't match your code (which
>> wants pf before, not after, interface).  I'm working on fixing that, but
>> it's taking me longer than I planned, since I also decided to add tests
>> of the parsing, and in those tests, it appears that pf did not get
>> parsed as expected.  I don't know if it's a flaw in the original patch
>> or in my touchups...
> 
> I think I understand the problem now.  You aren't parsing nForwardPfs
> unless there are exactly 0 nForwardIfs; but if you end up reparsing
> existing live XML, you don't want to lose the pf information.  That is,
> the parsing routine should always parse pf information; and perhaps even
> be taught to honor flags (just like the format routine), so that it
> skips parsing <interface> elements if doing an INACTIVE parse and a pf
> is present.

I got things to work without a flags on parse, for now, so this is what
I'm squashing into 3/5 before pushing:

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 755d510..907c046 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -226,7 +226,21 @@
   &lt;/forward&gt;
 ...
         </pre>
-        When a guest interface is being constructed, libvirt will pick
+        Additionally, <span class="since">since 0.9.10</span>, libvirt
+        allows a shorthand for specifying all virtual interfaces
+        associated with a single physical function, by using
+        the <code>&lt;pf&gt;</code> subelement to call out the
+        corresponding physical interface associated with multiple
+        virtual interfaces:
+        <pre>
+...
+  &lt;forward mode='passthrough'&gt;
+    &lt;pf dev='eth0'/&gt;
+  &lt;/forward&gt;
+...
+        </pre>
+
+        <p>When a guest interface is being constructed, libvirt will pick
         an interface from this list to use for the connection. In
         modes where physical interfaces can be shared by multiple
         guest interfaces, libvirt will choose the interface that
@@ -234,7 +248,7 @@
         that do not allow sharing of the physical device (in
         particular, 'passthrough' mode, and 'private' mode when using
         802.1Qbh), libvirt will choose an unused physical interface
-        or, if it can't find an unused interface, fail the operation.
+        or, if it can't find an unused interface, fail the operation.</p>
       </dd>
     </dl>
     <h5><a name="elementQoS">Quality of service</a></h5>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 9a5d575..6e1002f 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -85,20 +85,22 @@
                 </choice>
               </attribute>
             </optional>
-            <zeroOrMore>
-              <element name='interface'>
-                <attribute name='dev'>
-                  <ref name='deviceName'/>
-                </attribute>
-              </element>
-            </zeroOrMore>
-            <optional>
-              <element name='pf'>
-                <attribute name='dev'>
-                  <ref name='deviceName'/>
-                </attribute>
-              </element>
-            </optional>
+            <interleave>
+              <zeroOrMore>
+                <element name='interface'>
+                  <attribute name='dev'>
+                    <ref name='deviceName'/>
+                  </attribute>
+                </element>
+              </zeroOrMore>
+              <optional>
+                <element name='pf'>
+                  <attribute name='dev'>
+                    <ref name='deviceName'/>
+                  </attribute>
+                </element>
+              </optional>
+            </interleave>
           </element>
         </optional>

diff --git a/tests/networkxml2xmlin/passthrough-pf.xml
b/tests/networkxml2xmlin/passthrough-pf.xml
new file mode 100644
index 0000000..e63aae0
--- /dev/null
+++ b/tests/networkxml2xmlin/passthrough-pf.xml
@@ -0,0 +1,10 @@
+<network>
+  <name>local</name>
+  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+  <forward mode="passthrough">
+    <pf dev='eth0'/>
+    <interface dev='eth10'/>
+    <interface dev='eth11'/>
+  </forward>
+  <ip address="192.168.122.1" netmask="255.255.255.0"/>
+</network>
diff --git a/tests/networkxml2xmlout/passthrough-pf.xml
b/tests/networkxml2xmlout/passthrough-pf.xml
new file mode 100644
index 0000000..1a7e71e
--- /dev/null
+++ b/tests/networkxml2xmlout/passthrough-pf.xml
@@ -0,0 +1,9 @@
+<network>
+  <name>local</name>
+  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+  <forward mode='passthrough'>
+    <pf dev='eth0'/>
+  </forward>
+  <ip address='192.168.122.1' netmask='255.255.255.0'>
+  </ip>
+</network>
-- 
1.7.7.5



-- 
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]