[libvirt] [PATCH 7/7] qemu_domain: cleanup default input device code

Erik Skultety eskultet at redhat.com
Thu Dec 10 10:18:16 UTC 2015


On 04/12/15 20:30, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/qemu/qemu_domain.c                             | 28 ++++++++++------------
>  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
>  2 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 736624e..9cd3f4f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1038,8 +1038,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>      bool addPCIRoot = false;
>      bool addPCIeRoot = false;
>      bool addDefaultMemballoon = true;
> -    bool addDefaultUSBKBD = false;
> -    bool addDefaultUSBMouse = false;
> +    bool addDefaultUSBInput = false;
>      bool addPanicDevice = false;
>      int ret = -1;
>  
> @@ -1096,8 +1095,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>      case VIR_ARCH_PPC64:
>      case VIR_ARCH_PPC64LE:
>          addPCIRoot = true;
> -        addDefaultUSBKBD = true;
> -        addDefaultUSBMouse = true;
> +        addDefaultUSBInput = true;
>          /* For pSeries guests, the firmware provides the same
>           * functionality as the pvpanic device, so automatically
>           * add the definition if not already present */
> @@ -1177,19 +1175,17 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>          def->memballoon = memballoon;
>      }
>  
> -    if (addDefaultUSBKBD &&
> -        def->ngraphics > 0 &&
> -        virDomainDefMaybeAddInput(def,
> -                                  VIR_DOMAIN_INPUT_TYPE_KBD,
> -                                  VIR_DOMAIN_INPUT_BUS_USB) < 0)
> -        goto cleanup;
> +    if (def->ngraphics > 0 && addDefaultUSBInput) {
> +        if (virDomainDefMaybeAddInput(def,
> +                                      VIR_DOMAIN_INPUT_TYPE_MOUSE,
> +                                      VIR_DOMAIN_INPUT_BUS_USB) < 0)
> +            goto cleanup;
>  
> -    if (addDefaultUSBMouse &&
> -        def->ngraphics > 0 &&
> -        virDomainDefMaybeAddInput(def,
> -                                  VIR_DOMAIN_INPUT_TYPE_MOUSE,
> -                                  VIR_DOMAIN_INPUT_BUS_USB) < 0)
> -        goto cleanup;
> +        if (virDomainDefMaybeAddInput(def,
> +                                      VIR_DOMAIN_INPUT_TYPE_KBD,
> +                                      VIR_DOMAIN_INPUT_BUS_USB) < 0)
> +            goto cleanup;
> +    }
>  
>      if (addPanicDevice) {
>          size_t j;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
> index 39f4a1f..6472af5 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
> @@ -30,8 +30,8 @@
>      <controller type='usb' index='0'/>
>      <controller type='scsi' index='0'/>
>      <controller type='pci' index='0' model='pci-root'/>
> -    <input type='keyboard' bus='usb'/>
>      <input type='mouse' bus='usb'/>
> +    <input type='keyboard' bus='usb'/>
>      <graphics type='sdl'/>
>      <video>
>        <model type='cirrus' vram='16384' heads='1'/>
> 

I'm not convinced we want this, I discussed it with Jan to find some
corner cases that would break, there might be a slight problem with
saving and restoring an old domain on a new libvirt if user provided
some newxml to alter the host definition during restore which would have
those input devices removed, then migratable definition would be created
in qemuDomainSaveImageUpdateDef which would add these implicit devices
in reverse order if graphic device was present. The following  ABI
stability check would then fail.
Someone else could have at this as well and correct me, possibly NACKing
the patch. You could wait a while, maybe there will be a different review.

Erik
Erik




More information about the libvir-list mailing list