[libvirt] [PATCH] guest-agent-socket: don't generate default path to config XML
Martin Kletzander
mkletzan at redhat.com
Mon Nov 30 09:21:47 UTC 2015
On Mon, Nov 30, 2015 at 09:35:39AM +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.
>
In order to fix this also for domains that already exist, we should
ignore any socket paths starting with /var/lib/libvirt as those were
generated and users shouldn't explicitly specify them anyway.
>Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>---
> src/conf/domain_conf.c | 2 +-
> src/qemu/qemu_command.c | 28 ++++++++++++++++++++++++++++
> src/qemu/qemu_command.h | 1 +
> src/qemu/qemu_domain.c | 16 ----------------
> src/qemu/qemu_driver.c | 10 ++++++++++
> src/qemu/qemu_process.c | 3 +++
> tests/qemuhotplugtest.c | 5 +++++
> tests/qemuxml2argvtest.c | 5 +++++
> 8 files changed, 53 insertions(+), 17 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index cbfc41e..ebae828 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -9810,7 +9810,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> goto error;
> }
>
>- def->data.nix.listen = mode != NULL && STRNEQ(mode, "connect");
>+ def->data.nix.listen = mode == NULL || STRNEQ(mode, "connect");
>
This changes the default unix socket mode from 'connect' to 'bind', is
this just a leftover from some testing or is there a reason behind
that. We have no default documented anywhere, so it wouldn't be an
issue, I guess.
> def->data.nix.path = path;
> path = NULL;
>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;
>- }
>-
> /* forbid capabilities mode hostdev in this kind of hypervisor */
> if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
> dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) {
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index ae1d8e7..b4fac55 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -7254,6 +7254,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
> if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
> goto cleanup;
>
>+ if (qemuUnixSocketGenerate(def, cfg) < 0)
>+ goto cleanup;
>+
> if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
> goto cleanup;
>
>@@ -15846,6 +15849,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
> unsigned int flags)
> {
> virQEMUDriverPtr driver = conn->privateData;
>+ virQEMUDriverConfigPtr cfg = NULL;
> virDomainObjPtr vm = NULL;
> virDomainDefPtr def = NULL;
> virDomainPtr dom = NULL;
>@@ -15858,6 +15862,8 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
>
> virCheckFlags(0, NULL);
>
>+ cfg = virQEMUDriverGetConfig(driver);
>+
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> goto cleanup;
>
>@@ -15895,6 +15901,9 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
> if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
> goto cleanup;
>
>+ if (qemuUnixSocketGenerate(def, cfg) < 0)
>+ goto cleanup;
>+
Why would we generate some path for a qemu that's already running and
we are not passing that path to qemu? It would have no effect and
only provide false information to the user.
> if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
> goto cleanup;
>
>@@ -15936,6 +15945,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
> VIR_FREE(pidfile);
> virObjectUnref(caps);
> virObjectUnref(qemuCaps);
>+ virObjectUnref(cfg);
> return dom;
> }
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 192730c..29cf965 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -4663,6 +4663,9 @@ qemuProcessLaunch(virConnectPtr conn,
> if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
> goto cleanup;
>
>+ if (qemuUnixSocketGenerate(vm->def, cfg) < 0)
>+ goto cleanup;
>+
If you move it from here to qemuBuildCommandLine(), then you don't
need to mangle with tests. It sounds weird that we don't have a clear
split between completing the info in XML and building a command-line,
but that was never the case anyway.
> /* Get the advisory nodeset from numad if 'placement' of
> * either <vcpu> or <numatune> is 'auto'.
> */
>diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
>index 102e052..b17cca2 100644
>--- a/tests/qemuhotplugtest.c
>+++ b/tests/qemuhotplugtest.c
>@@ -61,6 +61,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
> {
> int ret = -1;
> qemuDomainObjPrivatePtr priv = NULL;
>+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(&driver);
>
> if (!(*vm = virDomainObjNew(xmlopt)))
> goto cleanup;
>@@ -94,10 +95,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
> if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0)
> goto cleanup;
>
>+ if (qemuUnixSocketGenerate((*vm)->def, cfg) < 0)
>+ goto cleanup;
>+
> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
>
> ret = 0;
> cleanup:
>+ virObjectUnref(cfg);
> return ret;
> }
>
>diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>index f7596a0..f6083db 100644
>--- a/tests/qemuxml2argvtest.c
>+++ b/tests/qemuxml2argvtest.c
>@@ -262,6 +262,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
> virCommandPtr cmd = NULL;
> size_t i;
> virBitmapPtr nodeset = NULL;
>+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(&driver);
>
> if (!(conn = virGetConnect()))
> goto out;
>@@ -324,6 +325,9 @@ static int testCompareXMLToArgvFiles(const char *xml,
> if (qemuAssignDeviceAliases(vmdef, extraFlags) < 0)
> goto out;
>
>+ if (qemuUnixSocketGenerate(vmdef, cfg) < 0)
>+ goto out;
>+
> for (i = 0; i < vmdef->nhostdevs; i++) {
> virDomainHostdevDefPtr hostdev = vmdef->hostdevs[i];
>
>@@ -387,6 +391,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
> virCommandFree(cmd);
> virDomainDefFree(vmdef);
> virObjectUnref(conn);
>+ virObjectUnref(cfg);
> virBitmapFree(nodeset);
> return ret;
> }
>--
>2.6.3
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- 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/20151130/4e32d633/attachment-0001.sig>
More information about the libvir-list
mailing list