Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
From: Michael Roth <mdroth@linux.vnet.ibm.com>
In-Reply-To: <20140522135653.6110.79891.stgit@hds.com>
References: <20140522135641.6110.28511.stgit@hds.com>
	<20140522135653.6110.79891.stgit@hds.com>
Message-ID: <20140603212147.3985.99693@loki>
Date: Tue, 03 Jun 2014 16:21:47 -0500
Subject: Re: [Qemu-devel] [PATCH v3 1/2] qga: Add 'mountpoints' argument to
 guest-fsfreeze-freeze command
To: Tomoki Sekiyama <tomoki.sekiyama@hds.com>, qemu-devel@nongnu.org
Cc: mitsuhiro.tanino@hds.com

Quoting Tomoki Sekiyama (2014-05-22 08:56:53)
> When an array of mount point paths is specified as 'mountpoints' argument
> of guest-fsfreeze-freeze, qemu-ga with this patch will only freeze the fi=
le
> systems mounted on specified paths in Linux.
> This would be useful when the host wants to create partial disk snapshots.

Since this isn't really applicable for win32, and it's hard to discover
optional params via guest-info without some extensive changes to how we han=
dle
capabilities negotiation, I think it makes more sense to introduce a new
command for this, something like guest-fsfreeze-freeze-filesystems, which we
can easily discover and properly mark as unsupported on win32.

Other than that looks good.

> =

> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
> ---
>  qga/commands-posix.c |   17 ++++++++++++++++-
>  qga/commands-win32.c |    3 ++-
>  qga/qapi-schema.json |    4 ++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
> =

> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 34ddba0..771f00c 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -714,9 +714,11 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error =
**errp)
>   * Walk list of mounted file systems in the guest, and freeze the ones w=
hich
>   * are real local file systems.
>   */
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_mountpoints, strList *mountpo=
ints,
> +                                  Error **errp)
>  {
>      int ret =3D 0, i =3D 0;
> +    strList *list;
>      FsMountList mounts;
>      struct FsMount *mount;
>      Error *local_err =3D NULL;
> @@ -741,6 +743,19 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>      ga_set_frozen(ga_state);
> =

>      QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, 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 =3D mountpoints; list; list =3D list->next) {
> +                if (strcmp(list->value, mount->dirname) =3D=3D 0) {
> +                    break;
> +                }
> +            }
> +            if (!list) {
> +                continue;
> +            }
> +        }
> +
>          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);
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index d793dd0..0c6296d 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -171,7 +171,8 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error *=
*errp)
>   * Freeze local file systems using Volume Shadow-copy Service.
>   * The frozen state is limited for up to 10 seconds by VSS.
>   */
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_mountpoints, strList *mountpo=
ints,
> +                                  Error **errp)
>  {
>      int i;
>      Error *local_err =3D NULL;
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index a8cdcb3..31c0dc8 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -378,12 +378,16 @@
>  #
>  # Sync and freeze all freezable, local guest filesystems
>  #
> +# @mountpoints: #optional an array of mountpoints of filesystems to be f=
rozen.
> +#               If omitted, every mounted filesystem is frozen. (Since: =
2.1)
> +#
>  # Returns: Number of file systems currently frozen. On error, all filesy=
stems
>  # will be thawed.
>  #
>  # Since: 0.15.0
>  ##
>  { 'command': 'guest-fsfreeze-freeze',
> +  'data':    { '*mountpoints': ['str'] },
>    'returns': 'int' }
> =

>  ##