| Message ID | 1437459483-24535-2-git-send-email-fan.du@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.du@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
To detach tap device automatically from bridge when exiting, just like what the reverse of "script" does. Signed-off-by: Fan Du <fan.du@intel.com> --- include/kvm/virtio-net.h | 1 + virtio/net.c | 49 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 38 insertions(+), 12 deletions(-)