Re: [PATCH V3 1/1] Drivers: hv: Implement the file copy service

[Olaf Hering <olaf@aepfle.de>]

On Tue, Jan 21, K. Y. Srinivasan wrote:

Looks better. A few comments below:


> +++ b/drivers/hv/hv_fcopy.c

> +static bool daemon_died;

This is a write only variable.

> +static u32 daemon_version;

This is a write only variable.

> +static int fcopy_handle_handshake(u32 version)
> +{
> +	pr_info("FCP: user-mode registering done. Daemon version: %d\n",
> +		version);
> +	fcopy_transaction.active = false;
> +	daemon_version = version;

I think fcopy_handle_handshake should proactivly reject requests if
version is not 0.

There is an unused FCOPY_VERSION as well. I think this should not be in
a header. Instead the kernel should handle all known versions and reject
everything it does not know about.

> +	syslog(LOG_INFO, "Target file name: %s\n", target_fname);

Newline in syslog

> +				if (mkdir((char *)smsg->path_name, 0755)) {
> +					syslog(LOG_ERR, "Failed to create %s\n",

Newline in syslog

> +		syslog(LOG_INFO, "File: %s exists\n", target_fname);

Newline in syslog

> +		syslog(LOG_INFO, "Open Failed: %s\n", strerror(errno));

Newline in syslog

> +				syslog(LOG_ERR, "Read error: %s\n",

Newline in syslog

> +			syslog(LOG_ERR, "Unknown operation: %d\n",

Newline in syslog

Olaf