On Wed, Apr 13, 2016 at 07:25:17AM -0400, John Ferlan wrote:
On 04/12/2016 08:43 AM, Martin Kletzander wrote:On Thu, Mar 31, 2016 at 08:35:43AM -0400, John Ferlan wrote: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 idI did that, but I wonder whether you haven't used more electrons by mentioning that than I just shaved off =DtoucheResolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210 Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- 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/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?Well, I just needed to initialize it to some string. I can change it to /tmp without any problems. However it is not used anywhere to access anything and we would need to change a lot of tests that make sense currently (as they generate those values -- one of them is even checking that it generates that particular value). Moreover, it should not introduce any leak as it is set only if the values are empty.oh right - although I imagine even this changes since commit id '1893b6df1' altered the API. I'm sure you know that already though ;-)
That commit didn't change it, fortunately =)
ACK for this though
Thanks for the review. Just to make sure, I can push the whole series?
JohnWhile 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.Oh, that reminded me that I should figure out why I can't start any machine since those patches went in... Anyway, I'm not aware about the fact that qemuProcessPrepareHost() is called in tests. And if it is, that's the problem by itself.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list