Re: [PATCH RFC 3/4] libxl: implement qemu-agent-command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/03/2017 12:15 AM, Jim Fehlig wrote:
> On 03/02/2017 11:46 AM, Jim Fehlig wrote:
>> On 02/08/2017 09:44 AM, Joao Martins wrote:
>>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> 
> Opps, forgot to mention that it would be helpful if the commit message contained 
> info on what Xen versions are supported and example config for the agent's 
> channel device.
Yeap, will add it.

> BTW, does this work with PV domUs that have a qemu process?
Yeap, it works.

> 
> Regards,
> Jim
> 
>>> ---
>>>  src/libxl/libxl_domain.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>  src/libxl/libxl_domain.h |  16 ++++
>>>  src/libxl/libxl_driver.c |  51 ++++++++++
>>>  3 files changed, 305 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>>> index ed73cd2..6bdd0ec 100644
>>> --- a/src/libxl/libxl_domain.c
>>> +++ b/src/libxl/libxl_domain.c
>>> @@ -782,6 +782,12 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
>>>          }
>>>      }
>>>
>>> +    if (priv->agent) {
>>> +        qemuAgentClose(priv->agent);
>>> +        priv->agent = NULL;
>>> +        priv->agentError = false;
>>> +    }
>>> +
>>>      if ((vm->def->nnets)) {
>>>          size_t i;
>>>
>>> @@ -940,6 +946,228 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config
>>> *d_config)
>>>      return -1;
>>>  }
>>>
>>> +/*
>>> + * This is a callback registered with a qemuAgentPtr instance,
>>> + * and to be invoked when the agent console hits an end of file
>>> + * condition, or error, thus indicating VM shutdown should be
>>> + * performed
>>> + */
>>> +static void
>>> +libxlHandleAgentEOF(qemuAgentPtr agent,
>>> +                          virDomainObjPtr vm)
>>
>> Whitespace is off.
>>
>>> +{
>>> +    libxlDomainObjPrivatePtr priv;
>>> +
>>> +    VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name);
>>> +
>>> +    virObjectLock(vm);
>>> +
>>> +    priv = vm->privateData;
>>> +
>>> +    if (!priv->agent) {
>>> +        VIR_DEBUG("Agent freed already");
>>> +        goto unlock;
>>> +    }
>>> +
>>> +    qemuAgentClose(agent);
>>> +    priv->agent = NULL;
>>
>> Should priv->agentError be reset to false?
>>
>>> +
>>> + unlock:
>>> +    virObjectUnlock(vm);
>>> +    return;
>>> +}
>>> +
>>> +
>>> +/*
>>> + * This is invoked when there is some kind of error
>>> + * parsing data to/from the agent. The VM can continue
>>> + * to run, but no further agent commands will be
>>> + * allowed
>>> + */
>>> +static void
>>> +libxlHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED,
>>> +                            virDomainObjPtr vm)
>>
>> Whitespace is off here too, and in a few other places below that I'll stop
>> nagging about.
>>
>>> +{
>>> +    libxlDomainObjPrivatePtr priv;
>>> +
>>> +    VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name);
>>> +
>>> +    virObjectLock(vm);
>>> +
>>> +    priv = vm->privateData;
>>> +
>>> +    priv->agentError = true;
>>> +
>>> +    virObjectUnlock(vm);
>>> +}
>>> +
>>> +static void libxlHandleAgentDestroy(qemuAgentPtr agent,
>>> +                                          virDomainObjPtr vm)
>>> +{
>>> +    VIR_DEBUG("Received destroy agent=%p vm=%p", agent, vm);
>>> +
>>> +    virObjectUnref(vm);
>>> +}
>>> +
>>> +static qemuAgentCallbacks agentCallbacks = {
>>> +    .destroy = libxlHandleAgentDestroy,
>>> +    .eofNotify = libxlHandleAgentEOF,
>>> +    .errorNotify = libxlHandleAgentError,
>>> +};
>>> +
>>> +static virDomainChrDefPtr
>>> +libxlFindAgentConfig(virDomainDefPtr def)
>>> +{
>>> +    size_t i;
>>> +
>>> +    for (i = 0; i < def->nchannels; i++) {
>>> +        virDomainChrDefPtr channel = def->channels[i];
>>> +
>>> +        if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN)
>>> +            continue;
>>> +
>>> +        if (STREQ_NULLABLE(channel->target.name, "org.qemu.guest_agent.0"))
>>> +            return channel;
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +bool
>>> +libxlDomainAgentAvailable(virDomainObjPtr vm, bool reportError)
>>
>> Is the reportError parameter needed? The callers in this patch and 4/4 pass 'true'.
>>
>>> +{
>>> +    libxlDomainObjPrivatePtr priv = vm->privateData;
>>> +
>>> +    if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
>>> +        if (reportError) {
>>> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>> +                           _("domain is not running"));
>>> +        }
>>> +        return false;
>>> +    }
>>> +    if (priv->agentError) {
>>> +        if (reportError) {
>>> +            virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
>>> +                           _("QEMU guest agent is not "
>>> +                             "available due to an error"));
>>> +        }
>>> +        return false;
>>> +    }
>>> +    if (!priv->agent) {
>>> +        if (libxlFindAgentConfig(vm->def)) {
>>> +            if (reportError) {
>>> +                virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
>>> +                               _("QEMU guest agent is not connected"));
>>> +            }
>>> +            return false;
>>> +        } else {
>>> +            if (reportError) {
>>> +                virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>>> +                               _("QEMU guest agent is not configured"));
>>> +            }
>>> +            return false;
>>> +        }
>>> +    }
>>> +    return true;
>>> +}
>>> +
>>> +static int
>>> +libxlConnectAgent(virDomainObjPtr vm)
>>> +{
>>> +    virDomainChrDefPtr config = libxlFindAgentConfig(vm->def);
>>> +    libxlDomainObjPrivatePtr priv = vm->privateData;
>>> +    qemuAgentPtr agent = NULL;
>>> +    int ret = -1;
>>> +
>>> +    if (!config)
>>> +        return 0;
>>> +
>>> +    if (priv->agent)
>>> +        return 0;
>>> +
>>> +    /* Hold an extra reference because we can't allow 'vm' to be
>>> +     * deleted while the agent is active */
>>> +    virObjectRef(vm);
>>> +
>>> +    ignore_value(virTimeMillisNow(&priv->agentStart));
>>> +    virObjectUnlock(vm);
>>> +
>>> +    agent = qemuAgentOpen(vm, config->source, &agentCallbacks);
>>> +
>>> +    virObjectLock(vm);
>>> +    priv->agentStart = 0;
>>> +
>>> +    if (agent == NULL)
>>> +        virObjectUnref(vm);
>>> +
>>> +    if (!virDomainObjIsActive(vm)) {
>>> +        qemuAgentClose(agent);
>>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +                       _("guest crashed while connecting to the guest agent"));
>>> +        ret = -2;
>>
>> This can be dropped, right? ret is initialized to -1 and the caller only checks
>> for ret < 0.
>>
>>> +        goto cleanup;
>>> +    }
>>> +
>>> +    priv->agent = agent;
>>> +
>>> +    if (priv->agent == NULL) {
>>> +        VIR_INFO("Failed to connect agent for %s", vm->def->name);
>>> +        goto cleanup;
>>> +    }
>>> +
>>> +    ret = 0;
>>> +
>>> + cleanup:
>>> +    return ret;
>>
>> Hmm, since there is nothing to cleanup might as well return -1 at failure points
>> and 0 on success.
>>
>>> +}
>>> +
>>> +/*
>>> + * obj must be locked before calling
>>> + *
>>> + * To be called immediately before any QEMU agent API call.
>>> + * Must have already called libxlDomainObjBeginJob() and checked
>>> + * that the VM is still active.
>>> + *
>>> + * To be followed with libxlDomainObjExitAgent() once complete
>>> + */
>>> +void
>>> +libxlDomainObjEnterAgent(virDomainObjPtr obj)
>>> +{
>>> +    libxlDomainObjPrivatePtr priv = obj->privateData;
>>> +
>>> +    VIR_DEBUG("Entering agent (agent=%p vm=%p name=%s)",
>>> +              priv->agent, obj, obj->def->name);
>>> +    virObjectLock(priv->agent);
>>> +    virObjectRef(priv->agent);
>>> +    ignore_value(virTimeMillisNow(&priv->agentStart));
>>> +    virObjectUnlock(obj);
>>> +}
>>> +
>>> +
>>> +/* obj must NOT be locked before calling
>>> + *
>>> + * Should be paired with an earlier qemuDomainObjEnterAgent() call
>>> + */
>>> +void
>>> +libxlDomainObjExitAgent(virDomainObjPtr obj)
>>> +{
>>> +    libxlDomainObjPrivatePtr priv = obj->privateData;
>>> +    bool hasRefs;
>>> +
>>> +    hasRefs = virObjectUnref(priv->agent);
>>> +
>>> +    if (hasRefs)
>>> +        virObjectUnlock(priv->agent);
>>> +
>>> +    virObjectLock(obj);
>>> +    VIR_DEBUG("Exited agent (agent=%p vm=%p name=%s)",
>>> +              priv->agent, obj, obj->def->name);
>>> +
>>> +    priv->agentStart = 0;
>>> +    if (!hasRefs)
>>> +        priv->agent = NULL;
>>> +}
>>> +
>>>  static int
>>>  libxlNetworkPrepareDevices(virDomainDefPtr def)
>>>  {
>>> @@ -1312,8 +1540,17 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>>>      libxlDomainCreateIfaceNames(vm->def, &d_config);
>>>
>>>  #ifdef LIBXL_HAVE_DEVICE_CHANNEL
>>> -    if (vm->def->nchannels > 0)
>>> +    if (vm->def->nchannels > 0) {
>>>          libxlDomainCreateChannelPTY(vm->def, cfg->ctx);
>>> +
>>> +        /* Failure to connect to agent shouldn't be fatal */
>>> +        if (libxlConnectAgent(vm) < 0) {
>>> +            VIR_WARN("Cannot connect to QEMU guest agent for %s",
>>> +                     vm->def->name);
>>> +            virResetLastError();
>>> +            priv->agentError = true;
>>> +        }
>>> +    }
>>>  #endif
>>>
>>>      if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL)
>>> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
>>> index 3a3890b..59f9f8d 100644
>>> --- a/src/libxl/libxl_domain.h
>>> +++ b/src/libxl/libxl_domain.h
>>> @@ -29,6 +29,7 @@
>>>  # include "domain_conf.h"
>>>  # include "libxl_conf.h"
>>>  # include "virchrdev.h"
>>> +# include "virqemuagent.h"
>>>
>>>  # define JOB_MASK(job)                  (1 << (job - 1))
>>>  # define DEFAULT_JOB_MASK               \
>>> @@ -62,6 +63,11 @@ typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
>>>  struct _libxlDomainObjPrivate {
>>>      virObjectLockable parent;
>>>
>>> +    /* agent */
>>> +    qemuAgentPtr agent;
>>> +    bool agentError;
>>> +    unsigned long long agentStart;
>>> +
>>>      /* console */
>>>      virChrdevsPtr devs;
>>>      libxl_evgen_domain_death *deathW;
>>> @@ -91,6 +97,16 @@ void
>>>  libxlDomainObjEndJob(libxlDriverPrivatePtr driver,
>>>                       virDomainObjPtr obj);
>>>
>>> +bool
>>> +libxlDomainAgentAvailable(virDomainObjPtr vm,
>>> +                          bool reportError);
>>> +
>>> +void
>>> +libxlDomainObjEnterAgent(virDomainObjPtr obj);
>>> +
>>> +void
>>> +libxlDomainObjExitAgent(virDomainObjPtr obj);
>>> +
>>>  int
>>>  libxlDomainJobUpdateTime(struct libxlDomainJobObj *job)
>>>      ATTRIBUTE_RETURN_CHECK;
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index 3a69720..cf5e702 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -57,6 +57,7 @@
>>>  #include "virstring.h"
>>>  #include "virsysinfo.h"
>>>  #include "viraccessapicheck.h"
>>> +#include "viraccessapicheckqemu.h"
>>>  #include "viratomic.h"
>>>  #include "virhostdev.h"
>>>  #include "network/bridge_driver.h"
>>> @@ -6415,6 +6416,55 @@ libxlConnectBaselineCPU(virConnectPtr conn,
>>>      return cpu;
>>>  }
>>>
>>> +static char *
>>> +libxlDomainQemuAgentCommand(virDomainPtr domain,
>>> +                           const char *cmd,
>>> +                           int timeout,
>>> +                           unsigned int flags)
>>> +{
>>> +    libxlDriverPrivatePtr driver = domain->conn->privateData;
>>> +    virDomainObjPtr vm;
>>> +    int ret = -1;
>>> +    char *result = NULL;
>>> +    libxlDomainObjPrivatePtr priv;
>>> +
>>> +    virCheckFlags(0, NULL);
>>> +
>>> +    if (!(vm = libxlDomObjFromDomain(domain)))
>>> +        goto cleanup;
>>> +
>>> +    priv = vm->privateData;
>>> +
>>> +    if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (!virDomainObjIsActive(vm)) {
>>> +        virReportError(VIR_ERR_OPERATION_INVALID,
>>> +                       "%s", _("domain is not running"));
>>> +        goto endjob;
>>> +    }
>>> +
>>> +    if (!libxlDomainAgentAvailable(vm, true))
>>> +        goto endjob;
>>> +
>>> +    libxlDomainObjEnterAgent(vm);
>>> +    ret = qemuAgentArbitraryCommand(priv->agent, cmd, &result, timeout);
>>> +    libxlDomainObjExitAgent(vm);
>>> +    if (ret < 0)
>>> +        VIR_FREE(result);
>>> +
>>> + endjob:
>>> +    libxlDomainObjEndJob(driver, vm);
>>> +
>>> + cleanup:
>>> +    virDomainObjEndAPI(&vm);
>>> +    return result;
>>> +}
>>> +
>>> +
>>>  static virHypervisorDriver libxlHypervisorDriver = {
>>>      .name = LIBXL_DRIVER_NAME,
>>>      .connectOpen = libxlConnectOpen, /* 0.9.0 */
>>> @@ -6522,6 +6572,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>>>      .connectGetDomainCapabilities = libxlConnectGetDomainCapabilities, /*
>>> 2.0.0 */
>>>      .connectCompareCPU = libxlConnectCompareCPU, /* 2.3.0 */
>>>      .connectBaselineCPU = libxlConnectBaselineCPU, /* 2.3.0 */
>>> +    .domainQemuAgentCommand = libxlDomainQemuAgentCommand, /* 3.1.0 */
>>
>> Heh, let's hope for 3.2.0. Apologies for the delay.
>>
>> Regards,
>> Jim
>>
>> --
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux