Re: [PATCH kvmtool 3/5] virtio/net: Warn if virtio_net is implicitly enabled

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Martin,

On Tue, Jan 18, 2022 at 12:12:01AM +0200, Martin Radev wrote:
> The virtio_net device is implicitly enabled if user doesn't
> explicitly specify that virtio_net is disabled. This is
> counter-intuitive to how the rest of the virtio commandline
> works.
> 
> For backwards-compatibility, the commandline parameters are
> not changed. Instead, this patch prints out a warning if
> virtio_net is implicitly enabled.
> 
> Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
> ---
>  virtio/net.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/virtio/net.c b/virtio/net.c
> index 9a25bfa..ab75d40 100644
> --- a/virtio/net.c
> +++ b/virtio/net.c
> @@ -1002,6 +1002,9 @@ int virtio_net__init(struct kvm *kvm)
>  
>  	if (kvm->cfg.num_net_devices == 0 && kvm->cfg.no_net == 0) {
>  		static struct virtio_net_params net_params;
> +		pr_warning(
> +			"No net devices configured, but no_net not specified. "
> +			"Enabling virtio_net with default network settings...\n");

I don't think this deserves a warning, as there's nothing inherently wrong
with having a default network device. In fact, this is quite helpful for
users who  want to get a VM up and running without complicated setup. I do
agree that at the moment there's nothing to let the user know that a virtio
network device is being created, not in the usage synopsis for the lkvm run
command, nor in the man page for kvmtool, but I don't think that printing a
warning is the solution here.

What I suggest is that instead you print the virtual network interface
parameters that can be useful for a user to know (like IP address and
netmask, or whatever you think might be useful) using pr_debug() after
virtio_net__init_one() is called successfully below.

Thanks,
Alex

>  
>  		net_params = (struct virtio_net_params) {
>  			.guest_ip	= kvm->cfg.guest_ip,
> -- 
> 2.25.1
>