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
