[PATCH 0/3] Fixes for kvmtool virtio-net part

[PATCH 0/3] Fixes for kvmtool virtio-net part

[Fan Du]

Hallo

This changset provide some fix and small enhancement
for kvm tool when I play it with virtio part.

Fan Du (3):
  kvm-tool: Introduce downscript option
  kvm-tool: Pass address of pointer to ioctl for tap
  kvm-tool: Restrict queue number to 1 when vhost on

 tools/kvm/include/kvm/virtio-net.h |  1 +
 tools/kvm/virtio/net.c             | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 1/3] kvm-tool: Introduce downscript option

[Fan Du]

To detach tap device automatically from bridge when exiting,
just like the reverse of "script" does.

Signed-off-by: Fan Du <fan.du@intel.com>
---
 tools/kvm/include/kvm/virtio-net.h |  1 +
 tools/kvm/virtio/net.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/tools/kvm/include/kvm/virtio-net.h b/tools/kvm/include/kvm/virtio-net.h
index f435cc3..d136a09 100644
--- a/tools/kvm/include/kvm/virtio-net.h
+++ b/tools/kvm/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/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 25b9496..f3f7200 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -696,6 +696,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) {
@@ -871,6 +873,32 @@ virtio_dev_init(virtio_net__init);
 
 int virtio_net__exit(struct kvm *kvm)
 {
+	int pid;
+	int status;
+	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")) {
+			pid = fork();
+			if (pid == 0) {
+				execl(ndev->params->downscript,
+				      params->downscript, ndev->tap_name, NULL);
+				_exit(1);
+			} else {
+				waitpid(pid, &status, 0);
+				if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
+					pr_warning("Fail to cleanup tap by %s",
+						   params->script);
+				}
+			}
+		}
+	}
 	return 0;
 }
 virtio_dev_exit(virtio_net__exit);
-- 
1.8.3.1

Re: [PATCH 1/3] kvm-tool: Introduce downscript option

[Andre Przywara]

Hi,

On 20/07/15 10:28, Fan Du wrote:
> To detach tap device automatically from bridge when exiting,
> just like the reverse of "script" does.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
>  tools/kvm/include/kvm/virtio-net.h |  1 +
>  tools/kvm/virtio/net.c             | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/tools/kvm/include/kvm/virtio-net.h b/tools/kvm/include/kvm/virtio-net.h
> index f435cc3..d136a09 100644
> --- a/tools/kvm/include/kvm/virtio-net.h
> +++ b/tools/kvm/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/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
> index 25b9496..f3f7200 100644
> --- a/tools/kvm/virtio/net.c
> +++ b/tools/kvm/virtio/net.c
> @@ -696,6 +696,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) {
> @@ -871,6 +873,32 @@ virtio_dev_init(virtio_net__init);
>  
>  int virtio_net__exit(struct kvm *kvm)
>  {
> +	int pid;
> +	int status;
> +	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 ||

Do you mean && here? Otherwise it may try to execute "none".

> +		    strcmp(params->downscript, "none")) {
> +			pid = fork();
> +			if (pid == 0) {
> +				execl(ndev->params->downscript,
> +				      params->downscript, ndev->tap_name, NULL);
> +				_exit(1);
> +			} else {
> +				waitpid(pid, &status, 0);
> +				if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
> +					pr_warning("Fail to cleanup tap by %s",
> +						   params->script);
> +				}
> +			}
> +		}
> +	}

I think this should be moved into a separate function to avoid code
duplication with the init script execution. Also it improves readability
and avoids too long lines.

Cheers,
Andre.

>  	return 0;
>  }
>  virtio_dev_exit(virtio_net__exit);
> 

[PATCH 2/3] kvm-tool: Pass address of pointer to ioctl for tap

[Fan Du]

When enable virtio tap mode as root previliege by:
lkvm run --disk linux-0.2.img  --kernel bzImage --network virtio

lkvm spits:
[    1.981352] loop: module loaded
[    1.986039]  vda:
  Warning: Config tap device error. Are you root?
You have requested a TAP device, but creation of one has failed because: Invalid argument

The last param of ioctl should be a pointer address

Signed-off-by: Fan Du <fan.du@intel.com>
Fixes: e325f3e77e78 ("kvmtool: Add minimal support for macvtap")
---
 tools/kvm/virtio/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index f3f7200..5678ff2 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -286,7 +286,7 @@ static int virtio_net_request_tap(struct net_dev *ndev, struct ifreq *ifr,
 	if (tapname)
 		strncpy(ifr->ifr_name, tapname, sizeof(ifr->ifr_name));
 
-	ret = ioctl(ndev->tap_fd, TUNSETIFF, &ifr);
+	ret = ioctl(ndev->tap_fd, TUNSETIFF, ifr);
 
 	if (ret >= 0)
 		strncpy(ndev->tap_name, ifr->ifr_name, sizeof(ndev->tap_name));
-- 
1.8.3.1

[PATCH 3/3] kvm-tool: Restrict queue number to 1 when vhost on

[Fan Du]

vhost kernel driver does not support mutiple queue yet,
Tweak queue number will fail with "--net mode=tap,vhost=1,mq=2"
as below when lkvm trying to set ring kick fd for queue 2:

VHOST_SET_VRING_KICK failed: No buffer space available

Error on this scenario, and overide with the default one queue
configuration.

Signed-off-by: Fan Du <fan.du@intel.com>
---
 tools/kvm/virtio/net.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 5678ff2..32159f1 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -714,6 +714,10 @@ static int set_net_param(struct kvm *kvm, struct virtio_net_params *p,
 		p->mq = atoi(val);
 	} else
 		die("Unknown network parameter %s", param);
+	if (p->vhost && p->mq > 1) {
+		p->mq = 1;
+		pr_err("Virtio vhost does not support mq yet, overide mq to 1.");
+	}
 
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH 0/3] Fixes for kvmtool virtio-net part

[Andre Przywara]

Hi,
> 
> This changset provide some fix and small enhancement
> for kvm tool when I play it with virtio part.

have you seen the new kvmtool repository Will created, which is a
stand-alone version outside of the Linux tree?

https://git.kernel.org/cgit/linux/kernel/git/will/kvmtool.git/

It seems like Pekka's old repository doesn't see many patches these days
any more.
I quickly checked and could apply your patches to the new repository by
just removing the tools/kvm path prefix.
So do you care to rebase your patches on top of Will's repository and
send them again (minus patch 2, which we have already in our branch
[1]). Please also see my comment on patch 1/3.

Thanks,
Andre.

[1]
https://git.kernel.org/cgit/linux/kernel/git/will/kvmtool.git/commit/?id=f83dc816a9c76f87ad90723f366700077fb367ea

> 
> Fan Du (3):
>   kvm-tool: Introduce downscript option
>   kvm-tool: Pass address of pointer to ioctl for tap
>   kvm-tool: Restrict queue number to 1 when vhost on
> 
>  tools/kvm/include/kvm/virtio-net.h |  1 +
>  tools/kvm/virtio/net.c             | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 

RE: [PATCH 0/3] Fixes for kvmtool virtio-net part

[Du, Fan]

>-----Original Message-----
>From: Andre Przywara [mailto:andre.przywara@arm.com]
>Sent: Monday, July 20, 2015 10:43 PM
>To: Du, Fan
>Cc: kvm@vger.kernel.org; penberg@kernel.org; Will Deacon; Marc Zyngier
>Subject: Re: [PATCH 0/3] Fixes for kvmtool virtio-net part
>
>Hi,
>>
>> This changset provide some fix and small enhancement
>> for kvm tool when I play it with virtio part.
>
>have you seen the new kvmtool repository Will created, which is a
>stand-alone version outside of the Linux tree?
>
>https://git.kernel.org/cgit/linux/kernel/git/will/kvmtool.git/

Thanks for the notice. I'm new to the kvm mail list. 
Already sent v2 after rebasing and incorporated your comments.
Please review.

>It seems like Pekka's old repository doesn't see many patches these days
>any more.
>I quickly checked and could apply your patches to the new repository by
>just removing the tools/kvm path prefix.
>So do you care to rebase your patches on top of Will's repository and
>send them again (minus patch 2, which we have already in our branch
>[1]). Please also see my comment on patch 1/3.
>
>Thanks,
>Andre.
>
>[1]
>https://git.kernel.org/cgit/linux/kernel/git/will/kvmtool.git/commit/?id=f83dc81
>6a9c76f87ad90723f366700077fb367ea
>
>>
>> Fan Du (3):
>>   kvm-tool: Introduce downscript option
>>   kvm-tool: Pass address of pointer to ioctl for tap
>>   kvm-tool: Restrict queue number to 1 when vhost on
>>
>>  tools/kvm/include/kvm/virtio-net.h |  1 +
>>  tools/kvm/virtio/net.c             | 34
>+++++++++++++++++++++++++++++++++-
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>