[libvirt] [PATCH] qemu: capabilities: Don't partially reprope caps on process reconnect

[Boris Fiuczynski]

Peter,
besides the trivial typo in the subject line (s/reprope/reprobe) I agree 
that this is the correct fix for the capability problem when 
reconnecting to the qemu processes. Thanks.

I suggest that you mention in the patch description that libvirt 
starting with version 2.2.0 is terminating all running qemu domains if 
the qemu binaries do not implement/have not enabled the qmp command 
'query-hotpluggable-cpus'.
Including the user observable internal error message for reference would 
maybe help as well. (internal error: unable to execute QEMU command 
'query-hotpluggable-cpus': The feature 'query-hotpluggable-cpus' is not 
enabled)

In addition this patch looks to me like a good candidate for the 
maintenance branches.

Tested-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>

On 11/25/2016 05:14 PM, Peter Krempa wrote:
> Thanks to the complex capability caching code virQEMUCapsProbeQMP was
> never called when we were starting a new qemu VM. On the other hand,
> when we are reconnecting to the qemu process we reload the capability
> list from the status XML file. This means that the flag preventing the
> function being called was not set and thus we partially reprobed some of
> the capabilities.
>
> The recent addition of CPU hotplug clears the
> QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS if the machine does not support it.
> The partial re-probe on reconnect results into attempting to call the
> unsupported command and then killing the VM.
>
> Remove the partial reprobe and depend on the stored capabilities. If it
> will be necessary to reprobe the capabilities in the future, we should
> do a full reprobe rather than this partial one.
> ---
>  src/qemu/qemu_capabilities.c | 17 -----------------
>  src/qemu/qemu_capabilities.h |  3 ---
>  src/qemu/qemu_process.c      |  4 ----
>  3 files changed, 24 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8901e7b..37e5302 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2937,23 +2937,6 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps,
>      return 0;
>  }
>
> -int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
> -                        qemuMonitorPtr mon)
> -{
> -    VIR_DEBUG("qemuCaps=%p mon=%p", qemuCaps, mon);
> -
> -    if (qemuCaps->usedQMP)
> -        return 0;
> -
> -    if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
> -        return -1;
> -
> -    if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
> -        return -1;
> -
> -    return 0;
> -}
> -
>
>  static bool
>  virQEMUCapsCPUFilterFeatures(const char *name,
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 5255815..be71507 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -403,9 +403,6 @@ virQEMUCapsPtr virQEMUCapsNew(void);
>  int virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>                                qemuMonitorPtr mon);
>
> -int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
> -                        qemuMonitorPtr mon);
> -
>  void virQEMUCapsSet(virQEMUCapsPtr qemuCaps,
>                      virQEMUCapsFlags flag) ATTRIBUTE_NONNULL(1);
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ab0c2c8..90f1101 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1723,10 +1723,6 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
>      if (qemuMonitorSetCapabilities(priv->mon) < 0)
>          goto cleanup;
>
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON) &&
> -        virQEMUCapsProbeQMP(priv->qemuCaps, priv->mon) < 0)
> -        goto cleanup;
> -
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) &&
>          qemuMonitorSetMigrationCapability(priv->mon,
>                                            QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
>


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294