2014-11-07 17:09:41

by Vitaly Kuznetsov

[permalink] [raw]
Subject: [PATCH 0/3] Tools: hv: vssdaemon: freeze/thaw logic improvement for the failure case

This patch series addresses the following issues:
- Wrong error reporting for multiple filesystems case.
- Skip all readonly-mounted filesystems instead of skipping iso9660.
- Thaw all filesystems after an unsuccessful freeze attempt.

Vitaly Kuznetsov (3):
Tools: hv: vssdaemon: consult with errno in case of failure only
Tools: hv: vssdaemon: skip all filesystems mounted readonly
Tools: hv: vssdaemon: thaw everything in case of freeze failure

tools/hv/hv_vss_daemon.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

--
1.9.3


2014-11-07 17:09:47

by Vitaly Kuznetsov

[permalink] [raw]
Subject: [PATCH 3/3] Tools: hv: vssdaemon: thaw everything in case of freeze failure

If one or more filesystems failed to freeze we need to thaw everything as
host doing backup won't issue THAW request after we return HV_E_FAIL and our
system will remain with frozen filesystems for ever.

There is no track of filesystems we freeze so in case there is some external
tool doing freeze/thaw requests at the same time they will collide with vss
daemon. This issue can be addressed by introducing a freeze/thaw transaction
and keeping track of what was actually frozen

Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
tools/hv/hv_vss_daemon.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 7be999a..e98c638 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -284,6 +284,12 @@ int main(int argc, char *argv[])
error = vss_operate(op);
if (error)
error = HV_E_FAIL;
+ if (error && op == VSS_OP_FREEZE) {
+ /* Need to thaw all frozen fylesystems */
+ syslog(LOG_ERR,
+ "Freeze failed, thaw everything");
+ vss_operate(VSS_OP_THAW);
+ }
break;
default:
syslog(LOG_ERR, "Illegal op:%d\n", op);
--
1.9.3

2014-11-07 17:09:46

by Vitaly Kuznetsov

[permalink] [raw]
Subject: [PATCH 2/3] Tools: hv: vssdaemon: skip all filesystems mounted readonly

Instead of making a list of exceptions for readonly filesystems
in addition to iso9660 we already have it is better to skip freeze
operation for all readonly-mounted filesystems.

Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
tools/hv/hv_vss_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 5f67858..7be999a 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -90,7 +90,7 @@ static int vss_operate(int operation)
while ((ent = getmntent(mounts))) {
if (strncmp(ent->mnt_fsname, match, strlen(match)))
continue;
- if (strcmp(ent->mnt_type, "iso9660") == 0)
+ if (hasmntopt(ent, MNTOPT_RO) != NULL)
continue;
if (strcmp(ent->mnt_type, "vfat") == 0)
continue;
--
1.9.3

2014-11-07 17:09:44

by Vitaly Kuznetsov

[permalink] [raw]
Subject: [PATCH 1/3] Tools: hv: vssdaemon: consult with errno in case of failure only

If ioctl() return 0 there is no point in examining errno and
it can actually produce misleading output. In case there was
no failure errno will contain the error code for previous failure
so user will see the following in the log:
Hyper-V VSS: VSS: freeze of /mnt/udf: Operation not supported
Hyper-V VSS: VSS: freeze of /: Operation not supported

We should also log errors with LOG_ERR instead of LOG_INFO.

Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
tools/hv/hv_vss_daemon.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 9ae2b6e..5f67858 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -52,7 +52,11 @@ static int vss_do_freeze(char *dir, unsigned int cmd, char *fs_op)
if (fd < 0)
return 1;
ret = ioctl(fd, cmd, 0);
- syslog(LOG_INFO, "VSS: %s of %s: %s\n", fs_op, dir, strerror(errno));
+ if (ret)
+ syslog(LOG_ERR, "VSS: %s of %s: %s\n", fs_op, dir,
+ strerror(errno));
+ else
+ syslog(LOG_INFO, "VSS: %s of %s succeeded\n", fs_op, dir);
close(fd);
return !!ret;
}
--
1.9.3

2014-11-08 01:24:28

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH 0/3] Tools: hv: vssdaemon: freeze/thaw logic improvement for the failure case

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:[email protected]]
> Sent: Saturday, November 8, 2014 1:09 AM
> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
> Cc: [email protected]; [email protected]; Dexuan Cui
> Subject: [PATCH 0/3] Tools: hv: vssdaemon: freeze/thaw logic improvement
> for the failure case
>
> This patch series addresses the following issues:
> - Wrong error reporting for multiple filesystems case.
> - Skip all readonly-mounted filesystems instead of skipping iso9660.
> - Thaw all filesystems after an unsuccessful freeze attempt.
>
> Vitaly Kuznetsov (3):
> Tools: hv: vssdaemon: consult with errno in case of failure only
> Tools: hv: vssdaemon: skip all filesystems mounted readonly
> Tools: hv: vssdaemon: thaw everything in case of freeze failure
>
> tools/hv/hv_vss_daemon.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)

Hi Vitaly,
Thanks for your patchset!

FYI: Greg checked in a patch of mine several hours ago -- my patch
implemented "thaw all filesytems on a failure of freeze" too. :-)

Please see my patch in Greg's char-misc-next tree:
https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-next&id=4f689190bb55d171d2f6614f8a6cbd4b868e48bd

Can you please rebase your patch(es) on Greg's tree?

Thanks,
-- Dexuan

2014-11-10 12:31:06

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH 0/3] Tools: hv: vssdaemon: freeze/thaw logic improvement for the failure case

Dexuan Cui <[email protected]> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:[email protected]]
>> Sent: Saturday, November 8, 2014 1:09 AM
>> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
>> Cc: [email protected]; [email protected]; Dexuan Cui
>> Subject: [PATCH 0/3] Tools: hv: vssdaemon: freeze/thaw logic improvement
>> for the failure case
>>
>> This patch series addresses the following issues:
>> - Wrong error reporting for multiple filesystems case.
>> - Skip all readonly-mounted filesystems instead of skipping iso9660.
>> - Thaw all filesystems after an unsuccessful freeze attempt.
>>
>> Vitaly Kuznetsov (3):
>> Tools: hv: vssdaemon: consult with errno in case of failure only
>> Tools: hv: vssdaemon: skip all filesystems mounted readonly
>> Tools: hv: vssdaemon: thaw everything in case of freeze failure
>>
>> tools/hv/hv_vss_daemon.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> Hi Vitaly,
> Thanks for your patchset!
>
> FYI: Greg checked in a patch of mine several hours ago -- my patch
> implemented "thaw all filesytems on a failure of freeze" too. :-)

Ah, sorry for stepping on your toes :-)

>
> Please see my patch in Greg's char-misc-next tree:
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-next&id=4f689190bb55d171d2f6614f8a6cbd4b868e48bd
>
> Can you please rebase your patch(es) on Greg's tree?

Sure, I'll throw away my patch#3, rebase, and repost.

>
> Thanks,
> -- Dexuan

--
Vitaly