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

Re: [libvirt] [RFC: PATCHv3 3/3] save: generate idempotent inactive xml for running domain



On 07/22/2011 12:21 AM, Eric Blake wrote:
Noticed by comparing the xml generated by virDomainSave with the
xml produced by reparsing and redumping that xml.

* src/conf/domain_conf.c (virDomainDeviceInfoIsSet): Add
parameter, and update all callers.  Make static.
(virDomainNetDefFormat): Skip generated ifname.
* src/conf/domain_conf.h (virDomainDeviceInfoIsSet): Delete.
* src/libvirt_private.syms (domain_conf.h): Update.
---

Sending this now, to get review started, but I still have some
more fixing to do - right now, active domains still include:

+<seclabel type='dynamic' model='selinux' relabel='yes'/>

which is not present on reparse, but I'm too tired to find out why.


I know the feeling :-)


So does it turn out that this is important, or not?


  src/conf/domain_conf.c   |   26 ++++++++++++++++----------
  src/conf/domain_conf.h   |    1 -
  src/libvirt_private.syms |    1 -
  3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 919a75a..52e8ada 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1388,11 +1388,12 @@ int virDomainDeviceVirtioSerialAddressIsValid(
  }


-int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info)
+static int
+virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags)
  {
      if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
          return 1;
-    if (info->alias)
+    if (info->alias&&  !(flags&  VIR_DOMAIN_XML_INACTIVE))
          return 1;
      return 0;
  }
@@ -8580,7 +8581,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
          break;
      }

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          virBufferAddLit(buf, ">\n");
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
@@ -8806,9 +8807,14 @@ virDomainNetDefFormat(virBufferPtr buf,
          break;
      }

-    if (def->ifname)
+
+    if (def->ifname&&
+        !((flags&  VIR_DOMAIN_XML_INACTIVE)&&
+          (STRPREFIX(def->ifname, "vnet")))) {
+        /* Skip auto-generated target names for inactive config. */


It's kind of bothersome that use of this magic device name prefix isn't self-contained in domain_conf.c (or somewhere else). Perhaps the string could be defined in domain_conf.h, then used here and in qemu_command.c (is it used any place else?).

Aside from that bit of ugliness, ACK. (And I could live with this. at least temporarily, as well, although making all the places work off a single string constant might hypothetically save someone lots of frustration in some uncharted future.)

          virBufferEscapeString(buf, "<target dev='%s'/>\n",
                                def->ifname);
+    }
      if (def->model) {
          virBufferEscapeString(buf, "<model type='%s'/>\n",
                                def->model);
@@ -9041,7 +9047,7 @@ virDomainChrDefFormat(virBufferPtr buf,
          break;
      }

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
      }
@@ -9069,7 +9075,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
      virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
      switch (def->type) {
      case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
-        if (!virDomainDeviceInfoIsSet(&def->info)) {
+        if (!virDomainDeviceInfoIsSet(&def->info, flags)) {
              virBufferAddLit(buf, "/>\n");
              return 0;
          }
@@ -9119,7 +9125,7 @@ virDomainSoundDefFormat(virBufferPtr buf,
      virBufferAsprintf(buf, "<sound model='%s'",
                        model);

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          virBufferAddLit(buf, ">\n");
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
@@ -9148,7 +9154,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
      virBufferAsprintf(buf, "<memballoon model='%s'",
                        model);

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          virBufferAddLit(buf, ">\n");
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
@@ -9197,7 +9203,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
      virBufferAsprintf(buf, "<watchdog model='%s' action='%s'",
                        model, action);

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          virBufferAddLit(buf, ">\n");
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
@@ -9280,7 +9286,7 @@ virDomainInputDefFormat(virBufferPtr buf,
      virBufferAsprintf(buf, "<input type='%s' bus='%s'",
                        type, bus);

-    if (virDomainDeviceInfoIsSet(&def->info)) {
+    if (virDomainDeviceInfoIsSet(&def->info, flags)) {
          virBufferAddLit(buf, ">\n");
          if (virDomainDeviceInfoFormat(buf,&def->info, flags)<  0)
              return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 551946b..b97a57a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1388,7 +1388,6 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
  int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
  int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr);
  int virDomainDeviceVirtioSerialAddressIsValid(virDomainDeviceVirtioSerialAddressPtr addr);
-int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info);
  void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
  void virDomainDefClearPCIAddresses(virDomainDefPtr def);
  void virDomainDefClearDeviceAliases(virDomainDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e7d2579..3996efa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -267,7 +267,6 @@ virDomainDeviceAddressIsValid;
  virDomainDeviceAddressTypeToString;
  virDomainDeviceDefFree;
  virDomainDeviceDefParse;
-virDomainDeviceInfoIsSet;
  virDomainDeviceInfoIterate;
  virDomainDevicePCIAddressIsValid;
  virDomainDeviceTypeToString;


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