qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).