All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] qga: Add FreeBSD support
@ 2022-10-13  9:23 Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 1/7] qga: Add initial " Alexander Ivanov
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

Add freeze/thaw, shutdown/halt/reboot, password setting and
guest-network-get-interfaces command support for FreeBSD.

v4:
6,7: Return bool instead int in guest_get_hw_addr().

v3:
1: Add a comment about echo suppressing.
5: Replace code moving by splitting the code into a few blocks under
   architecture conditions.
5,6: Move actions with dumb qmp_guest_set_user_password() to
     the appropriate patch.
6: Fix error/obtained return.

v2:
1: Reject the idea to move all the Linux-specific code to a separate file.
   First commit now adds initial support of FreeBSD. Fixed device paths
   and fixed virtio device initialization (disable echo). Add comment why
   we should disable the code under HAVE_GETIFADDRS in FreeBSD.
2: Replace the second commit (which now is the first) by moving
   Linux-specific freeze/thaw code to a separate file commands-linux.c.
3: Add error raising if stat() returns error. Replaced strcmp() calls by
   g_str_equal(). Add a comment explaining why UFSRESUME isn't necessary.
4: Replace #elifdef by #elif defined().
5: Now the code doesn't move from one file to aanother but still is
   moving inside file so the patch doesn't become easier to review. =(
   Fixed typos.
6,7: New patches. Add guest-network-get-interfaces command support.

Alexander Ivanov (7):
  qga: Add initial FreeBSD support
  qga: Move Linux-specific FS freeze/thaw code to a separate file
  qga: Add UFS freeze/thaw support for FreeBSD
  qga: Add shutdown/halt/reboot support for FreeBSD
  qga: Add support for user password setting in FreeBSD
  qga: Move HW address getting to a separate function
  qga: Add HW address getting for FreeBSD

 meson.build           |   2 +-
 qga/channel-posix.c   |  19 ++
 qga/commands-bsd.c    | 200 +++++++++++++
 qga/commands-common.h |  52 ++++
 qga/commands-linux.c  | 286 +++++++++++++++++++
 qga/commands-posix.c  | 641 ++++++++++++++----------------------------
 qga/main.c            |  13 +-
 qga/meson.build       |   6 +
 8 files changed, 780 insertions(+), 439 deletions(-)
 create mode 100644 qga/commands-bsd.c
 create mode 100644 qga/commands-linux.c

-- 
2.34.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/7] qga: Add initial FreeBSD support
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file Alexander Ivanov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

- Fix device path.
- Fix virtio-serial channel initialization.
- Make the code buildable in FreeBSD.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 meson.build          |  2 +-
 qga/channel-posix.c  | 19 +++++++++++++++++++
 qga/commands-posix.c |  8 ++++++++
 qga/main.c           |  6 +++++-
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index b686dfef75..71fe72ea06 100644
--- a/meson.build
+++ b/meson.build
@@ -75,7 +75,7 @@ have_tools = get_option('tools') \
   .allowed()
 have_ga = get_option('guest_agent') \
   .disable_auto_if(not have_system and not have_tools) \
-  .require(targetos in ['sunos', 'linux', 'windows'],
+  .require(targetos in ['sunos', 'linux', 'windows', 'freebsd'],
            error_message: 'unsupported OS for QEMU guest agent') \
   .allowed()
 have_block = have_system or have_tools
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 6796a02cff..568350ded4 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -149,6 +149,25 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
             return false;
         }
 #endif
+#ifdef __FreeBSD__
+        /*
+         * In the default state channel sends echo of every command to a
+         * client. The client programm doesn't expect this and raises an
+         * error. Suppress echo by resetting ECHO terminal flag.
+         */
+        struct termios tio;
+        if (tcgetattr(fd, &tio) < 0) {
+            error_setg_errno(errp, errno, "error getting channel termios attrs");
+            close(fd);
+            return false;
+        }
+        tio.c_lflag &= ~ECHO;
+        if (tcsetattr(fd, TCSAFLUSH, &tio) < 0) {
+            error_setg_errno(errp, errno, "error setting channel termios attrs");
+            close(fd);
+            return false;
+        }
+#endif /* __FreeBSD__ */
         ret = ga_channel_client_add(c, fd);
         if (ret) {
             error_setg(errp, "error adding channel to main loop");
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index eea819cff0..16d67e9f6d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -51,6 +51,14 @@
 #endif
 #endif
 
+#ifdef __FreeBSD__
+/*
+ * The code under HAVE_GETIFADDRS condition can't be compiled in FreeBSD.
+ * Fix it in one of the following patches.
+ */
+#undef HAVE_GETIFADDRS
+#endif
+
 #ifdef HAVE_GETIFADDRS
 #include <arpa/inet.h>
 #include <sys/socket.h>
diff --git a/qga/main.c b/qga/main.c
index 5a9d8252e0..0d27c97d38 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -45,9 +45,13 @@
 #endif
 
 #ifndef _WIN32
+#ifdef __FreeBSD__
+#define QGA_VIRTIO_PATH_DEFAULT "/dev/vtcon/org.qemu.guest_agent.0"
+#else /* __FreeBSD__ */
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
-#define QGA_STATE_RELATIVE_DIR  "run"
+#endif /* __FreeBSD__ */
 #define QGA_SERIAL_PATH_DEFAULT "/dev/ttyS0"
+#define QGA_STATE_RELATIVE_DIR  "run"
 #else
 #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
 #define QGA_STATE_RELATIVE_DIR  "qemu-ga"
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 1/7] qga: Add initial " Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13 12:10   ` Marc-André Lureau
  2022-10-13  9:23 ` [PATCH v4 3/7] qga: Add UFS freeze/thaw support for FreeBSD Alexander Ivanov
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

In the next patches we are going to add FreeBSD support for QEMU Guest
Agent. In the result, code in commands-posix.c will be too cumbersome.

Move Linux-specific FS freeze/thaw code to a separate file commands-linux.c
keeping common POSIX code in commands-posix.c.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-common.h |  35 +++++
 qga/commands-linux.c  | 286 +++++++++++++++++++++++++++++++++++++++++
 qga/commands-posix.c  | 289 +++---------------------------------------
 qga/meson.build       |   3 +
 4 files changed, 340 insertions(+), 273 deletions(-)
 create mode 100644 qga/commands-linux.c

diff --git a/qga/commands-common.h b/qga/commands-common.h
index d0e4a9696f..181fc330aa 100644
--- a/qga/commands-common.h
+++ b/qga/commands-common.h
@@ -10,6 +10,40 @@
 #define QGA_COMMANDS_COMMON_H
 
 #include "qga-qapi-types.h"
+#include "guest-agent-core.h"
+#include "qemu/queue.h"
+
+#if defined(__linux__)
+#include <linux/fs.h>
+#ifdef FIFREEZE
+#define CONFIG_FSFREEZE
+#endif
+#ifdef FITRIM
+#define CONFIG_FSTRIM
+#endif
+#endif /* __linux__ */
+
+#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
+typedef struct FsMount {
+    char *dirname;
+    char *devtype;
+    unsigned int devmajor, devminor;
+    QTAILQ_ENTRY(FsMount) next;
+} FsMount;
+
+typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
+
+bool build_fs_mount_list(FsMountList *mounts, Error **errp);
+void free_fs_mount_list(FsMountList *mounts);
+#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
+
+#if defined(CONFIG_FSFREEZE)
+int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
+                                          strList *mountpoints,
+                                          FsMountList mounts,
+                                          Error **errp);
+int qmp_guest_fsfreeze_do_thaw(Error **errp);
+#endif /* CONFIG_FSFREEZE */
 
 typedef struct GuestFileHandle GuestFileHandle;
 
@@ -29,4 +63,5 @@ GuestFileRead *guest_file_read_unsafe(GuestFileHandle *gfh,
  */
 char *qga_get_host_name(Error **errp);
 
+void ga_wait_child(pid_t pid, int *status, Error **errp);
 #endif
diff --git a/qga/commands-linux.c b/qga/commands-linux.c
new file mode 100644
index 0000000000..214e408fcd
--- /dev/null
+++ b/qga/commands-linux.c
@@ -0,0 +1,286 @@
+/*
+ * QEMU Guest Agent Linux-specific command implementations
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *  Michal Privoznik  <mprivozn@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "commands-common.h"
+#include "cutils.h"
+#include <mntent.h>
+#include <sys/ioctl.h>
+
+#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
+static int dev_major_minor(const char *devpath,
+                           unsigned int *devmajor, unsigned int *devminor)
+{
+    struct stat st;
+
+    *devmajor = 0;
+    *devminor = 0;
+
+    if (stat(devpath, &st) < 0) {
+        slog("failed to stat device file '%s': %s", devpath, strerror(errno));
+        return -1;
+    }
+    if (S_ISDIR(st.st_mode)) {
+        /* It is bind mount */
+        return -2;
+    }
+    if (S_ISBLK(st.st_mode)) {
+        *devmajor = major(st.st_rdev);
+        *devminor = minor(st.st_rdev);
+        return 0;
+    }
+    return -1;
+}
+
+static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
+{
+    struct mntent *ment;
+    FsMount *mount;
+    char const *mtab = "/proc/self/mounts";
+    FILE *fp;
+    unsigned int devmajor, devminor;
+
+    fp = setmntent(mtab, "r");
+    if (!fp) {
+        error_setg(errp, "failed to open mtab file: '%s'", mtab);
+        return false;
+    }
+
+    while ((ment = getmntent(fp))) {
+        /*
+         * An entry which device name doesn't start with a '/' is
+         * either a dummy file system or a network file system.
+         * Add special handling for smbfs and cifs as is done by
+         * coreutils as well.
+         */
+        if ((ment->mnt_fsname[0] != '/') ||
+            (strcmp(ment->mnt_type, "smbfs") == 0) ||
+            (strcmp(ment->mnt_type, "cifs") == 0)) {
+            continue;
+        }
+        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) == -2) {
+            /* Skip bind mounts */
+            continue;
+        }
+
+        mount = g_new0(FsMount, 1);
+        mount->dirname = g_strdup(ment->mnt_dir);
+        mount->devtype = g_strdup(ment->mnt_type);
+        mount->devmajor = devmajor;
+        mount->devminor = devminor;
+
+        QTAILQ_INSERT_TAIL(mounts, mount, next);
+    }
+
+    endmntent(fp);
+    return true;
+}
+
+static void decode_mntname(char *name, int len)
+{
+    int i, j = 0;
+    for (i = 0; i <= len; i++) {
+        if (name[i] != '\\') {
+            name[j++] = name[i];
+        } else if (name[i + 1] == '\\') {
+            name[j++] = '\\';
+            i++;
+        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
+                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
+                   name[i + 3] >= '0' && name[i + 3] <= '7') {
+            name[j++] = (name[i + 1] - '0') * 64 +
+                        (name[i + 2] - '0') * 8 +
+                        (name[i + 3] - '0');
+            i += 3;
+        } else {
+            name[j++] = name[i];
+        }
+    }
+}
+
+/*
+ * Walk the mount table and build a list of local file systems
+ */
+bool build_fs_mount_list(FsMountList *mounts, Error **errp)
+{
+    FsMount *mount;
+    char const *mountinfo = "/proc/self/mountinfo";
+    FILE *fp;
+    char *line = NULL, *dash;
+    size_t n;
+    char check;
+    unsigned int devmajor, devminor;
+    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
+
+    fp = fopen(mountinfo, "r");
+    if (!fp) {
+        return build_fs_mount_list_from_mtab(mounts, errp);
+    }
+
+    while (getline(&line, &n, fp) != -1) {
+        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
+                     &devmajor, &devminor, &dir_s, &dir_e, &check);
+        if (ret < 3) {
+            continue;
+        }
+        dash = strstr(line + dir_e, " - ");
+        if (!dash) {
+            continue;
+        }
+        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
+                     &type_s, &type_e, &dev_s, &dev_e, &check);
+        if (ret < 1) {
+            continue;
+        }
+        line[dir_e] = 0;
+        dash[type_e] = 0;
+        dash[dev_e] = 0;
+        decode_mntname(line + dir_s, dir_e - dir_s);
+        decode_mntname(dash + dev_s, dev_e - dev_s);
+        if (devmajor == 0) {
+            /* btrfs reports major number = 0 */
+            if (strcmp("btrfs", dash + type_s) != 0 ||
+                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0) {
+                continue;
+            }
+        }
+
+        mount = g_new0(FsMount, 1);
+        mount->dirname = g_strdup(line + dir_s);
+        mount->devtype = g_strdup(dash + type_s);
+        mount->devmajor = devmajor;
+        mount->devminor = devminor;
+
+        QTAILQ_INSERT_TAIL(mounts, mount, next);
+    }
+    free(line);
+
+    fclose(fp);
+    return true;
+}
+#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
+
+#ifdef CONFIG_FSFREEZE
+/*
+ * Walk list of mounted file systems in the guest, and freeze the ones which
+ * are real local file systems.
+ */
+int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
+                                          strList *mountpoints,
+                                          FsMountList mounts,
+                                          Error **errp)
+{
+    struct FsMount *mount;
+    strList *list;
+    int fd, ret, i = 0;
+
+    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
+        /* To issue fsfreeze in the reverse order of mounts, check if the
+         * mount is listed in the list here */
+        if (has_mountpoints) {
+            for (list = mountpoints; list; list = list->next) {
+                if (strcmp(list->value, mount->dirname) == 0) {
+                    break;
+                }
+            }
+            if (!list) {
+                continue;
+            }
+        }
+
+        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
+        if (fd == -1) {
+            error_setg_errno(errp, errno, "failed to open %s", mount->dirname);
+            return -1;
+        }
+
+        /* we try to cull filesystems we know won't work in advance, but other
+         * filesystems may not implement fsfreeze for less obvious reasons.
+         * these will report EOPNOTSUPP. we simply ignore these when tallying
+         * the number of frozen filesystems.
+         * if a filesystem is mounted more than once (aka bind mount) a
+         * consecutive attempt to freeze an already frozen filesystem will
+         * return EBUSY.
+         *
+         * any other error means a failure to freeze a filesystem we
+         * expect to be freezable, so return an error in those cases
+         * and return system to thawed state.
+         */
+        ret = ioctl(fd, FIFREEZE);
+        if (ret == -1) {
+            if (errno != EOPNOTSUPP && errno != EBUSY) {
+                error_setg_errno(errp, errno, "failed to freeze %s",
+                                 mount->dirname);
+                close(fd);
+                return -1;
+            }
+        } else {
+            i++;
+        }
+        close(fd);
+    }
+    return i;
+}
+
+int qmp_guest_fsfreeze_do_thaw(Error **errp)
+{
+    int ret;
+    FsMountList mounts;
+    FsMount *mount;
+    int fd, i = 0, logged;
+    Error *local_err = NULL;
+
+    QTAILQ_INIT(&mounts);
+    if (!build_fs_mount_list(&mounts, &local_err)) {
+        error_propagate(errp, local_err);
+        return -1;
+    }
+
+    QTAILQ_FOREACH(mount, &mounts, next) {
+        logged = false;
+        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
+        if (fd == -1) {
+            continue;
+        }
+        /* we have no way of knowing whether a filesystem was actually unfrozen
+         * as a result of a successful call to FITHAW, only that if an error
+         * was returned the filesystem was *not* unfrozen by that particular
+         * call.
+         *
+         * since multiple preceding FIFREEZEs require multiple calls to FITHAW
+         * to unfreeze, continuing issuing FITHAW until an error is returned,
+         * in which case either the filesystem is in an unfreezable state, or,
+         * more likely, it was thawed previously (and remains so afterward).
+         *
+         * also, since the most recent successful call is the one that did
+         * the actual unfreeze, we can use this to provide an accurate count
+         * of the number of filesystems unfrozen by guest-fsfreeze-thaw, which
+         * may * be useful for determining whether a filesystem was unfrozen
+         * during the freeze/thaw phase by a process other than qemu-ga.
+         */
+        do {
+            ret = ioctl(fd, FITHAW);
+            if (ret == 0 && !logged) {
+                i++;
+                logged = true;
+            }
+        } while (ret == 0);
+        close(fd);
+    }
+
+    free_fs_mount_list(&mounts);
+
+    return i;
+}
+#endif /* CONFIG_FSFREEZE */
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 16d67e9f6d..9574b83c92 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -16,11 +16,9 @@
 #include <sys/utsname.h>
 #include <sys/wait.h>
 #include <dirent.h>
-#include "guest-agent-core.h"
 #include "qga-qapi-commands.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
-#include "qemu/queue.h"
 #include "qemu/host-utils.h"
 #include "qemu/sockets.h"
 #include "qemu/base64.h"
@@ -70,7 +68,7 @@
 #endif
 #endif
 
-static void ga_wait_child(pid_t pid, int *status, Error **errp)
+void ga_wait_child(pid_t pid, int *status, Error **errp)
 {
     pid_t rpid;
 
@@ -629,16 +627,7 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
 #if defined(__linux__)
 
 #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
-typedef struct FsMount {
-    char *dirname;
-    char *devtype;
-    unsigned int devmajor, devminor;
-    QTAILQ_ENTRY(FsMount) next;
-} FsMount;
-
-typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
-
-static void free_fs_mount_list(FsMountList *mounts)
+void free_fs_mount_list(FsMountList *mounts)
 {
      FsMount *mount, *temp;
 
@@ -653,157 +642,6 @@ static void free_fs_mount_list(FsMountList *mounts)
          g_free(mount);
      }
 }
-
-static int dev_major_minor(const char *devpath,
-                           unsigned int *devmajor, unsigned int *devminor)
-{
-    struct stat st;
-
-    *devmajor = 0;
-    *devminor = 0;
-
-    if (stat(devpath, &st) < 0) {
-        slog("failed to stat device file '%s': %s", devpath, strerror(errno));
-        return -1;
-    }
-    if (S_ISDIR(st.st_mode)) {
-        /* It is bind mount */
-        return -2;
-    }
-    if (S_ISBLK(st.st_mode)) {
-        *devmajor = major(st.st_rdev);
-        *devminor = minor(st.st_rdev);
-        return 0;
-    }
-    return -1;
-}
-
-/*
- * Walk the mount table and build a list of local file systems
- */
-static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
-{
-    struct mntent *ment;
-    FsMount *mount;
-    char const *mtab = "/proc/self/mounts";
-    FILE *fp;
-    unsigned int devmajor, devminor;
-
-    fp = setmntent(mtab, "r");
-    if (!fp) {
-        error_setg(errp, "failed to open mtab file: '%s'", mtab);
-        return false;
-    }
-
-    while ((ment = getmntent(fp))) {
-        /*
-         * An entry which device name doesn't start with a '/' is
-         * either a dummy file system or a network file system.
-         * Add special handling for smbfs and cifs as is done by
-         * coreutils as well.
-         */
-        if ((ment->mnt_fsname[0] != '/') ||
-            (strcmp(ment->mnt_type, "smbfs") == 0) ||
-            (strcmp(ment->mnt_type, "cifs") == 0)) {
-            continue;
-        }
-        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) == -2) {
-            /* Skip bind mounts */
-            continue;
-        }
-
-        mount = g_new0(FsMount, 1);
-        mount->dirname = g_strdup(ment->mnt_dir);
-        mount->devtype = g_strdup(ment->mnt_type);
-        mount->devmajor = devmajor;
-        mount->devminor = devminor;
-
-        QTAILQ_INSERT_TAIL(mounts, mount, next);
-    }
-
-    endmntent(fp);
-    return true;
-}
-
-static void decode_mntname(char *name, int len)
-{
-    int i, j = 0;
-    for (i = 0; i <= len; i++) {
-        if (name[i] != '\\') {
-            name[j++] = name[i];
-        } else if (name[i + 1] == '\\') {
-            name[j++] = '\\';
-            i++;
-        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
-                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
-                   name[i + 3] >= '0' && name[i + 3] <= '7') {
-            name[j++] = (name[i + 1] - '0') * 64 +
-                        (name[i + 2] - '0') * 8 +
-                        (name[i + 3] - '0');
-            i += 3;
-        } else {
-            name[j++] = name[i];
-        }
-    }
-}
-
-static bool build_fs_mount_list(FsMountList *mounts, Error **errp)
-{
-    FsMount *mount;
-    char const *mountinfo = "/proc/self/mountinfo";
-    FILE *fp;
-    char *line = NULL, *dash;
-    size_t n;
-    char check;
-    unsigned int devmajor, devminor;
-    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
-
-    fp = fopen(mountinfo, "r");
-    if (!fp) {
-        return build_fs_mount_list_from_mtab(mounts, errp);
-    }
-
-    while (getline(&line, &n, fp) != -1) {
-        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
-                     &devmajor, &devminor, &dir_s, &dir_e, &check);
-        if (ret < 3) {
-            continue;
-        }
-        dash = strstr(line + dir_e, " - ");
-        if (!dash) {
-            continue;
-        }
-        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
-                     &type_s, &type_e, &dev_s, &dev_e, &check);
-        if (ret < 1) {
-            continue;
-        }
-        line[dir_e] = 0;
-        dash[type_e] = 0;
-        dash[dev_e] = 0;
-        decode_mntname(line + dir_s, dir_e - dir_s);
-        decode_mntname(dash + dev_s, dev_e - dev_s);
-        if (devmajor == 0) {
-            /* btrfs reports major number = 0 */
-            if (strcmp("btrfs", dash + type_s) != 0 ||
-                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0) {
-                continue;
-            }
-        }
-
-        mount = g_new0(FsMount, 1);
-        mount->dirname = g_strdup(line + dir_s);
-        mount->devtype = g_strdup(dash + type_s);
-        mount->devmajor = devmajor;
-        mount->devminor = devminor;
-
-        QTAILQ_INSERT_TAIL(mounts, mount, next);
-    }
-    free(line);
-
-    fclose(fp);
-    return true;
-}
 #endif
 
 #if defined(CONFIG_FSFREEZE)
@@ -1708,20 +1546,13 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
     return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
 }
 
-/*
- * Walk list of mounted file systems in the guest, and freeze the ones which
- * are real local file systems.
- */
 int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
                                        strList *mountpoints,
                                        Error **errp)
 {
-    int ret = 0, i = 0;
-    strList *list;
+    int ret;
     FsMountList mounts;
-    struct FsMount *mount;
     Error *local_err = NULL;
-    int fd;
 
     slog("guest-fsfreeze called");
 
@@ -1740,122 +1571,34 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
     /* cannot risk guest agent blocking itself on a write in this state */
     ga_set_frozen(ga_state);
 
-    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
-        /* To issue fsfreeze in the reverse order of mounts, check if the
-         * mount is listed in the list here */
-        if (has_mountpoints) {
-            for (list = mountpoints; list; list = list->next) {
-                if (strcmp(list->value, mount->dirname) == 0) {
-                    break;
-                }
-            }
-            if (!list) {
-                continue;
-            }
-        }
-
-        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
-        if (fd == -1) {
-            error_setg_errno(errp, errno, "failed to open %s", mount->dirname);
-            goto error;
-        }
-
-        /* we try to cull filesystems we know won't work in advance, but other
-         * filesystems may not implement fsfreeze for less obvious reasons.
-         * these will report EOPNOTSUPP. we simply ignore these when tallying
-         * the number of frozen filesystems.
-         * if a filesystem is mounted more than once (aka bind mount) a
-         * consecutive attempt to freeze an already frozen filesystem will
-         * return EBUSY.
-         *
-         * any other error means a failure to freeze a filesystem we
-         * expect to be freezable, so return an error in those cases
-         * and return system to thawed state.
-         */
-        ret = ioctl(fd, FIFREEZE);
-        if (ret == -1) {
-            if (errno != EOPNOTSUPP && errno != EBUSY) {
-                error_setg_errno(errp, errno, "failed to freeze %s",
-                                 mount->dirname);
-                close(fd);
-                goto error;
-            }
-        } else {
-            i++;
-        }
-        close(fd);
-    }
+    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
+                                            mounts, errp);
 
     free_fs_mount_list(&mounts);
     /* We may not issue any FIFREEZE here.
      * Just unset ga_state here and ready for the next call.
      */
-    if (i == 0) {
+    if (ret == 0) {
         ga_unset_frozen(ga_state);
+    } else if (ret < 0) {
+        qmp_guest_fsfreeze_thaw(NULL);
     }
-    return i;
-
-error:
-    free_fs_mount_list(&mounts);
-    qmp_guest_fsfreeze_thaw(NULL);
-    return 0;
+    return ret;
 }
 
-/*
- * Walk list of frozen file systems in the guest, and thaw them.
- */
 int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 {
     int ret;
-    FsMountList mounts;
-    FsMount *mount;
-    int fd, i = 0, logged;
-    Error *local_err = NULL;
 
-    QTAILQ_INIT(&mounts);
-    if (!build_fs_mount_list(&mounts, &local_err)) {
-        error_propagate(errp, local_err);
-        return 0;
+    ret = qmp_guest_fsfreeze_do_thaw(errp);
+    if (ret >= 0) {
+        ga_unset_frozen(ga_state);
+        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
+    } else {
+        ret = 0;
     }
 
-    QTAILQ_FOREACH(mount, &mounts, next) {
-        logged = false;
-        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
-        if (fd == -1) {
-            continue;
-        }
-        /* we have no way of knowing whether a filesystem was actually unfrozen
-         * as a result of a successful call to FITHAW, only that if an error
-         * was returned the filesystem was *not* unfrozen by that particular
-         * call.
-         *
-         * since multiple preceding FIFREEZEs require multiple calls to FITHAW
-         * to unfreeze, continuing issuing FITHAW until an error is returned,
-         * in which case either the filesystem is in an unfreezable state, or,
-         * more likely, it was thawed previously (and remains so afterward).
-         *
-         * also, since the most recent successful call is the one that did
-         * the actual unfreeze, we can use this to provide an accurate count
-         * of the number of filesystems unfrozen by guest-fsfreeze-thaw, which
-         * may * be useful for determining whether a filesystem was unfrozen
-         * during the freeze/thaw phase by a process other than qemu-ga.
-         */
-        do {
-            ret = ioctl(fd, FITHAW);
-            if (ret == 0 && !logged) {
-                i++;
-                logged = true;
-            }
-        } while (ret == 0);
-        close(fd);
-    }
-
-    ga_unset_frozen(ga_state);
-    free_fs_mount_list(&mounts);
-
-    execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
-
-    return i;
+    return ret;
 }
 
 static void guest_fsfreeze_cleanup(void)
diff --git a/qga/meson.build b/qga/meson.build
index a0ffd6d268..932b4e7ca8 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -72,6 +72,9 @@ qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
   'commands-posix.c',
   'commands-posix-ssh.c',
 ))
+qga_ss.add(when: 'CONFIG_LINUX', if_true: files(
+  'commands-linux.c',
+))
 qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
   'channel-win32.c',
   'commands-win32.c',
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/7] qga: Add UFS freeze/thaw support for FreeBSD
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 1/7] qga: Add initial " Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 4/7] qga: Add shutdown/halt/reboot " Alexander Ivanov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

UFS supports FS freezing through ioctl UFSSUSPEND on /dev/ufssuspend.
Frozen FS can be thawed by closing /dev/ufssuspend file descriptior.

Use getmntinfo to get a list of mounted FS.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-bsd.c    | 169 +++++++++++++++++++++++
 qga/commands-common.h |  11 ++
 qga/commands-posix.c  | 308 ++++++++++++++++++++----------------------
 qga/main.c            |   7 +-
 qga/meson.build       |   3 +
 5 files changed, 334 insertions(+), 164 deletions(-)
 create mode 100644 qga/commands-bsd.c

diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
new file mode 100644
index 0000000000..ca06692179
--- /dev/null
+++ b/qga/commands-bsd.c
@@ -0,0 +1,169 @@
+/*
+ * QEMU Guest Agent BSD-specific command implementations
+ *
+ * Copyright (c) Virtuozzo International GmbH.
+ *
+ * Authors:
+ *  Alexander Ivanov  <alexander.ivanov@virtuozzo.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qga-qapi-commands.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/error.h"
+#include "qemu/queue.h"
+#include "commands-common.h"
+#include <sys/ioctl.h>
+#include <sys/param.h>
+#include <sys/ucred.h>
+#include <sys/mount.h>
+#include <paths.h>
+
+#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
+bool build_fs_mount_list(FsMountList *mounts, Error **errp)
+{
+    FsMount *mount;
+    struct statfs *mntbuf, *mntp;
+    struct stat statbuf;
+    int i, count, ret;
+
+    count = getmntinfo(&mntbuf, MNT_NOWAIT);
+    if (count == 0) {
+        error_setg_errno(errp, errno, "getmntinfo failed");
+        return false;
+    }
+
+    for (i = 0; i < count; i++) {
+        mntp = &mntbuf[i];
+        ret = stat(mntp->f_mntonname, &statbuf);
+        if (ret != 0) {
+            error_setg_errno(errp, errno, "stat failed on %s",
+                             mntp->f_mntonname);
+            return false;
+        }
+
+        mount = g_new0(FsMount, 1);
+
+        mount->dirname = g_strdup(mntp->f_mntonname);
+        mount->devtype = g_strdup(mntp->f_fstypename);
+        mount->devmajor = major(mount->dev);
+        mount->devminor = minor(mount->dev);
+        mount->fsid = mntp->f_fsid;
+        mount->dev = statbuf.st_dev;
+
+        QTAILQ_INSERT_TAIL(mounts, mount, next);
+    }
+    return true;
+}
+#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
+
+#if defined(CONFIG_FSFREEZE)
+static int ufssuspend_fd = -1;
+static int ufssuspend_cnt;
+
+int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
+                                          strList *mountpoints,
+                                          FsMountList mounts,
+                                          Error **errp)
+{
+    int ret;
+    strList *list;
+    struct FsMount *mount;
+
+    if (ufssuspend_fd != -1) {
+        error_setg(errp, "filesystems have already frozen");
+        return -1;
+    }
+
+    ufssuspend_cnt = 0;
+    ufssuspend_fd = qemu_open(_PATH_UFSSUSPEND, O_RDWR, errp);
+    if (ufssuspend_fd == -1) {
+        return -1;
+    }
+
+    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
+        /*
+         * To issue fsfreeze in the reverse order of mounts, check if the
+         * mount is listed in the list here
+         */
+        if (has_mountpoints) {
+            for (list = mountpoints; list; list = list->next) {
+                if (g_str_equal(list->value, mount->dirname)) {
+                    break;
+                }
+            }
+            if (!list) {
+                continue;
+            }
+        }
+
+        /* Only UFS supports suspend */
+        if (!g_str_equal(mount->devtype, "ufs")) {
+            continue;
+        }
+
+        ret = ioctl(ufssuspend_fd, UFSSUSPEND, &mount->fsid);
+        if (ret == -1) {
+            /*
+             * ioctl returns EBUSY for all the FS except the first one
+             * that was suspended
+             */
+            if (errno == EBUSY) {
+                continue;
+            }
+            error_setg_errno(errp, errno, "failed to freeze %s",
+                             mount->dirname);
+            goto error;
+        }
+        ufssuspend_cnt++;
+    }
+    return ufssuspend_cnt;
+error:
+    close(ufssuspend_fd);
+    ufssuspend_fd = -1;
+    return -1;
+
+}
+
+/*
+ * We don't need to call UFSRESUME ioctl because all the frozen FS
+ * are thawed on /dev/ufssuspend closing.
+ */
+int qmp_guest_fsfreeze_do_thaw(Error **errp)
+{
+    int ret = ufssuspend_cnt;
+    ufssuspend_cnt = 0;
+    if (ufssuspend_fd != -1) {
+        close(ufssuspend_fd);
+        ufssuspend_fd = -1;
+    }
+    return ret;
+}
+
+GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+#endif /* CONFIG_FSFREEZE */
diff --git a/qga/commands-common.h b/qga/commands-common.h
index 181fc330aa..2d9878a634 100644
--- a/qga/commands-common.h
+++ b/qga/commands-common.h
@@ -23,11 +23,22 @@
 #endif
 #endif /* __linux__ */
 
+#ifdef __FreeBSD__
+#include <ufs/ffs/fs.h>
+#ifdef UFSSUSPEND
+#define CONFIG_FSFREEZE
+#endif
+#endif /* __FreeBSD__ */
+
 #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
 typedef struct FsMount {
     char *dirname;
     char *devtype;
     unsigned int devmajor, devminor;
+#if defined(__FreeBSD__)
+    dev_t dev;
+    fsid_t fsid;
+#endif
     QTAILQ_ENTRY(FsMount) next;
 } FsMount;
 
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 9574b83c92..49f9996a9c 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -33,20 +33,12 @@
 
 #if defined(__linux__)
 #include <mntent.h>
-#include <linux/fs.h>
 #include <sys/statvfs.h>
 #include <linux/nvme_ioctl.h>
 
 #ifdef CONFIG_LIBUDEV
 #include <libudev.h>
 #endif
-
-#ifdef FIFREEZE
-#define CONFIG_FSFREEZE
-#endif
-#ifdef FITRIM
-#define CONFIG_FSTRIM
-#endif
 #endif
 
 #ifdef __FreeBSD__
@@ -623,9 +615,6 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
     }
 }
 
-/* linux-specific implementations. avoid this if at all possible. */
-#if defined(__linux__)
-
 #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
 void free_fs_mount_list(FsMountList *mounts)
 {
@@ -644,6 +633,156 @@ void free_fs_mount_list(FsMountList *mounts)
 }
 #endif
 
+#if defined(CONFIG_FSFREEZE)
+typedef enum {
+    FSFREEZE_HOOK_THAW = 0,
+    FSFREEZE_HOOK_FREEZE,
+} FsfreezeHookArg;
+
+static const char *fsfreeze_hook_arg_string[] = {
+    "thaw",
+    "freeze",
+};
+
+static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **errp)
+{
+    int status;
+    pid_t pid;
+    const char *hook;
+    const char *arg_str = fsfreeze_hook_arg_string[arg];
+    Error *local_err = NULL;
+
+    hook = ga_fsfreeze_hook(ga_state);
+    if (!hook) {
+        return;
+    }
+    if (access(hook, X_OK) != 0) {
+        error_setg_errno(errp, errno, "can't access fsfreeze hook '%s'", hook);
+        return;
+    }
+
+    slog("executing fsfreeze hook with arg '%s'", arg_str);
+    pid = fork();
+    if (pid == 0) {
+        setsid();
+        reopen_fd_to_null(0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        execl(hook, hook, arg_str, NULL);
+        _exit(EXIT_FAILURE);
+    } else if (pid < 0) {
+        error_setg_errno(errp, errno, "failed to create child process");
+        return;
+    }
+
+    ga_wait_child(pid, &status, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (!WIFEXITED(status)) {
+        error_setg(errp, "fsfreeze hook has terminated abnormally");
+        return;
+    }
+
+    status = WEXITSTATUS(status);
+    if (status) {
+        error_setg(errp, "fsfreeze hook has failed with status %d", status);
+        return;
+    }
+}
+
+/*
+ * Return status of freeze/thaw
+ */
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
+{
+    if (ga_is_frozen(ga_state)) {
+        return GUEST_FSFREEZE_STATUS_FROZEN;
+    }
+
+    return GUEST_FSFREEZE_STATUS_THAWED;
+}
+
+int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+    return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+                                       strList *mountpoints,
+                                       Error **errp)
+{
+    int ret;
+    FsMountList mounts;
+    Error *local_err = NULL;
+
+    slog("guest-fsfreeze called");
+
+    execute_fsfreeze_hook(FSFREEZE_HOOK_FREEZE, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return -1;
+    }
+
+    QTAILQ_INIT(&mounts);
+    if (!build_fs_mount_list(&mounts, &local_err)) {
+        error_propagate(errp, local_err);
+        return -1;
+    }
+
+    /* cannot risk guest agent blocking itself on a write in this state */
+    ga_set_frozen(ga_state);
+
+    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
+                                            mounts, errp);
+
+    free_fs_mount_list(&mounts);
+    /* We may not issue any FIFREEZE here.
+     * Just unset ga_state here and ready for the next call.
+     */
+    if (ret == 0) {
+        ga_unset_frozen(ga_state);
+    } else if (ret < 0) {
+        qmp_guest_fsfreeze_thaw(NULL);
+    }
+    return ret;
+}
+
+int64_t qmp_guest_fsfreeze_thaw(Error **errp)
+{
+    int ret;
+
+    ret = qmp_guest_fsfreeze_do_thaw(errp);
+    if (ret >= 0) {
+        ga_unset_frozen(ga_state);
+        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
+    } else {
+        ret = 0;
+    }
+
+    return ret;
+}
+
+static void guest_fsfreeze_cleanup(void)
+{
+    Error *err = NULL;
+
+    if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
+        qmp_guest_fsfreeze_thaw(&err);
+        if (err) {
+            slog("failed to clean up frozen filesystems: %s",
+                 error_get_pretty(err));
+            error_free(err);
+        }
+    }
+}
+#endif
+
+/* linux-specific implementations. avoid this if at all possible. */
+#if defined(__linux__)
 #if defined(CONFIG_FSFREEZE)
 
 static char *get_pci_driver(char const *syspath, int pathlen, Error **errp)
@@ -1467,153 +1606,6 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
     free_fs_mount_list(&mounts);
     return ret;
 }
-
-
-typedef enum {
-    FSFREEZE_HOOK_THAW = 0,
-    FSFREEZE_HOOK_FREEZE,
-} FsfreezeHookArg;
-
-static const char *fsfreeze_hook_arg_string[] = {
-    "thaw",
-    "freeze",
-};
-
-static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **errp)
-{
-    int status;
-    pid_t pid;
-    const char *hook;
-    const char *arg_str = fsfreeze_hook_arg_string[arg];
-    Error *local_err = NULL;
-
-    hook = ga_fsfreeze_hook(ga_state);
-    if (!hook) {
-        return;
-    }
-    if (access(hook, X_OK) != 0) {
-        error_setg_errno(errp, errno, "can't access fsfreeze hook '%s'", hook);
-        return;
-    }
-
-    slog("executing fsfreeze hook with arg '%s'", arg_str);
-    pid = fork();
-    if (pid == 0) {
-        setsid();
-        reopen_fd_to_null(0);
-        reopen_fd_to_null(1);
-        reopen_fd_to_null(2);
-
-        execl(hook, hook, arg_str, NULL);
-        _exit(EXIT_FAILURE);
-    } else if (pid < 0) {
-        error_setg_errno(errp, errno, "failed to create child process");
-        return;
-    }
-
-    ga_wait_child(pid, &status, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    if (!WIFEXITED(status)) {
-        error_setg(errp, "fsfreeze hook has terminated abnormally");
-        return;
-    }
-
-    status = WEXITSTATUS(status);
-    if (status) {
-        error_setg(errp, "fsfreeze hook has failed with status %d", status);
-        return;
-    }
-}
-
-/*
- * Return status of freeze/thaw
- */
-GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
-{
-    if (ga_is_frozen(ga_state)) {
-        return GUEST_FSFREEZE_STATUS_FROZEN;
-    }
-
-    return GUEST_FSFREEZE_STATUS_THAWED;
-}
-
-int64_t qmp_guest_fsfreeze_freeze(Error **errp)
-{
-    return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
-}
-
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-                                       strList *mountpoints,
-                                       Error **errp)
-{
-    int ret;
-    FsMountList mounts;
-    Error *local_err = NULL;
-
-    slog("guest-fsfreeze called");
-
-    execute_fsfreeze_hook(FSFREEZE_HOOK_FREEZE, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return -1;
-    }
-
-    QTAILQ_INIT(&mounts);
-    if (!build_fs_mount_list(&mounts, &local_err)) {
-        error_propagate(errp, local_err);
-        return -1;
-    }
-
-    /* cannot risk guest agent blocking itself on a write in this state */
-    ga_set_frozen(ga_state);
-
-    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
-                                            mounts, errp);
-
-    free_fs_mount_list(&mounts);
-    /* We may not issue any FIFREEZE here.
-     * Just unset ga_state here and ready for the next call.
-     */
-    if (ret == 0) {
-        ga_unset_frozen(ga_state);
-    } else if (ret < 0) {
-        qmp_guest_fsfreeze_thaw(NULL);
-    }
-    return ret;
-}
-
-int64_t qmp_guest_fsfreeze_thaw(Error **errp)
-{
-    int ret;
-
-    ret = qmp_guest_fsfreeze_do_thaw(errp);
-    if (ret >= 0) {
-        ga_unset_frozen(ga_state);
-        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
-    } else {
-        ret = 0;
-    }
-
-    return ret;
-}
-
-static void guest_fsfreeze_cleanup(void)
-{
-    Error *err = NULL;
-
-    if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
-        qmp_guest_fsfreeze_thaw(&err);
-        if (err) {
-            slog("failed to clean up frozen filesystems: %s",
-                 error_get_pretty(err));
-            error_free(err);
-        }
-    }
-}
 #endif /* CONFIG_FSFREEZE */
 
 #if defined(CONFIG_FSTRIM)
diff --git a/qga/main.c b/qga/main.c
index 0d27c97d38..b3580508fa 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -37,12 +37,7 @@
 #include "qga/service-win32.h"
 #include "qga/vss-win32.h"
 #endif
-#ifdef __linux__
-#include <linux/fs.h>
-#ifdef FIFREEZE
-#define CONFIG_FSFREEZE
-#endif
-#endif
+#include "commands-common.h"
 
 #ifndef _WIN32
 #ifdef __FreeBSD__
diff --git a/qga/meson.build b/qga/meson.build
index 932b4e7ca8..3cfb9166e5 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -75,6 +75,9 @@ qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
 qga_ss.add(when: 'CONFIG_LINUX', if_true: files(
   'commands-linux.c',
 ))
+qga_ss.add(when: 'CONFIG_BSD', if_true: files(
+  'commands-bsd.c',
+))
 qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
   'channel-win32.c',
   'commands-win32.c',
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 4/7] qga: Add shutdown/halt/reboot support for FreeBSD
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
                   ` (2 preceding siblings ...)
  2022-10-13  9:23 ` [PATCH v4 3/7] qga: Add UFS freeze/thaw support for FreeBSD Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 5/7] qga: Add support for user password setting in FreeBSD Alexander Ivanov
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

Add appropriate shutdown command arguments to qmp_guest_shutdown()
for FreeBSD.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-posix.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 49f9996a9c..88e0d0fe24 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -90,6 +90,10 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
     const char *powerdown_flag = "-i5";
     const char *halt_flag = "-i0";
     const char *reboot_flag = "-i6";
+#elif defined(CONFIG_BSD)
+    const char *powerdown_flag = "-p";
+    const char *halt_flag = "-h";
+    const char *reboot_flag = "-r";
 #else
     const char *powerdown_flag = "-P";
     const char *halt_flag = "-H";
@@ -120,6 +124,9 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 #ifdef CONFIG_SOLARIS
         execl("/sbin/shutdown", "shutdown", shutdown_flag, "-g0", "-y",
               "hypervisor initiated shutdown", (char *)NULL);
+#elif defined(CONFIG_BSD)
+        execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
+               "hypervisor initiated shutdown", (char *)NULL);
 #else
         execl("/sbin/shutdown", "shutdown", "-h", shutdown_flag, "+0",
                "hypervisor initiated shutdown", (char *)NULL);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 5/7] qga: Add support for user password setting in FreeBSD
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
                   ` (3 preceding siblings ...)
  2022-10-13  9:23 ` [PATCH v4 4/7] qga: Add shutdown/halt/reboot " Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 6/7] qga: Move HW address getting to a separate function Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 7/7] qga: Add HW address getting for FreeBSD Alexander Ivanov
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

Move qmp_guest_set_user_password() from __linux__ condition to
(__linux__ || __FreeBSD__) condition. Add command and arguments
for password setting in FreeBSD.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-posix.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 88e0d0fe24..f5b9e5c530 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2122,7 +2122,9 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
 
     return processed;
 }
+#endif /* __linux__ */
 
+#if defined(__linux__) || defined(__FreeBSD__)
 void qmp_guest_set_user_password(const char *username,
                                  const char *password,
                                  bool crypted,
@@ -2156,10 +2158,15 @@ void qmp_guest_set_user_password(const char *username,
         goto out;
     }
 
+#ifdef __FreeBSD__
+    chpasswddata = g_strdup(rawpasswddata);
+    passwd_path = g_find_program_in_path("pw");
+#else
     chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata);
-    chpasswdlen = strlen(chpasswddata);
-
     passwd_path = g_find_program_in_path("chpasswd");
+#endif
+
+    chpasswdlen = strlen(chpasswddata);
 
     if (!passwd_path) {
         error_setg(errp, "cannot find 'passwd' program in PATH");
@@ -2180,11 +2187,17 @@ void qmp_guest_set_user_password(const char *username,
         reopen_fd_to_null(1);
         reopen_fd_to_null(2);
 
+#ifdef __FreeBSD__
+        const char *h_arg;
+        h_arg = (crypted) ? "-H" : "-h";
+        execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0", NULL);
+#else
         if (crypted) {
             execl(passwd_path, "chpasswd", "-e", NULL);
         } else {
             execl(passwd_path, "chpasswd", NULL);
         }
+#endif
         _exit(EXIT_FAILURE);
     } else if (pid < 0) {
         error_setg_errno(errp, errno, "failed to create child process");
@@ -2227,7 +2240,17 @@ out:
         close(datafd[1]);
     }
 }
+#else /* __linux__ || __FreeBSD__ */
+void qmp_guest_set_user_password(const char *username,
+                                 const char *password,
+                                 bool crypted,
+                                 Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+}
+#endif /* __linux__ || __FreeBSD__ */
 
+#ifdef __linux__
 static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf,
                                int size, Error **errp)
 {
@@ -2764,14 +2787,6 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
-void qmp_guest_set_user_password(const char *username,
-                                 const char *password,
-                                 bool crypted,
-                                 Error **errp)
-{
-    error_setg(errp, QERR_UNSUPPORTED);
-}
-
 GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
 {
     error_setg(errp, QERR_UNSUPPORTED);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 6/7] qga: Move HW address getting to a separate function
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
                   ` (4 preceding siblings ...)
  2022-10-13  9:23 ` [PATCH v4 5/7] qga: Add support for user password setting in FreeBSD Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  2022-10-13  9:23 ` [PATCH v4 7/7] qga: Add HW address getting for FreeBSD Alexander Ivanov
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

In the next patch FreeBSD support for guest-network-get-interfaces will be
added. Previously move Linux-specific code of HW address getting to a
separate functions and add a dumb function to commands-bsd.c.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-bsd.c    | 16 +++++++
 qga/commands-common.h |  6 +++
 qga/commands-posix.c  | 98 ++++++++++++++++++++++++-------------------
 3 files changed, 78 insertions(+), 42 deletions(-)

diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
index ca06692179..ebf0fb8b0f 100644
--- a/qga/commands-bsd.c
+++ b/qga/commands-bsd.c
@@ -167,3 +167,19 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
     return NULL;
 }
 #endif /* CONFIG_FSFREEZE */
+
+#ifdef HAVE_GETIFADDRS
+/*
+ * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a
+ * buffer with ETHER_ADDR_LEN length at least.
+ *
+ * Returns false in case of an error, otherwise true. "obtained" arguument
+ * is true if a MAC address was obtained successful, otherwise false.
+ */
+bool guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
+                       bool *obtained, Error **errp)
+{
+    *obtained = false;
+    return true;
+}
+#endif /* HAVE_GETIFADDRS */
diff --git a/qga/commands-common.h b/qga/commands-common.h
index 2d9878a634..05d1f7ccdd 100644
--- a/qga/commands-common.h
+++ b/qga/commands-common.h
@@ -56,6 +56,12 @@ int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
 int qmp_guest_fsfreeze_do_thaw(Error **errp);
 #endif /* CONFIG_FSFREEZE */
 
+#ifdef HAVE_GETIFADDRS
+#include <ifaddrs.h>
+bool guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
+                       bool *obtained, Error **errp);
+#endif
+
 typedef struct GuestFileHandle GuestFileHandle;
 
 GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f5b9e5c530..787ffb1562 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -41,20 +41,12 @@
 #endif
 #endif
 
-#ifdef __FreeBSD__
-/*
- * The code under HAVE_GETIFADDRS condition can't be compiled in FreeBSD.
- * Fix it in one of the following patches.
- */
-#undef HAVE_GETIFADDRS
-#endif
-
 #ifdef HAVE_GETIFADDRS
 #include <arpa/inet.h>
 #include <sys/socket.h>
 #include <net/if.h>
+#include <net/ethernet.h>
 #include <sys/types.h>
-#include <ifaddrs.h>
 #ifdef CONFIG_SOLARIS
 #include <sys/sockio.h>
 #endif
@@ -2889,6 +2881,57 @@ static int guest_get_network_stats(const char *name,
     return -1;
 }
 
+#ifndef __FreeBSD__
+/*
+ * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a
+ * buffer with ETHER_ADDR_LEN length at least.
+ *
+ * Returns false in case of an error, otherwise true. "obtained" argument
+ * is true if a MAC address was obtained successful, otherwise false.
+ */
+bool guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
+                       bool *obtained, Error **errp)
+{
+    struct ifreq ifr;
+    int sock;
+
+    *obtained = false;
+
+    /* we haven't obtained HW address yet */
+    sock = socket(PF_INET, SOCK_STREAM, 0);
+    if (sock == -1) {
+        error_setg_errno(errp, errno, "failed to create socket");
+        return false;
+    }
+
+    memset(&ifr, 0, sizeof(ifr));
+    pstrcpy(ifr.ifr_name, IF_NAMESIZE, ifa->ifa_name);
+    if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
+        /*
+         * We can't get the hw addr of this interface, but that's not a
+         * fatal error.
+         */
+        if (errno == EADDRNOTAVAIL) {
+            /* The interface doesn't have a hw addr (e.g. loopback). */
+            g_debug("failed to get MAC address of %s: %s",
+                    ifa->ifa_name, strerror(errno));
+        } else{
+            g_warning("failed to get MAC address of %s: %s",
+                      ifa->ifa_name, strerror(errno));
+        }
+    } else {
+#ifdef CONFIG_SOLARIS
+        memcpy(buf, &ifr.ifr_addr.sa_data, ETHER_ADDR_LEN);
+#else
+        memcpy(buf, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
+#endif
+        *obtained = true;
+    }
+    close(sock);
+    return true;
+}
+#endif /* __FreeBSD__ */
+
 /*
  * Build information about guest interfaces
  */
@@ -2909,9 +2952,8 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
         GuestNetworkInterfaceStat *interface_stat = NULL;
         char addr4[INET_ADDRSTRLEN];
         char addr6[INET6_ADDRSTRLEN];
-        int sock;
-        struct ifreq ifr;
-        unsigned char *mac_addr;
+        unsigned char mac_addr[ETHER_ADDR_LEN];
+        bool obtained;
         void *p;
 
         g_debug("Processing %s interface", ifa->ifa_name);
@@ -2926,45 +2968,17 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
         }
 
         if (!info->has_hardware_address) {
-            /* we haven't obtained HW address yet */
-            sock = socket(PF_INET, SOCK_STREAM, 0);
-            if (sock == -1) {
-                error_setg_errno(errp, errno, "failed to create socket");
+            if (!guest_get_hw_addr(ifa, mac_addr, &obtained, errp)) {
                 goto error;
             }
-
-            memset(&ifr, 0, sizeof(ifr));
-            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
-            if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
-                /*
-                 * We can't get the hw addr of this interface, but that's not a
-                 * fatal error. Don't set info->hardware_address, but keep
-                 * going.
-                 */
-                if (errno == EADDRNOTAVAIL) {
-                    /* The interface doesn't have a hw addr (e.g. loopback). */
-                    g_debug("failed to get MAC address of %s: %s",
-                            ifa->ifa_name, strerror(errno));
-                } else{
-                    g_warning("failed to get MAC address of %s: %s",
-                              ifa->ifa_name, strerror(errno));
-                }
-
-            } else {
-#ifdef CONFIG_SOLARIS
-                mac_addr = (unsigned char *) &ifr.ifr_addr.sa_data;
-#else
-                mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
-#endif
+            if (obtained) {
                 info->hardware_address =
                     g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
                                     (int) mac_addr[0], (int) mac_addr[1],
                                     (int) mac_addr[2], (int) mac_addr[3],
                                     (int) mac_addr[4], (int) mac_addr[5]);
-
                 info->has_hardware_address = true;
             }
-            close(sock);
         }
 
         if (ifa->ifa_addr &&
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 7/7] qga: Add HW address getting for FreeBSD
  2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
                   ` (5 preceding siblings ...)
  2022-10-13  9:23 ` [PATCH v4 6/7] qga: Move HW address getting to a separate function Alexander Ivanov
@ 2022-10-13  9:23 ` Alexander Ivanov
  6 siblings, 0 replies; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-13  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau

Replace a dumb function in commands-bsd.c by the code of HW address
getting.

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-bsd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
index ebf0fb8b0f..15cade2d4c 100644
--- a/qga/commands-bsd.c
+++ b/qga/commands-bsd.c
@@ -20,6 +20,8 @@
 #include <sys/param.h>
 #include <sys/ucred.h>
 #include <sys/mount.h>
+#include <net/if_dl.h>
+#include <net/ethernet.h>
 #include <paths.h>
 
 #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
@@ -179,7 +181,20 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
 bool guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
                        bool *obtained, Error **errp)
 {
+    struct sockaddr_dl *sdp;
+
     *obtained = false;
+
+    if (ifa->ifa_addr->sa_family != AF_LINK) {
+        /* We can get HW address only for AF_LINK family. */
+        g_debug("failed to get MAC address of %s", ifa->ifa_name);
+        return true;
+    }
+
+    sdp = (struct sockaddr_dl *)ifa->ifa_addr;
+    memcpy(buf, sdp->sdl_data + sdp->sdl_nlen, ETHER_ADDR_LEN);
+    *obtained = true;
+
     return true;
 }
 #endif /* HAVE_GETIFADDRS */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file
  2022-10-13  9:23 ` [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file Alexander Ivanov
@ 2022-10-13 12:10   ` Marc-André Lureau
  2022-10-14  7:55     ` Alexander Ivanov
  0 siblings, 1 reply; 11+ messages in thread
From: Marc-André Lureau @ 2022-10-13 12:10 UTC (permalink / raw)
  To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, kkostiuk

[-- Attachment #1: Type: text/plain, Size: 25389 bytes --]

Hi

On Thu, Oct 13, 2022 at 1:23 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:

> In the next patches we are going to add FreeBSD support for QEMU Guest
> Agent. In the result, code in commands-posix.c will be too cumbersome.
>
> Move Linux-specific FS freeze/thaw code to a separate file commands-linux.c
> keeping common POSIX code in commands-posix.c.
>
> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  qga/commands-common.h |  35 +++++
>  qga/commands-linux.c  | 286 +++++++++++++++++++++++++++++++++++++++++
>  qga/commands-posix.c  | 289 +++---------------------------------------
>  qga/meson.build       |   3 +
>  4 files changed, 340 insertions(+), 273 deletions(-)
>  create mode 100644 qga/commands-linux.c
>
> diff --git a/qga/commands-common.h b/qga/commands-common.h
> index d0e4a9696f..181fc330aa 100644
> --- a/qga/commands-common.h
> +++ b/qga/commands-common.h
> @@ -10,6 +10,40 @@
>  #define QGA_COMMANDS_COMMON_H
>
>  #include "qga-qapi-types.h"
> +#include "guest-agent-core.h"
> +#include "qemu/queue.h"
> +
> +#if defined(__linux__)
> +#include <linux/fs.h>
> +#ifdef FIFREEZE
> +#define CONFIG_FSFREEZE
> +#endif
> +#ifdef FITRIM
> +#define CONFIG_FSTRIM
> +#endif
> +#endif /* __linux__ */
> +
> +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
> +typedef struct FsMount {
> +    char *dirname;
> +    char *devtype;
> +    unsigned int devmajor, devminor;
> +    QTAILQ_ENTRY(FsMount) next;
> +} FsMount;
> +
> +typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
> +
> +bool build_fs_mount_list(FsMountList *mounts, Error **errp);
> +void free_fs_mount_list(FsMountList *mounts);
> +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
> +
> +#if defined(CONFIG_FSFREEZE)
> +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
> +                                          strList *mountpoints,
> +                                          FsMountList mounts,
> +                                          Error **errp);
> +int qmp_guest_fsfreeze_do_thaw(Error **errp);
> +#endif /* CONFIG_FSFREEZE */
>
>  typedef struct GuestFileHandle GuestFileHandle;
>
> @@ -29,4 +63,5 @@ GuestFileRead *guest_file_read_unsafe(GuestFileHandle
> *gfh,
>   */
>  char *qga_get_host_name(Error **errp);
>
> +void ga_wait_child(pid_t pid, int *status, Error **errp);
>

This doesn't belong here, afaict.



>  #endif
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> new file mode 100644
> index 0000000000..214e408fcd
> --- /dev/null
> +++ b/qga/commands-linux.c
> @@ -0,0 +1,286 @@
> +/*
> + * QEMU Guest Agent Linux-specific command implementations
> + *
> + * Copyright IBM Corp. 2011
> + *
> + * Authors:
> + *  Michael Roth      <mdroth@linux.vnet.ibm.com>
> + *  Michal Privoznik  <mprivozn@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "commands-common.h"
> +#include "cutils.h"
> +#include <mntent.h>
> +#include <sys/ioctl.h>
> +
> +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
> +static int dev_major_minor(const char *devpath,
> +                           unsigned int *devmajor, unsigned int *devminor)
> +{
> +    struct stat st;
> +
> +    *devmajor = 0;
> +    *devminor = 0;
> +
> +    if (stat(devpath, &st) < 0) {
> +        slog("failed to stat device file '%s': %s", devpath,
> strerror(errno));
> +        return -1;
> +    }
> +    if (S_ISDIR(st.st_mode)) {
> +        /* It is bind mount */
> +        return -2;
> +    }
> +    if (S_ISBLK(st.st_mode)) {
> +        *devmajor = major(st.st_rdev);
> +        *devminor = minor(st.st_rdev);
> +        return 0;
> +    }
> +    return -1;
> +}
> +
> +static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error
> **errp)
> +{
> +    struct mntent *ment;
> +    FsMount *mount;
> +    char const *mtab = "/proc/self/mounts";
> +    FILE *fp;
> +    unsigned int devmajor, devminor;
> +
> +    fp = setmntent(mtab, "r");
> +    if (!fp) {
> +        error_setg(errp, "failed to open mtab file: '%s'", mtab);
> +        return false;
> +    }
> +
> +    while ((ment = getmntent(fp))) {
> +        /*
> +         * An entry which device name doesn't start with a '/' is
> +         * either a dummy file system or a network file system.
> +         * Add special handling for smbfs and cifs as is done by
> +         * coreutils as well.
> +         */
> +        if ((ment->mnt_fsname[0] != '/') ||
> +            (strcmp(ment->mnt_type, "smbfs") == 0) ||
> +            (strcmp(ment->mnt_type, "cifs") == 0)) {
> +            continue;
> +        }
> +        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) ==
> -2) {
> +            /* Skip bind mounts */
> +            continue;
> +        }
> +
> +        mount = g_new0(FsMount, 1);
> +        mount->dirname = g_strdup(ment->mnt_dir);
> +        mount->devtype = g_strdup(ment->mnt_type);
> +        mount->devmajor = devmajor;
> +        mount->devminor = devminor;
> +
> +        QTAILQ_INSERT_TAIL(mounts, mount, next);
> +    }
> +
> +    endmntent(fp);
> +    return true;
> +}
> +
> +static void decode_mntname(char *name, int len)
> +{
> +    int i, j = 0;
> +    for (i = 0; i <= len; i++) {
> +        if (name[i] != '\\') {
> +            name[j++] = name[i];
> +        } else if (name[i + 1] == '\\') {
> +            name[j++] = '\\';
> +            i++;
> +        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
> +                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
> +                   name[i + 3] >= '0' && name[i + 3] <= '7') {
> +            name[j++] = (name[i + 1] - '0') * 64 +
> +                        (name[i + 2] - '0') * 8 +
> +                        (name[i + 3] - '0');
> +            i += 3;
> +        } else {
> +            name[j++] = name[i];
> +        }
> +    }
> +}
> +
> +/*
> + * Walk the mount table and build a list of local file systems
> + */
> +bool build_fs_mount_list(FsMountList *mounts, Error **errp)
> +{
> +    FsMount *mount;
> +    char const *mountinfo = "/proc/self/mountinfo";
> +    FILE *fp;
> +    char *line = NULL, *dash;
> +    size_t n;
> +    char check;
> +    unsigned int devmajor, devminor;
> +    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
> +
> +    fp = fopen(mountinfo, "r");
> +    if (!fp) {
> +        return build_fs_mount_list_from_mtab(mounts, errp);
> +    }
> +
> +    while (getline(&line, &n, fp) != -1) {
> +        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
> +                     &devmajor, &devminor, &dir_s, &dir_e, &check);
> +        if (ret < 3) {
> +            continue;
> +        }
> +        dash = strstr(line + dir_e, " - ");
> +        if (!dash) {
> +            continue;
> +        }
> +        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
> +                     &type_s, &type_e, &dev_s, &dev_e, &check);
> +        if (ret < 1) {
> +            continue;
> +        }
> +        line[dir_e] = 0;
> +        dash[type_e] = 0;
> +        dash[dev_e] = 0;
> +        decode_mntname(line + dir_s, dir_e - dir_s);
> +        decode_mntname(dash + dev_s, dev_e - dev_s);
> +        if (devmajor == 0) {
> +            /* btrfs reports major number = 0 */
> +            if (strcmp("btrfs", dash + type_s) != 0 ||
> +                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0) {
> +                continue;
> +            }
> +        }
> +
> +        mount = g_new0(FsMount, 1);
> +        mount->dirname = g_strdup(line + dir_s);
> +        mount->devtype = g_strdup(dash + type_s);
> +        mount->devmajor = devmajor;
> +        mount->devminor = devminor;
> +
> +        QTAILQ_INSERT_TAIL(mounts, mount, next);
> +    }
> +    free(line);
> +
> +    fclose(fp);
> +    return true;
> +}
> +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
> +
> +#ifdef CONFIG_FSFREEZE
> +/*
> + * Walk list of mounted file systems in the guest, and freeze the ones
> which
> + * are real local file systems.
> + */
> +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
> +                                          strList *mountpoints,
> +                                          FsMountList mounts,
> +                                          Error **errp)
> +{
> +    struct FsMount *mount;
> +    strList *list;
> +    int fd, ret, i = 0;
> +
> +    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
> +        /* To issue fsfreeze in the reverse order of mounts, check if the
> +         * mount is listed in the list here */
> +        if (has_mountpoints) {
> +            for (list = mountpoints; list; list = list->next) {
> +                if (strcmp(list->value, mount->dirname) == 0) {
> +                    break;
> +                }
> +            }
> +            if (!list) {
> +                continue;
> +            }
> +        }
> +
> +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
> +        if (fd == -1) {
> +            error_setg_errno(errp, errno, "failed to open %s",
> mount->dirname);
> +            return -1;
> +        }
> +
> +        /* we try to cull filesystems we know won't work in advance, but
> other
> +         * filesystems may not implement fsfreeze for less obvious
> reasons.
> +         * these will report EOPNOTSUPP. we simply ignore these when
> tallying
> +         * the number of frozen filesystems.
> +         * if a filesystem is mounted more than once (aka bind mount) a
> +         * consecutive attempt to freeze an already frozen filesystem will
> +         * return EBUSY.
> +         *
> +         * any other error means a failure to freeze a filesystem we
> +         * expect to be freezable, so return an error in those cases
> +         * and return system to thawed state.
> +         */
> +        ret = ioctl(fd, FIFREEZE);
> +        if (ret == -1) {
> +            if (errno != EOPNOTSUPP && errno != EBUSY) {
> +                error_setg_errno(errp, errno, "failed to freeze %s",
> +                                 mount->dirname);
> +                close(fd);
> +                return -1;
> +            }
> +        } else {
> +            i++;
> +        }
> +        close(fd);
> +    }
> +    return i;
> +}
> +
> +int qmp_guest_fsfreeze_do_thaw(Error **errp)
> +{
> +    int ret;
> +    FsMountList mounts;
> +    FsMount *mount;
> +    int fd, i = 0, logged;
> +    Error *local_err = NULL;
> +
> +    QTAILQ_INIT(&mounts);
> +    if (!build_fs_mount_list(&mounts, &local_err)) {
> +        error_propagate(errp, local_err);
> +        return -1;
> +    }
> +
> +    QTAILQ_FOREACH(mount, &mounts, next) {
> +        logged = false;
> +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
> +        if (fd == -1) {
> +            continue;
> +        }
> +        /* we have no way of knowing whether a filesystem was actually
> unfrozen
> +         * as a result of a successful call to FITHAW, only that if an
> error
> +         * was returned the filesystem was *not* unfrozen by that
> particular
> +         * call.
> +         *
> +         * since multiple preceding FIFREEZEs require multiple calls to
> FITHAW
> +         * to unfreeze, continuing issuing FITHAW until an error is
> returned,
> +         * in which case either the filesystem is in an unfreezable
> state, or,
> +         * more likely, it was thawed previously (and remains so
> afterward).
> +         *
> +         * also, since the most recent successful call is the one that did
> +         * the actual unfreeze, we can use this to provide an accurate
> count
> +         * of the number of filesystems unfrozen by guest-fsfreeze-thaw,
> which
> +         * may * be useful for determining whether a filesystem was
> unfrozen
> +         * during the freeze/thaw phase by a process other than qemu-ga.
> +         */
> +        do {
> +            ret = ioctl(fd, FITHAW);
> +            if (ret == 0 && !logged) {
> +                i++;
> +                logged = true;
> +            }
> +        } while (ret == 0);
> +        close(fd);
> +    }
> +
> +    free_fs_mount_list(&mounts);
> +
> +    return i;
> +}
> +#endif /* CONFIG_FSFREEZE */
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 16d67e9f6d..9574b83c92 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -16,11 +16,9 @@
>  #include <sys/utsname.h>
>  #include <sys/wait.h>
>  #include <dirent.h>
> -#include "guest-agent-core.h"
>  #include "qga-qapi-commands.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qemu/queue.h"
>  #include "qemu/host-utils.h"
>  #include "qemu/sockets.h"
>  #include "qemu/base64.h"
> @@ -70,7 +68,7 @@
>  #endif
>  #endif
>
> -static void ga_wait_child(pid_t pid, int *status, Error **errp)
> +void ga_wait_child(pid_t pid, int *status, Error **errp)
>  {
>      pid_t rpid;
>
> @@ -629,16 +627,7 @@ void qmp_guest_file_flush(int64_t handle, Error
> **errp)
>  #if defined(__linux__)
>
>  #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
> -typedef struct FsMount {
> -    char *dirname;
> -    char *devtype;
> -    unsigned int devmajor, devminor;
> -    QTAILQ_ENTRY(FsMount) next;
> -} FsMount;
> -
> -typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
> -
> -static void free_fs_mount_list(FsMountList *mounts)
> +void free_fs_mount_list(FsMountList *mounts)
>  {
>       FsMount *mount, *temp;
>
> @@ -653,157 +642,6 @@ static void free_fs_mount_list(FsMountList *mounts)
>           g_free(mount);
>       }
>  }
> -
> -static int dev_major_minor(const char *devpath,
> -                           unsigned int *devmajor, unsigned int *devminor)
> -{
> -    struct stat st;
> -
> -    *devmajor = 0;
> -    *devminor = 0;
> -
> -    if (stat(devpath, &st) < 0) {
> -        slog("failed to stat device file '%s': %s", devpath,
> strerror(errno));
> -        return -1;
> -    }
> -    if (S_ISDIR(st.st_mode)) {
> -        /* It is bind mount */
> -        return -2;
> -    }
> -    if (S_ISBLK(st.st_mode)) {
> -        *devmajor = major(st.st_rdev);
> -        *devminor = minor(st.st_rdev);
> -        return 0;
> -    }
> -    return -1;
> -}
> -
> -/*
> - * Walk the mount table and build a list of local file systems
> - */
> -static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error
> **errp)
> -{
> -    struct mntent *ment;
> -    FsMount *mount;
> -    char const *mtab = "/proc/self/mounts";
> -    FILE *fp;
> -    unsigned int devmajor, devminor;
> -
> -    fp = setmntent(mtab, "r");
> -    if (!fp) {
> -        error_setg(errp, "failed to open mtab file: '%s'", mtab);
> -        return false;
> -    }
> -
> -    while ((ment = getmntent(fp))) {
> -        /*
> -         * An entry which device name doesn't start with a '/' is
> -         * either a dummy file system or a network file system.
> -         * Add special handling for smbfs and cifs as is done by
> -         * coreutils as well.
> -         */
> -        if ((ment->mnt_fsname[0] != '/') ||
> -            (strcmp(ment->mnt_type, "smbfs") == 0) ||
> -            (strcmp(ment->mnt_type, "cifs") == 0)) {
> -            continue;
> -        }
> -        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) ==
> -2) {
> -            /* Skip bind mounts */
> -            continue;
> -        }
> -
> -        mount = g_new0(FsMount, 1);
> -        mount->dirname = g_strdup(ment->mnt_dir);
> -        mount->devtype = g_strdup(ment->mnt_type);
> -        mount->devmajor = devmajor;
> -        mount->devminor = devminor;
> -
> -        QTAILQ_INSERT_TAIL(mounts, mount, next);
> -    }
> -
> -    endmntent(fp);
> -    return true;
> -}
> -
> -static void decode_mntname(char *name, int len)
> -{
> -    int i, j = 0;
> -    for (i = 0; i <= len; i++) {
> -        if (name[i] != '\\') {
> -            name[j++] = name[i];
> -        } else if (name[i + 1] == '\\') {
> -            name[j++] = '\\';
> -            i++;
> -        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
> -                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
> -                   name[i + 3] >= '0' && name[i + 3] <= '7') {
> -            name[j++] = (name[i + 1] - '0') * 64 +
> -                        (name[i + 2] - '0') * 8 +
> -                        (name[i + 3] - '0');
> -            i += 3;
> -        } else {
> -            name[j++] = name[i];
> -        }
> -    }
> -}
> -
> -static bool build_fs_mount_list(FsMountList *mounts, Error **errp)
> -{
> -    FsMount *mount;
> -    char const *mountinfo = "/proc/self/mountinfo";
> -    FILE *fp;
> -    char *line = NULL, *dash;
> -    size_t n;
> -    char check;
> -    unsigned int devmajor, devminor;
> -    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
> -
> -    fp = fopen(mountinfo, "r");
> -    if (!fp) {
> -        return build_fs_mount_list_from_mtab(mounts, errp);
> -    }
> -
> -    while (getline(&line, &n, fp) != -1) {
> -        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
> -                     &devmajor, &devminor, &dir_s, &dir_e, &check);
> -        if (ret < 3) {
> -            continue;
> -        }
> -        dash = strstr(line + dir_e, " - ");
> -        if (!dash) {
> -            continue;
> -        }
> -        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
> -                     &type_s, &type_e, &dev_s, &dev_e, &check);
> -        if (ret < 1) {
> -            continue;
> -        }
> -        line[dir_e] = 0;
> -        dash[type_e] = 0;
> -        dash[dev_e] = 0;
> -        decode_mntname(line + dir_s, dir_e - dir_s);
> -        decode_mntname(dash + dev_s, dev_e - dev_s);
> -        if (devmajor == 0) {
> -            /* btrfs reports major number = 0 */
> -            if (strcmp("btrfs", dash + type_s) != 0 ||
> -                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0) {
> -                continue;
> -            }
> -        }
> -
> -        mount = g_new0(FsMount, 1);
> -        mount->dirname = g_strdup(line + dir_s);
> -        mount->devtype = g_strdup(dash + type_s);
> -        mount->devmajor = devmajor;
> -        mount->devminor = devminor;
> -
> -        QTAILQ_INSERT_TAIL(mounts, mount, next);
> -    }
> -    free(line);
> -
> -    fclose(fp);
> -    return true;
> -}
>  #endif
>
>  #if defined(CONFIG_FSFREEZE)
> @@ -1708,20 +1546,13 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>      return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
>  }
>
> -/*
> - * Walk list of mounted file systems in the guest, and freeze the ones
> which
> - * are real local file systems.
> - */
>  int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>                                         strList *mountpoints,
>                                         Error **errp)
>  {
> -    int ret = 0, i = 0;
> -    strList *list;
> +    int ret;
>      FsMountList mounts;
> -    struct FsMount *mount;
>      Error *local_err = NULL;
> -    int fd;
>
>      slog("guest-fsfreeze called");
>
> @@ -1740,122 +1571,34 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
>      /* cannot risk guest agent blocking itself on a write in this state */
>      ga_set_frozen(ga_state);
>
> -    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
> -        /* To issue fsfreeze in the reverse order of mounts, check if the
> -         * mount is listed in the list here */
> -        if (has_mountpoints) {
> -            for (list = mountpoints; list; list = list->next) {
> -                if (strcmp(list->value, mount->dirname) == 0) {
> -                    break;
> -                }
> -            }
> -            if (!list) {
> -                continue;
> -            }
> -        }
> -
> -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
> -        if (fd == -1) {
> -            error_setg_errno(errp, errno, "failed to open %s",
> mount->dirname);
> -            goto error;
> -        }
> -
> -        /* we try to cull filesystems we know won't work in advance, but
> other
> -         * filesystems may not implement fsfreeze for less obvious
> reasons.
> -         * these will report EOPNOTSUPP. we simply ignore these when
> tallying
> -         * the number of frozen filesystems.
> -         * if a filesystem is mounted more than once (aka bind mount) a
> -         * consecutive attempt to freeze an already frozen filesystem will
> -         * return EBUSY.
> -         *
> -         * any other error means a failure to freeze a filesystem we
> -         * expect to be freezable, so return an error in those cases
> -         * and return system to thawed state.
> -         */
> -        ret = ioctl(fd, FIFREEZE);
> -        if (ret == -1) {
> -            if (errno != EOPNOTSUPP && errno != EBUSY) {
> -                error_setg_errno(errp, errno, "failed to freeze %s",
> -                                 mount->dirname);
> -                close(fd);
> -                goto error;
> -            }
> -        } else {
> -            i++;
> -        }
> -        close(fd);
> -    }
> +    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
> +                                            mounts, errp);
>
>      free_fs_mount_list(&mounts);
>      /* We may not issue any FIFREEZE here.
>       * Just unset ga_state here and ready for the next call.
>       */
> -    if (i == 0) {
> +    if (ret == 0) {
>          ga_unset_frozen(ga_state);
> +    } else if (ret < 0) {
> +        qmp_guest_fsfreeze_thaw(NULL);
>      }
> -    return i;
> -
> -error:
> -    free_fs_mount_list(&mounts);
> -    qmp_guest_fsfreeze_thaw(NULL);
> -    return 0;
> +    return ret;
>  }
>
> -/*
> - * Walk list of frozen file systems in the guest, and thaw them.
> - */
>  int64_t qmp_guest_fsfreeze_thaw(Error **errp)
>  {
>      int ret;
> -    FsMountList mounts;
> -    FsMount *mount;
> -    int fd, i = 0, logged;
> -    Error *local_err = NULL;
>
> -    QTAILQ_INIT(&mounts);
> -    if (!build_fs_mount_list(&mounts, &local_err)) {
> -        error_propagate(errp, local_err);
> -        return 0;
> +    ret = qmp_guest_fsfreeze_do_thaw(errp);
> +    if (ret >= 0) {
> +        ga_unset_frozen(ga_state);
> +        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
> +    } else {
> +        ret = 0;
>      }
>
> -    QTAILQ_FOREACH(mount, &mounts, next) {
> -        logged = false;
> -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
> -        if (fd == -1) {
> -            continue;
> -        }
> -        /* we have no way of knowing whether a filesystem was actually
> unfrozen
> -         * as a result of a successful call to FITHAW, only that if an
> error
> -         * was returned the filesystem was *not* unfrozen by that
> particular
> -         * call.
> -         *
> -         * since multiple preceding FIFREEZEs require multiple calls to
> FITHAW
> -         * to unfreeze, continuing issuing FITHAW until an error is
> returned,
> -         * in which case either the filesystem is in an unfreezable
> state, or,
> -         * more likely, it was thawed previously (and remains so
> afterward).
> -         *
> -         * also, since the most recent successful call is the one that did
> -         * the actual unfreeze, we can use this to provide an accurate
> count
> -         * of the number of filesystems unfrozen by guest-fsfreeze-thaw,
> which
> -         * may * be useful for determining whether a filesystem was
> unfrozen
> -         * during the freeze/thaw phase by a process other than qemu-ga.
> -         */
> -        do {
> -            ret = ioctl(fd, FITHAW);
> -            if (ret == 0 && !logged) {
> -                i++;
> -                logged = true;
> -            }
> -        } while (ret == 0);
> -        close(fd);
> -    }
> -
> -    ga_unset_frozen(ga_state);
> -    free_fs_mount_list(&mounts);
> -
> -    execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
> -
> -    return i;
> +    return ret;
>  }
>
>  static void guest_fsfreeze_cleanup(void)
> diff --git a/qga/meson.build b/qga/meson.build
> index a0ffd6d268..932b4e7ca8 100644
> --- a/qga/meson.build
> +++ b/qga/meson.build
> @@ -72,6 +72,9 @@ qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
>    'commands-posix.c',
>    'commands-posix-ssh.c',
>  ))
> +qga_ss.add(when: 'CONFIG_LINUX', if_true: files(
> +  'commands-linux.c',
> +))
>  qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
>    'channel-win32.c',
>    'commands-win32.c',
> --
> 2.34.1
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 31190 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file
  2022-10-13 12:10   ` Marc-André Lureau
@ 2022-10-14  7:55     ` Alexander Ivanov
  2022-10-14  8:08       ` Marc-André Lureau
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Ivanov @ 2022-10-14  7:55 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: qemu-devel, den, michael.roth, kkostiuk

[-- Attachment #1: Type: text/plain, Size: 30309 bytes --]

Hi

On 13.10.2022 14:10, Marc-André Lureau wrote:
> Hi
>
> On Thu, Oct 13, 2022 at 1:23 PM Alexander Ivanov 
> <alexander.ivanov@virtuozzo.com> wrote:
>
>     In the next patches we are going to add FreeBSD support for QEMU Guest
>     Agent. In the result, code in commands-posix.c will be too cumbersome.
>
>     Move Linux-specific FS freeze/thaw code to a separate file
>     commands-linux.c
>     keeping common POSIX code in commands-posix.c.
>
>     Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>     Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>     ---
>      qga/commands-common.h |  35 +++++
>      qga/commands-linux.c  | 286 +++++++++++++++++++++++++++++++++++++++++
>      qga/commands-posix.c  | 289
>     +++---------------------------------------
>      qga/meson.build       |   3 +
>      4 files changed, 340 insertions(+), 273 deletions(-)
>      create mode 100644 qga/commands-linux.c
>
>     diff --git a/qga/commands-common.h b/qga/commands-common.h
>     index d0e4a9696f..181fc330aa 100644
>     --- a/qga/commands-common.h
>     +++ b/qga/commands-common.h
>     @@ -10,6 +10,40 @@
>      #define QGA_COMMANDS_COMMON_H
>
>      #include "qga-qapi-types.h"
>     +#include "guest-agent-core.h"
>     +#include "qemu/queue.h"
>     +
>     +#if defined(__linux__)
>     +#include <linux/fs.h>
>     +#ifdef FIFREEZE
>     +#define CONFIG_FSFREEZE
>     +#endif
>     +#ifdef FITRIM
>     +#define CONFIG_FSTRIM
>     +#endif
>     +#endif /* __linux__ */
>     +
>     +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>     +typedef struct FsMount {
>     +    char *dirname;
>     +    char *devtype;
>     +    unsigned int devmajor, devminor;
>     +    QTAILQ_ENTRY(FsMount) next;
>     +} FsMount;
>     +
>     +typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
>     +
>     +bool build_fs_mount_list(FsMountList *mounts, Error **errp);
>     +void free_fs_mount_list(FsMountList *mounts);
>     +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
>     +
>     +#if defined(CONFIG_FSFREEZE)
>     +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
>     +                                          strList *mountpoints,
>     +                                          FsMountList mounts,
>     +                                          Error **errp);
>     +int qmp_guest_fsfreeze_do_thaw(Error **errp);
>     +#endif /* CONFIG_FSFREEZE */
>
>      typedef struct GuestFileHandle GuestFileHandle;
>
>     @@ -29,4 +63,5 @@ GuestFileRead
>     *guest_file_read_unsafe(GuestFileHandle *gfh,
>       */
>      char *qga_get_host_name(Error **errp);
>
>     +void ga_wait_child(pid_t pid, int *status, Error **errp);
>
>
> This doesn't belong here, afaict.

What do you mean, this patch or this place in the header file?

I moved this function to the global scope because it was used in 
commands-posix.c

and commands-linux.c in the first version of the patchset. But now we 
can leave

the function static in commands-posix.c. Should I do it?

>
>      #endif
>     diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>     new file mode 100644
>     index 0000000000..214e408fcd
>     --- /dev/null
>     +++ b/qga/commands-linux.c
>     @@ -0,0 +1,286 @@
>     +/*
>     + * QEMU Guest Agent Linux-specific command implementations
>     + *
>     + * Copyright IBM Corp. 2011
>     + *
>     + * Authors:
>     + *  Michael Roth      <mdroth@linux.vnet.ibm.com>
>     + *  Michal Privoznik  <mprivozn@redhat.com>
>     + *
>     + * This work is licensed under the terms of the GNU GPL, version
>     2 or later.
>     + * See the COPYING file in the top-level directory.
>     + */
>     +
>     +#include "qemu/osdep.h"
>     +#include "qapi/error.h"
>     +#include "commands-common.h"
>     +#include "cutils.h"
>     +#include <mntent.h>
>     +#include <sys/ioctl.h>
>     +
>     +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>     +static int dev_major_minor(const char *devpath,
>     +                           unsigned int *devmajor, unsigned int
>     *devminor)
>     +{
>     +    struct stat st;
>     +
>     +    *devmajor = 0;
>     +    *devminor = 0;
>     +
>     +    if (stat(devpath, &st) < 0) {
>     +        slog("failed to stat device file '%s': %s", devpath,
>     strerror(errno));
>     +        return -1;
>     +    }
>     +    if (S_ISDIR(st.st_mode)) {
>     +        /* It is bind mount */
>     +        return -2;
>     +    }
>     +    if (S_ISBLK(st.st_mode)) {
>     +        *devmajor = major(st.st_rdev);
>     +        *devminor = minor(st.st_rdev);
>     +        return 0;
>     +    }
>     +    return -1;
>     +}
>     +
>     +static bool build_fs_mount_list_from_mtab(FsMountList *mounts,
>     Error **errp)
>     +{
>     +    struct mntent *ment;
>     +    FsMount *mount;
>     +    char const *mtab = "/proc/self/mounts";
>     +    FILE *fp;
>     +    unsigned int devmajor, devminor;
>     +
>     +    fp = setmntent(mtab, "r");
>     +    if (!fp) {
>     +        error_setg(errp, "failed to open mtab file: '%s'", mtab);
>     +        return false;
>     +    }
>     +
>     +    while ((ment = getmntent(fp))) {
>     +        /*
>     +         * An entry which device name doesn't start with a '/' is
>     +         * either a dummy file system or a network file system.
>     +         * Add special handling for smbfs and cifs as is done by
>     +         * coreutils as well.
>     +         */
>     +        if ((ment->mnt_fsname[0] != '/') ||
>     +            (strcmp(ment->mnt_type, "smbfs") == 0) ||
>     +            (strcmp(ment->mnt_type, "cifs") == 0)) {
>     +            continue;
>     +        }
>     +        if (dev_major_minor(ment->mnt_fsname, &devmajor,
>     &devminor) == -2) {
>     +            /* Skip bind mounts */
>     +            continue;
>     +        }
>     +
>     +        mount = g_new0(FsMount, 1);
>     +        mount->dirname = g_strdup(ment->mnt_dir);
>     +        mount->devtype = g_strdup(ment->mnt_type);
>     +        mount->devmajor = devmajor;
>     +        mount->devminor = devminor;
>     +
>     +        QTAILQ_INSERT_TAIL(mounts, mount, next);
>     +    }
>     +
>     +    endmntent(fp);
>     +    return true;
>     +}
>     +
>     +static void decode_mntname(char *name, int len)
>     +{
>     +    int i, j = 0;
>     +    for (i = 0; i <= len; i++) {
>     +        if (name[i] != '\\') {
>     +            name[j++] = name[i];
>     +        } else if (name[i + 1] == '\\') {
>     +            name[j++] = '\\';
>     +            i++;
>     +        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
>     +                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
>     +                   name[i + 3] >= '0' && name[i + 3] <= '7') {
>     +            name[j++] = (name[i + 1] - '0') * 64 +
>     +                        (name[i + 2] - '0') * 8 +
>     +                        (name[i + 3] - '0');
>     +            i += 3;
>     +        } else {
>     +            name[j++] = name[i];
>     +        }
>     +    }
>     +}
>     +
>     +/*
>     + * Walk the mount table and build a list of local file systems
>     + */
>     +bool build_fs_mount_list(FsMountList *mounts, Error **errp)
>     +{
>     +    FsMount *mount;
>     +    char const *mountinfo = "/proc/self/mountinfo";
>     +    FILE *fp;
>     +    char *line = NULL, *dash;
>     +    size_t n;
>     +    char check;
>     +    unsigned int devmajor, devminor;
>     +    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
>     +
>     +    fp = fopen(mountinfo, "r");
>     +    if (!fp) {
>     +        return build_fs_mount_list_from_mtab(mounts, errp);
>     +    }
>     +
>     +    while (getline(&line, &n, fp) != -1) {
>     +        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
>     +                     &devmajor, &devminor, &dir_s, &dir_e, &check);
>     +        if (ret < 3) {
>     +            continue;
>     +        }
>     +        dash = strstr(line + dir_e, " - ");
>     +        if (!dash) {
>     +            continue;
>     +        }
>     +        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
>     +                     &type_s, &type_e, &dev_s, &dev_e, &check);
>     +        if (ret < 1) {
>     +            continue;
>     +        }
>     +        line[dir_e] = 0;
>     +        dash[type_e] = 0;
>     +        dash[dev_e] = 0;
>     +        decode_mntname(line + dir_s, dir_e - dir_s);
>     +        decode_mntname(dash + dev_s, dev_e - dev_s);
>     +        if (devmajor == 0) {
>     +            /* btrfs reports major number = 0 */
>     +            if (strcmp("btrfs", dash + type_s) != 0 ||
>     +                dev_major_minor(dash + dev_s, &devmajor,
>     &devminor) < 0) {
>     +                continue;
>     +            }
>     +        }
>     +
>     +        mount = g_new0(FsMount, 1);
>     +        mount->dirname = g_strdup(line + dir_s);
>     +        mount->devtype = g_strdup(dash + type_s);
>     +        mount->devmajor = devmajor;
>     +        mount->devminor = devminor;
>     +
>     +        QTAILQ_INSERT_TAIL(mounts, mount, next);
>     +    }
>     +    free(line);
>     +
>     +    fclose(fp);
>     +    return true;
>     +}
>     +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
>     +
>     +#ifdef CONFIG_FSFREEZE
>     +/*
>     + * Walk list of mounted file systems in the guest, and freeze the
>     ones which
>     + * are real local file systems.
>     + */
>     +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
>     +                                          strList *mountpoints,
>     +                                          FsMountList mounts,
>     +                                          Error **errp)
>     +{
>     +    struct FsMount *mount;
>     +    strList *list;
>     +    int fd, ret, i = 0;
>     +
>     +    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
>     +        /* To issue fsfreeze in the reverse order of mounts,
>     check if the
>     +         * mount is listed in the list here */
>     +        if (has_mountpoints) {
>     +            for (list = mountpoints; list; list = list->next) {
>     +                if (strcmp(list->value, mount->dirname) == 0) {
>     +                    break;
>     +                }
>     +            }
>     +            if (!list) {
>     +                continue;
>     +            }
>     +        }
>     +
>     +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>     +        if (fd == -1) {
>     +            error_setg_errno(errp, errno, "failed to open %s",
>     mount->dirname);
>     +            return -1;
>     +        }
>     +
>     +        /* we try to cull filesystems we know won't work in
>     advance, but other
>     +         * filesystems may not implement fsfreeze for less
>     obvious reasons.
>     +         * these will report EOPNOTSUPP. we simply ignore these
>     when tallying
>     +         * the number of frozen filesystems.
>     +         * if a filesystem is mounted more than once (aka bind
>     mount) a
>     +         * consecutive attempt to freeze an already frozen
>     filesystem will
>     +         * return EBUSY.
>     +         *
>     +         * any other error means a failure to freeze a filesystem we
>     +         * expect to be freezable, so return an error in those cases
>     +         * and return system to thawed state.
>     +         */
>     +        ret = ioctl(fd, FIFREEZE);
>     +        if (ret == -1) {
>     +            if (errno != EOPNOTSUPP && errno != EBUSY) {
>     +                error_setg_errno(errp, errno, "failed to freeze %s",
>     +                                 mount->dirname);
>     +                close(fd);
>     +                return -1;
>     +            }
>     +        } else {
>     +            i++;
>     +        }
>     +        close(fd);
>     +    }
>     +    return i;
>     +}
>     +
>     +int qmp_guest_fsfreeze_do_thaw(Error **errp)
>     +{
>     +    int ret;
>     +    FsMountList mounts;
>     +    FsMount *mount;
>     +    int fd, i = 0, logged;
>     +    Error *local_err = NULL;
>     +
>     +    QTAILQ_INIT(&mounts);
>     +    if (!build_fs_mount_list(&mounts, &local_err)) {
>     +        error_propagate(errp, local_err);
>     +        return -1;
>     +    }
>     +
>     +    QTAILQ_FOREACH(mount, &mounts, next) {
>     +        logged = false;
>     +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>     +        if (fd == -1) {
>     +            continue;
>     +        }
>     +        /* we have no way of knowing whether a filesystem was
>     actually unfrozen
>     +         * as a result of a successful call to FITHAW, only that
>     if an error
>     +         * was returned the filesystem was *not* unfrozen by that
>     particular
>     +         * call.
>     +         *
>     +         * since multiple preceding FIFREEZEs require multiple
>     calls to FITHAW
>     +         * to unfreeze, continuing issuing FITHAW until an error
>     is returned,
>     +         * in which case either the filesystem is in an
>     unfreezable state, or,
>     +         * more likely, it was thawed previously (and remains so
>     afterward).
>     +         *
>     +         * also, since the most recent successful call is the one
>     that did
>     +         * the actual unfreeze, we can use this to provide an
>     accurate count
>     +         * of the number of filesystems unfrozen by
>     guest-fsfreeze-thaw, which
>     +         * may * be useful for determining whether a filesystem
>     was unfrozen
>     +         * during the freeze/thaw phase by a process other than
>     qemu-ga.
>     +         */
>     +        do {
>     +            ret = ioctl(fd, FITHAW);
>     +            if (ret == 0 && !logged) {
>     +                i++;
>     +                logged = true;
>     +            }
>     +        } while (ret == 0);
>     +        close(fd);
>     +    }
>     +
>     +    free_fs_mount_list(&mounts);
>     +
>     +    return i;
>     +}
>     +#endif /* CONFIG_FSFREEZE */
>     diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>     index 16d67e9f6d..9574b83c92 100644
>     --- a/qga/commands-posix.c
>     +++ b/qga/commands-posix.c
>     @@ -16,11 +16,9 @@
>      #include <sys/utsname.h>
>      #include <sys/wait.h>
>      #include <dirent.h>
>     -#include "guest-agent-core.h"
>      #include "qga-qapi-commands.h"
>      #include "qapi/error.h"
>      #include "qapi/qmp/qerror.h"
>     -#include "qemu/queue.h"
>      #include "qemu/host-utils.h"
>      #include "qemu/sockets.h"
>      #include "qemu/base64.h"
>     @@ -70,7 +68,7 @@
>      #endif
>      #endif
>
>     -static void ga_wait_child(pid_t pid, int *status, Error **errp)
>     +void ga_wait_child(pid_t pid, int *status, Error **errp)
>      {
>          pid_t rpid;
>
>     @@ -629,16 +627,7 @@ void qmp_guest_file_flush(int64_t handle,
>     Error **errp)
>      #if defined(__linux__)
>
>      #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>     -typedef struct FsMount {
>     -    char *dirname;
>     -    char *devtype;
>     -    unsigned int devmajor, devminor;
>     -    QTAILQ_ENTRY(FsMount) next;
>     -} FsMount;
>     -
>     -typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
>     -
>     -static void free_fs_mount_list(FsMountList *mounts)
>     +void free_fs_mount_list(FsMountList *mounts)
>      {
>           FsMount *mount, *temp;
>
>     @@ -653,157 +642,6 @@ static void free_fs_mount_list(FsMountList
>     *mounts)
>               g_free(mount);
>           }
>      }
>     -
>     -static int dev_major_minor(const char *devpath,
>     -                           unsigned int *devmajor, unsigned int
>     *devminor)
>     -{
>     -    struct stat st;
>     -
>     -    *devmajor = 0;
>     -    *devminor = 0;
>     -
>     -    if (stat(devpath, &st) < 0) {
>     -        slog("failed to stat device file '%s': %s", devpath,
>     strerror(errno));
>     -        return -1;
>     -    }
>     -    if (S_ISDIR(st.st_mode)) {
>     -        /* It is bind mount */
>     -        return -2;
>     -    }
>     -    if (S_ISBLK(st.st_mode)) {
>     -        *devmajor = major(st.st_rdev);
>     -        *devminor = minor(st.st_rdev);
>     -        return 0;
>     -    }
>     -    return -1;
>     -}
>     -
>     -/*
>     - * Walk the mount table and build a list of local file systems
>     - */
>     -static bool build_fs_mount_list_from_mtab(FsMountList *mounts,
>     Error **errp)
>     -{
>     -    struct mntent *ment;
>     -    FsMount *mount;
>     -    char const *mtab = "/proc/self/mounts";
>     -    FILE *fp;
>     -    unsigned int devmajor, devminor;
>     -
>     -    fp = setmntent(mtab, "r");
>     -    if (!fp) {
>     -        error_setg(errp, "failed to open mtab file: '%s'", mtab);
>     -        return false;
>     -    }
>     -
>     -    while ((ment = getmntent(fp))) {
>     -        /*
>     -         * An entry which device name doesn't start with a '/' is
>     -         * either a dummy file system or a network file system.
>     -         * Add special handling for smbfs and cifs as is done by
>     -         * coreutils as well.
>     -         */
>     -        if ((ment->mnt_fsname[0] != '/') ||
>     -            (strcmp(ment->mnt_type, "smbfs") == 0) ||
>     -            (strcmp(ment->mnt_type, "cifs") == 0)) {
>     -            continue;
>     -        }
>     -        if (dev_major_minor(ment->mnt_fsname, &devmajor,
>     &devminor) == -2) {
>     -            /* Skip bind mounts */
>     -            continue;
>     -        }
>     -
>     -        mount = g_new0(FsMount, 1);
>     -        mount->dirname = g_strdup(ment->mnt_dir);
>     -        mount->devtype = g_strdup(ment->mnt_type);
>     -        mount->devmajor = devmajor;
>     -        mount->devminor = devminor;
>     -
>     -        QTAILQ_INSERT_TAIL(mounts, mount, next);
>     -    }
>     -
>     -    endmntent(fp);
>     -    return true;
>     -}
>     -
>     -static void decode_mntname(char *name, int len)
>     -{
>     -    int i, j = 0;
>     -    for (i = 0; i <= len; i++) {
>     -        if (name[i] != '\\') {
>     -            name[j++] = name[i];
>     -        } else if (name[i + 1] == '\\') {
>     -            name[j++] = '\\';
>     -            i++;
>     -        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
>     -                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
>     -                   name[i + 3] >= '0' && name[i + 3] <= '7') {
>     -            name[j++] = (name[i + 1] - '0') * 64 +
>     -                        (name[i + 2] - '0') * 8 +
>     -                        (name[i + 3] - '0');
>     -            i += 3;
>     -        } else {
>     -            name[j++] = name[i];
>     -        }
>     -    }
>     -}
>     -
>     -static bool build_fs_mount_list(FsMountList *mounts, Error **errp)
>     -{
>     -    FsMount *mount;
>     -    char const *mountinfo = "/proc/self/mountinfo";
>     -    FILE *fp;
>     -    char *line = NULL, *dash;
>     -    size_t n;
>     -    char check;
>     -    unsigned int devmajor, devminor;
>     -    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
>     -
>     -    fp = fopen(mountinfo, "r");
>     -    if (!fp) {
>     -        return build_fs_mount_list_from_mtab(mounts, errp);
>     -    }
>     -
>     -    while (getline(&line, &n, fp) != -1) {
>     -        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
>     -                     &devmajor, &devminor, &dir_s, &dir_e, &check);
>     -        if (ret < 3) {
>     -            continue;
>     -        }
>     -        dash = strstr(line + dir_e, " - ");
>     -        if (!dash) {
>     -            continue;
>     -        }
>     -        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
>     -                     &type_s, &type_e, &dev_s, &dev_e, &check);
>     -        if (ret < 1) {
>     -            continue;
>     -        }
>     -        line[dir_e] = 0;
>     -        dash[type_e] = 0;
>     -        dash[dev_e] = 0;
>     -        decode_mntname(line + dir_s, dir_e - dir_s);
>     -        decode_mntname(dash + dev_s, dev_e - dev_s);
>     -        if (devmajor == 0) {
>     -            /* btrfs reports major number = 0 */
>     -            if (strcmp("btrfs", dash + type_s) != 0 ||
>     -                dev_major_minor(dash + dev_s, &devmajor,
>     &devminor) < 0) {
>     -                continue;
>     -            }
>     -        }
>     -
>     -        mount = g_new0(FsMount, 1);
>     -        mount->dirname = g_strdup(line + dir_s);
>     -        mount->devtype = g_strdup(dash + type_s);
>     -        mount->devmajor = devmajor;
>     -        mount->devminor = devminor;
>     -
>     -        QTAILQ_INSERT_TAIL(mounts, mount, next);
>     -    }
>     -    free(line);
>     -
>     -    fclose(fp);
>     -    return true;
>     -}
>      #endif
>
>      #if defined(CONFIG_FSFREEZE)
>     @@ -1708,20 +1546,13 @@ int64_t qmp_guest_fsfreeze_freeze(Error
>     **errp)
>          return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
>      }
>
>     -/*
>     - * Walk list of mounted file systems in the guest, and freeze the
>     ones which
>     - * are real local file systems.
>     - */
>      int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>                                             strList *mountpoints,
>                                             Error **errp)
>      {
>     -    int ret = 0, i = 0;
>     -    strList *list;
>     +    int ret;
>          FsMountList mounts;
>     -    struct FsMount *mount;
>          Error *local_err = NULL;
>     -    int fd;
>
>          slog("guest-fsfreeze called");
>
>     @@ -1740,122 +1571,34 @@ int64_t
>     qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>          /* cannot risk guest agent blocking itself on a write in this
>     state */
>          ga_set_frozen(ga_state);
>
>     -    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
>     -        /* To issue fsfreeze in the reverse order of mounts,
>     check if the
>     -         * mount is listed in the list here */
>     -        if (has_mountpoints) {
>     -            for (list = mountpoints; list; list = list->next) {
>     -                if (strcmp(list->value, mount->dirname) == 0) {
>     -                    break;
>     -                }
>     -            }
>     -            if (!list) {
>     -                continue;
>     -            }
>     -        }
>     -
>     -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>     -        if (fd == -1) {
>     -            error_setg_errno(errp, errno, "failed to open %s",
>     mount->dirname);
>     -            goto error;
>     -        }
>     -
>     -        /* we try to cull filesystems we know won't work in
>     advance, but other
>     -         * filesystems may not implement fsfreeze for less
>     obvious reasons.
>     -         * these will report EOPNOTSUPP. we simply ignore these
>     when tallying
>     -         * the number of frozen filesystems.
>     -         * if a filesystem is mounted more than once (aka bind
>     mount) a
>     -         * consecutive attempt to freeze an already frozen
>     filesystem will
>     -         * return EBUSY.
>     -         *
>     -         * any other error means a failure to freeze a filesystem we
>     -         * expect to be freezable, so return an error in those cases
>     -         * and return system to thawed state.
>     -         */
>     -        ret = ioctl(fd, FIFREEZE);
>     -        if (ret == -1) {
>     -            if (errno != EOPNOTSUPP && errno != EBUSY) {
>     -                error_setg_errno(errp, errno, "failed to freeze %s",
>     -                                 mount->dirname);
>     -                close(fd);
>     -                goto error;
>     -            }
>     -        } else {
>     -            i++;
>     -        }
>     -        close(fd);
>     -    }
>     +    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints,
>     mountpoints,
>     +                                            mounts, errp);
>
>          free_fs_mount_list(&mounts);
>          /* We may not issue any FIFREEZE here.
>           * Just unset ga_state here and ready for the next call.
>           */
>     -    if (i == 0) {
>     +    if (ret == 0) {
>              ga_unset_frozen(ga_state);
>     +    } else if (ret < 0) {
>     +        qmp_guest_fsfreeze_thaw(NULL);
>          }
>     -    return i;
>     -
>     -error:
>     -    free_fs_mount_list(&mounts);
>     -    qmp_guest_fsfreeze_thaw(NULL);
>     -    return 0;
>     +    return ret;
>      }
>
>     -/*
>     - * Walk list of frozen file systems in the guest, and thaw them.
>     - */
>      int64_t qmp_guest_fsfreeze_thaw(Error **errp)
>      {
>          int ret;
>     -    FsMountList mounts;
>     -    FsMount *mount;
>     -    int fd, i = 0, logged;
>     -    Error *local_err = NULL;
>
>     -    QTAILQ_INIT(&mounts);
>     -    if (!build_fs_mount_list(&mounts, &local_err)) {
>     -        error_propagate(errp, local_err);
>     -        return 0;
>     +    ret = qmp_guest_fsfreeze_do_thaw(errp);
>     +    if (ret >= 0) {
>     +        ga_unset_frozen(ga_state);
>     +        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
>     +    } else {
>     +        ret = 0;
>          }
>
>     -    QTAILQ_FOREACH(mount, &mounts, next) {
>     -        logged = false;
>     -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>     -        if (fd == -1) {
>     -            continue;
>     -        }
>     -        /* we have no way of knowing whether a filesystem was
>     actually unfrozen
>     -         * as a result of a successful call to FITHAW, only that
>     if an error
>     -         * was returned the filesystem was *not* unfrozen by that
>     particular
>     -         * call.
>     -         *
>     -         * since multiple preceding FIFREEZEs require multiple
>     calls to FITHAW
>     -         * to unfreeze, continuing issuing FITHAW until an error
>     is returned,
>     -         * in which case either the filesystem is in an
>     unfreezable state, or,
>     -         * more likely, it was thawed previously (and remains so
>     afterward).
>     -         *
>     -         * also, since the most recent successful call is the one
>     that did
>     -         * the actual unfreeze, we can use this to provide an
>     accurate count
>     -         * of the number of filesystems unfrozen by
>     guest-fsfreeze-thaw, which
>     -         * may * be useful for determining whether a filesystem
>     was unfrozen
>     -         * during the freeze/thaw phase by a process other than
>     qemu-ga.
>     -         */
>     -        do {
>     -            ret = ioctl(fd, FITHAW);
>     -            if (ret == 0 && !logged) {
>     -                i++;
>     -                logged = true;
>     -            }
>     -        } while (ret == 0);
>     -        close(fd);
>     -    }
>     -
>     -    ga_unset_frozen(ga_state);
>     -    free_fs_mount_list(&mounts);
>     -
>     -    execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
>     -
>     -    return i;
>     +    return ret;
>      }
>
>      static void guest_fsfreeze_cleanup(void)
>     diff --git a/qga/meson.build b/qga/meson.build
>     index a0ffd6d268..932b4e7ca8 100644
>     --- a/qga/meson.build
>     +++ b/qga/meson.build
>     @@ -72,6 +72,9 @@ qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
>        'commands-posix.c',
>        'commands-posix-ssh.c',
>      ))
>     +qga_ss.add(when: 'CONFIG_LINUX', if_true: files(
>     +  'commands-linux.c',
>     +))
>      qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
>        'channel-win32.c',
>        'commands-win32.c',
>     -- 
>     2.34.1
>
>
>
> -- 
> Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 50763 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file
  2022-10-14  7:55     ` Alexander Ivanov
@ 2022-10-14  8:08       ` Marc-André Lureau
  0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2022-10-14  8:08 UTC (permalink / raw)
  To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, kkostiuk

[-- Attachment #1: Type: text/plain, Size: 26854 bytes --]

Hi

On Fri, Oct 14, 2022 at 11:56 AM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:

> Hi
> On 13.10.2022 14:10, Marc-André Lureau wrote:
>
> Hi
>
> On Thu, Oct 13, 2022 at 1:23 PM Alexander Ivanov <
> alexander.ivanov@virtuozzo.com> wrote:
>
>> In the next patches we are going to add FreeBSD support for QEMU Guest
>> Agent. In the result, code in commands-posix.c will be too cumbersome.
>>
>> Move Linux-specific FS freeze/thaw code to a separate file
>> commands-linux.c
>> keeping common POSIX code in commands-posix.c.
>>
>> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>> ---
>>  qga/commands-common.h |  35 +++++
>>  qga/commands-linux.c  | 286 +++++++++++++++++++++++++++++++++++++++++
>>  qga/commands-posix.c  | 289 +++---------------------------------------
>>  qga/meson.build       |   3 +
>>  4 files changed, 340 insertions(+), 273 deletions(-)
>>  create mode 100644 qga/commands-linux.c
>>
>> diff --git a/qga/commands-common.h b/qga/commands-common.h
>> index d0e4a9696f..181fc330aa 100644
>> --- a/qga/commands-common.h
>> +++ b/qga/commands-common.h
>> @@ -10,6 +10,40 @@
>>  #define QGA_COMMANDS_COMMON_H
>>
>>  #include "qga-qapi-types.h"
>> +#include "guest-agent-core.h"
>> +#include "qemu/queue.h"
>> +
>> +#if defined(__linux__)
>> +#include <linux/fs.h>
>> +#ifdef FIFREEZE
>> +#define CONFIG_FSFREEZE
>> +#endif
>> +#ifdef FITRIM
>> +#define CONFIG_FSTRIM
>> +#endif
>> +#endif /* __linux__ */
>> +
>> +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>> +typedef struct FsMount {
>> +    char *dirname;
>> +    char *devtype;
>> +    unsigned int devmajor, devminor;
>> +    QTAILQ_ENTRY(FsMount) next;
>> +} FsMount;
>> +
>> +typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
>> +
>> +bool build_fs_mount_list(FsMountList *mounts, Error **errp);
>> +void free_fs_mount_list(FsMountList *mounts);
>> +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
>> +
>> +#if defined(CONFIG_FSFREEZE)
>> +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
>> +                                          strList *mountpoints,
>> +                                          FsMountList mounts,
>> +                                          Error **errp);
>> +int qmp_guest_fsfreeze_do_thaw(Error **errp);
>> +#endif /* CONFIG_FSFREEZE */
>>
>>  typedef struct GuestFileHandle GuestFileHandle;
>>
>> @@ -29,4 +63,5 @@ GuestFileRead *guest_file_read_unsafe(GuestFileHandle
>> *gfh,
>>   */
>>  char *qga_get_host_name(Error **errp);
>>
>> +void ga_wait_child(pid_t pid, int *status, Error **errp);
>>
>
> This doesn't belong here, afaict.
>
> What do you mean, this patch or this place in the header file?
>

I mean that this change doesn't look necessary with this patch.

> I moved this function to the global scope because it was used in
> commands-posix.c
>
> and commands-linux.c in the first version of the patchset. But now we can
> leave
>
> the function static in commands-posix.c. Should I do it?
>

yes, don't change it if not needed, thanks !


>
>
>
>>  #endif
>> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>> new file mode 100644
>> index 0000000000..214e408fcd
>> --- /dev/null
>> +++ b/qga/commands-linux.c
>> @@ -0,0 +1,286 @@
>> +/*
>> + * QEMU Guest Agent Linux-specific command implementations
>> + *
>> + * Copyright IBM Corp. 2011
>> + *
>> + * Authors:
>> + *  Michael Roth      <mdroth@linux.vnet.ibm.com>
>> + *  Michal Privoznik  <mprivozn@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "commands-common.h"
>> +#include "cutils.h"
>> +#include <mntent.h>
>> +#include <sys/ioctl.h>
>> +
>> +#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>> +static int dev_major_minor(const char *devpath,
>> +                           unsigned int *devmajor, unsigned int
>> *devminor)
>> +{
>> +    struct stat st;
>> +
>> +    *devmajor = 0;
>> +    *devminor = 0;
>> +
>> +    if (stat(devpath, &st) < 0) {
>> +        slog("failed to stat device file '%s': %s", devpath,
>> strerror(errno));
>> +        return -1;
>> +    }
>> +    if (S_ISDIR(st.st_mode)) {
>> +        /* It is bind mount */
>> +        return -2;
>> +    }
>> +    if (S_ISBLK(st.st_mode)) {
>> +        *devmajor = major(st.st_rdev);
>> +        *devminor = minor(st.st_rdev);
>> +        return 0;
>> +    }
>> +    return -1;
>> +}
>> +
>> +static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error
>> **errp)
>> +{
>> +    struct mntent *ment;
>> +    FsMount *mount;
>> +    char const *mtab = "/proc/self/mounts";
>> +    FILE *fp;
>> +    unsigned int devmajor, devminor;
>> +
>> +    fp = setmntent(mtab, "r");
>> +    if (!fp) {
>> +        error_setg(errp, "failed to open mtab file: '%s'", mtab);
>> +        return false;
>> +    }
>> +
>> +    while ((ment = getmntent(fp))) {
>> +        /*
>> +         * An entry which device name doesn't start with a '/' is
>> +         * either a dummy file system or a network file system.
>> +         * Add special handling for smbfs and cifs as is done by
>> +         * coreutils as well.
>> +         */
>> +        if ((ment->mnt_fsname[0] != '/') ||
>> +            (strcmp(ment->mnt_type, "smbfs") == 0) ||
>> +            (strcmp(ment->mnt_type, "cifs") == 0)) {
>> +            continue;
>> +        }
>> +        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) ==
>> -2) {
>> +            /* Skip bind mounts */
>> +            continue;
>> +        }
>> +
>> +        mount = g_new0(FsMount, 1);
>> +        mount->dirname = g_strdup(ment->mnt_dir);
>> +        mount->devtype = g_strdup(ment->mnt_type);
>> +        mount->devmajor = devmajor;
>> +        mount->devminor = devminor;
>> +
>> +        QTAILQ_INSERT_TAIL(mounts, mount, next);
>> +    }
>> +
>> +    endmntent(fp);
>> +    return true;
>> +}
>> +
>> +static void decode_mntname(char *name, int len)
>> +{
>> +    int i, j = 0;
>> +    for (i = 0; i <= len; i++) {
>> +        if (name[i] != '\\') {
>> +            name[j++] = name[i];
>> +        } else if (name[i + 1] == '\\') {
>> +            name[j++] = '\\';
>> +            i++;
>> +        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
>> +                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
>> +                   name[i + 3] >= '0' && name[i + 3] <= '7') {
>> +            name[j++] = (name[i + 1] - '0') * 64 +
>> +                        (name[i + 2] - '0') * 8 +
>> +                        (name[i + 3] - '0');
>> +            i += 3;
>> +        } else {
>> +            name[j++] = name[i];
>> +        }
>> +    }
>> +}
>> +
>> +/*
>> + * Walk the mount table and build a list of local file systems
>> + */
>> +bool build_fs_mount_list(FsMountList *mounts, Error **errp)
>> +{
>> +    FsMount *mount;
>> +    char const *mountinfo = "/proc/self/mountinfo";
>> +    FILE *fp;
>> +    char *line = NULL, *dash;
>> +    size_t n;
>> +    char check;
>> +    unsigned int devmajor, devminor;
>> +    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
>> +
>> +    fp = fopen(mountinfo, "r");
>> +    if (!fp) {
>> +        return build_fs_mount_list_from_mtab(mounts, errp);
>> +    }
>> +
>> +    while (getline(&line, &n, fp) != -1) {
>> +        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
>> +                     &devmajor, &devminor, &dir_s, &dir_e, &check);
>> +        if (ret < 3) {
>> +            continue;
>> +        }
>> +        dash = strstr(line + dir_e, " - ");
>> +        if (!dash) {
>> +            continue;
>> +        }
>> +        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
>> +                     &type_s, &type_e, &dev_s, &dev_e, &check);
>> +        if (ret < 1) {
>> +            continue;
>> +        }
>> +        line[dir_e] = 0;
>> +        dash[type_e] = 0;
>> +        dash[dev_e] = 0;
>> +        decode_mntname(line + dir_s, dir_e - dir_s);
>> +        decode_mntname(dash + dev_s, dev_e - dev_s);
>> +        if (devmajor == 0) {
>> +            /* btrfs reports major number = 0 */
>> +            if (strcmp("btrfs", dash + type_s) != 0 ||
>> +                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0)
>> {
>> +                continue;
>> +            }
>> +        }
>> +
>> +        mount = g_new0(FsMount, 1);
>> +        mount->dirname = g_strdup(line + dir_s);
>> +        mount->devtype = g_strdup(dash + type_s);
>> +        mount->devmajor = devmajor;
>> +        mount->devminor = devminor;
>> +
>> +        QTAILQ_INSERT_TAIL(mounts, mount, next);
>> +    }
>> +    free(line);
>> +
>> +    fclose(fp);
>> +    return true;
>> +}
>> +#endif /* CONFIG_FSFREEZE || CONFIG_FSTRIM */
>> +
>> +#ifdef CONFIG_FSFREEZE
>> +/*
>> + * Walk list of mounted file systems in the guest, and freeze the ones
>> which
>> + * are real local file systems.
>> + */
>> +int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints,
>> +                                          strList *mountpoints,
>> +                                          FsMountList mounts,
>> +                                          Error **errp)
>> +{
>> +    struct FsMount *mount;
>> +    strList *list;
>> +    int fd, ret, i = 0;
>> +
>> +    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
>> +        /* To issue fsfreeze in the reverse order of mounts, check if the
>> +         * mount is listed in the list here */
>> +        if (has_mountpoints) {
>> +            for (list = mountpoints; list; list = list->next) {
>> +                if (strcmp(list->value, mount->dirname) == 0) {
>> +                    break;
>> +                }
>> +            }
>> +            if (!list) {
>> +                continue;
>> +            }
>> +        }
>> +
>> +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>> +        if (fd == -1) {
>> +            error_setg_errno(errp, errno, "failed to open %s",
>> mount->dirname);
>> +            return -1;
>> +        }
>> +
>> +        /* we try to cull filesystems we know won't work in advance, but
>> other
>> +         * filesystems may not implement fsfreeze for less obvious
>> reasons.
>> +         * these will report EOPNOTSUPP. we simply ignore these when
>> tallying
>> +         * the number of frozen filesystems.
>> +         * if a filesystem is mounted more than once (aka bind mount) a
>> +         * consecutive attempt to freeze an already frozen filesystem
>> will
>> +         * return EBUSY.
>> +         *
>> +         * any other error means a failure to freeze a filesystem we
>> +         * expect to be freezable, so return an error in those cases
>> +         * and return system to thawed state.
>> +         */
>> +        ret = ioctl(fd, FIFREEZE);
>> +        if (ret == -1) {
>> +            if (errno != EOPNOTSUPP && errno != EBUSY) {
>> +                error_setg_errno(errp, errno, "failed to freeze %s",
>> +                                 mount->dirname);
>> +                close(fd);
>> +                return -1;
>> +            }
>> +        } else {
>> +            i++;
>> +        }
>> +        close(fd);
>> +    }
>> +    return i;
>> +}
>> +
>> +int qmp_guest_fsfreeze_do_thaw(Error **errp)
>> +{
>> +    int ret;
>> +    FsMountList mounts;
>> +    FsMount *mount;
>> +    int fd, i = 0, logged;
>> +    Error *local_err = NULL;
>> +
>> +    QTAILQ_INIT(&mounts);
>> +    if (!build_fs_mount_list(&mounts, &local_err)) {
>> +        error_propagate(errp, local_err);
>> +        return -1;
>> +    }
>> +
>> +    QTAILQ_FOREACH(mount, &mounts, next) {
>> +        logged = false;
>> +        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>> +        if (fd == -1) {
>> +            continue;
>> +        }
>> +        /* we have no way of knowing whether a filesystem was actually
>> unfrozen
>> +         * as a result of a successful call to FITHAW, only that if an
>> error
>> +         * was returned the filesystem was *not* unfrozen by that
>> particular
>> +         * call.
>> +         *
>> +         * since multiple preceding FIFREEZEs require multiple calls to
>> FITHAW
>> +         * to unfreeze, continuing issuing FITHAW until an error is
>> returned,
>> +         * in which case either the filesystem is in an unfreezable
>> state, or,
>> +         * more likely, it was thawed previously (and remains so
>> afterward).
>> +         *
>> +         * also, since the most recent successful call is the one that
>> did
>> +         * the actual unfreeze, we can use this to provide an accurate
>> count
>> +         * of the number of filesystems unfrozen by guest-fsfreeze-thaw,
>> which
>> +         * may * be useful for determining whether a filesystem was
>> unfrozen
>> +         * during the freeze/thaw phase by a process other than qemu-ga.
>> +         */
>> +        do {
>> +            ret = ioctl(fd, FITHAW);
>> +            if (ret == 0 && !logged) {
>> +                i++;
>> +                logged = true;
>> +            }
>> +        } while (ret == 0);
>> +        close(fd);
>> +    }
>> +
>> +    free_fs_mount_list(&mounts);
>> +
>> +    return i;
>> +}
>> +#endif /* CONFIG_FSFREEZE */
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 16d67e9f6d..9574b83c92 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -16,11 +16,9 @@
>>  #include <sys/utsname.h>
>>  #include <sys/wait.h>
>>  #include <dirent.h>
>> -#include "guest-agent-core.h"
>>  #include "qga-qapi-commands.h"
>>  #include "qapi/error.h"
>>  #include "qapi/qmp/qerror.h"
>> -#include "qemu/queue.h"
>>  #include "qemu/host-utils.h"
>>  #include "qemu/sockets.h"
>>  #include "qemu/base64.h"
>> @@ -70,7 +68,7 @@
>>  #endif
>>  #endif
>>
>> -static void ga_wait_child(pid_t pid, int *status, Error **errp)
>> +void ga_wait_child(pid_t pid, int *status, Error **errp)
>>  {
>>      pid_t rpid;
>>
>> @@ -629,16 +627,7 @@ void qmp_guest_file_flush(int64_t handle, Error
>> **errp)
>>  #if defined(__linux__)
>>
>>  #if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)
>> -typedef struct FsMount {
>> -    char *dirname;
>> -    char *devtype;
>> -    unsigned int devmajor, devminor;
>> -    QTAILQ_ENTRY(FsMount) next;
>> -} FsMount;
>> -
>> -typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
>> -
>> -static void free_fs_mount_list(FsMountList *mounts)
>> +void free_fs_mount_list(FsMountList *mounts)
>>  {
>>       FsMount *mount, *temp;
>>
>> @@ -653,157 +642,6 @@ static void free_fs_mount_list(FsMountList *mounts)
>>           g_free(mount);
>>       }
>>  }
>> -
>> -static int dev_major_minor(const char *devpath,
>> -                           unsigned int *devmajor, unsigned int
>> *devminor)
>> -{
>> -    struct stat st;
>> -
>> -    *devmajor = 0;
>> -    *devminor = 0;
>> -
>> -    if (stat(devpath, &st) < 0) {
>> -        slog("failed to stat device file '%s': %s", devpath,
>> strerror(errno));
>> -        return -1;
>> -    }
>> -    if (S_ISDIR(st.st_mode)) {
>> -        /* It is bind mount */
>> -        return -2;
>> -    }
>> -    if (S_ISBLK(st.st_mode)) {
>> -        *devmajor = major(st.st_rdev);
>> -        *devminor = minor(st.st_rdev);
>> -        return 0;
>> -    }
>> -    return -1;
>> -}
>> -
>> -/*
>> - * Walk the mount table and build a list of local file systems
>> - */
>> -static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error
>> **errp)
>> -{
>> -    struct mntent *ment;
>> -    FsMount *mount;
>> -    char const *mtab = "/proc/self/mounts";
>> -    FILE *fp;
>> -    unsigned int devmajor, devminor;
>> -
>> -    fp = setmntent(mtab, "r");
>> -    if (!fp) {
>> -        error_setg(errp, "failed to open mtab file: '%s'", mtab);
>> -        return false;
>> -    }
>> -
>> -    while ((ment = getmntent(fp))) {
>> -        /*
>> -         * An entry which device name doesn't start with a '/' is
>> -         * either a dummy file system or a network file system.
>> -         * Add special handling for smbfs and cifs as is done by
>> -         * coreutils as well.
>> -         */
>> -        if ((ment->mnt_fsname[0] != '/') ||
>> -            (strcmp(ment->mnt_type, "smbfs") == 0) ||
>> -            (strcmp(ment->mnt_type, "cifs") == 0)) {
>> -            continue;
>> -        }
>> -        if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) ==
>> -2) {
>> -            /* Skip bind mounts */
>> -            continue;
>> -        }
>> -
>> -        mount = g_new0(FsMount, 1);
>> -        mount->dirname = g_strdup(ment->mnt_dir);
>> -        mount->devtype = g_strdup(ment->mnt_type);
>> -        mount->devmajor = devmajor;
>> -        mount->devminor = devminor;
>> -
>> -        QTAILQ_INSERT_TAIL(mounts, mount, next);
>> -    }
>> -
>> -    endmntent(fp);
>> -    return true;
>> -}
>> -
>> -static void decode_mntname(char *name, int len)
>> -{
>> -    int i, j = 0;
>> -    for (i = 0; i <= len; i++) {
>> -        if (name[i] != '\\') {
>> -            name[j++] = name[i];
>> -        } else if (name[i + 1] == '\\') {
>> -            name[j++] = '\\';
>> -            i++;
>> -        } else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
>> -                   name[i + 2] >= '0' && name[i + 2] <= '7' &&
>> -                   name[i + 3] >= '0' && name[i + 3] <= '7') {
>> -            name[j++] = (name[i + 1] - '0') * 64 +
>> -                        (name[i + 2] - '0') * 8 +
>> -                        (name[i + 3] - '0');
>> -            i += 3;
>> -        } else {
>> -            name[j++] = name[i];
>> -        }
>> -    }
>> -}
>> -
>> -static bool build_fs_mount_list(FsMountList *mounts, Error **errp)
>> -{
>> -    FsMount *mount;
>> -    char const *mountinfo = "/proc/self/mountinfo";
>> -    FILE *fp;
>> -    char *line = NULL, *dash;
>> -    size_t n;
>> -    char check;
>> -    unsigned int devmajor, devminor;
>> -    int ret, dir_s, dir_e, type_s, type_e, dev_s, dev_e;
>> -
>> -    fp = fopen(mountinfo, "r");
>> -    if (!fp) {
>> -        return build_fs_mount_list_from_mtab(mounts, errp);
>> -    }
>> -
>> -    while (getline(&line, &n, fp) != -1) {
>> -        ret = sscanf(line, "%*u %*u %u:%u %*s %n%*s%n%c",
>> -                     &devmajor, &devminor, &dir_s, &dir_e, &check);
>> -        if (ret < 3) {
>> -            continue;
>> -        }
>> -        dash = strstr(line + dir_e, " - ");
>> -        if (!dash) {
>> -            continue;
>> -        }
>> -        ret = sscanf(dash, " - %n%*s%n %n%*s%n%c",
>> -                     &type_s, &type_e, &dev_s, &dev_e, &check);
>> -        if (ret < 1) {
>> -            continue;
>> -        }
>> -        line[dir_e] = 0;
>> -        dash[type_e] = 0;
>> -        dash[dev_e] = 0;
>> -        decode_mntname(line + dir_s, dir_e - dir_s);
>> -        decode_mntname(dash + dev_s, dev_e - dev_s);
>> -        if (devmajor == 0) {
>> -            /* btrfs reports major number = 0 */
>> -            if (strcmp("btrfs", dash + type_s) != 0 ||
>> -                dev_major_minor(dash + dev_s, &devmajor, &devminor) < 0)
>> {
>> -                continue;
>> -            }
>> -        }
>> -
>> -        mount = g_new0(FsMount, 1);
>> -        mount->dirname = g_strdup(line + dir_s);
>> -        mount->devtype = g_strdup(dash + type_s);
>> -        mount->devmajor = devmajor;
>> -        mount->devminor = devminor;
>> -
>> -        QTAILQ_INSERT_TAIL(mounts, mount, next);
>> -    }
>> -    free(line);
>> -
>> -    fclose(fp);
>> -    return true;
>> -}
>>  #endif
>>
>>  #if defined(CONFIG_FSFREEZE)
>> @@ -1708,20 +1546,13 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>>      return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
>>  }
>>
>> -/*
>> - * Walk list of mounted file systems in the guest, and freeze the ones
>> which
>> - * are real local file systems.
>> - */
>>  int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>>                                         strList *mountpoints,
>>                                         Error **errp)
>>  {
>> -    int ret = 0, i = 0;
>> -    strList *list;
>> +    int ret;
>>      FsMountList mounts;
>> -    struct FsMount *mount;
>>      Error *local_err = NULL;
>> -    int fd;
>>
>>      slog("guest-fsfreeze called");
>>
>> @@ -1740,122 +1571,34 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
>> has_mountpoints,
>>      /* cannot risk guest agent blocking itself on a write in this state
>> */
>>      ga_set_frozen(ga_state);
>>
>> -    QTAILQ_FOREACH_REVERSE(mount, &mounts, next) {
>> -        /* To issue fsfreeze in the reverse order of mounts, check if the
>> -         * mount is listed in the list here */
>> -        if (has_mountpoints) {
>> -            for (list = mountpoints; list; list = list->next) {
>> -                if (strcmp(list->value, mount->dirname) == 0) {
>> -                    break;
>> -                }
>> -            }
>> -            if (!list) {
>> -                continue;
>> -            }
>> -        }
>> -
>> -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>> -        if (fd == -1) {
>> -            error_setg_errno(errp, errno, "failed to open %s",
>> mount->dirname);
>> -            goto error;
>> -        }
>> -
>> -        /* we try to cull filesystems we know won't work in advance, but
>> other
>> -         * filesystems may not implement fsfreeze for less obvious
>> reasons.
>> -         * these will report EOPNOTSUPP. we simply ignore these when
>> tallying
>> -         * the number of frozen filesystems.
>> -         * if a filesystem is mounted more than once (aka bind mount) a
>> -         * consecutive attempt to freeze an already frozen filesystem
>> will
>> -         * return EBUSY.
>> -         *
>> -         * any other error means a failure to freeze a filesystem we
>> -         * expect to be freezable, so return an error in those cases
>> -         * and return system to thawed state.
>> -         */
>> -        ret = ioctl(fd, FIFREEZE);
>> -        if (ret == -1) {
>> -            if (errno != EOPNOTSUPP && errno != EBUSY) {
>> -                error_setg_errno(errp, errno, "failed to freeze %s",
>> -                                 mount->dirname);
>> -                close(fd);
>> -                goto error;
>> -            }
>> -        } else {
>> -            i++;
>> -        }
>> -        close(fd);
>> -    }
>> +    ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
>> +                                            mounts, errp);
>>
>>      free_fs_mount_list(&mounts);
>>      /* We may not issue any FIFREEZE here.
>>       * Just unset ga_state here and ready for the next call.
>>       */
>> -    if (i == 0) {
>> +    if (ret == 0) {
>>          ga_unset_frozen(ga_state);
>> +    } else if (ret < 0) {
>> +        qmp_guest_fsfreeze_thaw(NULL);
>>      }
>> -    return i;
>> -
>> -error:
>> -    free_fs_mount_list(&mounts);
>> -    qmp_guest_fsfreeze_thaw(NULL);
>> -    return 0;
>> +    return ret;
>>  }
>>
>> -/*
>> - * Walk list of frozen file systems in the guest, and thaw them.
>> - */
>>  int64_t qmp_guest_fsfreeze_thaw(Error **errp)
>>  {
>>      int ret;
>> -    FsMountList mounts;
>> -    FsMount *mount;
>> -    int fd, i = 0, logged;
>> -    Error *local_err = NULL;
>>
>> -    QTAILQ_INIT(&mounts);
>> -    if (!build_fs_mount_list(&mounts, &local_err)) {
>> -        error_propagate(errp, local_err);
>> -        return 0;
>> +    ret = qmp_guest_fsfreeze_do_thaw(errp);
>> +    if (ret >= 0) {
>> +        ga_unset_frozen(ga_state);
>> +        execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
>> +    } else {
>> +        ret = 0;
>>      }
>>
>> -    QTAILQ_FOREACH(mount, &mounts, next) {
>> -        logged = false;
>> -        fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
>> -        if (fd == -1) {
>> -            continue;
>> -        }
>> -        /* we have no way of knowing whether a filesystem was actually
>> unfrozen
>> -         * as a result of a successful call to FITHAW, only that if an
>> error
>> -         * was returned the filesystem was *not* unfrozen by that
>> particular
>> -         * call.
>> -         *
>> -         * since multiple preceding FIFREEZEs require multiple calls to
>> FITHAW
>> -         * to unfreeze, continuing issuing FITHAW until an error is
>> returned,
>> -         * in which case either the filesystem is in an unfreezable
>> state, or,
>> -         * more likely, it was thawed previously (and remains so
>> afterward).
>> -         *
>> -         * also, since the most recent successful call is the one that
>> did
>> -         * the actual unfreeze, we can use this to provide an accurate
>> count
>> -         * of the number of filesystems unfrozen by guest-fsfreeze-thaw,
>> which
>> -         * may * be useful for determining whether a filesystem was
>> unfrozen
>> -         * during the freeze/thaw phase by a process other than qemu-ga.
>> -         */
>> -        do {
>> -            ret = ioctl(fd, FITHAW);
>> -            if (ret == 0 && !logged) {
>> -                i++;
>> -                logged = true;
>> -            }
>> -        } while (ret == 0);
>> -        close(fd);
>> -    }
>> -
>> -    ga_unset_frozen(ga_state);
>> -    free_fs_mount_list(&mounts);
>> -
>> -    execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
>> -
>> -    return i;
>> +    return ret;
>>  }
>>
>>  static void guest_fsfreeze_cleanup(void)
>> diff --git a/qga/meson.build b/qga/meson.build
>> index a0ffd6d268..932b4e7ca8 100644
>> --- a/qga/meson.build
>> +++ b/qga/meson.build
>> @@ -72,6 +72,9 @@ qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
>>    'commands-posix.c',
>>    'commands-posix-ssh.c',
>>  ))
>> +qga_ss.add(when: 'CONFIG_LINUX', if_true: files(
>> +  'commands-linux.c',
>> +))
>>  qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
>>    'channel-win32.c',
>>    'commands-win32.c',
>> --
>> 2.34.1
>>
>>
>
> --
> Marc-André Lureau
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 43464 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-10-14  8:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13  9:23 [PATCH v4 0/7] qga: Add FreeBSD support Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 1/7] qga: Add initial " Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 2/7] qga: Move Linux-specific FS freeze/thaw code to a separate file Alexander Ivanov
2022-10-13 12:10   ` Marc-André Lureau
2022-10-14  7:55     ` Alexander Ivanov
2022-10-14  8:08       ` Marc-André Lureau
2022-10-13  9:23 ` [PATCH v4 3/7] qga: Add UFS freeze/thaw support for FreeBSD Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 4/7] qga: Add shutdown/halt/reboot " Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 5/7] qga: Add support for user password setting in FreeBSD Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 6/7] qga: Move HW address getting to a separate function Alexander Ivanov
2022-10-13  9:23 ` [PATCH v4 7/7] qga: Add HW address getting for FreeBSD Alexander Ivanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.