Re: [PATCHv3 1/2] kvmtool: Introduce downscript option for virtio-net

2015-07-24 Thread Will Deacon 
On Tue, Jul 21, 2015 at 10:44:31AM +0100, Andre Przywara wrote:
 thanks for the rework, that looks good to me, some minor nits below.
 Not sure if that requires a new version, maybe Will can fix that up when
 he applies it.

Nah, please send a new version with these points addressed.

Will
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 1/2] kvmtool: Introduce downscript option for virtio-net

2015-07-21 Thread Fan Du 
To detach tap device automatically from bridge when exiting,
just like what the reverse of script does.

Signed-off-by: Fan Du fan...@intel.com
---
 include/kvm/virtio-net.h |  1 +
 virtio/net.c | 49 
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/include/kvm/virtio-net.h b/include/kvm/virtio-net.h
index f435cc3..d136a09 100644
--- a/include/kvm/virtio-net.h
+++ b/include/kvm/virtio-net.h
@@ -9,6 +9,7 @@ struct virtio_net_params {
const char *guest_ip;
const char *host_ip;
const char *script;
+   const char *downscript;
const char *trans;
const char *tapif;
char guest_mac[6];
diff --git a/virtio/net.c b/virtio/net.c
index 4a6a855..d343615 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -294,10 +294,29 @@ static int virtio_net_request_tap(struct net_dev *ndev, 
struct ifreq *ifr,
return ret;
 }
 
+static int virtio_net_exec_script(const char* script, char *tap_name)
+{
+   int pid;
+   int status;
+
+   pid = fork();
+   if (pid == 0) {
+   execl(script, script, tap_name, NULL);
+   _exit(1);
+   } else {
+   waitpid(pid, status, 0);
+   if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
+   pr_warning(Fail to setup tap by %s, script);
+   return -1;
+   }
+   }
+   return 0;
+}
+
 static bool virtio_net__tap_init(struct net_dev *ndev)
 {
int sock = socket(AF_INET, SOCK_STREAM, 0);
-   int pid, status, offload, hdr_len;
+   int offload, hdr_len;
struct sockaddr_in sin = {0};
struct ifreq ifr;
const struct virtio_net_params *params = ndev-params;
@@ -339,17 +358,8 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
}
 
if (strcmp(params-script, none)) {
-   pid = fork();
-   if (pid == 0) {
-   execl(params-script, params-script, ndev-tap_name, 
NULL);
-   _exit(1);
-   } else {
-   waitpid(pid, status, 0);
-   if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
-   pr_warning(Fail to setup tap by %s, 
params-script);
-   goto fail;
-   }
-   }
+   if(virtio_net_exec_script(params-script, ndev-tap_name)  0)
+   goto fail;
} else if (!skipconf) {
memset(ifr, 0, sizeof(ifr));
strncpy(ifr.ifr_name, ndev-tap_name, sizeof(ndev-tap_name));
@@ -702,6 +712,8 @@ static int set_net_param(struct kvm *kvm, struct 
virtio_net_params *p,
die(Unknown network mode %s, please use user, tap or 
none, kvm-cfg.network);
} else if (strcmp(param, script) == 0) {
p-script = strdup(val);
+   } else if (strcmp(param, downscript) == 0) {
+   p-downscript = strdup(val);
} else if (strcmp(param, guest_ip) == 0) {
p-guest_ip = strdup(val);
} else if (strcmp(param, host_ip) == 0) {
@@ -877,6 +889,19 @@ virtio_dev_init(virtio_net__init);
 
 int virtio_net__exit(struct kvm *kvm)
 {
+   struct virtio_net_params *params;
+   struct net_dev *ndev;
+   struct list_head *ptr;
+
+   list_for_each(ptr, ndevs) {
+   ndev = list_entry(ptr, struct net_dev, list);
+   params = ndev-params;
+   /* Cleanup any tap device which attached to bridge */
+   if (ndev-mode == NET_MODE_TAP 
+   strcmp(params-downscript, none)) {
+   virtio_net_exec_script(params-downscript, 
ndev-tap_name);
+   }
+   }
return 0;
 }
 virtio_dev_exit(virtio_net__exit);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 1/2] kvmtool: Introduce downscript option for virtio-net

2015-07-21 Thread Andre Przywara 
Hi,

thanks for the rework, that looks good to me, some minor nits below.
Not sure if that requires a new version, maybe Will can fix that up when
he applies it.

On 21/07/15 07:18, Fan Du wrote:
 To detach tap device automatically from bridge when exiting,
 just like what the reverse of script does.
 
 Signed-off-by: Fan Du fan...@intel.com
 ---
  include/kvm/virtio-net.h |  1 +
  virtio/net.c | 49 
 
  2 files changed, 38 insertions(+), 12 deletions(-)
 
 diff --git a/include/kvm/virtio-net.h b/include/kvm/virtio-net.h
 index f435cc3..d136a09 100644
 --- a/include/kvm/virtio-net.h
 +++ b/include/kvm/virtio-net.h
 @@ -9,6 +9,7 @@ struct virtio_net_params {
   const char *guest_ip;
   const char *host_ip;
   const char *script;
 + const char *downscript;
   const char *trans;
   const char *tapif;
   char guest_mac[6];
 diff --git a/virtio/net.c b/virtio/net.c
 index 4a6a855..d343615 100644
 --- a/virtio/net.c
 +++ b/virtio/net.c
 @@ -294,10 +294,29 @@ static int virtio_net_request_tap(struct net_dev *ndev, 
 struct ifreq *ifr,
   return ret;
  }
  
 +static int virtio_net_exec_script(const char* script, char *tap_name)

can you make tap_name a const char * as well?

 +{
 + int pid;

fork() returns a value of type pid_t, so pid should be of that type
too. I know it was int before, but let's fix it when we rewrite it anyway.

 + int status;
 +
 + pid = fork();
 + if (pid == 0) {
 + execl(script, script, tap_name, NULL);
 + _exit(1);
 + } else {
 + waitpid(pid, status, 0);
 + if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
 + pr_warning(Fail to setup tap by %s, script);
 + return -1;
 + }
 + }
 + return 0;
 +}
 +
  static bool virtio_net__tap_init(struct net_dev *ndev)
  {
   int sock = socket(AF_INET, SOCK_STREAM, 0);
 - int pid, status, offload, hdr_len;
 + int offload, hdr_len;
   struct sockaddr_in sin = {0};
   struct ifreq ifr;
   const struct virtio_net_params *params = ndev-params;
 @@ -339,17 +358,8 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
   }
  
   if (strcmp(params-script, none)) {
 - pid = fork();
 - if (pid == 0) {
 - execl(params-script, params-script, ndev-tap_name, 
 NULL);
 - _exit(1);
 - } else {
 - waitpid(pid, status, 0);
 - if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
 - pr_warning(Fail to setup tap by %s, 
 params-script);
 - goto fail;
 - }
 - }
 + if(virtio_net_exec_script(params-script, ndev-tap_name)  0)
 + goto fail;
   } else if (!skipconf) {
   memset(ifr, 0, sizeof(ifr));
   strncpy(ifr.ifr_name, ndev-tap_name, sizeof(ndev-tap_name));
 @@ -702,6 +712,8 @@ static int set_net_param(struct kvm *kvm, struct 
 virtio_net_params *p,
   die(Unknown network mode %s, please use user, tap or 
 none, kvm-cfg.network);
   } else if (strcmp(param, script) == 0) {
   p-script = strdup(val);
 + } else if (strcmp(param, downscript) == 0) {
 + p-downscript = strdup(val);
   } else if (strcmp(param, guest_ip) == 0) {
   p-guest_ip = strdup(val);
   } else if (strcmp(param, host_ip) == 0) {
 @@ -877,6 +889,19 @@ virtio_dev_init(virtio_net__init);
  
  int virtio_net__exit(struct kvm *kvm)
  {
 + struct virtio_net_params *params;
 + struct net_dev *ndev;
 + struct list_head *ptr;
 +
 + list_for_each(ptr, ndevs) {
 + ndev = list_entry(ptr, struct net_dev, list);
 + params = ndev-params;
 + /* Cleanup any tap device which attached to bridge */
 + if (ndev-mode == NET_MODE_TAP 
 + strcmp(params-downscript, none)) {
 + virtio_net_exec_script(params-downscript, 
 ndev-tap_name);
 + }

You don't need the braces here since the condition is only a one-liner.

Cheers,
Andre.

 + }
   return 0;
  }
  virtio_dev_exit(virtio_net__exit);
 
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html