[libvirt] [PATCH RFC 3/4] libxl: implement qemu-agent-command
Jim Fehlig
jfehlig at suse.com
Fri Mar 3 00:15:16 UTC 2017
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 at oracle.com>
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.
BTW, does this work with PV domUs that have a qemu process?
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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
More information about the libvir-list
mailing list