Re: [PATCH v4 1/4] nwfilter: Remove unnecessary UUID comparison bypass

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

 



On 12/08/2017 09:01 AM, John Ferlan wrote:
Commit id '46a811db07' added code to check if the filter by Name
already existed with a different UUID, to go along with the existing
found filter by UUID and compare the Names match thus making it
impossible to reach this find by Name condition without both the
Name and UUID already matching, so remove the need to "filter"
out the UUID for the virNWFilterDefEqual.

Sorry, but difficult to parse this sentence.


Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
  src/conf/virnwfilterobj.c | 18 ++++--------------
  1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 408e575ca..87d7e7270 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -287,18 +287,11 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj)

  static bool
  virNWFilterDefEqual(const virNWFilterDef *def1,
-                    virNWFilterDefPtr def2,
-                    bool cmpUUIDs)
+                    virNWFilterDefPtr def2)
  {
      bool ret = false;
-    unsigned char rem_uuid[VIR_UUID_BUFLEN];
-    char *xml1, *xml2 = NULL;
-
-    if (!cmpUUIDs) {
-        /* make sure the UUIDs are equal */
-        memcpy(rem_uuid, def2->uuid, sizeof(rem_uuid));
-        memcpy(def2->uuid, def1->uuid, sizeof(def2->uuid));
-    }
+    char *xml1 = NULL;
+    char *xml2 = NULL;

      if (!(xml1 = virNWFilterDefFormat(def1)) ||
          !(xml2 = virNWFilterDefFormat(def2)))
@@ -307,9 +300,6 @@ virNWFilterDefEqual(const virNWFilterDef *def1,
      ret = STREQ(xml1, xml2);

Thinking out lout here (and below): virNWFilterDefFormat() will write the UUID into the XML. The STREQ will only ever be equal if def1 and def2 have the same UUID.


   cleanup:
-    if (!cmpUUIDs)
-        memcpy(def2->uuid, rem_uuid, sizeof(rem_uuid));
-
      VIR_FREE(xml1);
      VIR_FREE(xml2);

@@ -360,7 +350,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
      if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {

We will not get here if a filter with the same uuid and name exists or same name and different uuid. So we can have the same uuid and a different name at this point.

          objdef = obj->def;
-        if (virNWFilterDefEqual(def, objdef, false)) {
+        if (virNWFilterDefEqual(def, objdef)) {

This call doesn't need to 'shadow' the uuid. It's the only caller. so your change seems to be correct.

Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>

              virNWFilterDefFree(objdef);
              obj->def = def;
              return obj;



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

  Powered by Linux