Re: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
On 03/30/2016 11:14 AM, Martin Kletzander wrote:
Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
channel cannot be plugged in because we won't generate its path
automatically. Let's not only fix that, but also add tests for it so
next time it's checked for.
Save some electrons, shorten the commit id
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++
tests/qemuhotplugtest.c | 15 ++++++
.../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+qemu-agent.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-qemu-agent-detach.xml | 5 ++
.../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
6 files changed, 144 insertions(+)
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e82dbf5448dc..b7741e15d445 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1565,6 +1565,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
+ goto cleanup;
+
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2b0de94fb4a6..384b7b9592b9 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
(*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
+ if (qemuDomainSetPrivatePaths(&priv->libDir,
+ &priv->channelTargetDir,
+ "/var/lib/libvirt",
+ "/var/lib/libvirt/qemu/channel/target",
+ (*vm)->def->name,
+ (*vm)->def->id) < 0)
I believe this overwrites qemuTestDriverInit - since it's a test I'm not
concerned about the memory leak, just the processing consistency since
you're not really starting a guest, why change the paths?
While it's fresh in my mind (still) using /tmp/* in *DriverInit when I
was generating patches the domain master secret key file caused problems
if I actually tried to check for the existence, especially since
qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
to create the directory structure. Perhaps if the tests driver created
"tmp/*" paths rather than "/tmp/*" paths that'd work, but is more
or
less unrelated.
I'll wait for your thoughts on this before an official ACK -
John
+ goto cleanup;
+
ret = 0;
cleanup:
return ret;
@@ -495,6 +503,13 @@ mymain(void)
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5")
QMP_OK,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_LIVE("hotplug-base", "qemu-agent", false, true,
+ "chardev-add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("hotplug-base", "qemu-agent-detach", false,
false,
+ "device_del", QMP_OK,
+ "chardev-remove", QMP_OK);
+
qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
[...]