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

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

[Vitaly Kuznetsov]

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

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

[Vitaly Kuznetsov]

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 <vkuznets@redhat.com>
---
 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

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

[Vitaly Kuznetsov]

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 <vkuznets@redhat.com>
---
 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

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

[Vitaly Kuznetsov]

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 <vkuznets@redhat.com>
---
 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

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

[Dexuan Cui]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Saturday, November 8, 2014 1:09 AM
> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
> Cc: devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; 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

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

[Vitaly Kuznetsov]

Dexuan Cui <decui@microsoft.com> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Saturday, November 8, 2014 1:09 AM
>> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
>> Cc: devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; 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