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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
