[Qemu-devel] Re: [PATCH 2/5] virtio-console: Add support for multiple ports for generic guest-host communication

[Anthony Liguori <anthony@codemonkey.ws>]

Amit Shah wrote:
> This interface extends the virtio-console device to handle
> multiple ports that present a char device from which bits can
> be sent and read.
>
> Sample uses for such a device can be obtaining info from the
> guest like the file systems used, apps installed, etc. for
> offline usage and logged-in users, clipboard copy-paste, etc.
> for online usage.
>
> Each port is to be assigned a unique function, for example, the
> first 4 ports may be reserved for libvirt usage, the next 4 for
> generic streaming data and so on. This port-function mapping
> isn't finalised yet.
>
>   

I thought the consensus was that we wanted strings to identify ports 
such that a scheme like reverse fqdn could be used?

> diff --git a/hw/pc.c b/hw/pc.c
> index d96d756..d5d2542 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1422,11 +1422,17 @@ static void pc_init1(ram_addr_t ram_size,
>      }
>  
>      /* Add virtio console devices */
> -    if (pci_enabled) {
> -        for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
> -            if (virtcon_hds[i]) {
> -                pci_create_simple(pci_bus, -1, "virtio-console-pci");
> -            }
> +    if (pci_enabled && virtcon_nr_ports) {
> +        void *dev;
> +
> +        dev = pci_create_simple(pci_bus, -1, "virtio-console-pci");
> +        if (!dev) {
> +            fprintf(stderr, "qemu: could not create virtio console pci device\n");
> +            exit(1);
> +        }
> +
> +        for (i = 0; i < virtcon_nr_ports; i++) {
> +                virtio_console_new_port(dev, virtcon_idx[i]);
>          }
>      }
>  }
>   

If a user does:

qemu -M pc-0.11.0 -virtiocon vc -virtiocon vc

This patch will break that guest.  I think the best solution to this is 
to properly integrate with qdev and the new chardev infrastructure.  
virtio-console-pci-0.11 should have the old semantics, 
virtio-console-pci-0.12 should be a bus.

>  #include "hw.h"
> +#include "monitor.h"
> +#include "pci.h"
>   

We don't want to add PCI dependency to virtio console.  It isn't always 
used on platforms with PCI.

> +static VirtIOConsole *virtio_console;
> +static struct virtio_console_config virtcon_config;
> +static VirtIOConsolePort virtcon_ports[MAX_VIRTIO_CONSOLE_PORTS]
>   

Introducing statics like this indicates something isn't right.  Again, I 
think a proper qdev bus conversion would take care of that.

> +/* This function gets called from vl.c during initial options
> + * parsing as well as from the monitor to parse the options.
> + * So it's a good idea to not print out anything and just
> + * return values which can become meaningful.
> + */
> +int init_virtio_console_port(int port, const char *opts)
> +{
> +    char dev[256];
> +    const char *prot;
> +    const char *idx;
> +    uint32_t port_nr;
> +    int j, k;
> +
> +    memset(dev, 0, sizeof(dev));
> +    prot = strstr(opts, ",protocol=");
> +    idx  = strstr(opts, ",port=");
>   

Need to integrate with QemuOpts.

> +
> +    register_savevm("virtio-console", -1, 2, virtio_console_save, virtio_console_load, s);
>   

Should integrate with VMState.

Regards,

Anthony Liguori