[libvirt] [PATCH] Report original error when QMP probing fails with new QEMU

[Jiri Denemark]

On Thu, Dec 04, 2014 at 15:33:38 +0000, Daniel Berrange wrote:
> If probing capabilities via QMP fails, we now have a check
> that prevents us falling back to -help parsing. Unfortunately
> the error message
> 
>   "Failed to probe capabilities for /usr/bin/qemu-kvm:
>    unsupported configuration: QEMU 2.1.2 is too new for help parsing"
> 
> is proving rather unhelpful to the user. We need to be telling
> them why QMP failed (the root cause), rather than they can't
> use -help (the side effect).
> 
> To do this we should capture stderr during QMP probing, and
> if -help parsing then sees a new QEMU version, we know that
> QMP should have worked, and so we can show the messages from
> stderr. The message thus becomes
> 
>   "Failed to probe capabilities for /usr/bin/qemu-kvm:
>    internal error: QEMU / QMP failed: Could not access
>    KVM kernel module: No such file or directory
>    failed to initialize KVM: No such file or directory"
> ---
>  src/qemu/qemu_capabilities.c | 37 +++++++++++++++++++++++++++----------
>  src/qemu/qemu_capabilities.h |  3 ++-
>  src/qemu/qemu_process.c      |  4 ++--
>  tests/qemuhelptest.c         |  2 +-
>  4 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index d2c046d..f777484 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1331,7 +1331,8 @@ int virQEMUCapsParseHelpStr(const char *qemu,
>                              unsigned int *version,
>                              bool *is_kvm,
>                              unsigned int *kvm_version,
> -                            bool check_yajl)
> +                            bool check_yajl,
> +                            const char *qemuerr)

qmperr, qmperror, or qmpError (basically "qmp" instead of "qemu") would
be a better name for this I think as it would be obvious this is the
error output from QMP probing. But not a big deal.

...
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a14b6f7..68777e8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3554,12 +3554,12 @@ qemuProcessReconnect(void *opaque)
>      struct qemuProcessReconnectData *data = opaque;
>      virQEMUDriverPtr driver = data->driver;
>      virDomainObjPtr obj = data->obj;
> -    qemuDomainObjPrivatePtr priv;
> +    qemuDomainObjPrivatePtr priv = NULL;
>      virConnectPtr conn = data->conn;
>      struct qemuDomainJobObj oldjob;
>      int state;
>      int reason;
> -    virQEMUDriverConfigPtr cfg;
> +    virQEMUDriverConfigPtr cfg = NULL;
>      size_t i;
>      int ret;
>  

Remove this hunk, the build was already fixed by Peter.

> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index 0c68367..6d9525c 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -59,7 +59,7 @@ static int testHelpStrParsing(const void *data)
>          goto cleanup;
>  
>      if (virQEMUCapsParseHelpStr("QEMU", help, flags,
> -                                &version, &is_kvm, &kvm_version, false) == -1) {
> +                                &version, &is_kvm, &kvm_version, false, NULL) == -1) {
>          if (info->error && virGetLastError()->code == info->error)
>              ret = 0;
>          goto cleanup;

ACK with the hunk removed.

Jirka