From: Dexuan Cui <decui@microsoft.com>
To: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "driverdev-devel@linuxdriverproject.org" 
	<driverdev-devel@linuxdriverproject.org>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "apw@canonical.com" <apw@canonical.com>,
        "jasowang@redhat.com" <jasowang@redhat.com>
Subject: RE: [PATCH v3 RESEND] Tools: hv: vssdaemon: ignore the EBUSY on
 multiple freezing the same partition
Thread-Topic: [PATCH v3 RESEND] Tools: hv: vssdaemon: ignore the EBUSY on
 multiple freezing the same partition
Thread-Index: AQHP7QvtQ+R06uDVdE+EY9U8uNYDEpxPz+Zw
Date: Tue, 4 Nov 2014 02:16:50 +0000
Message-ID: <F792CF86EFE20D4AB8064279AFBA51C610587577@HKNPRD3002MB017.064d.mgd.msft.net>
References: <1413885374-12148-1-git-send-email-decui@microsoft.com>
In-Reply-To: <1413885374-12148-1-git-send-email-decui@microsoft.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of Dexuan Cui
> Sent: Tuesday, October 21, 2014 17:56 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Cc: Haiyang Zhang
> Subject: [PATCH v3 RESEND] Tools: hv: vssdaemon: ignore the EBUSY on
> multiple freezing the same partition
> 
> If a partition appears mounted more than once in /proc/mounts,
> vss_do_freeze()
> succeeds only for the first time and gets EBUSY (on freeze) or EINVAL (on
> thaw) for the second time. The patch ignores these to make the backup
> feature
> work.
> 
> Also improved the error handling in case a freeze operation fails.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> 
> v2: Add "errno = 0;" before the ioctl()
> (Unnecessary and removed now since we remove syslog() in vss_do_freeze()
> in v3)
> 
> v3: Remove the unsafe syslog() in vss_do_freeze(): that could write the disk.
>     Thaw the filesystems in case the freezing operation fails.
>     In main(), add syslog() when we check the return value of vss_operate().
> 
> [Oct 21, 2014] This is just a RESEND since the previous one I sent about a
> month
> ago seems neglected. :-(
> 
>  tools/hv/hv_vss_daemon.c | 48
> ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
> index 6a213b8..1db9430 100644
> --- a/tools/hv/hv_vss_daemon.c
> +++ b/tools/hv/hv_vss_daemon.c
> @@ -44,21 +44,39 @@ static struct sockaddr_nl addr;
>  #endif
> 
> 
> -static int vss_do_freeze(char *dir, unsigned int cmd, char *fs_op)
> +/* Don't use syslog() in the function since that can cause write to disk */
> +static int vss_do_freeze(char *dir, unsigned int cmd)
>  {
>  	int ret, fd = open(dir, O_RDONLY);
> 
>  	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 a partition is mounted more than once, only the first
> +	 * FREEZE/THAW can succeed and the later ones will get
> +	 * EBUSY/EINVAL respectively: there could be 2 cases:
> +	 * 1) a user may mount the same partition to differnt directories
> +	 *  by mistake or on purpose;
> +	 * 2) The subvolume of btrfs appears to have the same partition
> +	 * mounted more than once.
> +	 */
> +	if (ret) {
> +		if ((cmd == FIFREEZE && errno == EBUSY) ||
> +		    (cmd == FITHAW && errno == EINVAL)) {
> +			close(fd);
> +			return 0;
> +		}
> +	}
> +
>  	close(fd);
>  	return !!ret;
>  }
> 
>  static int vss_operate(int operation)
>  {
> -	char *fs_op;
>  	char match[] = "/dev/";
>  	FILE *mounts;
>  	struct mntent *ent;
> @@ -68,11 +86,9 @@ static int vss_operate(int operation)
>  	switch (operation) {
>  	case VSS_OP_FREEZE:
>  		cmd = FIFREEZE;
> -		fs_op = "freeze";
>  		break;
>  	case VSS_OP_THAW:
>  		cmd = FITHAW;
> -		fs_op = "thaw";
>  		break;
>  	default:
>  		return -1;
> @@ -93,15 +109,23 @@ static int vss_operate(int operation)
>  			root_seen = 1;
>  			continue;
>  		}
> -		error |= vss_do_freeze(ent->mnt_dir, cmd, fs_op);
> +		error |= vss_do_freeze(ent->mnt_dir, cmd);
> +		if (error && operation == VSS_OP_FREEZE)
> +			goto err;
>  	}
>  	endmntent(mounts);
> 
>  	if (root_seen) {
> -		error |= vss_do_freeze("/", cmd, fs_op);
> +		error |= vss_do_freeze("/", cmd);
> +		if (error && operation == VSS_OP_FREEZE)
> +			goto err;
>  	}
> 
>  	return error;
> +err:
> +	endmntent(mounts);
> +	vss_operate(VSS_OP_THAW);
> +	return error;
>  }
> 
>  static int netlink_send(int fd, struct cn_msg *msg)
> @@ -249,8 +273,16 @@ int main(void)
>  		case VSS_OP_FREEZE:
>  		case VSS_OP_THAW:
>  			error = vss_operate(op);
> -			if (error)
> +			syslog(LOG_INFO, "VSS: op=%s: %s\n",
> +				op == VSS_OP_FREEZE ? "FREEZE" : "THAW",
> +				error ? "failed" : "succeeded");
> +
> +			if (error) {
>  				error = HV_E_FAIL;
> +				syslog(LOG_ERR, "op=%d failed!", op);
> +				syslog(LOG_ERR, "report it with these files:");
> +				syslog(LOG_ERR, "/etc/fstab and
> /proc/mounts");
> +			}
>  			break;
>  		default:
>  			syslog(LOG_ERR, "Illegal op:%d\n", op);
> --
> 1.9.1

Hi Greg,
ping. 
(I saw you replying in the lists 3 minutes ago. :-)

Thanks,
-- Dexuan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/