[libvirt] [PATCH v2] guest-agent-socket: don't generate default path to config XML
Martin Kletzander
mkletzan at redhat.com
Wed Dec 9 10:25:48 UTC 2015
On Mon, Nov 30, 2015 at 12:37:58PM +0100, Pavel Hrdina wrote:
>While we started using for all unix sockets as default one common
>directory based on a guest name it introduced several issues, for example
>with renaming the guest or cloning it. In general it's not entirely
>bad, but in this case it would be best to hide the auto-generated socket
>path from user and don't export it in the config XML.
>
>Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>---
>
>Version 2:
> - removed unnecessary changes
>
> src/qemu/qemu_command.c | 28 ++++++++++++++++++++++++++++
> src/qemu/qemu_command.h | 1 +
> src/qemu/qemu_domain.c | 16 ----------------
> src/qemu/qemu_driver.c | 3 +++
> src/qemu/qemu_process.c | 3 +++
> tests/qemuhotplugtest.c | 5 +++++
> tests/qemuxml2argvtest.c | 5 +++++
> 7 files changed, 45 insertions(+), 16 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 2a9fab5..f1bb621 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -1262,6 +1262,34 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> }
>
>
>+int
>+qemuUnixSocketGenerate(virDomainDefPtr def,
>+ virQEMUDriverConfigPtr cfg)
>+{
>+ size_t i;
>+
>+ for (i = 0; i < def->nchannels; i++) {
>+ virDomainChrDefPtr channel = def->channels[i];
>+
>+ if (channel->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>+ channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>+ channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>+ !channel->source.data.nix.path) {
>+ if (virAsprintf(&channel->source.data.nix.path,
>+ "%s/domain-%s/%s",
>+ cfg->channelTargetDir, def->name,
>+ channel->target.name ? channel->target.name
>+ : "unknown.sock") < 0)
>+ return -1;
>+
>+ channel->source.data.nix.listen = true;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
>+
> static void
> qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,
> virDomainDeviceAddressType type)
>diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
>index bebdd27..0512a23 100644
>--- a/src/qemu/qemu_command.h
>+++ b/src/qemu/qemu_command.h
>@@ -283,6 +283,7 @@ int qemuAssignDevicePCISlots(virDomainDefPtr def,
> virDomainPCIAddressSetPtr addrs);
>
> int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
>+int qemuUnixSocketGenerate(virDomainDefPtr def, virQEMUDriverConfigPtr cfg);
> int qemuDomainNetVLAN(virDomainNetDefPtr def);
> int qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx);
> int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index ed21245..7e05289 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -1329,22 +1329,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> ARCH_IS_S390(def->os.arch))
> dev->data.controller->model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
>
>- /* auto generate unix socket path */
>- if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
>- dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>- dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>- dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>- !dev->data.chr->source.data.nix.path) {
>- if (virAsprintf(&dev->data.chr->source.data.nix.path,
>- "%s/domain-%s/%s",
>- cfg->channelTargetDir, def->name,
>- dev->data.chr->target.name ? dev->data.chr->target.name
>- : "unknown.sock") < 0)
>- goto cleanup;
>-
>- dev->data.chr->source.data.nix.listen = true;
>- }
>-
For the sake of upgradibility, here we should disregard any paths that
start with cfg->channelTargetDir, maybe even with cfg->configBaseDir.
Users should not be using paths in libvirt's directories if they need to
access it anyway. Of course we'll need to document that as well.
I just wonder how come we don't need any tests fixed. We should've been
testing the auto-generation, I believe. And even if not, we should test
that we don't generate it into the XML now.
Otherwise looks fine, weak ACK due to tests, but I believe they can be
part of the next patch for the removal of these paths.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151209/a7942556/attachment-0001.sig>
More information about the libvir-list
mailing list