Re: [PATCHv2 5/7] conf: new function virDomainActualNetDefContentsFormat

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

 



On 21.02.2014 17:37, Laine Stump wrote:
This function is currently only called from one place, but in a
subsequent patch will be called from a 2nd place.

The new function exactly replicates the original behavior of the part
of virDomainActualNetDefFormat() that it replaces, but takes a
virDomainNetDefPtr instead of virDomainActualNetDefPtr, and uses the
virDomainNetGetActual*() functions whenever possible, rather than
reaching into def->data.network.actual - this is to be sure that we
are reporting exactly what is being used internally, just in case
there are any discrepancies (there shouldn't be).
---
Change from V1:

   I noticed during testing that bandwidth from a portgroup wasn't
   properly displayed if the network was forward
   mode='nat|route'. Fixing this required changing the conditions for
   using virDomainActualNetDefContentsFormat() - it must be used for
   those types of network as well, and when that is the case, we need
   to be sure to output the <source> element, as it otherwise would be
   skipped.

  src/conf/domain_conf.c | 113 +++++++++++++++++++++++++++++++++----------------
  1 file changed, 76 insertions(+), 37 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 67fc372..8291b73 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15427,78 +15427,112 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf,
      return 0;
  }

+/* virDomainActualNetDefContentsFormat() - format just the subelements
+ * of <interface> that may be overridden by what is in the
+ * virDomainActualNetDef, but inside the current element, rather
+ * than enclosed in an <actual> subelement.
+ */
  static int
-virDomainActualNetDefFormat(virBufferPtr buf,
-                            virDomainActualNetDefPtr def,
-                            unsigned int flags)
+virDomainActualNetDefContentsFormat(virBufferPtr buf,
+                                    virDomainNetDefPtr def,
+                                    const char *typeStr,
+                                    bool inSubelement,
+                                    unsigned int flags)
  {
-    const char *type;
      const char *mode;

-    if (!def)
-        return 0;
-
-    type = virDomainNetTypeToString(def->type);
-    if (!type) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unexpected net type %d"), def->type);
-        return -1;
-    }
-
-    virBufferAsprintf(buf, "<actual type='%s'", type);
-    if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV &&
-        def->data.hostdev.def.managed) {
-        virBufferAddLit(buf, " managed='yes'");
-    }
-    virBufferAddLit(buf, ">\n");
-
-    virBufferAdjustIndent(buf, 2);
-    switch (def->type) {
+    switch (virDomainNetGetActualType(def)) {
      case VIR_DOMAIN_NET_TYPE_BRIDGE:
          virBufferEscapeString(buf, "<source bridge='%s'/>\n",
-                              def->data.bridge.brname);
+                              virDomainNetGetActualBridgeName(def));
          break;

      case VIR_DOMAIN_NET_TYPE_DIRECT:
          virBufferAddLit(buf, "<source");
-        if (def->data.direct.linkdev)
-            virBufferEscapeString(buf, " dev='%s'",
-                                  def->data.direct.linkdev);
+        virBufferEscapeString(buf, " dev='%s'",
+                              virDomainNetGetActualDirectDev(def));

-        mode = virNetDevMacVLanModeTypeToString(def->data.direct.mode);
+        mode = virNetDevMacVLanModeTypeToString(virDomainNetGetActualDirectMode(def));
          if (!mode) {
              virReportError(VIR_ERR_INTERNAL_ERROR,
                             _("unexpected source mode %d"),
-                           def->data.direct.mode);
+                           virDomainNetGetActualDirectMode(def));
              return -1;
          }
          virBufferAsprintf(buf, " mode='%s'/>\n", mode);
          break;

      case VIR_DOMAIN_NET_TYPE_HOSTDEV:
-        if (virDomainHostdevDefFormatSubsys(buf, &def->data.hostdev.def,
+        if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def),
                                              flags, true) < 0) {
              return -1;
          }
          break;

      case VIR_DOMAIN_NET_TYPE_NETWORK:
-        if (def->class_id)
-            virBufferAsprintf(buf, "<class id='%u'/>", def->class_id);
+        if (!inSubelement) {
+            /* the <source> element isn't included in <actual>
+             * (i.e. when we're putting our output into a subelement
+             * rather than inline) because the main element has the
+             * same info already. If we're outputting inline, though,
+             * we *do* need to output <source>, because the caller
+             * won't have done it.
+             */
+            virBufferEscapeString(buf, "<source network='%s'/>\n",
+                                  def->data.network.name);
+        }
+        if (def->data.network.actual && def->data.network.actual->class_id)
+            virBufferAsprintf(buf, "<class id='%u'/>\n",
+                              def->data.network.actual->class_id);
          break;
      default:
          virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unexpected net type %s"), type);
+                       _("unexpected actual net type %s"), typeStr);
          return -1;
      }

-    if (virNetDevVlanFormat(&def->vlan, buf) < 0)
+    if (virNetDevVlanFormat(virDomainNetGetActualVlan(def), buf) < 0)
          return -1;
-    if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
+    if (virNetDevVPortProfileFormat(virDomainNetGetActualVirtPortProfile(def), buf) < 0)
          return -1;
-    if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
+    if (virNetDevBandwidthFormat(virDomainNetGetActualBandwidth(def), buf) < 0)
+        return -1;
+    return 0;
+}
+
+
+/* virDomainActualNetDefFormat() - format the ActualNetDef
+ * info inside an <actual> element, as required for internal storage
+ * of domain status
+ */
+static int
+virDomainActualNetDefFormat(virBufferPtr buf,
+                            virDomainNetDefPtr def,
+                            unsigned int flags)
+{
+    unsigned int type = virDomainNetGetActualType(def);
+    const char *typeStr = virDomainNetTypeToString(type);
+
+    if (!def)
+        return 0;
+
+    if (!typeStr) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected net type %d"), def->type);
          return -1;
+    }

+    virBufferAsprintf(buf, "<actual type='%s'", typeStr);
+    if (type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+        virDomainHostdevDefPtr hostdef = virDomainNetGetActualHostdev(def);
+        if  (hostdef && hostdef->managed)
+            virBufferAddLit(buf, " managed='yes'");
+    }
+    virBufferAddLit(buf, ">\n");
+
+    virBufferAdjustIndent(buf, 2);
+    if (virDomainActualNetDefContentsFormat(buf, def, typeStr, true, flags) < 0)
+       return -1;
      virBufferAdjustIndent(buf, -2);
      virBufferAddLit(buf, "</actual>\n");
      return 0;
@@ -15536,8 +15570,13 @@ virDomainNetDefFormat(virBufferPtr buf,
          virBufferEscapeString(buf, " portgroup='%s'",
                                def->data.network.portgroup);
          virBufferAddLit(buf, "/>\n");
+
+        /* ONLY for internal status storage - format the ActualNetDef
+         * as a subelement of <interface> so that no persistent config
+         * data is overwritten.
+         */
          if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) &&
-            (virDomainActualNetDefFormat(buf, def->data.network.actual, flags) < 0))
+            (virDomainActualNetDefFormat(buf, def, flags) < 0))
              return -1;
          break;



ACK

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]