[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/2] qga/qmp_guest_fstrim: Return per path fs
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/2] qga/qmp_guest_fstrim: Return per path fstrim result |
Date: |
Tue, 26 May 2015 23:13:17 -0500 |
User-agent: |
alot/0.3.6 |
Quoting Justin Ossevoort (2015-05-11 01:58:45)
> The current guest-fstrim support only returns an error if some
> mountpoint was unable to be trimmed, skipping any possible additional
> mountpoints. The result of the TRIM operation itself is also discarded.
>
> This change returns a per mountpoint result of the TRIM operation. If an
> error occurs on some mountpoints that error is returned and the
> guest-fstrim continue with any additional mountpoints.
>
> The returned values for errors, minimum and trimmed are dependant on the
> filesystem, storage stacks and kernel version.
>
> Signed-off-by: Justin Ossevoort <address@hidden>
Looks good other than s/type/struct/ in schema that Olga pointed out.
No need to respin, can fix it up in my tree.
> ---
> qga/commands-posix.c | 54
> ++++++++++++++++++++++++++++++++++++++--------------
> qga/commands-win32.c | 4 +++-
> qga/qapi-schema.json | 32 ++++++++++++++++++++++++++++---
> 3 files changed, 72 insertions(+), 18 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 4449628..ec0d69e 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1325,8 +1325,12 @@ static void guest_fsfreeze_cleanup(void)
> /*
> * Walk list of mounted file systems in the guest, and trim them.
> */
> -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> +GuestFilesystemTrimResponse *
> +qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> {
> + GuestFilesystemTrimResponse *response;
> + GuestFilesystemTrimResultList *list;
> + GuestFilesystemTrimResult *result;
> int ret = 0;
> FsMountList mounts;
> struct FsMount *mount;
> @@ -1340,39 +1344,59 @@ void qmp_guest_fstrim(bool has_minimum, int64_t
> minimum, Error **errp)
> build_fs_mount_list(&mounts, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> - return;
> + return NULL;
> }
>
> + response = g_malloc0(sizeof(*response));
> +
> QTAILQ_FOREACH(mount, &mounts, next) {
> + result = g_malloc0(sizeof(*result));
> + result->path = g_strdup(mount->dirname);
> +
> + list = g_malloc0(sizeof(*list));
> + list->value = result;
> + list->next = response->paths;
> + response->paths = list;
> +
> fd = qemu_open(mount->dirname, O_RDONLY);
> if (fd == -1) {
> - error_setg_errno(errp, errno, "failed to open %s",
> mount->dirname);
> - goto error;
> + result->error = g_strdup_printf("failed to open: %s",
> + strerror(errno));
> + result->has_error = true;
> + continue;
> }
>
> /* We try to cull filesytems we know won't work in advance, but other
> * filesytems may not implement fstrim for less obvious reasons.
> These
> - * will report EOPNOTSUPP; we simply ignore these errors. Any other
> - * error means an unexpected error, so return it in those cases. In
> - * some other cases ENOTTY will be reported (e.g. CD-ROMs).
> + * will report EOPNOTSUPP; while in some other cases ENOTTY will be
> + * reported (e.g. CD-ROMs).
> + * Any other error means an unexpected error.
> */
> r.start = 0;
> r.len = -1;
> r.minlen = has_minimum ? minimum : 0;
> ret = ioctl(fd, FITRIM, &r);
> if (ret == -1) {
> - if (errno != ENOTTY && errno != EOPNOTSUPP) {
> - error_setg_errno(errp, errno, "failed to trim %s",
> - mount->dirname);
> - close(fd);
> - goto error;
> + result->has_error = true;
> + if (errno == ENOTTY || errno == EOPNOTSUPP) {
> + result->error = g_strdup("trim not supported");
> + } else {
> + result->error = g_strdup_printf("failed to trim: %s",
> + strerror(errno));
> }
> + close(fd);
> + continue;
> }
> +
> + result->has_minimum = true;
> + result->minimum = r.minlen;
> + result->has_trimmed = true;
> + result->trimmed = r.len;
> close(fd);
> }
>
> -error:
> free_fs_mount_list(&mounts);
> + return response;
> }
> #endif /* CONFIG_FSTRIM */
>
> @@ -2401,9 +2425,11 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
> #endif /* CONFIG_FSFREEZE */
>
> #if !defined(CONFIG_FSTRIM)
> -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> +GuestFilesystemTrimResponse *
> +qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> {
> error_set(errp, QERR_UNSUPPORTED);
> + return NULL;
> }
> #endif
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3ef0549..cc407f3 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -493,9 +493,11 @@ static void guest_fsfreeze_cleanup(void)
> * Walk list of mounted file systems in the guest, and discard unused
> * areas.
> */
> -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> +GuestFilesystemTrimResponse *
> +qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> {
> error_set(errp, QERR_UNSUPPORTED);
> + return NULL;
> }
>
> typedef enum {
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 95f49e3..b4f4b93 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -425,6 +425,30 @@
> 'returns': 'int' }
>
> ##
> +# @GuestFilesystemTrimResult
> +#
> +# @path: path that was trimmed
> +# @error: an error message when trim failed
> +# @trimmed: bytes trimmed for this path
> +# @minimum: reported effective minimum for this path
> +#
> +# Since: 2.4
> +##
> +{ 'type': 'GuestFilesystemTrimResult',
> + 'data': {'path': 'str',
> + '*trimmed': 'int', '*minimum': 'int', '*error': 'str'} }
> +
> +##
> +# @GuestFilesystemTrimResponse
> +#
> +# @paths: list of @GuestFilesystemTrimResult per path that was trimmed
> +#
> +# Since: 2.4
> +##
> +{ 'type': 'GuestFilesystemTrimResponse',
> + 'data': {'paths': ['GuestFilesystemTrimResult']} }
> +
> +##
> # @guest-fstrim:
> #
> # Discard (or "trim") blocks which are not in use by the filesystem.
> @@ -437,12 +461,14 @@
> # fragmented free space, although not all blocks will be discarded.
> # The default value is zero, meaning "discard every free block".
> #
> -# Returns: Nothing.
> +# Returns: A @GuestFilesystemTrimResponse which contains the
> +# status of all trimmed paths.
> #
> -# Since: 1.2
> +# Since: 2.4
> ##
> { 'command': 'guest-fstrim',
> - 'data': { '*minimum': 'int' } }
> + 'data': { '*minimum': 'int' },
> + 'returns': 'GuestFilesystemTrimResponse' }
>
> ##
> # @guest-suspend-disk
> --
> 2.1.4
>
>