9:52 a.m.
Clean up some parts of the agent code, fix a recently added bug and add support
for guest agent hotplug, all this while removing more lines than adding.
Peter Krempa (5):
qemu: agent: Reuse virJSONValueObjectCreateVArgs in
qemuAgentMakeCommand
qemu: Fix domain object leak in qemuDomainInterfaceAddresses
qemu: Reuse qemuDomainAgentAvailable in qemuDomainInterfaceAddresses
qemu: agent: Differentiate errors when the agent channel was
hotplugged
qemu: Connect to guest agent after channel hotplug
src/qemu/qemu_agent.c | 73 ++-----------------------------------------------
src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++---
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_driver.c | 56 +++++++++++++++++++++----------------
src/qemu/qemu_process.c | 22 +--------------
src/qemu/qemu_process.h | 2 ++
6 files changed, 81 insertions(+), 119 deletions(-)
--
2.3.5
9:52 a.m.
New subject: [libvirt] [PATCH 1/5] qemu: agent: Reuse virJSONValueObjectCreateVArgs in qemuAgentMakeCommand
Since the code is now separated into the common helper, we can reuse it
instead of maintaining two copies.
---
src/qemu/qemu_agent.c | 73 ++-------------------------------------------------
1 file changed, 2 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 548d580..3f6a9bf 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1136,7 +1136,6 @@ qemuAgentMakeCommand(const char *cmdname,
virJSONValuePtr obj;
virJSONValuePtr jargs = NULL;
va_list args;
- char *key;
va_start(args, cmdname);
@@ -1146,76 +1145,8 @@ qemuAgentMakeCommand(const char *cmdname,
if (virJSONValueObjectAppendString(obj, "execute", cmdname) < 0)
goto error;
- while ((key = va_arg(args, char *)) != NULL) {
- int ret;
- char type;
-
- if (strlen(key) < 3) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("argument key '%s' is too short, missing type
prefix"),
- key);
- goto error;
- }
-
- /* Keys look like s:name the first letter is a type code */
- type = key[0];
- key += 2;
-
- if (!jargs &&
- !(jargs = virJSONValueNewObject()))
- goto error;
-
- /* This doesn't support maps/arrays. This hasn't
- * proved to be a problem..... yet :-) */
- switch (type) {
- case 's': {
- char *val = va_arg(args, char *);
- ret = virJSONValueObjectAppendString(jargs, key, val);
- } break;
- case 'i': {
- int val = va_arg(args, int);
- ret = virJSONValueObjectAppendNumberInt(jargs, key, val);
- } break;
- case 'u': {
- unsigned int val = va_arg(args, unsigned int);
- ret = virJSONValueObjectAppendNumberUint(jargs, key, val);
- } break;
- case 'I': {
- long long val = va_arg(args, long long);
- ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
- } break;
- case 'U': {
- /* qemu silently truncates numbers larger than LLONG_MAX,
- * so passing the full range of unsigned 64 bit integers
- * is not safe here. Pass them as signed 64 bit integers
- * instead.
- */
- long long val = va_arg(args, long long);
- ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
- } break;
- case 'd': {
- double val = va_arg(args, double);
- ret = virJSONValueObjectAppendNumberDouble(jargs, key, val);
- } break;
- case 'b': {
- int val = va_arg(args, int);
- ret = virJSONValueObjectAppendBoolean(jargs, key, val);
- } break;
- case 'n': {
- ret = virJSONValueObjectAppendNull(jargs, key);
- } break;
- case 'a': {
- virJSONValuePtr val = va_arg(args, virJSONValuePtr);
- ret = virJSONValueObjectAppend(jargs, key, val);
- } break;
- default:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unsupported data type '%c' for arg
'%s'"), type, key - 2);
- goto error;
- }
- if (ret < 0)
- goto error;
- }
+ if (virJSONValueObjectCreateVArgs(&jargs, args) < 0)
+ goto error;
if (jargs &&
virJSONValueObjectAppend(obj, "arguments", jargs) < 0)
--
2.3.5
9:52 a.m.
New subject: [libvirt] [PATCH 2/5] qemu: Fix domain object leak in qemuDomainInterfaceAddresses
The API didn't use virDomainObjEndAPI to release the domain object thus
it leaked a reference to it.
---
src/qemu/qemu_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 989c20c..7d2c5ed 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19789,8 +19789,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
}
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
--
2.3.5
9:52 a.m.
New subject: [libvirt] [PATCH 3/5] qemu: Reuse qemuDomainAgentAvailable in qemuDomainInterfaceAddresses
---
src/qemu/qemu_driver.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7d2c5ed..ce38e36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19750,27 +19750,11 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
break;
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
- if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("QEMU guest agent is not "
- "available due to an error"));
- goto cleanup;
- }
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is not running"));
- goto endjob;
- }
-
- if (!priv->agent) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("QEMU guest agent is not configured"));
+ if (qemuDomainAgentAvailable(vm, true) < 0)
goto endjob;
- }
qemuDomainObjEnterAgent(vm);
ret = qemuAgentGetInterfaces(priv->agent, ifaces);
--
2.3.5
9:52 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: agent: Differentiate errors when the agent channel was hotplugged
When the guest agent channel gets hotplugged to a VM, libvirt would
still report that "QEMU guest agent is not configured" rather than
stating that the connection was not established yet.
Currently the code won't be able to connect to the agent after hotplug
but that will change in a later patch.
As the qemuFindAgentConfig() helper is quite helpful in this case move
it to a more usable place and export it.
---
src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_process.c | 20 --------------------
3 files changed, 43 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6c480c6..f57768c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2976,11 +2976,19 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
return false;
}
if (!priv->agent) {
- if (reportError) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("QEMU guest agent is not configured"));
+ if (qemuFindAgentConfig(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 false;
}
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
if (reportError) {
@@ -3078,3 +3086,32 @@ qemuDomainSupportsBlockJobs(virDomainObjPtr vm,
return 0;
}
+
+
+/**
+ * qemuFindAgentConfig:
+ * @def: domain definition
+ *
+ * Returns the pointer to the channel definition that is used to access the
+ * guest agent if the agent is configured or NULL otherwise.
+ */
+virDomainChrSourceDefPtr
+qemuFindAgentConfig(virDomainDefPtr def)
+{
+ virDomainChrSourceDefPtr config = NULL;
+ size_t i;
+
+ for (i = 0; i < def->nchannels; i++) {
+ virDomainChrDefPtr channel = def->channels[i];
+
+ if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)
+ continue;
+
+ if (STREQ_NULLABLE(channel->target.name, "org.qemu.guest_agent.0"))
{
+ config = &channel->source;
+ break;
+ }
+ }
+
+ return config;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d550ae3..76a4819 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -439,4 +439,6 @@ bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk);
int qemuDomainAlignMemorySizes(virDomainDefPtr def);
void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem);
+virDomainChrSourceDefPtr qemuFindAgentConfig(virDomainDefPtr def);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6707170..7ce8fa5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -201,26 +201,6 @@ static qemuAgentCallbacks agentCallbacks = {
.errorNotify = qemuProcessHandleAgentError,
};
-static virDomainChrSourceDefPtr
-qemuFindAgentConfig(virDomainDefPtr def)
-{
- virDomainChrSourceDefPtr config = NULL;
- size_t i;
-
- for (i = 0; i < def->nchannels; i++) {
- virDomainChrDefPtr channel = def->channels[i];
-
- if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)
- continue;
-
- if (STREQ_NULLABLE(channel->target.name, "org.qemu.guest_agent.0"))
{
- config = &channel->source;
- break;
- }
- }
-
- return config;
-}
static int
qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm)
--
2.3.5
9:52 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: Connect to guest agent after channel hotplug
If a user hot-attaches the guest agent channel libvirt would ignore it
until the restart of libvirtd or shutdown/destroy and start of the VM
itself.
This patch adds code that opens or closes the guest agent connection
according to the state of the guest agent channel according to
connect/disconnect events.
To allow opening the channel from the event handler qemuConnectAgent
needed to be exported.
---
src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++----
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 2 ++
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce38e36..28e27f2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4419,6 +4419,8 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
virDomainChrDeviceState newstate;
virObjectEventPtr event = NULL;
virDomainDeviceDef dev;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rc;
if (connected)
newstate = VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED;
@@ -4445,10 +4447,35 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
dev.data.chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)
goto endjob;
- if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")
&&
- (event = virDomainEventAgentLifecycleNewFromObj(vm, newstate,
-
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL)))
- qemuDomainEventQueue(driver, event);
+ if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0"))
{
+ switch (newstate) {
+ case VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED:
+ if (!priv->agent) {
+ if ((rc = qemuConnectAgent(driver, vm)) == -2)
+ goto endjob;
+
+ if (rc < 0)
+ priv->agentError = true;
+ }
+ break;
+
+ case VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED:
+ if (priv->agent) {
+ qemuAgentClose(priv->agent);
+ priv->agent = NULL;
+ priv->agentError = false;
+ }
+ break;
+
+ case VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT:
+ case VIR_DOMAIN_CHR_DEVICE_STATE_LAST:
+ break;
+ };
+
+ if ((event = virDomainEventAgentLifecycleNewFromObj(vm, newstate,
+
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL)))
+ qemuDomainEventQueue(driver, event);
+ }
dev.data.chr->state = newstate;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7ce8fa5..3105101 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -202,7 +202,7 @@ static qemuAgentCallbacks agentCallbacks = {
};
-static int
+int
qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c67b0cb..d40f68d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -113,4 +113,6 @@ int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm,
const char *alias);
+int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm);
+
#endif /* __QEMU_PROCESS_H__ */
--
2.3.5
10:09 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: Connect to guest agent after channel hotplug
On 24.04.2015 16:52, Peter Krempa wrote:
If a user hot-attaches the guest agent channel libvirt would ignore
it
until the restart of libvirtd or shutdown/destroy and start of the VM
itself.
This patch adds code that opens or closes the guest agent connection
according to the state of the guest agent channel according to
connect/disconnect events.
To allow opening the channel from the event handler qemuConnectAgent
needed to be exported.
---
src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++----
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 2 ++
3 files changed, 34 insertions(+), 5 deletions(-)
Finally. I had this in my mind for a while, but always thought it's
gonna be more complex so I've put it aside. But now that I see we have
everything prepared, how silly my thinking was. Good job.
Michal
10:09 a.m.
On 24.04.2015 16:52, Peter Krempa wrote:
Clean up some parts of the agent code, fix a recently added bug and
add support
for guest agent hotplug, all this while removing more lines than adding.
Peter Krempa (5):
qemu: agent: Reuse virJSONValueObjectCreateVArgs in
qemuAgentMakeCommand
qemu: Fix domain object leak in qemuDomainInterfaceAddresses
qemu: Reuse qemuDomainAgentAvailable in qemuDomainInterfaceAddresses
qemu: agent: Differentiate errors when the agent channel was
hotplugged
qemu: Connect to guest agent after channel hotplug
src/qemu/qemu_agent.c | 73 ++-----------------------------------------------
src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++---
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_driver.c | 56 +++++++++++++++++++++----------------
src/qemu/qemu_process.c | 22 +--------------
src/qemu/qemu_process.h | 2 ++
6 files changed, 81 insertions(+), 119 deletions(-)
ACK series.
Michal
10:23 a.m.
On Fri, Apr 24, 2015 at 17:09:13 +0200, Michal Privoznik wrote:
On 24.04.2015 16:52, Peter Krempa wrote:
> Clean up some parts of the agent code, fix a recently added bug and add support
> for guest agent hotplug, all this while removing more lines than adding.
>
> Peter Krempa (5):
> qemu: agent: Reuse virJSONValueObjectCreateVArgs in
> qemuAgentMakeCommand
> qemu: Fix domain object leak in qemuDomainInterfaceAddresses
> qemu: Reuse qemuDomainAgentAvailable in qemuDomainInterfaceAddresses
> qemu: agent: Differentiate errors when the agent channel was
> hotplugged
> qemu: Connect to guest agent after channel hotplug
>
> src/qemu/qemu_agent.c | 73 ++-----------------------------------------------
> src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++---
> src/qemu/qemu_domain.h | 2 ++
> src/qemu/qemu_driver.c | 56 +++++++++++++++++++++----------------
> src/qemu/qemu_process.c | 22 +--------------
> src/qemu/qemu_process.h | 2 ++
> 6 files changed, 81 insertions(+), 119 deletions(-)
>
ACK series.
Thanks, I've pushed this series.
Peter
3498
days inactive
3500
days old
8 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa