[libvirt] [PATCH 0/3] properly handle '=' in the VNC socket path
by Pavel Hrdina
Pavel Hrdina (3):
qemu: capabilities: introduce QEMU_CAPS_VNC_MULTI_SERVERS
qemu: properly handle '=' in the VNC socket path
tests: add test case for VNC unix path with '='
src/qemu/qemu_capabilities.c | 4 ++
src/qemu/qemu_capabilities.h | 3 ++
src/qemu/qemu_command.c | 5 ++-
src/qemu/qemu_process.c | 51 ++++++++++++++++++++--
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
...muxml2argv-graphics-vnc-socket-new-cmdline.args | 22 ++++++++++
...emuxml2argv-graphics-vnc-socket-new-cmdline.xml | 20 +++++++++
...argv-graphics-vnc-socket-path-not-supported.xml | 1 +
.../qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +-
tests/qemuxml2argvtest.c | 5 +++
22 files changed, 121 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket-new-cmdline.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket-new-cmdline.xml
create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket-path-not-supported.xml
--
2.13.3
7 years, 5 months
[libvirt] [PATCH] qemu_cgroup: Remove unnecessary virQEMUDriverPtr arguments
by Martin Kletzander
Since commit 2e6ecba1bcac, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.
This is a first (example) patch of this kind of clean up, others will
hopefully follow.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 33 ++++++++++++++-------------------
src/qemu/qemu_cgroup.h | 6 ++----
src/qemu/qemu_process.c | 4 ++--
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 7355527c1cc5..6fc413098ae8 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -604,8 +604,7 @@ qemuTeardownChardevCgroup(virDomainObjPtr vm,
static int
-qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm)
+qemuSetupDevicesCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = NULL;
@@ -644,7 +643,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
if (rv < 0)
goto cleanup;
- cfg = virQEMUDriverGetConfig(driver);
+ cfg = virQEMUDriverGetConfig(priv->driver);
deviceACL = cfg->cgroupDeviceACL ?
(const char *const *)cfg->cgroupDeviceACL :
defaultDeviceACL;
@@ -769,8 +768,7 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm)
static int
-qemuSetupCpuCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm)
+qemuSetupCpuCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
virObjectEventPtr event = NULL;
@@ -806,7 +804,7 @@ qemuSetupCpuCgroup(virQEMUDriverPtr driver,
event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
}
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(priv->driver, event);
}
return 0;
@@ -814,16 +812,15 @@ qemuSetupCpuCgroup(virQEMUDriverPtr driver,
static int
-qemuInitCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+qemuInitCgroup(virDomainObjPtr vm,
size_t nnicindexes,
int *nicindexes)
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver);
- if (!virQEMUDriverIsPrivileged(driver))
+ if (!virQEMUDriverIsPrivileged(priv->driver))
goto done;
if (!virCgroupAvailable())
@@ -954,14 +951,13 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
}
int
-qemuConnectCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm)
+qemuConnectCgroup(virDomainObjPtr vm)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver);
int ret = -1;
- if (!virQEMUDriverIsPrivileged(driver))
+ if (!virQEMUDriverIsPrivileged(priv->driver))
goto done;
if (!virCgroupAvailable())
@@ -991,8 +987,7 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
}
int
-qemuSetupCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+qemuSetupCgroup(virDomainObjPtr vm,
size_t nnicindexes,
int *nicindexes)
{
@@ -1005,13 +1000,13 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
return -1;
}
- if (qemuInitCgroup(driver, vm, nnicindexes, nicindexes) < 0)
+ if (qemuInitCgroup(vm, nnicindexes, nicindexes) < 0)
return -1;
if (!priv->cgroup)
return 0;
- if (qemuSetupDevicesCgroup(driver, vm) < 0)
+ if (qemuSetupDevicesCgroup(vm) < 0)
goto cleanup;
if (qemuSetupBlkioCgroup(vm) < 0)
@@ -1020,7 +1015,7 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
if (qemuSetupMemoryCgroup(vm) < 0)
goto cleanup;
- if (qemuSetupCpuCgroup(driver, vm) < 0)
+ if (qemuSetupCpuCgroup(vm) < 0)
goto cleanup;
if (qemuSetupCpusetCgroup(vm) < 0)
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index d016ce29d87e..3fc1583612e1 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -55,10 +55,8 @@ int qemuSetupChardevCgroup(virDomainObjPtr vm,
virDomainChrDefPtr dev);
int qemuTeardownChardevCgroup(virDomainObjPtr vm,
virDomainChrDefPtr dev);
-int qemuConnectCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm);
-int qemuSetupCgroup(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+int qemuConnectCgroup(virDomainObjPtr vm);
+int qemuSetupCgroup(virDomainObjPtr vm,
size_t nnicindexes,
int *nicindexes);
int qemuSetupCpusetMems(virDomainObjPtr vm);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 87ab85fdb476..535c16b03705 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5595,7 +5595,7 @@ qemuProcessLaunch(virConnectPtr conn,
}
VIR_DEBUG("Setting up domain cgroup (if required)");
- if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0)
+ if (qemuSetupCgroup(vm, nnicindexes, nicindexes) < 0)
goto cleanup;
if (!(priv->perf = virPerfNew()))
@@ -6821,7 +6821,7 @@ qemuProcessReconnect(void *opaque)
if (qemuHostdevUpdateActiveDomainDevices(driver, obj->def) < 0)
goto error;
- if (qemuConnectCgroup(driver, obj) < 0)
+ if (qemuConnectCgroup(obj) < 0)
goto error;
if (qemuDomainPerfRestart(obj) < 0)
--
2.13.3
7 years, 5 months
[libvirt] [PATCH v3] qemu: Check for existence of provided *_tls_x509_cert_dir
by John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1458630
Introduce virQEMUDriverConfigTLSDirValidateResetDefault to validate
that if any of the *_tls_x509_cert_dir values were set properly and
reset the default value if the default_tls_x509_cert_dir changed.
Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
v2: https://www.redhat.com/archives/libvir-list/2017-June/msg01341.html
Follow-ups in July though.
Changes since v2 - reduced verbosity in qemu.conf and adjusted the logic
to create/call virQEMUDriverConfigTLSDirValidateResetDefault after all
the values are read in order to validate the values and adjust the default
if necessary.
Tested by
1. Having everything commented out, w/ no /etc/pki/qemu:
Works as expected
2. Uncomment the default_tls_x509_cert_dir, w/ no /etc/pki/qemu:
Fails as expected
3. Uncomment each of the other *_tls_x509_cert_dir's when directory not exist:
Fails as expected
4. Use a directory that exists for other _*tls_x509_cert_dirs:
Works as expected
5. Change the default_tls_x509_cert_dir to an existing directory:
Works as expected, each of the other uncommented *_tls_x509_cert_dirs
will get the new value (tested via debug code) while one that used an
existing default didn't change from it's default /etc/pki/libvirt-*
src/qemu/qemu.conf | 8 +++++++
src/qemu/qemu_conf.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_conf.h | 4 ++++
src/qemu/qemu_driver.c | 3 +++
4 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e6c0832..9526aed 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -13,6 +13,14 @@
#
# dh-params.pem - the DH params configuration file
#
+# If the directory does not exist or contain the necessary files, QEMU
+# domains will fail to start if they are configured to use TLS.
+#
+# In order to overwrite the default path alter the following. This path
+# definition will be used as the default path for other *_tls_x509_cert_dir
+# configuration settings if their default path does not exist or is not
+# specifically set.
+#
#default_tls_x509_cert_dir = "/etc/pki/qemu"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 6f44cbf..87d2c2d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -451,8 +451,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (!(conf = virConfReadFile(filename, 0)))
goto cleanup;
- if (virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir) < 0)
+ if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir)) < 0)
goto cleanup;
+ cfg->checkdefaultTLSx509certdir = (rv == 1);
if (virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify) < 0)
goto cleanup;
if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
@@ -872,6 +873,68 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
return ret;
}
+
+/**
+ * @cfg: Recently config values
+ *
+ * Validate the recently read *_tls_x509_cert_dir values and if necessary
+ * update the default value to match the default_tls_x509_cert_dir
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virQEMUDriverConfigTLSDirValidateResetDefault(virQEMUDriverConfigPtr cfg)
+{
+ bool newDefault = false;
+
+ /* If the default entry was uncommented, then validate existence */
+ if (cfg->checkdefaultTLSx509certdir) {
+ if (!virFileExists(cfg->defaultTLSx509certdir)) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("default_tls_x509_cert_dir directory '%s' "
+ "does not exist"),
+ cfg->defaultTLSx509certdir);
+ return -1;
+ }
+ if (STRNEQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu"))
+ newDefault = true;
+ }
+
+ /* We know virQEMUDriverConfigNew set the particular value to either
+ * it's default or default_tls_x509_cert_dir's default. So, if not the
+ * default default and the directory doesn't exist, then the entry was
+ * set in the config file to something that doesn't exist, so error.
+ *
+ * Also, if the defaultTLSx509certdir value was changed from the default,
+ * then we need to update the default for each setting as well to match
+ * the default_tls_x509_cert_dir.
+ */
+#define VALIDATE_TLS_X509_CERT_DIR(val) \
+ do { \
+ if (STRNEQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu") && \
+ !virFileExists(cfg->val ## TLSx509certdir)) { \
+ virReportError(VIR_ERR_CONF_SYNTAX, \
+ _(#val"_tls_x509_cert_dir directory '%s' " \
+ "does not exist"), \
+ cfg->val ## TLSx509certdir); \
+ return -1; \
+ } else if (newDefault) { \
+ VIR_FREE(cfg->val ## TLSx509certdir); \
+ if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
+ cfg->defaultTLSx509certdir) < 0) \
+ return -1; \
+ } \
+ } while (0)
+
+ VALIDATE_TLS_X509_CERT_DIR(vnc);
+ VALIDATE_TLS_X509_CERT_DIR(spice);
+ VALIDATE_TLS_X509_CERT_DIR(chardev);
+ VALIDATE_TLS_X509_CERT_DIR(migrate);
+
+ return 0;
+}
+
+
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
{
virQEMUDriverConfigPtr conf;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 1407eef..fffa871 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -112,6 +112,7 @@ struct _virQEMUDriverConfig {
char *nvramDir;
char *defaultTLSx509certdir;
+ bool checkdefaultTLSx509certdir;
bool defaultTLSx509verify;
char *defaultTLSx509secretUUID;
@@ -301,6 +302,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
const char *filename,
bool privileged);
+int
+virQEMUDriverConfigTLSDirValidateResetDefault(virQEMUDriverConfigPtr cfg);
+
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6568def..2731f8e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -667,6 +667,9 @@ qemuStateInitialize(bool privileged,
goto error;
VIR_FREE(driverConf);
+ if (virQEMUDriverConfigTLSDirValidateResetDefault(cfg) < 0)
+ goto error;
+
if (virFileMakePath(cfg->stateDir) < 0) {
virReportSystemError(errno, _("Failed to create state dir %s"),
cfg->stateDir);
--
2.9.4
7 years, 5 months
[libvirt] [PATCH v2 0/7] introduce virFileCache and use it for qemu capabilities
by Pavel Hrdina
Pavel Hrdina (7):
util: introduce virFileCache
tests: add virfilecachetest
qemu: introduce struct _virQEMUCapsCachePriv
tests: rewrite host CPU mocking
qemu: pass only host arch instead of the whole virCaps
qemu: switch QEMU capabilities to use virFileCache
qemu: privatize _virQEMUCapsCachePriv struct
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 9 +
src/qemu/qemu_capabilities.c | 372 ++++++-----------
src/qemu/qemu_capabilities.h | 20 +-
src/qemu/qemu_capspriv.h | 17 +-
src/qemu/qemu_conf.h | 3 +-
src/qemu/qemu_domain.c | 17 +-
src/qemu/qemu_driver.c | 10 +-
src/qemu/qemu_process.c | 9 +-
src/util/virfilecache.c | 439 +++++++++++++++++++++
src/util/virfilecache.h | 137 +++++++
tests/Makefile.am | 12 +
tests/domaincapstest.c | 55 +--
tests/qemucapsprobe.c | 3 +-
tests/qemucpumock.c | 14 +-
tests/qemuxml2argvtest.c | 9 +-
tests/testutilshostcpus.h | 148 +++++++
tests/testutilsqemu.c | 107 +----
tests/testutilsqemu.h | 3 +-
tests/testutilsxen.c | 29 +-
...a15b1658aa16923e46497dd8deeb6be287ddb0ca0.cache | 1 +
...ae4bc3a28e75bc3e262757001e8b953580f5e75ef.cache | 1 +
tests/virfilecachemock.c | 31 ++
tests/virfilecachetest.c | 238 +++++++++++
25 files changed, 1200 insertions(+), 486 deletions(-)
create mode 100644 src/util/virfilecache.c
create mode 100644 src/util/virfilecache.h
create mode 100644 tests/testutilshostcpus.h
create mode 100644 tests/virfilecachedata/5f3154560c130108b282a2aa15b1658aa16923e46497dd8deeb6be287ddb0ca0.cache
create mode 120000 tests/virfilecachedata/9ca150bf3119b75dcac8e8bae4bc3a28e75bc3e262757001e8b953580f5e75ef.cache
create mode 100644 tests/virfilecachemock.c
create mode 100644 tests/virfilecachetest.c
--
2.13.3
7 years, 5 months
[libvirt] [PATCH glib] Don't set LC_ALL=C during build as that breaks python apps
by Daniel P. Berrange
Setting LC_ALL=C breaks python apps doing I/O on UTF-8 source
files. In particular this broke glib-mkenums
GEN libvirt-gconfig-enum-types.h
Traceback (most recent call last):
File "/usr/bin/glib-mkenums", line 669, in <module>
process_file(fname)
File "/usr/bin/glib-mkenums", line 406, in process_file
line = curfile.readline()
File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 849: ordinal not in range(128)
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
Pushed to fix rawhide build
maint.mk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/maint.mk b/maint.mk
index 405c6d0..ef72b4f 100644
--- a/maint.mk
+++ b/maint.mk
@@ -117,8 +117,8 @@ news-check-lines-spec ?= 1,10
news-check-regexp ?= '^\*.* $(VERSION_REGEXP) \($(today)\)'
# Prevent programs like 'sort' from considering distinct strings to be equal.
-# Doing it here saves us from having to set LC_ALL elsewhere in this file.
-export LC_ALL = C
+# Doing it here saves us from having to set LC_COLLATE elsewhere in this file.
+export LC_COLLATE = C
## --------------- ##
## Sanity checks. ##
--
2.13.3
7 years, 5 months
[libvirt] [PATCH 0/3] Introduce RW locks to virDomainObjList
by Michal Privoznik
While this is not that critical (hash tables have O(1) time complexity for
lookups), it lays down path towards making virDomainObj using RW locks instead
of mutexes. Still, in my limited testing this showed slight improvement.
Michal Privoznik (3):
virthread: Introduce virRWLockInitPreferWriter
virobject: Introduce virObjectRWLockable
virdomainobjlist: Use virObjectRWLockable
src/conf/virdomainobjlist.c | 24 ++++----
src/libvirt_private.syms | 4 ++
src/util/virobject.c | 144 ++++++++++++++++++++++++++++++++++----------
src/util/virobject.h | 16 +++++
src/util/virthread.c | 35 +++++++++++
src/util/virthread.h | 1 +
6 files changed, 180 insertions(+), 44 deletions(-)
--
2.13.0
7 years, 5 months
[libvirt] [PATCH] qemu: we prefer C89 comment styles over C99
by Pavel Hrdina
Introduced by commit 'a7bc2c8cfd6f'.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
Pushed under trivial rule.
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index dcdbfc9701..5c073027db 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7264,7 +7264,8 @@ qemuDomainPrepareChannel(virDomainChrDefPtr channel,
"%s/%s", domainChannelTargetDir,
channel->target.name) < 0)
return -1;
- } else { // Generate a unique name
+ } else {
+ /* Generate a unique name */
if (virAsprintf(&channel->source->data.nix.path,
"%s/vioser-%02d-%02d-%02d.sock",
domainChannelTargetDir,
--
2.13.3
7 years, 5 months
[libvirt] [PATCH v2] Generate unique socket file
by Scott Garfinkle
It's possible to have more than one unnamed virtio-serial unix channel.
We need to generate a unique name for each channel. Currently, we use
".../unknown.sock" for all of them. Better practice would be to specify
an explicit target path name; however, in the absence of that, we need
uniqueness in the names we generate internally.
Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
for each instance of
<channel type='unix'>
<source mode='bind'/>
<target type='virtio'/>
</channel>
Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.
Changes from v1: new socket name
Signed-off-by: Scott Garfinkle <seg(a)us.ibm.com>
---
src/qemu/qemu_domain.c | 24 +++++++++++++++-------
.../qemuxml2argv-channel-virtio-unix.args | 2 +-
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78e75f1..7a9958d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7178,18 +7178,28 @@ int
qemuDomainPrepareChannel(virDomainChrDefPtr channel,
const char *domainChannelTargetDir)
{
- if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
- channel->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
- !channel->source->data.nix.path) {
+ if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
+ channel->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
+ channel->source->data.nix.path)
+ return 0;
+
+ if (channel->target.name) {
if (virAsprintf(&channel->source->data.nix.path,
"%s/%s", domainChannelTargetDir,
- channel->target.name ? channel->target.name
- : "unknown.sock") < 0)
+ channel->target.name) < 0)
+ return -1;
+ } else { // Generate a unique name
+ if (virAsprintf(&channel->source->data.nix.path,
+ "%s/vioser-%02d-%02d-%02d.sock",
+ domainChannelTargetDir,
+ channel->info.addr.vioserial.controller,
+ channel->info.addr.vioserial.bus,
+ channel->info.addr.vioserial.port) < 0)
return -1;
-
- channel->source->data.nix.listen = true;
}
+ channel->source->data.nix.listen = true;
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
index 2b72965..8e0452a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
@@ -29,7 +29,7 @@ path=/tmp/channel/domain--1-QEMUGuest1/org.qemu.guest_agent.0,server,nowait \
-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\
id=channel0,name=org.qemu.guest_agent.0 \
-chardev socket,id=charchannel1,\
-path=/tmp/channel/domain--1-QEMUGuest1/unknown.sock,server,nowait \
+path=/tmp/channel/domain--1-QEMUGuest1/vioser-00-00-02.sock,server,nowait \
-device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\
id=channel1 \
-chardev socket,id=charchannel2,path=/tmp/channel/domain--1-QEMUGuest1/ble,\
--
1.8.3.1
7 years, 5 months
[libvirt] [PATCH v3] storage: Disallow usage of the HBA for a fc_host backing
by John Ferlan
Disallow providing the wwnn/wwpn of the HBA in the adapter XML:
<adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
wwpn='HBA_wwpn'/>
This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:
<adapter type='scsi_host' name='scsi_hostN'/>
So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
v2: https://www.redhat.com/archives/libvir-list/2017-July/msg01019.html
Changes since v2, from review, simplify logic even more.
docs/formatstorage.html.in | 27 +++++++++++++++------------
src/storage/storage_backend_scsi.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 4946ddf..27578e8 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -207,18 +207,21 @@
</dl>
<dl>
<dt><code>wwnn</code> and <code>wwpn</code></dt>
- <dd>The "World Wide Node Name" (<code>wwnn</code>) and "World Wide
- Port Name" (<code>wwpn</code>) are used by the "fc_host" adapter
- to uniquely identify the device in the Fibre Channel storage fabric
- (the device can be either a HBA or vHBA). Both wwnn and wwpn should
- be specified. Use the command 'virsh nodedev-dumpxml' to determine
- how to set the values for the wwnn/wwpn of a (v)HBA. The wwnn and
- wwpn have very specific numerical format requirements based on the
- hypervisor being used, thus care should be taken if you decide to
- generate your own to follow the standards; otherwise, the pool
- will fail to start with an opaque error message indicating failure
- to write to the vport_create file during vport create/delete due
- to "No such file or directory".
+ <dd>The required "World Wide Node Name" (<code>wwnn</code>) and
+ "World Wide Port Name" (<code>wwpn</code>) are used by the
+ "fc_host" adapter to uniquely identify the vHBA device in the
+ Fibre Channel storage fabric. If the vHBA device already exists
+ as a Node Device, then libvirt will use it; otherwise, the vHBA
+ will be created using the provided values. It is considered a
+ configuration error use the values from the HBA as those would
+ be for a "scsi_host" <code>type</code> pool instead. The
+ <code>wwnn</code> and <code>wwpn</code> have very specific
+ format requirements based on the hypervisor being used, thus
+ care should be taken if you decide to generate your own to
+ follow the standards; otherwise, the pool will fail to start
+ with an opaque error message indicating failure to write to
+ the vport_create file during vport create/delete due to
+ "No such file or directory".
<span class="since">Since 1.0.4</span>
</dd>
</dl>
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index af12889..575e6a6 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -211,6 +211,33 @@ getAdapterName(virStorageAdapterPtr adapter)
}
+/**
+ * @name: Name from a wwnn/wwpn lookup
+ *
+ * Validate that the @name fetched from the wwnn/wwpn is a vHBA
+ * and not an HBA as that should be a configuration error. It's only
+ * possible to use an existing wwnn/wwpn of a vHBA because that's
+ * what someone would have created using the node device create via XML
+ * functionality. Using the HBA "just because" it has a wwnn/wwpn and
+ * the characteristics of a vHBA is just not valid
+ *
+ * Returns true if the @name is OK, false on error
+ */
+static bool
+checkName(const char *name)
+{
+ unsigned int host_num;
+
+ if (virSCSIHostGetNumber(name, &host_num) &&
+ virVHBAIsVportCapable(NULL, host_num))
+ return true;
+
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("the wwnn/wwpn for '%s' are assigned to an HBA"), name);
+ return false;
+}
+
+
/*
* Using the host# name found via wwnn/wwpn lookup in the fc_host
* sysfs tree to get the parent 'scsi_host#' to ensure it matches.
@@ -288,6 +315,9 @@ createVport(virConnectPtr conn,
* this pool and we don't have to create the vHBA
*/
if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
+ if (!(checkName(name)))
+ goto cleanup;
+
/* If a parent was provided, let's make sure the 'name' we've
* retrieved has the same parent. If not this will cause failure. */
if (!fchost->parent || checkParent(conn, name, fchost->parent))
--
2.9.4
7 years, 5 months
[libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing
by John Ferlan
Disallow providing the wwnn/wwpn of the HBA in the adapter XML:
<adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
wwpn='HBA_wwpn'/>
This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:
<adapter type='scsi_host' name='scsi_hostN'/>
So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
This used to be patch 4 of the series:
https://www.redhat.com/archives/libvir-list/2017-July/msg00837.html
which while the series was in it's 3rd review cycle, this particular
patch was new to the series and thus becomes a v2 since the three other
patches from the series were pushed.
docs/formatstorage.html.in | 27 +++++++++--------
src/storage/storage_backend_scsi.c | 62 +++++++++++++++++++++++++++++++++-----
2 files changed, 69 insertions(+), 20 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 4946ddf..27578e8 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -207,18 +207,21 @@
</dl>
<dl>
<dt><code>wwnn</code> and <code>wwpn</code></dt>
- <dd>The "World Wide Node Name" (<code>wwnn</code>) and "World Wide
- Port Name" (<code>wwpn</code>) are used by the "fc_host" adapter
- to uniquely identify the device in the Fibre Channel storage fabric
- (the device can be either a HBA or vHBA). Both wwnn and wwpn should
- be specified. Use the command 'virsh nodedev-dumpxml' to determine
- how to set the values for the wwnn/wwpn of a (v)HBA. The wwnn and
- wwpn have very specific numerical format requirements based on the
- hypervisor being used, thus care should be taken if you decide to
- generate your own to follow the standards; otherwise, the pool
- will fail to start with an opaque error message indicating failure
- to write to the vport_create file during vport create/delete due
- to "No such file or directory".
+ <dd>The required "World Wide Node Name" (<code>wwnn</code>) and
+ "World Wide Port Name" (<code>wwpn</code>) are used by the
+ "fc_host" adapter to uniquely identify the vHBA device in the
+ Fibre Channel storage fabric. If the vHBA device already exists
+ as a Node Device, then libvirt will use it; otherwise, the vHBA
+ will be created using the provided values. It is considered a
+ configuration error use the values from the HBA as those would
+ be for a "scsi_host" <code>type</code> pool instead. The
+ <code>wwnn</code> and <code>wwpn</code> have very specific
+ format requirements based on the hypervisor being used, thus
+ care should be taken if you decide to generate your own to
+ follow the standards; otherwise, the pool will fail to start
+ with an opaque error message indicating failure to write to
+ the vport_create file during vport create/delete due to
+ "No such file or directory".
<span class="since">Since 1.0.4</span>
</dd>
</dl>
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index af12889..8892822 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -211,21 +211,66 @@ getAdapterName(virStorageAdapterPtr adapter)
}
+/**
+ * @name: Name from a wwnn/wwpn lookup
+ *
+ * Validate that the @name fetched from the wwnn/wwpn is a vHBA and not
+ * an HBA as that should be a configuration error. It's only possible to
+ * use an existing wwnn/wwpn of a vHBA because that's what someone would
+ * have created using the node device create functionality. Using the
+ * HBA "just because" it has a wwnn/wwpn and the characteristics of a
+ * vHBA is just not valid
+ *
+ * Returns the @scsi_host_name to be used or NULL on errror
+ */
+static char *
+checkName(const char *name)
+{
+ char *scsi_host_name = NULL;
+ unsigned int host_num;
+
+ if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
+ return NULL;
+
+ if (virSCSIHostGetNumber(scsi_host_name, &host_num) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("host name '%s' is not properly formatted"),
+ name);
+ goto error;
+ }
+
+ /* If scsi_host_name is vport capable, then it's an HBA. This is
+ * a configuration error as the wwnn/wwpn should only be for a vHBA */
+ if (virVHBAIsVportCapable(NULL, host_num)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("the wwnn/wwpn for '%s' are assigned to an HBA"),
+ scsi_host_name);
+ goto error;
+ }
+
+ return scsi_host_name;
+
+ error:
+ VIR_FREE(scsi_host_name);
+ return NULL;
+}
+
+
/*
* Using the host# name found via wwnn/wwpn lookup in the fc_host
* sysfs tree to get the parent 'scsi_host#' to ensure it matches.
*/
static bool
checkParent(virConnectPtr conn,
- const char *name,
+ const char *scsi_host_name,
const char *parent_name)
{
unsigned int host_num;
- char *scsi_host_name = NULL;
char *vhba_parent = NULL;
bool retval = false;
- VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name);
+ VIR_DEBUG("conn=%p, scsi_host_name=%s, parent_name=%s",
+ conn, scsi_host_name, parent_name);
/* autostarted pool - assume we're OK */
if (!conn)
@@ -245,9 +290,6 @@ checkParent(virConnectPtr conn,
goto cleanup;
}
- if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
- goto cleanup;
-
if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name)))
goto cleanup;
@@ -255,7 +297,7 @@ checkParent(virConnectPtr conn,
virReportError(VIR_ERR_XML_ERROR,
_("Parent attribute '%s' does not match parent '%s' "
"determined for the '%s' wwnn/wwpn lookup."),
- parent_name, vhba_parent, name);
+ parent_name, vhba_parent, scsi_host_name);
goto cleanup;
}
@@ -263,7 +305,6 @@ checkParent(virConnectPtr conn,
cleanup:
VIR_FREE(vhba_parent);
- VIR_FREE(scsi_host_name);
return retval;
}
@@ -275,6 +316,7 @@ createVport(virConnectPtr conn,
virStorageAdapterFCHostPtr fchost)
{
char *name = NULL;
+ char *scsi_host_name = NULL;
virStoragePoolFCRefreshInfoPtr cbdata = NULL;
virThread thread;
int ret = -1;
@@ -288,6 +330,9 @@ createVport(virConnectPtr conn,
* this pool and we don't have to create the vHBA
*/
if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
+ if (!(scsi_host_name = checkName(name)))
+ goto cleanup;
+
/* If a parent was provided, let's make sure the 'name' we've
* retrieved has the same parent. If not this will cause failure. */
if (!fchost->parent || checkParent(conn, name, fchost->parent))
@@ -333,6 +378,7 @@ createVport(virConnectPtr conn,
cleanup:
VIR_FREE(name);
+ VIR_FREE(scsi_host_name);
return ret;
}
--
2.9.4
7 years, 5 months
