Hi

On Fri, Oct 14, 2022 at 11:56 AM Alexander Ivanov <
[email protected]> wrote:

> Hi
> On 13.10.2022 14:10, Marc-André Lureau wrote:
>
> Hi
>
> On Thu, Oct 13, 2022 at 1:23 PM Alexander Ivanov <
> [email protected]> 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 <[email protected]>
>> Reviewed-by: Marc-André Lureau <[email protected]>
>> Signed-off-by: Alexander Ivanov <[email protected]>
>> ---
>>  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      <[email protected]>
>> + *  Michal Privoznik  <[email protected]>
>> + *
>> + * 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

Reply via email to