RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-20 Thread KY Srinivasan 
Olaf,

I am cleaning up the code based on your feedback. By the time I am done with my 
cleanup, I doubt if
this patch would apply. Do you mind if I were to include your changes here as 
part of my cleanup?

Thank you,

K. Y

 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Thursday, January 16, 2014 2:49 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
  Implement the file copy service for Linux guests on Hyper-V. This permits 
  the
  host to copy a file (over VMBUS) into the guest. This facility is part of
  guest integration services supported on the Windows platform.
  Here is a link that provides additional details on this functionality:
 
 The change below fixes some warnings in the daemon code.
 Compile tested only.
 I also think the newlines in some of the syslog calls should be removed.
 
 Olaf
 
 
 hv_fcopy_daemon.c: In function 'hv_start_fcopy':
 hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
smsg-file_name);
^
 hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
 *', but argument 5 has type '__u16 *' [-Wformat=]
 hv_fcopy_daemon.c:57:6: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
   errno, strerror(errno));
   ^
 hv_fcopy_daemon.c:61:4: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
 syslog(LOG_ERR, Invalid path: %s\n, smsg-path_name);
 ^
 hv_fcopy_daemon.c: In function 'main':
 hv_fcopy_daemon.c:117:8: warning: ignoring return value of 'daemon', declared
 with attribute warn_unused_result [-Wunused-result]
   daemon(1, 0);
 ^
 hv_fcopy_daemon.c:132:7: warning: ignoring return value of 'write', declared
 with attribute warn_unused_result [-Wunused-result]
   write(fcopy_fd, version, sizeof(int));
^
 hv_fcopy_daemon.c:171:9: warning: ignoring return value of 'pwrite', declared
 with attribute warn_unused_result [-Wunused-result]
pwrite(fcopy_fd, error, sizeof(int), 0);
  ^
 
 Signed-off-by: Olaf Hering o...@aepfle.de
 
 diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
 index c0e5c90..d1fadb7 100644
 --- a/tools/hv/hv_fcopy_daemon.c
 +++ b/tools/hv/hv_fcopy_daemon.c
 @@ -35,14 +35,14 @@
  #include dirent.h
 
  static int target_fd;
 -char target_fname[W_MAX_PATH];
 +static char target_fname[W_MAX_PATH];
 
  static int hv_start_fcopy(struct hv_start_fcopy *smsg)
  {
   int error = HV_E_FAIL;
 
 - sprintf(target_fname, %s%s%s, smsg-path_name, /,
 - smsg-file_name);
 + snprintf(target_fname, sizeof(target_fname), %s/%s,
 + (char *)smsg-path_name, (char*)smsg-file_name);
 
   syslog(LOG_INFO, Target file name: %s\n, target_fname);
   /*
 @@ -54,12 +54,12 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
   if (mkdir((char *)smsg-path_name, 0755)) {
   syslog(LOG_ERR,
   Failed to create '%s'; error: %d %s\n,
 - smsg-path_name,
 + (char *)smsg-path_name,
   errno, strerror(errno));
   goto done;
   }
   } else {
 - syslog(LOG_ERR, Invalid path: %s\n, smsg-
 path_name);
 + syslog(LOG_ERR, Invalid path: %s, (char *)smsg-
 path_name);
   goto done;
   }
   }
 @@ -115,7 +115,8 @@ int main(void)
   char *buffer[4096 * 2];
   struct hv_fcopy_hdr *in_msg;
 
 - daemon(1, 0);
 + if (daemon(1, 0))
 + return 1;
   openlog(HV_FCOPY, 0, LOG_USER);
   syslog(LOG_INFO, HV_FCOPY starting; pid is:%d, getpid());
 
 @@ -130,7 +131,10 @@ int main(void)
   /*
* Register with the kernel.
*/
 - write(fcopy_fd, version, sizeof(int));
 + if (write(fcopy_fd, version, sizeof(int)) != sizeof(int)) {
 + syslog(LOG_ERR, write failed: %s,strerror(errno));
 + exit(EXIT_FAILURE);
 + }
 
   while (1) {
   /*
 @@ -169,6 +173,9 @@ int main(void)
 
   }
 
 - pwrite(fcopy_fd, error, sizeof(int), 0);
 + if (pwrite(fcopy_fd, error, sizeof(int), 0) != sizeof(int)) {
 + syslog(LOG_ERR, pwrite failed: %s,strerror(errno));
 + exit(EXIT_FAILURE);
 + }
   }
  }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-20 Thread Olaf Hering 
On Mon, Jan 20, KY Srinivasan wrote:

 I am cleaning up the code based on your feedback. By the time I am
 done with my cleanup, I doubt if this patch would apply. Do you mind
 if I were to include your changes here as part of my cleanup?

Yes, thats fine.


Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-20 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Monday, January 20, 2014 2:16 PM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Mon, Jan 20, KY Srinivasan wrote:
 
  I am cleaning up the code based on your feedback. By the time I am
  done with my cleanup, I doubt if this patch would apply. Do you mind
  if I were to include your changes here as part of my cleanup?
 
 Yes, thats fine.

Thanks Olaf.

K. Y
 
 
 Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Olaf Hering 
On Tue, Jan 14, K. Y. Srinivasan wrote:

 +enum hv_fcopy_op {
 + START_FILE_COPY = 0,
 + WRITE_TO_FILE,
 + COMPLETE_FCOPY,
 + CANCEL_FCOPY,
 +};
 +
 +struct hv_fcopy_hdr {
 + enum hv_fcopy_op operation;
 + uuid_le service_id0; /* currently unused */
 + uuid_le service_id1; /* currently unused */
 +} __attribute__((packed));

Is enum a fixed size? This struct is used in other structs, so I wonder
what will happen to the kernel/user protocol if any of that changes. Or
with a 64bit kernel and 32bit daemon. Maybe operation should be __u32?


Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Olaf Hering 
On Tue, Jan 14, K. Y. Srinivasan wrote:

 Implement the file copy service for Linux guests on Hyper-V. This permits the
 host to copy a file (over VMBUS) into the guest. This facility is part of
 guest integration services supported on the Windows platform.
 Here is a link that provides additional details on this functionality:

The change below fixes some warnings in the daemon code.
Compile tested only.
I also think the newlines in some of the syslog calls should be removed.

Olaf


hv_fcopy_daemon.c: In function 'hv_start_fcopy':
hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char *', 
but argument 3 has type '__u16 *' [-Wformat=]
   smsg-file_name);
   ^
hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char *', 
but argument 5 has type '__u16 *' [-Wformat=]
hv_fcopy_daemon.c:57:6: warning: format '%s' expects argument of type 'char *', 
but argument 3 has type '__u16 *' [-Wformat=]
  errno, strerror(errno));
  ^
hv_fcopy_daemon.c:61:4: warning: format '%s' expects argument of type 'char *', 
but argument 3 has type '__u16 *' [-Wformat=]
syslog(LOG_ERR, Invalid path: %s\n, smsg-path_name);
^
hv_fcopy_daemon.c: In function 'main':
hv_fcopy_daemon.c:117:8: warning: ignoring return value of 'daemon', declared 
with attribute warn_unused_result [-Wunused-result]
  daemon(1, 0);
^
hv_fcopy_daemon.c:132:7: warning: ignoring return value of 'write', declared 
with attribute warn_unused_result [-Wunused-result]
  write(fcopy_fd, version, sizeof(int));
   ^
hv_fcopy_daemon.c:171:9: warning: ignoring return value of 'pwrite', declared 
with attribute warn_unused_result [-Wunused-result]
   pwrite(fcopy_fd, error, sizeof(int), 0);
 ^

Signed-off-by: Olaf Hering o...@aepfle.de

diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index c0e5c90..d1fadb7 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -35,14 +35,14 @@
 #include dirent.h
 
 static int target_fd;
-char target_fname[W_MAX_PATH];
+static char target_fname[W_MAX_PATH];
 
 static int hv_start_fcopy(struct hv_start_fcopy *smsg)
 {
int error = HV_E_FAIL;
 
-   sprintf(target_fname, %s%s%s, smsg-path_name, /,
-   smsg-file_name);
+   snprintf(target_fname, sizeof(target_fname), %s/%s,
+   (char *)smsg-path_name, (char*)smsg-file_name);
 
syslog(LOG_INFO, Target file name: %s\n, target_fname);
/*
@@ -54,12 +54,12 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
if (mkdir((char *)smsg-path_name, 0755)) {
syslog(LOG_ERR,
Failed to create '%s'; error: %d %s\n,
-   smsg-path_name,
+   (char *)smsg-path_name,
errno, strerror(errno));
goto done;
}
} else {
-   syslog(LOG_ERR, Invalid path: %s\n, smsg-path_name);
+   syslog(LOG_ERR, Invalid path: %s, (char 
*)smsg-path_name);
goto done;
}
}
@@ -115,7 +115,8 @@ int main(void)
char *buffer[4096 * 2];
struct hv_fcopy_hdr *in_msg;
 
-   daemon(1, 0);
+   if (daemon(1, 0))
+   return 1;
openlog(HV_FCOPY, 0, LOG_USER);
syslog(LOG_INFO, HV_FCOPY starting; pid is:%d, getpid());
 
@@ -130,7 +131,10 @@ int main(void)
/*
 * Register with the kernel.
 */
-   write(fcopy_fd, version, sizeof(int));
+   if (write(fcopy_fd, version, sizeof(int)) != sizeof(int)) {
+   syslog(LOG_ERR, write failed: %s,strerror(errno));
+   exit(EXIT_FAILURE);
+   }
 
while (1) {
/*
@@ -169,6 +173,9 @@ int main(void)
 
}
 
-   pwrite(fcopy_fd, error, sizeof(int), 0);
+   if (pwrite(fcopy_fd, error, sizeof(int), 0) != sizeof(int)) {
+   syslog(LOG_ERR, pwrite failed: %s,strerror(errno));
+   exit(EXIT_FAILURE);
+   }
}
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Olaf Hering 
On Tue, Jan 14, K. Y. Srinivasan wrote:

This function should return valid numbers:

 +static ssize_t fcopy_write(struct file *file, const char __user *buf,
 + size_t count, loff_t *ppos)
 +{
 + int error = 0;
 +
 + if (count != sizeof(int))
 + return 0;

Should be an error.

 +
 + if (copy_from_user(error, buf, sizeof(int)))
 + return -EFAULT;
 +
 + if (in_hand_shake) {
 + fcopy_handle_handshake();
 + return 0;

Should be sizeof(int).

 + }
 +
 + /*
 +  * Complete the transaction by forwarding the result
 +  * to the host. But first, cancel the timeout.
 +  */
 + if (cancel_delayed_work_sync(fcopy_work))
 + fcopy_respond_to_host(error);
 +
 + return sizeof(int);
 +}

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-16 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Thursday, January 16, 2014 1:42 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
  +enum hv_fcopy_op {
  +   START_FILE_COPY = 0,
  +   WRITE_TO_FILE,
  +   COMPLETE_FCOPY,
  +   CANCEL_FCOPY,
  +};
  +
  +struct hv_fcopy_hdr {
  +   enum hv_fcopy_op operation;
  +   uuid_le service_id0; /* currently unused */
  +   uuid_le service_id1; /* currently unused */
  +} __attribute__((packed));
 
 Is enum a fixed size? This struct is used in other structs, so I wonder
 what will happen to the kernel/user protocol if any of that changes. Or
 with a 64bit kernel and 32bit daemon. Maybe operation should be __u32?

I will check and make the necessary changes.

Thank you,

K. Y
 
 
 Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-16 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Thursday, January 16, 2014 2:49 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
  Implement the file copy service for Linux guests on Hyper-V. This permits 
  the
  host to copy a file (over VMBUS) into the guest. This facility is part of
  guest integration services supported on the Windows platform.
  Here is a link that provides additional details on this functionality:
 
 The change below fixes some warnings in the daemon code.
 Compile tested only.
 I also think the newlines in some of the syslog calls should be removed.
 
 Olaf
 
 
 hv_fcopy_daemon.c: In function 'hv_start_fcopy':
 hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
smsg-file_name);
^
 hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
 *', but argument 5 has type '__u16 *' [-Wformat=]
 hv_fcopy_daemon.c:57:6: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
   errno, strerror(errno));
   ^
 hv_fcopy_daemon.c:61:4: warning: format '%s' expects argument of type 'char
 *', but argument 3 has type '__u16 *' [-Wformat=]
 syslog(LOG_ERR, Invalid path: %s\n, smsg-path_name);
 ^
 hv_fcopy_daemon.c: In function 'main':
 hv_fcopy_daemon.c:117:8: warning: ignoring return value of 'daemon', declared
 with attribute warn_unused_result [-Wunused-result]
   daemon(1, 0);
 ^
 hv_fcopy_daemon.c:132:7: warning: ignoring return value of 'write', declared
 with attribute warn_unused_result [-Wunused-result]
   write(fcopy_fd, version, sizeof(int));
^
 hv_fcopy_daemon.c:171:9: warning: ignoring return value of 'pwrite', declared
 with attribute warn_unused_result [-Wunused-result]
pwrite(fcopy_fd, error, sizeof(int), 0);
  ^
 
 Signed-off-by: Olaf Hering o...@aepfle.de
 
 diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
 index c0e5c90..d1fadb7 100644
 --- a/tools/hv/hv_fcopy_daemon.c
 +++ b/tools/hv/hv_fcopy_daemon.c
 @@ -35,14 +35,14 @@
  #include dirent.h
 
  static int target_fd;
 -char target_fname[W_MAX_PATH];
 +static char target_fname[W_MAX_PATH];
 
  static int hv_start_fcopy(struct hv_start_fcopy *smsg)
  {
   int error = HV_E_FAIL;
 
 - sprintf(target_fname, %s%s%s, smsg-path_name, /,
 - smsg-file_name);
 + snprintf(target_fname, sizeof(target_fname), %s/%s,
 + (char *)smsg-path_name, (char*)smsg-file_name);
 
   syslog(LOG_INFO, Target file name: %s\n, target_fname);
   /*
 @@ -54,12 +54,12 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
   if (mkdir((char *)smsg-path_name, 0755)) {
   syslog(LOG_ERR,
   Failed to create '%s'; error: %d %s\n,
 - smsg-path_name,
 + (char *)smsg-path_name,
   errno, strerror(errno));
   goto done;
   }
   } else {
 - syslog(LOG_ERR, Invalid path: %s\n, smsg-
 path_name);
 + syslog(LOG_ERR, Invalid path: %s, (char *)smsg-
 path_name);
   goto done;
   }
   }
 @@ -115,7 +115,8 @@ int main(void)
   char *buffer[4096 * 2];
   struct hv_fcopy_hdr *in_msg;
 
 - daemon(1, 0);
 + if (daemon(1, 0))
 + return 1;
   openlog(HV_FCOPY, 0, LOG_USER);
   syslog(LOG_INFO, HV_FCOPY starting; pid is:%d, getpid());
 
 @@ -130,7 +131,10 @@ int main(void)
   /*
* Register with the kernel.
*/
 - write(fcopy_fd, version, sizeof(int));
 + if (write(fcopy_fd, version, sizeof(int)) != sizeof(int)) {
 + syslog(LOG_ERR, write failed: %s,strerror(errno));
 + exit(EXIT_FAILURE);
 + }
 
   while (1) {
   /*
 @@ -169,6 +173,9 @@ int main(void)
 
   }
 
 - pwrite(fcopy_fd, error, sizeof(int), 0);
 + if (pwrite(fcopy_fd, error, sizeof(int), 0) != sizeof(int)) {
 + syslog(LOG_ERR, pwrite failed: %s,strerror(errno));
 + exit(EXIT_FAILURE);
 + }
   }
  }

Thanks Olaf; I will include these changes in the next version.

K. Y
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-16 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Thursday, January 16, 2014 3:27 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
 This function should return valid numbers:
 
  +static ssize_t fcopy_write(struct file *file, const char __user *buf,
  +   size_t count, loff_t *ppos)
  +{
  +   int error = 0;
  +
  +   if (count != sizeof(int))
  +   return 0;
 
 Should be an error.
 
  +
  +   if (copy_from_user(error, buf, sizeof(int)))
  +   return -EFAULT;
  +
  +   if (in_hand_shake) {
  +   fcopy_handle_handshake();
  +   return 0;
 
 Should be sizeof(int).
 
  +   }
  +
  +   /*
  +* Complete the transaction by forwarding the result
  +* to the host. But first, cancel the timeout.
  +*/
  +   if (cancel_delayed_work_sync(fcopy_work))
  +   fcopy_respond_to_host(error);
  +
  +   return sizeof(int);
  +}
Olaf,

I will make the changes in the next version of this patch.

Thank you,

K. Y
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Dan Carpenter 
On Thu, Jan 16, 2014 at 10:42:01AM +0100, Olaf Hering wrote:
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
  +enum hv_fcopy_op {
  +   START_FILE_COPY = 0,
  +   WRITE_TO_FILE,
  +   COMPLETE_FCOPY,
  +   CANCEL_FCOPY,
  +};
  +
  +struct hv_fcopy_hdr {
  +   enum hv_fcopy_op operation;
  +   uuid_le service_id0; /* currently unused */
  +   uuid_le service_id1; /* currently unused */
  +} __attribute__((packed));
 
 Is enum a fixed size?

No, it's not.  The compiler has a lot of latitude in choosing the size
for enums.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-15 Thread Olaf Hering 
On Tue, Jan 14, K. Y. Srinivasan wrote:

 +static int hv_start_fcopy(struct hv_start_fcopy *smsg)

 + if (access((char *)smsg-path_name, F_OK)) {
 + if (smsg-copy_flags  CREATE_PATH) {
 + if (mkdir((char *)smsg-path_name, 0755)) {

KY,

I guess this needs a loop over every path compoment to implement
'mkdir -p', if the -CreateFullPath option is used?

Hopefully -DestinationPath is an arbitrary string and does not require
some sort of C: prefix. ;-)


Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-15 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Wednesday, January 15, 2014 8:33 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
  +static int hv_start_fcopy(struct hv_start_fcopy *smsg)
 
  +   if (access((char *)smsg-path_name, F_OK)) {
  +   if (smsg-copy_flags  CREATE_PATH) {
  +   if (mkdir((char *)smsg-path_name, 0755)) {
 
 KY,
 
 I guess this needs a loop over every path compoment to implement
 'mkdir -p', if the -CreateFullPath option is used?

Yes; I will make the change. The good news is that for the destination path and 
file name,
the host does not interpret the path components.

Regards,

K. Y
 
 Hopefully -DestinationPath is an arbitrary string and does not require
 some sort of C: prefix. ;-)
 
 
 Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-14 Thread K. Y. Srinivasan 
Implement the file copy service for Linux guests on Hyper-V. This permits the
host to copy a file (over VMBUS) into the guest. This facility is part of
guest integration services supported on the Windows platform.
Here is a link that provides additional details on this functionality:

http://technet.microsoft.com/en-us/library/dn464282.aspx

In V1 version of the patch I have addressed comments from
Olaf Hering o...@aepfle.de and Dan Carpenter dan.carpen...@oracle.com

In this version of the patch I have made globals static.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
---
 drivers/hv/Makefile|2 +-
 drivers/hv/hv_fcopy.c  |  422 
 drivers/hv/hv_util.c   |   10 +
 include/linux/hyperv.h |   60 +++
 tools/hv/hv_fcopy_daemon.c |  174 ++
 5 files changed, 667 insertions(+), 1 deletions(-)
 create mode 100644 drivers/hv/hv_fcopy.c
 create mode 100644 tools/hv/hv_fcopy_daemon.c

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 0a74b56..5e4dfa4 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -5,4 +5,4 @@ obj-$(CONFIG_HYPERV_BALLOON)+= hv_balloon.o
 hv_vmbus-y := vmbus_drv.o \
 hv.o connection.o channel.o \
 channel_mgmt.o ring_buffer.o
-hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o
+hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
new file mode 100644
index 000..f108a6e
--- /dev/null
+++ b/drivers/hv/hv_fcopy.c
@@ -0,0 +1,422 @@
+/*
+ * An implementation of file copy service.
+ *
+ * Copyright (C) 2014, Microsoft, Inc.
+ *
+ * Author : K. Y. Srinivasan ksriniva...@novell.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
+#include linux/semaphore.h
+#include linux/fs.h
+#include linux/nls.h
+#include linux/workqueue.h
+#include linux/cdev.h
+#include linux/hyperv.h
+#include linux/sched.h
+#include linux/uaccess.h
+
+#define WIN8_SRV_MAJOR 1
+#define WIN8_SRV_MINOR 1
+#define WIN8_SRV_VERSION   (WIN8_SRV_MAJOR  16 | WIN8_SRV_MINOR)
+
+/*
+ * Global state maintained for transaction that is being processed.
+ * For a class of integration services, including the file copy service,
+ * the specified protocol is a request/response protocol which means that
+ * there can only be single outstanding transaction from the host at any
+ * given point in time. We use this to simplify memory management in this
+ * driver - we cache and process only one message at a time.
+ *
+ * While the request/response protocol is guaranteed by the host, we further
+ * ensure this by serializing packet processing in this driver - we do not
+ * read additional packets from the VMBUs until the current packet is fully
+ * handled.
+ *
+ * The transaction active state is set when we receive a request from the
+ * host and we cleanup this state when the transaction is completed - when we
+ * respond to the host with our response. When the transaction active state is
+ * set, we defer handling incoming packets.
+ */
+
+static struct {
+   bool active; /* transaction status - active or not */
+   int recv_len; /* number of bytes received. */
+   struct hv_fcopy_hdr  *fcopy_msg; /* current message */
+   struct hv_start_fcopy  message; /*  sent to daemon */
+   struct vmbus_channel *recv_channel; /* chn we got the request */
+   u64 recv_req_id; /* request ID. */
+   void *fcopy_context; /* for the channel callback */
+   struct semaphore read_sema;
+} fcopy_transaction;
+
+static dev_t fcopy_dev;
+static bool daemon_died;
+static bool opened; /* currently device opened */
+static struct task_struct *dtp; /* daemon task ptr */
+
+/*
+ * Before we can accept copy messages from the host, we need
+ * to handshake with the user level daemon. This state tracks
+ * if we are in the handshake phase.
+ */
+static bool in_hand_shake = true;
+static void fcopy_send_data(void);
+static void fcopy_respond_to_host(int error);
+static void fcopy_work_func(struct work_struct *dummy);
+static DECLARE_DELAYED_WORK(fcopy_work, fcopy_work_func);
+static u8 *recv_buffer;
+
+static void fcopy_work_func(struct work_struct *dummy)
+{
+   /*
+* If the timer fires, the user-mode component has not responded;
+* process the pending transaction.
+*/
+   fcopy_respond_to_host(HV_E_FAIL);
+}
+
+static void fcopy_handle_handshake(void)
+{
+   pr_info(FCP: user-mode registering

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

2014-01-14 Thread Olaf Hering 
On Tue, Jan 14, K. Y. Srinivasan wrote:


 +static ssize_t fcopy_write(struct file *file, const char __user *buf,
 + size_t count, loff_t *ppos)
 +{
 + int error = 0;
 +
 + if (count != sizeof(int))
 + return 0;
 +
 + if (copy_from_user(error, buf, sizeof(int)))
 + return -EFAULT;
 +
 + if (in_hand_shake) {
 + fcopy_handle_handshake();
 + return 0;
 + }


 + /*
 +  * Register with the kernel.
 +  */
 + write(fcopy_fd, version, sizeof(int));


Shouldnt there be some version check already even in this initial
implementation? What if there will be a newer daemon running on an older
kernel?

Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

2014-01-14 Thread KY Srinivasan 


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Tuesday, January 14, 2014 2:13 PM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
 
 On Tue, Jan 14, K. Y. Srinivasan wrote:
 
 
  +static ssize_t fcopy_write(struct file *file, const char __user *buf,
  +   size_t count, loff_t *ppos)
  +{
  +   int error = 0;
  +
  +   if (count != sizeof(int))
  +   return 0;
  +
  +   if (copy_from_user(error, buf, sizeof(int)))
  +   return -EFAULT;
  +
  +   if (in_hand_shake) {
  +   fcopy_handle_handshake();
  +   return 0;
  +   }
 
 
  +   /*
  +* Register with the kernel.
  +*/
  +   write(fcopy_fd, version, sizeof(int));
 
 
 Shouldnt there be some version check already even in this initial
 implementation? What if there will be a newer daemon running on an older
 kernel?

Currently, the daemon registers with the kernel passing its version number. I 
will make the
version check real.

Thanks,

K. Y
 
 Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel