From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47782) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxSij-0003FZ-6e for qemu-devel@nongnu.org; Wed, 27 May 2015 00:13:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxSif-0002Qb-US for qemu-devel@nongnu.org; Wed, 27 May 2015 00:13:33 -0400 Received: from mail-oi0-x233.google.com ([2607:f8b0:4003:c06::233]:32848) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxSif-0002QU-Ne for qemu-devel@nongnu.org; Wed, 27 May 2015 00:13:29 -0400 Received: by oiww2 with SMTP id w2so93756555oiw.0 for ; Tue, 26 May 2015 21:13:29 -0700 (PDT) Sender: fluxion Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1431327525-25625-3-git-send-email-justin@quarantainenet.nl> References: <1431327525-25625-1-git-send-email-justin@quarantainenet.nl> <1431327525-25625-3-git-send-email-justin@quarantainenet.nl> Message-ID: <20150527041317.5594.666@loki> Date: Tue, 26 May 2015 23:13:17 -0500 Subject: Re: [Qemu-devel] [PATCH v4 2/2] qga/qmp_guest_fstrim: Return per path fstrim result List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Justin Ossevoort , qemu-devel@nongnu.org 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 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 =3D 0; > FsMountList mounts; > struct FsMount *mount; > @@ -1340,39 +1344,59 @@ void qmp_guest_fstrim(bool has_minimum, int64_t m= inimum, Error **errp) > build_fs_mount_list(&mounts, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return; > + return NULL; > } > = > + response =3D g_malloc0(sizeof(*response)); > + > QTAILQ_FOREACH(mount, &mounts, next) { > + result =3D g_malloc0(sizeof(*result)); > + result->path =3D g_strdup(mount->dirname); > + > + list =3D g_malloc0(sizeof(*list)); > + list->value =3D result; > + list->next =3D response->paths; > + response->paths =3D list; > + > fd =3D qemu_open(mount->dirname, O_RDONLY); > if (fd =3D=3D -1) { > - error_setg_errno(errp, errno, "failed to open %s", mount->di= rname); > - goto error; > + result->error =3D g_strdup_printf("failed to open: %s", > + strerror(errno)); > + result->has_error =3D 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 o= ther > - * 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 =3D 0; > r.len =3D -1; > r.minlen =3D has_minimum ? minimum : 0; > ret =3D ioctl(fd, FITRIM, &r); > if (ret =3D=3D -1) { > - if (errno !=3D ENOTTY && errno !=3D EOPNOTSUPP) { > - error_setg_errno(errp, errno, "failed to trim %s", > - mount->dirname); > - close(fd); > - goto error; > + result->has_error =3D true; > + if (errno =3D=3D ENOTTY || errno =3D=3D EOPNOTSUPP) { > + result->error =3D g_strdup("trim not supported"); > + } else { > + result->error =3D g_strdup_printf("failed to trim: %s", > + strerror(errno)); > } > + close(fd); > + continue; > } > + > + result->has_minimum =3D true; > + result->minimum =3D r.minlen; > + result->has_trimmed =3D true; > + result->trimmed =3D 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 > = >=20