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(a)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.