[libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well

[John Ferlan]

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 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/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;
>  }
[...]