At 2018-02-16 02:41:25, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote: >Quoting Chen Hanxiao (2018-02-08 19:35:42) >> From: Chen Hanxiao <chenhanx...@gmail.com> >> >> If we set mountpoints to qmp_guest_fsfreeze_freeze_list, >> we may got nothing to freeze as all mountpoints are >> not valid. >> Call ga_unset_frozen in this senario. >> >> Cc: Michael Roth <mdr...@linux.vnet.ibm.com> >> Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com> >> --- >> Rebase on master >> >> qga/commands-posix.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index e809e382eb..9fd51f1d7a 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool >> has_mountpoints, >> } >> >> free_fs_mount_list(&mounts); >> + /* We may not issue any FIFREEZE here when had mountpoints. >> + * Just unset ga_state here and ready for the next call. >> + */ >> + if (has_mountpoints && i == 0) { >> + ga_unset_frozen(ga_state); >> + } > >It seems odd to special-case has_mountpoints. Wouldn't: > > if (i == 0) { > ... > } > >be more consistent? Then management could infer i==0 leaves qga in >unfrozen state. Otherwise I'd rather just stick with expecting a >gratuitous unfreeze() since it requires less special-casing on the >management/ side. > >And if we do change this, we'd probably want to update >qga/qapi-schema.json to reflect the behavior. I.e.: > >## ># @guest-fsfreeze-freeze: ># ># Sync and freeze all freezable, local guest filesystems. If this ># command succeeded, you may call @guest-fsfreeze-thaw later to ># unfreeze. >... ># Returns: Number of file systems currently frozen. On error, all filesystems >-# will be thawed. >+# will be thawed. If no filesystems are frozen as a result of this call, >+# then @guest-fsfreeze-status will remain "thawed" and calling >+# @guest-fsfreeze-thaw is not necessary. >
Thanks for the review. Will be fixed in v2. Regards, - Chen