July 2017 - Devel - libvirt List Archives

[libvirt] [PATCH] docs: Fix documentation related to memory locking
by Nitesh Konkar 26 Jul '17

26 Jul '17

Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com> --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bceddd2..91195be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -964,7 +964,7 @@ option opens up to a potential security risk: the host will be unable to reclaim the locked memory back from the guest when it's running out of memory, which means a malicious guest allocating large amounts of - locked memory could cause a denial-of-service attach on the host. + locked memory could cause a denial-of-service attack on the host. Because of this, using this option is discouraged unless your workload demands it; even then, it's highly recommended to set an <code>hard_limit</code> (see -- 2.9.4

2 1

[libvirt] [PATCH 0/3] properly handle '=' in the VNC socket path
by Pavel Hrdina 26 Jul '17

26 Jul '17

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

2 5

[libvirt] [PATCH] qemu_cgroup: Remove unnecessary virQEMUDriverPtr arguments
by Martin Kletzander 26 Jul '17

26 Jul '17

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

2 1

[libvirt] [PATCH v3] qemu: Check for existence of provided *_tls_x509_cert_dir
by John Ferlan 26 Jul '17

26 Jul '17

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

2 1

[libvirt] [PATCH v2 0/7] introduce virFileCache and use it for qemu capabilities
by Pavel Hrdina 26 Jul '17

26 Jul '17

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

2 14

[libvirt] [PATCH glib] Don't set LC_ALL=C during build as that breaks python apps
by Daniel P. Berrange 26 Jul '17

26 Jul '17

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

2 2

[libvirt] [PATCH 0/3] Introduce RW locks to virDomainObjList
by Michal Privoznik 26 Jul '17

26 Jul '17

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

4 28

[libvirt] [PATCH] qemu: we prefer C89 comment styles over C99
by Pavel Hrdina 26 Jul '17

26 Jul '17

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

2 1

[libvirt] [PATCH v2] Generate unique socket file
by Scott Garfinkle 25 Jul '17

25 Jul '17

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

3 2

[libvirt] [PATCH v3] storage: Disallow usage of the HBA for a fc_host backing
by John Ferlan 25 Jul '17

25 Jul '17

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

2 1