[libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
Martin Kletzander
mkletzan at redhat.com
Wed Apr 13 13:06:31 UTC 2016
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 id
>>>
>>
>> I did that, but I wonder whether you haven't used more electrons by
>> mentioning that than I just shaved off =D
>>
>
>touche
>
>>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
>>>>
>>>> Signed-off-by: Martin Kletzander <mkletzan at 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/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?
>John
>
>>> 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.
>>>
>>
>> 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.
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160413/7e529fc1/attachment-0001.sig>
More information about the libvir-list
mailing list