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

[Amit Shah <amit.shah@redhat.com>]

On (Fri) Sep 11 2009 [11:17:23], Anthony Liguori wrote:
> 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?

There wasn't any consensus; the discussion just ended abruptly.

That said, I'll have to revisit the discussion to see what the fqdn idea
was about.

That, or some other alternative will have to 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  

If there are multiple virtiocon devices specified, the first one will
default to port #0. The second one will error out saying port 0 is
taken. Isn't different from the current behaviour though.

> 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.

OK; there's a place where I need the qdev pointer from the PCIDevice.
Any other way of obtaining that?

>> +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.

Wondering if it already entered master.. I'll sync up with kraxel if
not.

>> +
>> +    register_savevm("virtio-console", -1, 2, virtio_console_save, virtio_console_load, s);
>>   
>
> Should integrate with VMState.

I don't think virtio devices have been converted yet.

		Amit