[libvirt] [PATCH v2 17/17] qemu: assume various QEMU 0.10 features are always available

[John Ferlan]

On 11/09/2015 11:24 AM, Daniel P. Berrange wrote:
> The -sdl and -net ...name=XXX arguments were both introduced
> in QEMU 0.10, so the QEMU driver can assume they are always
> available.
> 

The -sdl wasn't really removed it seems - although it did me peeking
into the rabbit hole for a make check failure...

> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c                       |  4 ----
>  src/qemu/qemu_capabilities.h                       |  6 +----
>  src/qemu/qemu_command.c                            | 27 ++++++++--------------
>  src/qemu/qemu_hotplug.c                            |  9 +-------
>  tests/qemucapabilitiesdata/caps_1.2.2-1.caps       |  1 -
>  tests/qemucapabilitiesdata/caps_1.3.1-1.caps       |  1 -
>  tests/qemucapabilitiesdata/caps_1.4.2-1.caps       |  1 -
>  tests/qemucapabilitiesdata/caps_1.5.3-1.caps       |  1 -
>  tests/qemucapabilitiesdata/caps_1.6.0-1.caps       |  1 -
>  tests/qemucapabilitiesdata/caps_1.6.50-1.caps      |  1 -
>  tests/qemucapabilitiesdata/caps_2.1.1-1.caps       |  1 -
>  tests/qemucaps2xmldata/all_1.6.0-1.caps            |  1 -
>  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps |  1 -
>  tests/qemuhelptest.c                               |  8 -------
>  tests/qemuhotplugtest.c                            |  1 -
>  .../qemuxml2argv-graphics-sdl-fullscreen.args      |  1 +
>  .../qemuxml2argv-graphics-sdl.args                 |  1 +
>  .../qemuxml2argvdata/qemuxml2argv-net-client.args  |  4 ++--
>  .../qemuxml2argv-net-eth-ifname.args               |  4 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-net-eth.args   |  4 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args |  4 ++--
>  .../qemuxml2argvdata/qemuxml2argv-net-server.args  |  4 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-net-udp.args   |  5 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-net-user.args  |  4 ++--
>  .../qemuxml2argvdata/qemuxml2argv-net-virtio.args  |  4 ++--
>  tests/qemuxml2argvtest.c                           |  6 ++---
>  26 files changed, 33 insertions(+), 72 deletions(-)
> 

Having "-sdl" in the args for qemuxml2argv-graphics-sdl.args and
qemuxml2argv-graphics-sdl-fullscreen.arg caused 'qemuargv2xmltest' to fail.

Running with debug on showed:

48) QEMU ARGV-2-XML graphics-sdl
... Got unexpected warning from qemuParseCommandLineString:
2015-11-09 21:52:58.744+0000: 7908: info : libvirt version: 1.2.22
2015-11-09 21:52:58.744+0000: 7908: warning : qemuParseCommandLine:13603
: unknown QEMU argument 'std', adding to the qemu namespace
^[[31m^[[1mFAILED^[[0m
49) QEMU ARGV-2-XML graphics-sdl-fullscreen
... Got unexpected warning from qemuParseCommandLineString:
2015-11-09 21:52:58.744+0000: 7908: warning : qemuParseCommandLine:13603
: unknown QEMU argument 'cirrus', adding to the qemu namespace
^[[31m^[[1mFAILED^[[0m

So someone was stripping (or not stripping) an argument...

Since graphics-sdl.args has:

 -parallel none \
+-sdl \
 -vga std

and graphics-sdl-fullscreen has:

 -full-screen \
+-sdl \
 -vga cirrus

After a bit of debugging - qemuParseCommandLine has the following:

       } else if (STRPREFIX(arg, "-hd") ||
                   STRPREFIX(arg, "-sd") ||
                   STRPREFIX(arg, "-fd") ||
                   STREQ(arg, "-cdrom")) {
            WANT_VALUE();

If I add:

        } else if (STREQ(arg, "-sdl")) {
            /* Ignore */

Just before that, then things are happy again.


While looking for the cause I also noted the following:

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a29cd08..5086c40 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1164,17 +1164,14 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
>          if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0)
>              return -1;
>      }
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NET_NAME) ||
> -        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> -        for (i = 0; i < def->nnets; i++) {
> -            /* type='hostdev' interfaces are also on the hostdevs list,
> -             * and will have their alias assigned with other hostdevs.
> -             */
> -            if (virDomainNetGetActualType(def->nets[i])
> -                != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> -                qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0) {
> -                return -1;
> -            }
> +    for (i = 0; i < def->nnets; i++) {
> +        /* type='hostdev' interfaces are also on the hostdevs list,
> +         * and will have their alias assigned with other hostdevs.
> +         */
> +        if (virDomainNetGetActualType(def->nets[i])
> +            != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> +            qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0) {
> +            return -1;
>          }
>      }
>  
> @@ -8514,8 +8511,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
>  {
>      switch ((virDomainGraphicsType) graphics->type) {
>      case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) &&
> -            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL)) {
> +        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL)) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("sdl not supported by '%s'"), def->emulator);
>              return -1;

later in this code there's a:

       if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL))
            virCommandAddArg(cmd, "-sdl");


The second caps check is probably unnecessary now.

John
> @@ -10551,11 +10547,6 @@ qemuBuildCommandLine(virConnectPtr conn,
>          }
>      }
>  
> -    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) && sdl + vnc + spice > 1) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("only 1 graphics device is supported"));
> -        goto error;
> -    }
>      if (sdl > 1 || vnc > 1 || spice > 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("only 1 graphics device of each type "

[...]