networkMigrateStateFiles was added nearly 5 years ago when the network
state directory was moved from /var/lib/libvirt to /var/run/libvirt
just prior to libvirt-1.2.4). It was only required to maintain proper
state information for networks that were active during an upgrade that
didn't involve rebooting the host. At this point the likelyhood of
anyone upgrading their libvirt from pre-1.2.4 directly to 5.0.0 or
later *without rebooting the host* is probably so close to 0 that no
properly informed bookie would take *any* odds on it happening, so it
seems appropriate to remove this pointless code.

Signed-off-by: Laine Stump <laine@laine.org>
---

Change from V1 - the now-unused #include <dirent.h> was causing make
syntax-check to fail.


 src/network/bridge_driver.c | 93 -------------------------------------
 1 file changed, 93 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index aed80c04d5..660a93666c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -33,7 +33,6 @@
 #include <sys/wait.h>
 #include <sys/ioctl.h>
 #include <net/if.h>
-#include <dirent.h>
 #if HAVE_SYS_SYSCTL_H
 # include <sys/sysctl.h>
 #endif
@@ -559,92 +558,6 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
 #endif
 
 
-static int
-networkMigrateStateFiles(virNetworkDriverStatePtr driver)
-{
-    /* Due to a change in location of network state xml beginning in
-     * libvirt 1.2.4 (from /var/lib/libvirt/network to
-     * /var/run/libvirt/network), we must check for state files in two
-     * locations. Anything found in the old location must be written
-     * to the new location, then erased from the old location. (Note
-     * that we read/write the file rather than calling rename()
-     * because the old and new state directories are likely in
-     * different filesystems).
-     */
-    int ret = -1;
-    const char *oldStateDir = LOCALSTATEDIR "/lib/libvirt/network";
-    DIR *dir;
-    int direrr;
-    struct dirent *entry;
-    char *oldPath = NULL, *newPath = NULL;
-    char *contents = NULL;
-    int rc;
-
-    if ((rc = virDirOpenIfExists(&dir, oldStateDir)) <= 0)
-        return rc;
-
-    if (virFileMakePath(driver->stateDir) < 0) {
-        virReportSystemError(errno, _("cannot create directory %s"),
-                             driver->stateDir);
-        goto cleanup;
-    }
-
-    while ((direrr = virDirRead(dir, &entry, oldStateDir)) > 0) {
-        if (entry->d_type != DT_UNKNOWN &&
-            entry->d_type != DT_REG)
-            continue;
-
-        if (virAsprintf(&oldPath, "%s/%s",
-                        oldStateDir, entry->d_name) < 0)
-            goto cleanup;
-
-        if (entry->d_type == DT_UNKNOWN) {
-            struct stat st;
-
-            if (lstat(oldPath, &st) < 0) {
-                virReportSystemError(errno,
-                                     _("failed to stat network status file '%s'"),
-                                     oldPath);
-                goto cleanup;
-            }
-
-            if (!S_ISREG(st.st_mode)) {
-                VIR_FREE(oldPath);
-                continue;
-            }
-        }
-
-        if (virFileReadAll(oldPath, 1024*1024, &contents) < 0)
-            goto cleanup;
-
-        if (virAsprintf(&newPath, "%s/%s",
-                        driver->stateDir, entry->d_name) < 0)
-            goto cleanup;
-        if (virFileWriteStr(newPath, contents, S_IRUSR | S_IWUSR) < 0) {
-            virReportSystemError(errno,
-                                 _("failed to write network status file '%s'"),
-                                 newPath);
-            goto cleanup;
-        }
-
-        unlink(oldPath);
-        VIR_FREE(oldPath);
-        VIR_FREE(newPath);
-        VIR_FREE(contents);
-    }
-    if (direrr < 0)
-        goto cleanup;
-
-    ret = 0;
- cleanup:
-    VIR_DIR_CLOSE(dir);
-    VIR_FREE(oldPath);
-    VIR_FREE(newPath);
-    VIR_FREE(contents);
-    return ret;
-}
-
-
 /**
  * networkStateInitialize:
  *
@@ -691,12 +604,6 @@ networkStateInitialize(bool privileged,
                        LOCALSTATEDIR "/lib/libvirt/radvd") < 0)
             goto error;
 
-        /* migration from old to new location is only applicable for
-         * privileged mode - unprivileged mode directories haven't
-         * changed location.
-         */
-        if (networkMigrateStateFiles(network_driver) < 0)
-            goto error;
     } else {
         configdir = virGetUserConfigDirectory();
         rundir = virGetUserRuntimeDirectory();
-- 
2.20.1

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

On Wed, 2019-01-09 at 21:47 -0500, Laine Stump wrote:
> networkMigrateStateFiles was added nearly 5 years ago when the network
> state directory was moved from /var/lib/libvirt to /var/run/libvirt
> just prior to libvirt-1.2.4). It was only required to maintain proper
> state information for networks that were active during an upgrade that
> didn't involve rebooting the host. At this point the likelyhood of
> anyone upgrading their libvirt from pre-1.2.4 directly to 5.0.0 or
> later *without rebooting the host* is probably so close to 0 that no
> properly informed bookie would take *any* odds on it happening, so it
> seems appropriate to remove this pointless code.

The rationale makes sense to me.

[...]
> @@ -691,12 +604,6 @@ networkStateInitialize(bool privileged,
>                         LOCALSTATEDIR "/lib/libvirt/radvd") < 0)
>              goto error;
>  

Please remove this empty line as well.

> -        /* migration from old to new location is only applicable for
> -         * privileged mode - unprivileged mode directories haven't
> -         * changed location.
> -         */
> -        if (networkMigrateStateFiles(network_driver) < 0)
> -            goto error;

With the above addressed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Maybe wait until 5.0.0 is released before pushing, in order to give
other people a chance to voice any concerns they might have.

-- 
Andrea Bolognani / Red Hat / Virtualization

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