[libvirt] [PATCH v3 3/6] Remove PS2 mouse device for non-X86 platforms

[Li Zhang]

On 2013年12月11日 00:17, Ján Tomko wrote:
> On 12/10/2013 07:02 AM, Li Zhang wrote:
>> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>
>> PS2 device only works for X86 platform, other platforms may need
>> USB mouse. Athough it doesn't influence the QEMU command line, but
>> It's not right to add one PS2 mouse for non-X86 platform.
>>
>> This patch is to remove PS2 device definition from other platforms.
>> Add one default USB mouse for PPC64. It can be also added for other
>> platforms if necessary.
>>
>> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>> ---
>>   src/conf/domain_conf.c                             | 59 ++++++++--------------
>>   src/qemu/qemu_domain.c                             | 20 +++++++-
>>   src/util/virarch.h                                 |  2 +
>>   .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
>>   4 files changed, 42 insertions(+), 41 deletions(-)
>> @@ -12260,30 +12263,6 @@ virDomainDefParseXML(xmlDocPtr xml,
>>       }
>>       VIR_FREE(nodes);
>>   
>> -    /* If graphics are enabled, there's an implicit PS2 mouse */
>> -    if (def->ngraphics > 0) {
>> -        virDomainInputDefPtr input;
>> -
>> -        if (VIR_ALLOC(input) < 0) {
>> -            goto error;
>> -        }
>> -        if (STREQ(def->os.type, "hvm")) {
>> -            input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
>> -            input->bus = VIR_DOMAIN_INPUT_BUS_PS2;
>> -        } else {
>> -            input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
>> -            input->bus = VIR_DOMAIN_INPUT_BUS_XEN;
>> -        }
>> -
>> -        if (VIR_REALLOC_N(def->inputs, def->ninputs + 1) < 0) {
>> -            virDomainInputDefFree(input);
>> -            goto error;
>> -        }
>> -        def->inputs[def->ninputs] = input;
>> -        def->ninputs++;
>> -    }
>> -
>> -
> Here you remove auto-adding of the mouse for all drivers, but only re-add it
> to the QEMU driver, leading to failures in sexpr2xmltest.
Sorry, I made some mistakes.
Let me correct it.

>
>
>>       /* analysis of the sound devices */
>>       if ((n = virXPathNodeSet("./devices/sound", ctxt, &nodes)) < 0) {
>>           goto error;
>> @@ -17201,15 +17180,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>   
>>       if (def->ngraphics > 0) {
>>           /* If graphics is enabled, add the implicit mouse */
>> -        virDomainInputDef autoInput = {
>> -            VIR_DOMAIN_INPUT_TYPE_MOUSE,
>> -            STREQ(def->os.type, "hvm") ?
>> -            VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
>> -            { .alias = NULL },
>> -        };
>> -
>> -        if (virDomainInputDefFormat(buf, &autoInput, flags) < 0)
>> -            goto error;
>> +        if (ARCH_IS_X86(def->os.arch)) {
>> +            virDomainInputDef autoInput = {
>> +                VIR_DOMAIN_INPUT_TYPE_MOUSE,
>> +                STREQ(def->os.type, "hvm") ?
>> +                VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
>> +                { .alias = NULL },
>> +            };
>> +            if (virDomainInputDefFormat(buf, &autoInput, flags) < 0)
>> +                goto error;
>> +        }
>> +
>>   
>>           for (n = 0; n < def->ngraphics; n++)
>>               if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0)
> Hmm, if non-USB input devices get skipped when generating the command line and
> when formatting the XML, I wonder why are we adding it there in the first place.

I also have some confusion about it.
For PS2 mouse, what I have seen is that it doesn't create QEMU command line.
But it is configured in XML file. This device is added by QEMU if XEN is 
not enabled (hw/pc_piix.c).
I am not sure whether how this device is added on other kind of 
virtualization.
This may be not necessary to add this device in libvirt, it is not 
configurable.

Thanks.
-Li

>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 346fec3..75e615a 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -691,6 +691,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>>       bool addPCIRoot = false;
>>       bool addPCIeRoot = false;
>>       bool addDefaultMemballoon = true;
>> +    bool addDefaultMouse = false;
>> +    int  mouse_bus = VIR_DOMAIN_INPUT_BUS_XEN;
>>   
>>       /* check for emulator and create a default one if needed */
>>       if (!def->emulator &&
>> @@ -721,6 +723,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>>               !STRPREFIX(def->os.machine, "rhel"))
>>               break;
>>           addPCIRoot = true;
>> +        addDefaultMouse = true;
>> +        if (STREQ(def->os.type, "hvm"))
>> +            mouse_bus = VIR_DOMAIN_INPUT_BUS_PS2;
> os.type has to be "hvm" in the qemu driver.

OK, I will remove this check.

>
>>           break;
>>   
>>       case VIR_ARCH_ARMV7L:
> Jan
>
>