Compare commits

..

6 Commits

Author SHA1 Message Date
Cole Robinson 9e23d5e3a9 nwfilter: increase pcap buffer size to be compatible with TPACKET_V3 (bz #1547237) 2018-07-03 12:15:26 -04:00
Daniel P. Berrangé c5bb6a7965 Adapt to changed wireshark plugin install dir
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-06-21 10:56:18 +01:00
Daniel P. Berrangé ac335adbbc Add new CPU features for CVE-2017-5715 and CVE-2018-3639
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-06-21 09:16:26 +01:00
Cole Robinson 7294ce1ae2 CVE-2018-5748: resource exhaustion via qemuMonitorIORead() (bz #1535785)
CVE-2018-6764: code injection via libvirt_lxc (bz #1542815)
Fix hotplug disk failure (bz #1540872)
2018-02-13 14:26:06 -05:00
Cole Robinson c23de3143a CVE-2017-1000256: libvirt: TLS certificate verification disabled for clients (bz #1503687)
Fix qemu image locking with shared disks (bz #1513447)
2017-12-04 12:11:03 -05:00
Cole Robinson 7042f56045 Fix TPM2 passthrough (bz #1486240)
Fix spice GL qemu:///system rendernode permissions (bz #1460804)
2017-09-15 19:06:08 -04:00
74 changed files with 3438 additions and 4681 deletions
@@ -1,31 +0,0 @@
From: Erik Skultety <eskultet@redhat.com>
Date: Mon, 2 Sep 2019 13:25:09 +0200
Subject: [PATCH] src: security: Replace bitwise OR with logical OR
Typo introduced by commit d73f3f58360.
https://bugzilla.redhat.com/show_bug.cgi?id=1738483
Signed-off-by: Erik Skultety <eskultet@redhat.com>
(cherry picked from commit 5801ef06ec29fdbac6718e455015450f7c243c1b)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740024
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/security_util.c b/src/security/security_util.c
index ad265b0bc5..9d3f483f6b 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -268,7 +268,7 @@ virSecurityMoveRememberedLabel(const char *name,
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) attr_value = NULL;
- if (!(ref_name = virSecurityGetRefCountAttrName(name)) |
+ if (!(ref_name = virSecurityGetRefCountAttrName(name)) ||
!(attr_name = virSecurityGetAttrName(name)))
return -1;
@@ -0,0 +1,34 @@
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Date: Thu, 29 Jun 2017 14:01:11 -0400
Subject: [PATCH] tpm: Use /dev/null for cancel path if none was found
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
TPM 2 does not implement sysfs files for cancellation of commands.
We therefore use /dev/null for the cancel path passed to QEMU.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit dfbb15b75433e520fb1b905c1c3e28753e53e4a5)
---
src/util/virtpm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 6d9b0657a..d5c10da38 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -61,9 +61,7 @@ virTPMCreateCancelPath(const char *devpath)
VIR_FREE(path);
}
if (!path)
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("No usable sysfs TPM cancel file could be "
- "found"));
+ ignore_value(VIR_STRDUP(path, "/dev/null"));
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("TPM device path %s is invalid"), devpath);
@@ -0,0 +1,108 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Sun, 27 Aug 2017 11:23:47 -0400
Subject: [PATCH] security: add MANAGER_MOUNT_NAMESPACE flag
The VIR_SECURITY_MANAGER_MOUNT_NAMESPACE flag informs the DAC driver
if mount namespaces are in use for the VM. Will be used for future
changes.
Wire it up in the qemu driver
(cherry picked from commit 321031e482425dfeae0f125cdac6df870f079efd)
---
src/qemu/qemu_driver.c | 2 ++
src/security/security_dac.c | 10 ++++++++++
src/security/security_dac.h | 3 +++
src/security/security_manager.c | 4 +++-
src/security/security_manager.h | 1 +
5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b7824512c..1f9264639 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -419,6 +419,8 @@ qemuSecurityInit(virQEMUDriverPtr driver)
if (virQEMUDriverIsPrivileged(driver)) {
if (cfg->dynamicOwnership)
flags |= VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP;
+ if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT))
+ flags |= VIR_SECURITY_MANAGER_MOUNT_NAMESPACE;
if (!(mgr = qemuSecurityNewDAC(QEMU_DRIVER_NAME,
cfg->user,
cfg->group,
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index ca7a6af6d..507be44a2 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -57,6 +57,7 @@ struct _virSecurityDACData {
gid_t *groups;
int ngroups;
bool dynamicOwnership;
+ bool mountNamespace;
char *baselabel;
virSecurityManagerDACChownCallback chownCallback;
};
@@ -237,6 +238,15 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
priv->dynamicOwnership = dynamicOwnership;
}
+void
+virSecurityDACSetMountNamespace(virSecurityManagerPtr mgr,
+ bool mountNamespace)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ priv->mountNamespace = mountNamespace;
+}
+
+
void
virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
virSecurityManagerDACChownCallback chownCallback)
diff --git a/src/security/security_dac.h b/src/security/security_dac.h
index 846cefbb5..97681c961 100644
--- a/src/security/security_dac.h
+++ b/src/security/security_dac.h
@@ -32,6 +32,9 @@ int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
bool dynamic);
+void virSecurityDACSetMountNamespace(virSecurityManagerPtr mgr,
+ bool mountNamespace);
+
void virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
virSecurityManagerDACChownCallback chownCallback);
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 95b995230..e43c99d4f 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -146,7 +146,8 @@ virSecurityManagerNewDAC(const char *virtDriver,
virSecurityManagerPtr mgr;
virCheckFlags(VIR_SECURITY_MANAGER_NEW_MASK |
- VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP, NULL);
+ VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP |
+ VIR_SECURITY_MANAGER_MOUNT_NAMESPACE, NULL);
mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC,
virtDriver,
@@ -161,6 +162,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
}
virSecurityDACSetDynamicOwnership(mgr, flags & VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP);
+ virSecurityDACSetMountNamespace(mgr, flags & VIR_SECURITY_MANAGER_MOUNT_NAMESPACE);
virSecurityDACSetChownCallback(mgr, chownCallback);
return mgr;
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 01296d339..08fb89203 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -36,6 +36,7 @@ typedef enum {
VIR_SECURITY_MANAGER_REQUIRE_CONFINED = 1 << 2,
VIR_SECURITY_MANAGER_PRIVILEGED = 1 << 3,
VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP = 1 << 4,
+ VIR_SECURITY_MANAGER_MOUNT_NAMESPACE = 1 << 5,
} virSecurityManagerNewFlags;
# define VIR_SECURITY_MANAGER_NEW_MASK \
@@ -1,176 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:10 +0200
Subject: [PATCH] security_util: Use more VIR_AUTOFREE()
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 8ced0909e5a0c41a44c2110d7060f6518b65f0db)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740024
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_util.c | 78 +++++++++++++++---------------------
1 file changed, 32 insertions(+), 46 deletions(-)
diff --git a/src/security/security_util.c b/src/security/security_util.c
index 9d3f483f6b..04347f51e5 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -113,34 +113,32 @@ virSecurityGetRememberedLabel(const char *name,
const char *path,
char **label)
{
- char *ref_name = NULL;
- char *attr_name = NULL;
- char *value = NULL;
+ VIR_AUTOFREE(char *) ref_name = NULL;
+ VIR_AUTOFREE(char *) attr_name = NULL;
+ VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
- int ret = -1;
*label = NULL;
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
- goto cleanup;
+ return -1;
if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
- if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP) {
- ret = -2;
- } else {
- virReportSystemError(errno,
- _("Unable to get XATTR %s on %s"),
- ref_name,
- path);
- }
- goto cleanup;
+ if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP)
+ return -2;
+
+ virReportSystemError(errno,
+ _("Unable to get XATTR %s on %s"),
+ ref_name,
+ path);
+ return -1;
}
if (virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("malformed refcount %s on %s"),
value, path);
- goto cleanup;
+ return -1;
}
VIR_FREE(value);
@@ -149,30 +147,25 @@ virSecurityGetRememberedLabel(const char *name,
if (refcount > 0) {
if (virAsprintf(&value, "%u", refcount) < 0)
- goto cleanup;
+ return -1;
if (virFileSetXAttr(path, ref_name, value) < 0)
- goto cleanup;
+ return -1;
} else {
if (virFileRemoveXAttr(path, ref_name) < 0)
- goto cleanup;
+ return -1;
if (!(attr_name = virSecurityGetAttrName(name)))
- goto cleanup;
+ return -1;
if (virFileGetXAttr(path, attr_name, label) < 0)
- goto cleanup;
+ return -1;
if (virFileRemoveXAttr(path, attr_name) < 0)
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(value);
- VIR_FREE(attr_name);
- VIR_FREE(ref_name);
- return ret;
+ return 0;
}
@@ -201,25 +194,23 @@ virSecuritySetRememberedLabel(const char *name,
const char *path,
const char *label)
{
- char *ref_name = NULL;
- char *attr_name = NULL;
- char *value = NULL;
+ VIR_AUTOFREE(char *) ref_name = NULL;
+ VIR_AUTOFREE(char *) attr_name = NULL;
+ VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
- int ret = -1;
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
- goto cleanup;
+ return -1;
if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
if (errno == ENOSYS || errno == ENOTSUP) {
- ret = -2;
- goto cleanup;
+ return -2;
} else if (errno != ENODATA) {
virReportSystemError(errno,
_("Unable to get XATTR %s on %s"),
ref_name,
path);
- goto cleanup;
+ return -1;
}
}
@@ -228,7 +219,7 @@ virSecuritySetRememberedLabel(const char *name,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("malformed refcount %s on %s"),
value, path);
- goto cleanup;
+ return -1;
}
VIR_FREE(value);
@@ -237,24 +228,19 @@ virSecuritySetRememberedLabel(const char *name,
if (refcount == 1) {
if (!(attr_name = virSecurityGetAttrName(name)))
- goto cleanup;
+ return -1;
if (virFileSetXAttr(path, attr_name, label) < 0)
- goto cleanup;
+ return -1;
}
if (virAsprintf(&value, "%u", refcount) < 0)
- goto cleanup;
+ return -1;
if (virFileSetXAttr(path, ref_name, value) < 0)
- goto cleanup;
+ return -1;
- ret = refcount;
- cleanup:
- VIR_FREE(value);
- VIR_FREE(attr_name);
- VIR_FREE(ref_name);
- return ret;
+ return refcount;
}
@@ -0,0 +1,101 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Mon, 17 Jul 2017 08:57:57 -0400
Subject: [PATCH] security: dac: relabel spice rendernode
For a logged in user this a path like /dev/dri/renderD128 will have
default ownership root:video which won't work for the qemu:qemu user,
so we need to chown it.
We only do this when mount namespaces are enabled in the qemu driver,
so the chown'ing doesn't interfere with other users of the shared
render node path
https://bugzilla.redhat.com/show_bug.cgi?id=1460804
(cherry picked from commit 98931187eefdec6f2dea5cb82ab6d23a3ffa6634)
---
src/security/security_dac.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 507be44a2..349dbe81d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1380,6 +1380,54 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
}
+static int
+virSecurityDACSetGraphicsLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainGraphicsDefPtr gfx)
+
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ uid_t user;
+ gid_t group;
+
+ /* Skip chowning the shared render file if namespaces are disabled */
+ if (!priv->mountNamespace)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+ if (seclabel && !seclabel->relabel)
+ return 0;
+
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0)
+ return -1;
+
+ if (gfx->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+ gfx->data.spice.gl == VIR_TRISTATE_BOOL_YES &&
+ gfx->data.spice.rendernode) {
+ if (virSecurityDACSetOwnership(priv, NULL,
+ gfx->data.spice.rendernode,
+ user, group) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static int
+virSecurityDACRestoreGraphicsLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainGraphicsDefPtr gfx ATTRIBUTE_UNUSED)
+
+{
+ /* The only graphics labelling we do is dependent on mountNamespaces,
+ in which case 'restoring' the label doesn't actually accomplish
+ anything, so there's nothing to do here */
+ return 0;
+}
+
+
static int
virSecurityDACSetInputLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
@@ -1491,6 +1539,11 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
rc = -1;
}
+ for (i = 0; i < def->ngraphics; i++) {
+ if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0)
+ return -1;
+ }
+
for (i = 0; i < def->ninputs; i++) {
if (virSecurityDACRestoreInputLabel(mgr, def, def->inputs[i]) < 0)
rc = -1;
@@ -1611,6 +1664,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
return -1;
}
+ for (i = 0; i < def->ngraphics; i++) {
+ if (virSecurityDACSetGraphicsLabel(mgr, def, def->graphics[i]) < 0)
+ return -1;
+ }
+
for (i = 0; i < def->ninputs; i++) {
if (virSecurityDACSetInputLabel(mgr, def, def->inputs[i]) < 0)
return -1;
@@ -1,39 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:11 +0200
Subject: [PATCH] security_util: Document virSecurityMoveRememberedLabel
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 359c7c1e94e0c54bf7e7a5a8365f066904b4387b)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740024
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_util.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/security/security_util.c b/src/security/security_util.c
index 04347f51e5..365b2dd2d6 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -244,6 +244,19 @@ virSecuritySetRememberedLabel(const char *name,
}
+/**
+ * virSecurityMoveRememberedLabel:
+ * @name: security driver name
+ * @src: source file
+ * @dst: destination file
+ *
+ * For given security driver @name, move all XATTRs related to seclabel
+ * remembering from @src to @dst. However, if @dst is NULL, then XATTRs
+ * are just removed from @src.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise.
+ */
int
virSecurityMoveRememberedLabel(const char *name,
const char *src,
@@ -0,0 +1,72 @@
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Thu, 5 Oct 2017 17:54:28 +0100
Subject: [PATCH] qemu: ensure TLS clients always verify the server certificate
The default_tls_x509_verify (and related) parameters in qemu.conf
control whether the QEMU TLS servers request & verify certificates
from clients. This works as a simple access control system for
servers by requiring the CA to issue certs to permitted clients.
This use of client certificates is disabled by default, since it
requires extra work to issue client certificates.
Unfortunately the code was using this configuration parameter when
setting up both TLS clients and servers in QEMU. The result was that
TLS clients for character devices and disk devices had verification
turned off, meaning they would ignore errors while validating the
server certificate.
This allows for trivial MITM attacks between client and server,
as any certificate returned by the attacker will be accepted by
the client.
This is assigned CVE-2017-1000256 / LSN-2017-0002
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 441d3eb6d1be940a67ce45a286602a967601b157)
(cherry picked from commit dc6c41798d1eb5c52c75365ffa22f7672709dfa7)
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args | 2 +-
.../qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9a27987d4..ae78cd17e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -718,7 +718,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
if (virJSONValueObjectCreate(propsret,
"s:dir", path,
"s:endpoint", (isListen ? "server": "client"),
- "b:verify-peer", verifypeer,
+ "b:verify-peer", (isListen ? verifypeer : true),
NULL) < 0)
goto cleanup;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
index 5aff7734e..ab5f7e27f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
@@ -26,7 +26,7 @@ server,nowait \
localport=1111 \
-device isa-serial,chardev=charserial0,id=serial0 \
-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
-endpoint=client,verify-peer=no \
+endpoint=client,verify-peer=yes \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
index 91f1fe0cd..2567abbfa 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
@@ -31,7 +31,7 @@ localport=1111 \
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
-endpoint=client,verify-peer=no,passwordid=charserial1-secret0 \
+endpoint=client,verify-peer=yes,passwordid=charserial1-secret0 \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
@@ -1,164 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:12 +0200
Subject: [PATCH] security: Don't increase XATTRs refcounter on failure
If user has two domains, each have the same disk (configured for
RW) but each runs with different seclabel then we deny start of
the second domain because in order to do that we would need to
relabel the disk but that would cut the first domain off. Even if
we did not do that, qemu would fail to start because it would be
unable to lock the disk image for the second time. So far, this
behaviour is expected. But what is not expected is that we
increase the refcounter in XATTRs and leave it like that.
What happens is that when the second domain starts,
virSecuritySetRememberedLabel() is called, and since there are
XATTRs from the first domain it increments the refcounter and
returns it (refcounter == 2 at this point). Then callers
(virSecurityDACSetOwnership() and
virSecuritySELinuxSetFileconHelper()) realize that refcounter is
greater than 1 and desired seclabel doesn't match the one the
disk image already has and an error is produced. But the
refcounter is never decremented.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740024
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 6a2806fd549c0bfdcb3eb83f130ee99ea8d213e1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_dac.c | 41 ++++++++++++++++++---------------
src/security/security_selinux.c | 18 ++++++++++-----
2 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 137daf5d28..4b4afef18a 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -751,6 +751,7 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
bool remember)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virErrorPtr origerr;
struct stat sb;
int refcount;
int rc;
@@ -784,12 +785,13 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
* is @refcount domains using the @path. Do not
* change the label (as it would almost certainly
* cause the other domains to lose access to the
- * @path). */
+ * @path). However, the refcounter was incremented in
+ * XATTRs so decrease it. */
if (sb.st_uid != uid || sb.st_gid != gid) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("Setting different DAC user or group on %s "
"which is already in use"), path);
- return -1;
+ goto error;
}
}
}
@@ -797,25 +799,26 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
NULLSTR(src ? src->path : path), (long)uid, (long)gid);
- if (virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0) {
- virErrorPtr origerr;
-
- virErrorPreserveLast(&origerr);
- /* Try to restore the label. This is done so that XATTRs
- * are left in the same state as when the control entered
- * this function. However, if our attempt fails, there's
- * not much we can do. XATTRs refcounting is fubar'ed and
- * the only option we have is warn users. */
- if (virSecurityDACRestoreFileLabelInternal(mgr, src, path, remember) < 0)
- VIR_WARN("Unable to restore label on '%s'. "
- "XATTRs might have been left in inconsistent state.",
- NULLSTR(src ? src->path : path));
-
- virErrorRestore(&origerr);
- return -1;
- }
+ if (virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0)
+ goto error;
return 0;
+
+ error:
+ virErrorPreserveLast(&origerr);
+ /* Try to restore the label. This is done so that XATTRs
+ * are left in the same state as when the control entered
+ * this function. However, if our attempt fails, there's
+ * not much we can do. XATTRs refcounting is fubar'ed and
+ * the only option we have is warn users. */
+ if (virSecurityDACRestoreFileLabelInternal(mgr, src, path, remember) < 0)
+ VIR_WARN("Unable to restore label on '%s'. "
+ "XATTRs might have been left in inconsistent state.",
+ NULLSTR(src ? src->path : path));
+
+ virErrorRestore(&origerr);
+
+ return -1;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index ea20373a90..9857223bbf 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1334,6 +1334,7 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
security_context_t econ = NULL;
int refcount;
int rc;
+ bool rollback = false;
int ret = -1;
if ((rc = virSecuritySELinuxTransactionAppend(path, tcon,
@@ -1353,6 +1354,8 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
if (econ) {
refcount = virSecuritySELinuxRememberLabel(path, econ);
+ if (refcount > 0)
+ rollback = true;
if (refcount == -2) {
/* Not supported. Don't error though. */
} else if (refcount < 0) {
@@ -1362,7 +1365,8 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
* is @refcount domains using the @path. Do not
* change the label (as it would almost certainly
* cause the other domains to lose access to the
- * @path). */
+ * @path). However, the refcounter was
+ * incremented in XATTRs so decrease it. */
if (STRNEQ(econ, tcon)) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("Setting different SELinux label on %s "
@@ -1373,7 +1377,12 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
}
}
- if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0) {
+ if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ if (ret < 0 && rollback) {
virErrorPtr origerr;
virErrorPreserveLast(&origerr);
@@ -1388,11 +1397,8 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
path);
virErrorRestore(&origerr);
- goto cleanup;
- }
- ret = 0;
- cleanup:
+ }
freecon(econ);
return ret;
}
@@ -0,0 +1,177 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 15 Nov 2017 13:15:57 +0100
Subject: [PATCH] qemu: Move snapshot disk validation functions into one
Move the code so that both the new image and old image can be verified
in the same function.
(cherry picked from commit 8ffdeed455650557df531aafc66c20b31bd4e0c4)
---
src/qemu/qemu_driver.c | 91 ++++++++++++++++++++------------------------------
1 file changed, 36 insertions(+), 55 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1f9264639..57f0c2bf4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13793,17 +13793,19 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn,
static int
-qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
+qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk,
+ virDomainDiskDefPtr domdisk)
{
- int actualType = virStorageSourceGetActualType(disk->src);
+ int domDiskType = virStorageSourceGetActualType(domdisk->src);
+ int snapDiskType = virStorageSourceGetActualType(snapdisk->src);
- switch ((virStorageType) actualType) {
+ switch ((virStorageType) domDiskType) {
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
- return 0;
+ break;
case VIR_STORAGE_TYPE_NETWORK:
- switch ((virStorageNetProtocol) disk->src->protocol) {
+ switch ((virStorageNetProtocol) domdisk->src->protocol) {
case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_RBD:
@@ -13820,7 +13822,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("external inactive snapshots are not supported on "
"'network' disks using '%s' protocol"),
- virStorageNetProtocolTypeToString(disk->src->protocol));
+ virStorageNetProtocolTypeToString(domdisk->src->protocol));
return -1;
}
break;
@@ -13831,7 +13833,23 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("external inactive snapshots are not supported on "
- "'%s' disks"), virStorageTypeToString(actualType));
+ "'%s' disks"), virStorageTypeToString(domDiskType));
+ return -1;
+ }
+
+ switch ((virStorageType) snapDiskType) {
+ case VIR_STORAGE_TYPE_BLOCK:
+ case VIR_STORAGE_TYPE_FILE:
+ break;
+
+ case VIR_STORAGE_TYPE_NETWORK:
+ case VIR_STORAGE_TYPE_DIR:
+ case VIR_STORAGE_TYPE_VOLUME:
+ case VIR_STORAGE_TYPE_NONE:
+ case VIR_STORAGE_TYPE_LAST:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("external inactive snapshots are not supported on "
+ "'%s' disks"), virStorageTypeToString(snapDiskType));
return -1;
}
@@ -13840,33 +13858,27 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
static int
-qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk)
+qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk,
+ virDomainDiskDefPtr domdisk)
{
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
+ int actualType = virStorageSourceGetActualType(snapdisk->src);
+
+ if (domdisk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("external active snapshots are not supported on scsi "
"passthrough devices"));
return -1;
}
- return 0;
-}
-
-
-static int
-qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr disk)
-{
- int actualType = virStorageSourceGetActualType(disk->src);
-
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
- return 0;
+ break;
case VIR_STORAGE_TYPE_NETWORK:
- switch ((virStorageNetProtocol) disk->src->protocol) {
+ switch ((virStorageNetProtocol) snapdisk->src->protocol) {
case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
- return 0;
+ break;
case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_NBD:
@@ -13883,7 +13895,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
virReportError(VIR_ERR_INTERNAL_ERROR,
_("external active snapshots are not supported on "
"'network' disks using '%s' protocol"),
- virStorageNetProtocolTypeToString(disk->src->protocol));
+ virStorageNetProtocolTypeToString(snapdisk->src->protocol));
return -1;
}
@@ -13903,31 +13915,6 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
}
-static int
-qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr disk)
-{
- int actualType = virStorageSourceGetActualType(disk->src);
-
- switch ((virStorageType) actualType) {
- case VIR_STORAGE_TYPE_BLOCK:
- case VIR_STORAGE_TYPE_FILE:
- return 0;
-
- case VIR_STORAGE_TYPE_NETWORK:
- case VIR_STORAGE_TYPE_DIR:
- case VIR_STORAGE_TYPE_VOLUME:
- case VIR_STORAGE_TYPE_NONE:
- case VIR_STORAGE_TYPE_LAST:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("external inactive snapshots are not supported on "
- "'%s' disks"), virStorageTypeToString(actualType));
- return -1;
- }
-
- return 0;
-}
-
-
static int
qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
virDomainDiskDefPtr disk,
@@ -13945,16 +13932,10 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
return -1;
- if (qemuDomainSnapshotPrepareDiskExternalBackingInactive(disk) < 0)
- return -1;
-
- if (qemuDomainSnapshotPrepareDiskExternalOverlayInactive(snapdisk) < 0)
+ if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0)
return -1;
} else {
- if (qemuDomainSnapshotPrepareDiskExternalBackingActive(disk) < 0)
- return -1;
-
- if (qemuDomainSnapshotPrepareDiskExternalOverlayActive(snapdisk) < 0)
+ if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0)
return -1;
}
-183
View File
@@ -1,183 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:13 +0200
Subject: [PATCH] util: Introduce virhostuptime
This module contains function to get host boot time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 8b802f13cb47817706cba101f5d52e2c8957698d)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741140
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
configure.ac | 1 +
src/libvirt_private.syms | 4 ++
src/util/Makefile.inc.am | 2 +
src/util/virhostuptime.c | 81 ++++++++++++++++++++++++++++++++++++++++
src/util/virhostuptime.h | 27 ++++++++++++++
5 files changed, 115 insertions(+)
create mode 100644 src/util/virhostuptime.c
create mode 100644 src/util/virhostuptime.h
diff --git a/configure.ac b/configure.ac
index d18d427695..771dd961a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -337,6 +337,7 @@ AC_CHECK_FUNCS_ONCE([\
getpwuid_r \
getrlimit \
getuid \
+ getutxid \
if_indextoname \
mmap \
newlocale \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c323f679b3..9fd778272f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2135,6 +2135,10 @@ virHostMemGetStats;
virHostMemSetParameters;
+# util/virhostuptime.h
+virHostGetBootTime;
+
+
# util/viridentity.h
virIdentityGetAttr;
virIdentityGetCurrent;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index a47f333a98..46866cf213 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -91,6 +91,8 @@ UTIL_SOURCES = \
util/virhostdev.h \
util/virhostmem.c \
util/virhostmem.h \
+ util/virhostuptime.c \
+ util/virhostuptime.h \
util/viridentity.c \
util/viridentity.h \
util/virinitctl.c \
diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
new file mode 100644
index 0000000000..62b781acd5
--- /dev/null
+++ b/src/util/virhostuptime.c
@@ -0,0 +1,81 @@
+/*
+ * virhostuptime.c: helper APIs for host uptime
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#ifdef HAVE_GETUTXID
+# include <utmpx.h>
+#endif
+
+#include "virhostuptime.h"
+#include "virthread.h"
+
+static unsigned long long bootTime;
+static int bootTimeErrno;
+static virOnceControl virHostGetBootTimeOnce = VIR_ONCE_CONTROL_INITIALIZER;
+
+#ifdef HAVE_GETUTXID
+static void
+virHostGetBootTimeOnceInit(void)
+{
+ struct utmpx id = {.ut_type = BOOT_TIME};
+ struct utmpx *res = NULL;
+
+ if (!(res = getutxid(&id))) {
+ bootTimeErrno = errno;
+ } else {
+ bootTime = res->ut_tv.tv_sec;
+ }
+
+ endutxent();
+}
+
+#else /* !HAVE_GETUTXID */
+
+static void
+virHostGetBootTimeOnceInit(void)
+{
+ bootTimeErrno = ENOSYS;
+}
+#endif /* HAVE_GETUTXID */
+
+/**
+ * virHostGetBootTime:
+ * @when: UNIX timestamp of boot time
+ *
+ * Get a UNIX timestamp of host boot time and store it at @when.
+ *
+ * Return: 0 on success,
+ * -1 otherwise.
+ */
+int
+virHostGetBootTime(unsigned long long *when)
+{
+ if (virOnce(&virHostGetBootTimeOnce, virHostGetBootTimeOnceInit) < 0)
+ return -1;
+
+ if (bootTimeErrno) {
+ errno = bootTimeErrno;
+ return -1;
+ }
+
+ *when = bootTime;
+ return 0;
+}
diff --git a/src/util/virhostuptime.h b/src/util/virhostuptime.h
new file mode 100644
index 0000000000..03c1517a64
--- /dev/null
+++ b/src/util/virhostuptime.h
@@ -0,0 +1,27 @@
+/*
+ * virhostuptime.h: helper APIs for host uptime
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "internal.h"
+
+int
+virHostGetBootTime(unsigned long long *when)
+ ATTRIBUTE_NOINLINE;
@@ -0,0 +1,55 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 14 Nov 2017 15:34:46 +0100
Subject: [PATCH] qemu: block: Add function to check if storage source allows
concurrent access
Storage source format backing a shared device (e.g. running a cluster
filesystem) needs to support the sharing so that metadata are not
corrupted. Add a central function for checking this.
(cherry picked from commit 1fc3cd8731640aefc48bbd9fc489f21cb99c6f67)
---
src/qemu/qemu_block.c | 15 +++++++++++++++
src/qemu/qemu_block.h | 3 +++
2 files changed, 18 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 7fb12ea5a..4c0a5eb78 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -379,6 +379,21 @@ qemuBlockGetNodeData(virJSONValuePtr data)
}
+/**
+ * qemuBlockStorageSourceSupportsConcurrentAccess:
+ * @src: disk storage source
+ *
+ * Returns true if the given storage format supports concurrent access from two
+ * separate processes.
+ */
+bool
+qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src)
+{
+ /* no need to check in backing chain since only RAW storage supports this */
+ return src->format == VIR_STORAGE_FILE_RAW;
+}
+
+
/**
* qemuBlockStorageSourceBuildHostsJSONSocketAddress:
* @src: disk storage source
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index f0a2c9aa7..ebf3149ce 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -53,6 +53,9 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
virHashTablePtr
qemuBlockGetNodeData(virJSONValuePtr data);
+bool
+qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src);
+
virJSONValuePtr
qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src);
@@ -1,344 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:14 +0200
Subject: [PATCH] security_util: Remove stale XATTRs
It may happen that we leave some XATTRs behind. For instance, on
a sudden power loss, the host just shuts down without calling
restore on domain paths. This creates a problem, because when the
host starts up again, the XATTRs are there but they don't reflect
the true state and this may result in libvirt denying start of a
domain.
To solve this, save a unique timestamp (host boot time) among
with our XATTRs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741140
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 7cfb7aab573a031880a1f4fd20747843fea109ba)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_util.c | 194 +++++++++++++++++++++++++++++++-
tests/qemusecuritymock.c | 12 ++
tools/libvirt_recover_xattrs.sh | 2 +-
3 files changed, 206 insertions(+), 2 deletions(-)
diff --git a/src/security/security_util.c b/src/security/security_util.c
index 365b2dd2d6..31f41cedfd 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -22,11 +22,16 @@
#include "virfile.h"
#include "virstring.h"
#include "virerror.h"
+#include "virlog.h"
+#include "viruuid.h"
+#include "virhostuptime.h"
#include "security_util.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
+VIR_LOG_INIT("security.security_util");
+
/* There are four namespaces available on Linux (xattr(7)):
*
* user - can be modified by anybody,
@@ -83,6 +88,153 @@ virSecurityGetRefCountAttrName(const char *name ATTRIBUTE_UNUSED)
}
+#ifdef XATTR_NAMESPACE
+static char *
+virSecurityGetTimestampAttrName(const char *name)
+{
+ char *ret = NULL;
+ ignore_value(virAsprintf(&ret, XATTR_NAMESPACE ".libvirt.security.timestamp_%s", name));
+ return ret;
+}
+#else /* !XATTR_NAMESPACE */
+static char *
+virSecurityGetTimestampAttrName(const char *name ATTRIBUTE_UNUSED)
+{
+ errno = ENOSYS;
+ virReportSystemError(errno, "%s",
+ _("Extended attributes are not supported on this system"));
+ return NULL;
+}
+#endif /* !XATTR_NAMESPACE */
+
+
+static char *
+virSecurityGetTimestamp(void)
+{
+ unsigned long long boottime = 0;
+ char *ret = NULL;
+
+ if (virHostGetBootTime(&boottime) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to get host boot time"));
+ return NULL;
+ }
+
+ ignore_value(virAsprintf(&ret, "%llu", boottime));
+ return ret;
+}
+
+
+/**
+ * virSecurityValidateTimestamp:
+ * @name: security driver name
+ * @path: file name
+ *
+ * Check if remembered label on @path for security driver @name
+ * is valid, i.e. the label has been set since the last boot. If
+ * the label was set in previous runs, all XATTRs related to
+ * @name are removed so that clean slate is restored.
+ *
+ * This is done having extra attribute timestamp_$SECDRIVER which
+ * contains the host boot time. Its value is then compared to
+ * actual host boot time. If these two values don't match then
+ * XATTRs are considered as stale and thus invalid.
+ *
+ * In ideal world, where there network file systems have XATTRs
+ * using plain host boot time is not enough as it may lead to a
+ * situation where a freshly started host sees XATTRs, sees the
+ * timestamp put there by some longer running host and considers
+ * the XATTRs invalid. Well, there is not an easy way out. We
+ * would need to somehow check if the longer running host is
+ * still there and uses the @path (how?).
+ * Fortunately, there is only one network file system which
+ * supports XATTRs currently (GlusterFS via FUSE) and it is used
+ * so rarely that it's almost a corner case.
+ * The worst thing that happens there is that we remove XATTRs
+ * and thus return @path to the default label for $SECDRIVER.
+ *
+ * Returns: 0 if remembered label is valid,
+ * 1 if remembered label was not valid,
+ * -2 if underlying file system doesn't support XATTRs,
+ * -1 otherwise.
+ */
+static int
+virSecurityValidateTimestamp(const char *name,
+ const char *path)
+{
+ VIR_AUTOFREE(char *) expected_timestamp = NULL;
+ VIR_AUTOFREE(char *) timestamp_name = NULL;
+ VIR_AUTOFREE(char *) value = NULL;
+
+ if (!(expected_timestamp = virSecurityGetTimestamp()) ||
+ !(timestamp_name = virSecurityGetTimestampAttrName(name)))
+ return -1;
+
+ errno = 0;
+ if (virFileGetXAttrQuiet(path, timestamp_name, &value) < 0) {
+ if (errno == ENOSYS || errno == ENOTSUP) {
+ return -2;
+ } else if (errno != ENODATA) {
+ virReportSystemError(errno,
+ _("Unable to get XATTR %s on %s"),
+ timestamp_name,
+ path);
+ return -1;
+ }
+
+ /* Timestamp is missing. We could continue and claim a valid timestamp.
+ * But then we would never remove stale XATTRs. Therefore, claim it
+ * invalid and have the code below remove all XATTRs. This of course
+ * means that we will not restore the original owner, but the plus side
+ * is that we reset refcounter which will represent the true state.
+ */
+ }
+
+ if (STREQ_NULLABLE(value, expected_timestamp)) {
+ VIR_DEBUG("XATTRs on %s secdriver=%s are valid", path, name);
+ return 0;
+ }
+
+ VIR_WARN("Invalid XATTR timestamp detected on %s secdriver=%s", path, name);
+
+ if (virSecurityMoveRememberedLabel(name, path, NULL) < 0)
+ return -1;
+
+ return 1;
+}
+
+
+static int
+virSecurityAddTimestamp(const char *name,
+ const char *path)
+{
+ VIR_AUTOFREE(char *) timestamp_name = NULL;
+ VIR_AUTOFREE(char *) timestamp_value = NULL;
+
+ if (!(timestamp_value = virSecurityGetTimestamp()) ||
+ !(timestamp_name = virSecurityGetTimestampAttrName(name)))
+ return -1;
+
+ return virFileSetXAttr(path, timestamp_name, timestamp_value);
+}
+
+
+static int
+virSecurityRemoveTimestamp(const char *name,
+ const char *path)
+{
+ VIR_AUTOFREE(char *) timestamp_name = NULL;
+
+ if (!(timestamp_name = virSecurityGetTimestampAttrName(name)))
+ return -1;
+
+ if (virFileRemoveXAttr(path, timestamp_name) < 0 && errno != ENOENT)
+ return -1;
+
+ return 0;
+}
+
+
/**
* virSecurityGetRememberedLabel:
* @name: security driver name
@@ -117,9 +269,13 @@ virSecurityGetRememberedLabel(const char *name,
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
+ int rc;
*label = NULL;
+ if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
+ return rc;
+
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
return -1;
@@ -163,6 +319,9 @@ virSecurityGetRememberedLabel(const char *name,
if (virFileRemoveXAttr(path, attr_name) < 0)
return -1;
+
+ if (virSecurityRemoveTimestamp(name, path) < 0)
+ return -1;
}
return 0;
@@ -198,6 +357,10 @@ virSecuritySetRememberedLabel(const char *name,
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
+ int rc;
+
+ if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
+ return rc;
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
return -1;
@@ -232,6 +395,9 @@ virSecuritySetRememberedLabel(const char *name,
if (virFileSetXAttr(path, attr_name, label) < 0)
return -1;
+
+ if (virSecurityAddTimestamp(name, path) < 0)
+ return -1;
}
if (virAsprintf(&value, "%u", refcount) < 0)
@@ -266,9 +432,12 @@ virSecurityMoveRememberedLabel(const char *name,
VIR_AUTOFREE(char *) ref_value = NULL;
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) attr_value = NULL;
+ VIR_AUTOFREE(char *) timestamp_name = NULL;
+ VIR_AUTOFREE(char *) timestamp_value = NULL;
if (!(ref_name = virSecurityGetRefCountAttrName(name)) ||
- !(attr_name = virSecurityGetAttrName(name)))
+ !(attr_name = virSecurityGetAttrName(name)) ||
+ !(timestamp_name = virSecurityGetTimestampAttrName(name)))
return -1;
if (virFileGetXAttrQuiet(src, ref_name, &ref_value) < 0) {
@@ -293,6 +462,17 @@ virSecurityMoveRememberedLabel(const char *name,
}
}
+ if (virFileGetXAttrQuiet(src, timestamp_name, &timestamp_value) < 0) {
+ if (errno == ENOSYS || errno == ENOTSUP) {
+ return -2;
+ } else if (errno != ENODATA) {
+ virReportSystemError(errno,
+ _("Unable to get XATTR %s on %s"),
+ attr_name, src);
+ return -1;
+ }
+ }
+
if (ref_value &&
virFileRemoveXAttr(src, ref_name) < 0) {
return -1;
@@ -303,6 +483,11 @@ virSecurityMoveRememberedLabel(const char *name,
return -1;
}
+ if (timestamp_value &&
+ virFileRemoveXAttr(src, timestamp_name) < 0) {
+ return -1;
+ }
+
if (dst) {
if (ref_value &&
virFileSetXAttr(dst, ref_name, ref_value) < 0) {
@@ -314,6 +499,13 @@ virSecurityMoveRememberedLabel(const char *name,
ignore_value(virFileRemoveXAttr(dst, ref_name));
return -1;
}
+
+ if (timestamp_value &&
+ virFileSetXAttr(dst, timestamp_name, timestamp_value) < 0) {
+ ignore_value(virFileRemoveXAttr(dst, ref_name));
+ ignore_value(virFileRemoveXAttr(dst, attr_name));
+ return -1;
+ }
}
return 0;
diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c
index a15eef29c9..373d64305a 100644
--- a/tests/qemusecuritymock.c
+++ b/tests/qemusecuritymock.c
@@ -32,6 +32,7 @@
#include "viralloc.h"
#include "qemusecuritytest.h"
#include "security/security_manager.h"
+#include "virhostuptime.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -488,3 +489,14 @@ virProcessRunInFork(virProcessForkCallback cb,
{
return cb(-1, opaque);
}
+
+
+/* We don't really need to mock this function. The qemusecuritytest doesn't
+ * care about the actual value. However, travis runs build and tests in a
+ * container where utmp is missing and thus this function fails. */
+int
+virHostGetBootTime(unsigned long long *when)
+{
+ *when = 1234567890;
+ return 0;
+}
diff --git a/tools/libvirt_recover_xattrs.sh b/tools/libvirt_recover_xattrs.sh
index 58f02f8dfb..3907413c63 100755
--- a/tools/libvirt_recover_xattrs.sh
+++ b/tools/libvirt_recover_xattrs.sh
@@ -74,7 +74,7 @@ fi
declare -a XATTRS
for i in "dac" "selinux"; do
for p in ${LIBVIRT_XATTR_PREFIXES[@]}; do
- XATTRS+=("$p.$i" "$p.ref_$i")
+ XATTRS+=("$p.$i" "$p.ref_$i" "$p.timestamp_$i")
done
done
@@ -0,0 +1,146 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 14 Nov 2017 15:37:09 +0100
Subject: [PATCH] qemu: domain: Reject shared disk access if backing format
does not support it
Disk sharing between two VMs may corrupt the images if the format driver
does not support it. Check that the user declared use of a supported
storage format when they want to share the disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
(cherry picked from commit 3b03a27cd00c2f032661d2bf8905795425752fc7)
---
src/qemu/qemu_domain.c | 29 +++++++++++++++++++++-
.../qemuxml2argv-disk-drive-shared-qcow.xml | 28 +++++++++++++++++++++
.../qemuxml2argv-disk-drive-shared.args | 2 +-
.../qemuxml2argv-disk-drive-shared.xml | 2 +-
tests/qemuxml2argvtest.c | 1 +
5 files changed, 59 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b98ffffae..42d17c1b0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -25,6 +25,7 @@
#include "qemu_domain.h"
#include "qemu_alias.h"
+#include "qemu_block.h"
#include "qemu_cgroup.h"
#include "qemu_command.h"
#include "qemu_process.h"
@@ -3299,6 +3300,29 @@ qemuDomainRedirdevDefValidate(const virDomainRedirdevDef *def)
}
+static int
+qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
+{
+ if (disk->src->shared && !disk->src->readonly) {
+ if (disk->src->format <= VIR_STORAGE_FILE_AUTO) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("shared access for disk '%s' requires use of "
+ "explicitly specified disk format"), disk->dst);
+ return -1;
+ }
+
+ if (!qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("shared access for disk '%s' requires use of "
+ "supported storage format"), disk->dst);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def ATTRIBUTE_UNUSED,
@@ -3308,7 +3332,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
- if (dev->type == VIR_DOMAIN_DEVICE_NET) {
+ if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+ if (qemuDomainDeviceDefValidateDisk(dev->data.disk) < 0)
+ goto cleanup;
+ } else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
const virDomainNetDef *net = dev->data.net;
if (net->guestIP.nroutes || net->guestIP.nips) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml
new file mode 100644
index 000000000..ca88a944b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <shareable/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args
index 502157bf8..326fde1b3 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args
@@ -19,7 +19,7 @@ server,nowait \
-no-acpi \
-boot c \
-usb \
--drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0,\
serial=XYZXYZXYZYXXYZYZYXYZY,cache=none \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml
index 9f7472378..677c2b0b7 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml
@@ -15,7 +15,7 @@
<devices>
<emulator>/usr/bin/qemu-system-i686</emulator>
<disk type='block' device='disk'>
- <driver name='qemu' type='qcow2'/>
+ <driver name='qemu' type='raw'/>
<source dev='/dev/HostVG/QEMUGuest1'/>
<target dev='hda' bus='ide'/>
<shareable/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 18f06e5aa..93f892229 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -895,6 +895,7 @@ mymain(void)
QEMU_CAPS_DRIVE_BOOT);
DO_TEST("disk-drive-shared",
QEMU_CAPS_DRIVE_SERIAL);
+ DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE);
DO_TEST("disk-drive-error-policy-stop",
QEMU_CAPS_MONITOR_JSON);
DO_TEST("disk-drive-error-policy-enospace",
@@ -1,96 +0,0 @@
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Date: Mon, 2 Sep 2019 13:25:15 +0200
Subject: [PATCH] security_util: verify xattrs only if ref is present
After 7cfb7aab573 commit starting a domain pullutes logs with
warnings like [1]. The reason is resource files do not
have timestamp before starting a domain and after destroying
domain the timestamp is cleared. Let's check the timestamp
only if attribute with refcounter is found.
[1] warning : virSecurityValidateTimestamp:198 : Invalid XATTR timestamp detected on \
/some/path secdriver=dac
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 7c40211a5a0fd15f5885e38dc571e81b9a3fd699)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741140
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_util.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/src/security/security_util.c b/src/security/security_util.c
index 31f41cedfd..865b3ec905 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -269,13 +269,9 @@ virSecurityGetRememberedLabel(const char *name,
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
- int rc;
*label = NULL;
- if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
- return rc;
-
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
return -1;
@@ -290,6 +286,20 @@ virSecurityGetRememberedLabel(const char *name,
return -1;
}
+ if (value) {
+ int rc;
+
+ /* Do this after we've tried to get refcounter to ensure underlying FS
+ * supports XATTRs and @path has refcounter attribute set, because
+ * validator might throws a warning. */
+ if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
+ return rc;
+
+ /* Invalid label is like a non-existent one */
+ if (rc == 1)
+ return -2;
+ }
+
if (virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("malformed refcount %s on %s"),
@@ -357,10 +367,6 @@ virSecuritySetRememberedLabel(const char *name,
VIR_AUTOFREE(char *) attr_name = NULL;
VIR_AUTOFREE(char *) value = NULL;
unsigned int refcount = 0;
- int rc;
-
- if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
- return rc;
if (!(ref_name = virSecurityGetRefCountAttrName(name)))
return -1;
@@ -377,6 +383,20 @@ virSecuritySetRememberedLabel(const char *name,
}
}
+ if (value) {
+ int rc;
+
+ /* Do this after we've tried to get refcounter to ensure underlying FS
+ * supports XATTRs and @path has refcounter attribute set, because
+ * validator might throws a warning. */
+ if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
+ return rc;
+
+ /* Invalid label is like a non-existent one */
+ if (rc == 1)
+ VIR_FREE(value);
+ }
+
if (value &&
virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -0,0 +1,63 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 15 Nov 2017 13:41:01 +0100
Subject: [PATCH] qemu: snapshot: Disallow snapshot of unsupported shared disks
Creating a snapshot would introduce a possibly unsupported member for
sharing into the backing chain. Add a check to prevent that from
happening.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
(cherry picked from commit 9b2fbfa6f6b535b9f41a7503531d43d86d7a8868)
---
src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 57f0c2bf4..91119a494 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13792,6 +13792,24 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn,
}
+static int
+qemuDomainSnapshotPrepareDiskShared(virDomainSnapshotDiskDefPtr snapdisk,
+ virDomainDiskDefPtr domdisk)
+{
+ if (!domdisk->src->shared || domdisk->src->readonly)
+ return 0;
+
+ if (!qemuBlockStorageSourceSupportsConcurrentAccess(snapdisk->src)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("shared access for disk '%s' requires use of "
+ "supported storage format"), domdisk->dst);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk,
virDomainDiskDefPtr domdisk)
@@ -13853,6 +13871,9 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi
return -1;
}
+ if (qemuDomainSnapshotPrepareDiskShared(snapdisk, domdisk) < 0)
+ return -1;
+
return 0;
}
@@ -13911,6 +13932,9 @@ qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk
return -1;
}
+ if (qemuDomainSnapshotPrepareDiskShared(snapdisk, domdisk) < 0)
+ return -1;
+
return 0;
}
@@ -1,97 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:16 +0200
Subject: [PATCH] virSecuritySELinuxSetFileconImpl: Drop @optional argument
The only thing that the @optional argument does is that it makes
the function return 1 instead of 0 if setting SELinux context
failed in a non-critical fashion. Drop the argument then and
return 1 in that case. This enables caller to learn if SELinux
context was set or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 34712a5e3b313fb9ca2223c54ea46f626429a6c4)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740506
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_selinux.c | 35 +++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 9857223bbf..766f334faf 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1257,12 +1257,27 @@ virSecuritySELinuxGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return 0;
}
-/* Attempt to change the label of PATH to TCON. If OPTIONAL is true,
- * return 1 if labelling was not possible. Otherwise, require a label
- * change, and return 0 for success, -1 for failure. */
+/**
+ * virSecuritySELinuxSetFileconImpl:
+ * @path: path to the file to set context on
+ * @tcon: target context to set
+ * @privileged: whether running as privileged user
+ *
+ * Set @tcon SELinux context on @path. If unable to do so, check SELinux
+ * configuration and produce sensible error message suggesting solution.
+ * It may happen that setting context fails but hypervisor will be able to
+ * open the @path successfully. This is because some file systems don't
+ * support SELinux, are RO, or the @path had the correct context from the
+ * start. If that is the case, a positive one is returned.
+ *
+ * Returns: 0 if context was set successfully
+ * 1 if setting the context failed in a non-critical fashion
+ * -1 in case of error
+ */
static int
-virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon,
- bool optional, bool privileged)
+virSecuritySELinuxSetFileconImpl(const char *path,
+ const char *tcon,
+ bool privileged)
{
security_context_t econ;
@@ -1278,7 +1293,7 @@ virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon,
if (STREQ(tcon, econ)) {
freecon(econ);
/* It's alright, there's nothing to change anyway. */
- return optional ? 1 : 0;
+ return 1;
}
freecon(econ);
}
@@ -1315,9 +1330,9 @@ virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon,
VIR_INFO("Setting security context '%s' on '%s' not supported",
tcon, path);
}
- if (optional)
- return 1;
}
+
+ return 1;
}
return 0;
}
@@ -1377,7 +1392,7 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
}
}
- if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0)
+ if (virSecuritySELinuxSetFileconImpl(path, tcon, privileged) < 0)
goto cleanup;
ret = 0;
@@ -1542,7 +1557,7 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
}
}
- if (virSecuritySELinuxSetFileconImpl(newpath, fcon, false, privileged) < 0)
+ if (virSecuritySELinuxSetFileconImpl(newpath, fcon, privileged) < 0)
goto cleanup;
ret = 0;
@@ -0,0 +1,34 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 15 Nov 2017 14:33:11 +0100
Subject: [PATCH] qemu: Disallow pivot of shared disks to unsupported storage
Pivoting to a unsupported storage type might break the assumption that
shared disks will not corrupt metadata.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
(cherry picked from commit 2b41c86294786c07f53afa633fe3dce703debc3c)
---
src/qemu/qemu_driver.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91119a494..208ccc9bc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16325,6 +16325,16 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
goto cleanup;
}
+ /* When pivoting to a shareable disk we need to make sure that the disk can
+ * be safely shared, since block copy might have changed the format. */
+ if (disk->src->shared && !disk->src->readonly &&
+ !qemuBlockStorageSourceSupportsConcurrentAccess(disk->mirror)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("can't pivot a shared disk to a storage volume not "
+ "supporting sharing"));
+ goto cleanup;
+ }
+
/* For active commit, the mirror is part of the already labeled
* chain. For blockcopy, we previously labeled only the top-level
* image; but if the user is reusing an external image that
@@ -1,106 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:17 +0200
Subject: [PATCH] security_selinux: Drop virSecuritySELinuxSetFileconOptional()
There is no real difference between
virSecuritySELinuxSetFilecon() and
virSecuritySELinuxSetFileconOptional(). Drop the latter in favour
of the former.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 079c1d6a291869ab4ee5d7f70bfd9a0f716c508e)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740506
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_selinux.c | 53 ++++++++++++++-------------------
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 766f334faf..87e1ba202d 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1419,15 +1419,6 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
}
-static int
-virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr,
- const char *path,
- const char *tcon,
- bool remember)
-{
- return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, true, remember);
-}
-
static int
virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
const char *path,
@@ -1884,28 +1875,28 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
parent_seclabel->label, remember);
} else if (!parent || parent == src) {
if (src->shared) {
- ret = virSecuritySELinuxSetFileconOptional(mgr,
- src->path,
- data->file_context,
- remember);
+ ret = virSecuritySELinuxSetFilecon(mgr,
+ src->path,
+ data->file_context,
+ remember);
} else if (src->readonly) {
- ret = virSecuritySELinuxSetFileconOptional(mgr,
- src->path,
- data->content_context,
- remember);
+ ret = virSecuritySELinuxSetFilecon(mgr,
+ src->path,
+ data->content_context,
+ remember);
} else if (secdef->imagelabel) {
- ret = virSecuritySELinuxSetFileconOptional(mgr,
- src->path,
- secdef->imagelabel,
- remember);
+ ret = virSecuritySELinuxSetFilecon(mgr,
+ src->path,
+ secdef->imagelabel,
+ remember);
} else {
ret = 0;
}
} else {
- ret = virSecuritySELinuxSetFileconOptional(mgr,
- src->path,
- data->content_context,
- remember);
+ ret = virSecuritySELinuxSetFilecon(mgr,
+ src->path,
+ data->content_context,
+ remember);
}
if (ret == 1 && !disk_seclabel) {
@@ -2045,14 +2036,14 @@ virSecuritySELinuxSetSCSILabel(virSCSIDevicePtr dev,
return 0;
if (virSCSIDeviceGetShareable(dev))
- return virSecuritySELinuxSetFileconOptional(mgr, file,
- data->file_context, true);
+ return virSecuritySELinuxSetFilecon(mgr, file,
+ data->file_context, true);
else if (virSCSIDeviceGetReadonly(dev))
- return virSecuritySELinuxSetFileconOptional(mgr, file,
- data->content_context, true);
+ return virSecuritySELinuxSetFilecon(mgr, file,
+ data->content_context, true);
else
- return virSecuritySELinuxSetFileconOptional(mgr, file,
- secdef->imagelabel, true);
+ return virSecuritySELinuxSetFilecon(mgr, file,
+ secdef->imagelabel, true);
}
static int
@@ -0,0 +1,126 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 15 Nov 2017 15:02:58 +0100
Subject: [PATCH] qemu: caps: Add capability for 'share-rw' disk option
'share-rw' for the disk device configures qemu to allow concurrent
access to the backing storage.
The capability is checked in various supported disk frontend buses since
it does not make sense to partially backport it.
(cherry picked from commit 860a3c4bea1d24773d8a495f213d5de3ac48a462)
---
src/qemu/qemu_capabilities.c | 14 ++++++++++++++
src/qemu/qemu_capabilities.h | 10 ++++++++++
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 +
5 files changed, 27 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e7ea6f47c..2de84715e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -439,6 +439,16 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"virtio-net.tx_queue_size",
"chardev-reconnect",
"virtio-gpu.max_outputs",
+
+ /* 270 */
+ "vxhs",
+ "virtio-blk.num-queues",
+ "machine.pseries.resize-hpt",
+ "vmcoreinfo",
+ "spapr-vty",
+
+ /* 275 */
+ "disk-share-rw",
);
@@ -1702,6 +1712,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
{ "event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX },
{ "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI },
{ "logical_block_size", QEMU_CAPS_BLOCKIO },
+ { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = {
@@ -1732,10 +1743,12 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVfioPCI[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSCSIDisk[] = {
{ "channel", QEMU_CAPS_SCSI_DISK_CHANNEL },
{ "wwn", QEMU_CAPS_SCSI_DISK_WWN },
+ { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIDEDrive[] = {
{ "wwn", QEMU_CAPS_IDE_DRIVE_WWN },
+ { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPiix4PM[] = {
@@ -1766,6 +1779,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PCIHost[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBStorage[] = {
{ "removable", QEMU_CAPS_USB_STORAGE_REMOVABLE },
+ { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsKVMPit[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f32687d4a..9c92d6b46 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -426,6 +426,16 @@ typedef enum {
QEMU_CAPS_CHARDEV_RECONNECT, /* -chardev reconnect */
QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, /* -device virtio-(vga|gpu-*),max-outputs= */
+ /* 270 */
+ QEMU_CAPS_VXHS, /* -drive file.driver=vxhs via query-qmp-schema */
+ QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES, /* virtio-blk-*.num-queues */
+ QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT, /* -machine pseries,resize-hpt */
+ QEMU_CAPS_DEVICE_VMCOREINFO, /* -device vmcoreinfo */
+ QEMU_CAPS_DEVICE_SPAPR_VTY, /* -device spapr-vty */
+
+ /* 275 */
+ QEMU_CAPS_DISK_SHARE_RW, /* share-rw=on for concurrent disk access */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
index a373a6db6..9551907c6 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
@@ -172,6 +172,7 @@
<flag name='vnc-multi-servers'/>
<flag name='chardev-reconnect'/>
<flag name='virtio-gpu.max_outputs'/>
+ <flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<package> (v2.9.0)</package>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index e80782cfb..0a6fbd077 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -137,6 +137,7 @@
<flag name='vnc-multi-servers'/>
<flag name='chardev-reconnect'/>
<flag name='virtio-gpu.max_outputs'/>
+ <flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
index 3641d0332..1294ebdb3 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
@@ -220,6 +220,7 @@
<flag name='vnc-multi-servers'/>
<flag name='chardev-reconnect'/>
<flag name='virtio-gpu.max_outputs'/>
+ <flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<package> (v2.9.0)</package>
@@ -1,126 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:18 +0200
Subject: [PATCH] security_selinux: Drop @optional from
_virSecuritySELinuxContextItem
Now, that we don't need to remember if setting context is
'optional' (the argument only made
virSecuritySELinuxSetFileconImpl() return a different success
code), we can drop it from the _virSecuritySELinuxContextItem
structure as we don't need to remember it in transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit b71d54f44707e80aae6fd01a766bb254882f5163)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740506
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_selinux.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 87e1ba202d..e5b55fccb4 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -81,7 +81,6 @@ typedef virSecuritySELinuxContextItem *virSecuritySELinuxContextItemPtr;
struct _virSecuritySELinuxContextItem {
char *path;
char *tcon;
- bool optional;
bool remember; /* Whether owner remembering should be done for @path/@src */
bool restore; /* Whether current operation is 'set' or 'restore' */
};
@@ -122,7 +121,6 @@ static int
virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list,
const char *path,
const char *tcon,
- bool optional,
bool remember,
bool restore)
{
@@ -135,7 +133,6 @@ virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list,
if (VIR_STRDUP(item->path, path) < 0 || VIR_STRDUP(item->tcon, tcon) < 0)
goto cleanup;
- item->optional = optional;
item->remember = remember;
item->restore = restore;
@@ -170,7 +167,6 @@ virSecuritySELinuxContextListFree(void *opaque)
* virSecuritySELinuxTransactionAppend:
* @path: Path to chown
* @tcon: target context
- * @optional: true if setting @tcon is optional
* @remember: if the original owner should be recorded/recalled
* @restore: if current operation is set or restore
*
@@ -187,7 +183,6 @@ virSecuritySELinuxContextListFree(void *opaque)
static int
virSecuritySELinuxTransactionAppend(const char *path,
const char *tcon,
- bool optional,
bool remember,
bool restore)
{
@@ -198,7 +193,7 @@ virSecuritySELinuxTransactionAppend(const char *path,
return 0;
if (virSecuritySELinuxContextListAppend(list, path, tcon,
- optional, remember, restore) < 0)
+ remember, restore) < 0)
return -1;
return 1;
@@ -234,7 +229,6 @@ virSecuritySELinuxRecallLabel(const char *path,
static int virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
const char *path,
const char *tcon,
- bool optional,
bool remember);
@@ -290,7 +284,6 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
rv = virSecuritySELinuxSetFileconHelper(list->manager,
item->path,
item->tcon,
- item->optional,
remember);
} else {
rv = virSecuritySELinuxRestoreFileLabel(list->manager,
@@ -1342,7 +1335,6 @@ static int
virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
const char *path,
const char *tcon,
- bool optional,
bool remember)
{
bool privileged = virSecurityManagerGetPrivileged(mgr);
@@ -1353,7 +1345,7 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
int ret = -1;
if ((rc = virSecuritySELinuxTransactionAppend(path, tcon,
- optional, remember, false)) < 0)
+ remember, false)) < 0)
return -1;
else if (rc > 0)
return 0;
@@ -1425,7 +1417,7 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
const char *tcon,
bool remember)
{
- return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, false, remember);
+ return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, remember);
}
static int
@@ -1512,7 +1504,7 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
}
if ((rc = virSecuritySELinuxTransactionAppend(path, NULL,
- false, recall, true)) < 0) {
+ recall, true)) < 0) {
goto cleanup;
} else if (rc > 0) {
ret = 0;
@@ -0,0 +1,133 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 15 Nov 2017 15:21:14 +0100
Subject: [PATCH] qemu: command: Mark <shared/> disks as such in qemu
Qemu has now an internal mechanism for locking images to fix specific
cases of disk corruption. This requires libvirt to mark the image as
shared so that qemu lifts certain restrictions.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378242
(cherry picked from commit 28907b0043fbf71085a798372ab9c816ba043b93)
---
src/qemu/qemu_command.c | 4 +++
.../qemuxml2argv-disk-drive-shared-locking.args | 32 +++++++++++++++++
.../qemuxml2argv-disk-drive-shared-locking.xml | 42 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
4 files changed, 80 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ae78cd17e..883525752 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2075,6 +2075,10 @@ qemuBuildDriveDevStr(const virDomainDef *def,
goto error;
}
+ if (disk->src->shared &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_SHARE_RW))
+ virBufferAddLit(&opt, ",share-rw=on");
+
if (!(drivealias = qemuAliasFromDisk(disk)))
goto error;
virBufferAsprintf(&opt, ",drive=%s,id=%s", drivealias, disk->info.alias);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args
new file mode 100644
index 000000000..cdf17f26d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/ide,format=raw,if=none,id=drive-ide0-0-0,cache=none \
+-device ide-drive,bus=ide.0,unit=0,share-rw=on,drive=drive-ide0-0-0,\
+id=ide0-0-0 \
+-drive file=/dev/scsi,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none \
+-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,share-rw=on,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-drive file=/dev/virtio,format=raw,if=none,id=drive-virtio-disk0,cache=none \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,share-rw=on,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml
new file mode 100644
index 000000000..dd48857a3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/ide'/>
+ <target dev='hda' bus='ide'/>
+ <shareable/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/scsi'/>
+ <target dev='sda' bus='scsi'/>
+ <shareable/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/virtio'/>
+ <target dev='vda' bus='virtio'/>
+ <shareable/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='scsi' index='0' model='virtio-scsi'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 93f892229..9585fdb70 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -896,6 +896,8 @@ mymain(void)
DO_TEST("disk-drive-shared",
QEMU_CAPS_DRIVE_SERIAL);
DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE);
+ DO_TEST("disk-drive-shared-locking",
+ QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DISK_SHARE_RW);
DO_TEST("disk-drive-error-policy-stop",
QEMU_CAPS_MONITOR_JSON);
DO_TEST("disk-drive-error-policy-enospace",
@@ -1,83 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:19 +0200
Subject: [PATCH] security_selinux: Drop virSecuritySELinuxSetFileconHelper
This function is no longer needed because after previous commits
it's just an alias to virSecuritySELinuxSetFilecon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit eaa2a064fa9a7cf6220349ac8706a22593cbf607)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740506
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_selinux.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e5b55fccb4..1df04d7358 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -226,10 +226,10 @@ virSecuritySELinuxRecallLabel(const char *path,
}
-static int virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
- const char *path,
- const char *tcon,
- bool remember);
+static int virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
+ const char *path,
+ const char *tcon,
+ bool remember);
static int virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
@@ -281,10 +281,10 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
const bool remember = item->remember && list->lock;
if (!item->restore) {
- rv = virSecuritySELinuxSetFileconHelper(list->manager,
- item->path,
- item->tcon,
- remember);
+ rv = virSecuritySELinuxSetFilecon(list->manager,
+ item->path,
+ item->tcon,
+ remember);
} else {
rv = virSecuritySELinuxRestoreFileLabel(list->manager,
item->path,
@@ -1332,10 +1332,10 @@ virSecuritySELinuxSetFileconImpl(const char *path,
static int
-virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
- const char *path,
- const char *tcon,
- bool remember)
+virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
+ const char *path,
+ const char *tcon,
+ bool remember)
{
bool privileged = virSecurityManagerGetPrivileged(mgr);
security_context_t econ = NULL;
@@ -1411,15 +1411,6 @@ virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
}
-static int
-virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
- const char *path,
- const char *tcon,
- bool remember)
-{
- return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, remember);
-}
-
static int
virSecuritySELinuxFSetFilecon(int fd, char *tcon)
{
@@ -1,49 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:20 +0200
Subject: [PATCH] security_selinux: Play nicely with network FS that only
emulates SELinux
There are some network file systems that do support XATTRs (e.g.
gluster via FUSE). And they appear to support SELinux too.
However, not really. Problem is, that it is impossible to change
SELinux label of a file stored there, and yet we claim success
(rightfully - hypervisor succeeds in opening the file). But this
creates a problem for us - from XATTR bookkeeping POV, we haven't
changed the label and thus if we remembered any label, we must
roll back and remove it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740506
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 8fe953805a535097cf3b819b3b68fbfb166efed4)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/security/security_selinux.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 1df04d7358..39d616ba44 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1384,12 +1384,18 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
}
}
- if (virSecuritySELinuxSetFileconImpl(path, tcon, privileged) < 0)
+ rc = virSecuritySELinuxSetFileconImpl(path, tcon, privileged);
+ if (rc < 0)
goto cleanup;
+ /* Do not try restoring the label if it was not changed
+ * (setting it failed in a non-critical fashion) */
+ if (rc == 0)
+ rollback = false;
+
ret = 0;
cleanup:
- if (ret < 0 && rollback) {
+ if (rollback) {
virErrorPtr origerr;
virErrorPreserveLast(&origerr);
@@ -1,42 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:21 +0200
Subject: [PATCH] qemu_blockjob: Print image path on failed security metadata
move too
When a block job is completed, the security image metadata are
moved to the new image. If this fails an warning is printed, but
the message contains only domain name and lacks image paths. Put
them both into the warning message.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 7f99d8a739c01a280ca0c173ecd6051a92f0c897)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/qemu/qemu_blockjob.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index a5b558b9ab..b3c0f784cd 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -584,8 +584,14 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver,
virDomainLockImageDetach(driver->lockManager, vm, disk->src);
/* Move secret driver metadata */
- if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror) < 0)
- VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);
+ if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror) < 0) {
+ VIR_WARN("Unable to move disk metadata on "
+ "vm %s from %s to %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ NULLSTR(disk->mirror->path),
+ disk->dst);
+ }
virObjectUnref(disk->src);
disk->src = disk->mirror;
@@ -1,79 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 2 Sep 2019 13:25:22 +0200
Subject: [PATCH] qemu_blockjob: Remove secdriver metadata more frequently
If a block job reaches failed/cancelled state, or is completed
without pivot then we must remove security driver metadata
associated to the backing chain so that we don't leave any
metadata behind.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 16fb3c8b83fb39f9931e942f7d1738ffe024d234)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Conflicts:
- src/qemu/qemu_blockjob.c:
qemuBlockJobProcessEventFailedActiveCommit() doesn't exist as
v5.7.0-rc2~2 isn't backported (blockdev is not going to be
enabled with RHEL-AV-8.1.0).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/qemu/qemu_blockjob.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index b3c0f784cd..beda9bd64e 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -597,7 +597,23 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver,
disk->src = disk->mirror;
} else {
if (disk->mirror) {
+ virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+ /* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+ for (n = disk->mirror; virStorageSourceIsBacking(n); n = n->backingStore) {
+ if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+ VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+ }
+ }
+
virObjectUnref(disk->mirror);
}
}
@@ -666,7 +682,23 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
case VIR_DOMAIN_BLOCK_JOB_FAILED:
case VIR_DOMAIN_BLOCK_JOB_CANCELED:
if (disk->mirror) {
+ virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+ /* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+ for (n = disk->mirror; virStorageSourceIsBacking(n); n = n->backingStore) {
+ if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+ VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+ }
+ }
+
virObjectUnref(disk->mirror);
disk->mirror = NULL;
}
@@ -1,192 +0,0 @@
From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>
Date: Fri, 9 Aug 2019 10:48:35 +0200
Subject: [PATCH] Revert "tpm: Check TPM XML device configuration changes after
edit"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Redefining a domain via virDomainDefineXML should not give different results
based on an already existing definition.
Also, there's a crasher somewhere in the code:
https://bugzilla.redhat.com/show_bug.cgi?id=1739338
This reverts commit 94b3aa55f83ada33a9fdda66068d58ef1a56c0a5
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit d8326cb8826170019c74018511d76c58665ab282)
---
src/conf/domain_conf.c | 56 ---------------------------------------
src/conf/domain_conf.h | 3 ---
src/libvirt_private.syms | 1 -
src/qemu/qemu_driver.c | 28 --------------------
src/qemu/qemu_extdevice.c | 2 +-
src/qemu/qemu_extdevice.h | 3 ---
6 files changed, 1 insertion(+), 92 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 441eb1a5a2..ac0561ddc5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31436,59 +31436,3 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics)
return true;
}
-
-
-static int
-virDomainCheckTPMChanges(virDomainDefPtr def,
- virDomainDefPtr newDef)
-{
- bool oldEnc, newEnc;
-
- if (!def->tpm)
- return 0;
-
- switch (def->tpm->type) {
- case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- if (virFileExists(def->tpm->data.emulator.storagepath)) {
- /* VM has been started */
- /* Once a VM was started with an encrypted state we allow
- * less configuration changes.
- */
- oldEnc = def->tpm->data.emulator.hassecretuuid;
- if (oldEnc && def->tpm->type != newDef->tpm->type) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Changing the type of TPM is not allowed"));
- return -1;
- }
- if (oldEnc && !newDef->tpm) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Removing an encrypted TPM is not allowed"));
- return -1;
- }
- newEnc = newDef->tpm->data.emulator.hassecretuuid;
- if (oldEnc != newEnc) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("TPM state encryption cannot be changed "
- "once VM was started"));
- return -1;
- }
- }
- break;
- case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- case VIR_DOMAIN_TPM_TYPE_LAST:
- break;
- }
-
- return 0;
-}
-
-
-int
-virDomainCheckDeviceChanges(virDomainDefPtr def,
- virDomainDefPtr newDef)
-{
- if (!def || !newDef)
- return 0;
-
- return virDomainCheckTPMChanges(def, newDef);
-}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8a4425df64..d5190a20db 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3638,6 +3638,3 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics);
bool
virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics);
-
-int
-virDomainCheckDeviceChanges(virDomainDefPtr def, virDomainDefPtr newDef);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9fd778272f..bbc5144aa2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -216,7 +216,6 @@ virDomainBootTypeFromString;
virDomainBootTypeToString;
virDomainCapabilitiesPolicyTypeToString;
virDomainCapsFeatureTypeToString;
-virDomainCheckDeviceChanges;
virDomainChrConsoleTargetTypeFromString;
virDomainChrConsoleTargetTypeToString;
virDomainChrDefForeach;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4ca3eb7bde..1ba6def419 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -53,7 +53,6 @@
#include "qemu_migration_params.h"
#include "qemu_blockjob.h"
#include "qemu_security.h"
-#include "qemu_extdevice.h"
#include "virerror.h"
#include "virlog.h"
@@ -7760,30 +7759,6 @@ qemuDomainCreate(virDomainPtr dom)
return qemuDomainCreateWithFlags(dom, 0);
}
-static int
-qemuDomainCheckDeviceChanges(virQEMUDriverPtr driver,
- virDomainDefPtr def)
-{
- virDomainObjPtr vm;
- int ret;
-
- vm = virDomainObjListFindByUUID(driver->domains, def->uuid);
- if (!vm)
- return 0;
-
- if (qemuExtDevicesInitPaths(driver, vm->def) < 0) {
- ret = -1;
- goto cleanup;
- }
-
- ret = virDomainCheckDeviceChanges(vm->def, def);
-
- cleanup:
- virDomainObjEndAPI(&vm);
-
- return ret;
-}
-
static virDomainPtr
qemuDomainDefineXMLFlags(virConnectPtr conn,
const char *xml,
@@ -7820,9 +7795,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
goto cleanup;
- if (qemuDomainCheckDeviceChanges(driver, def) < 0)
- goto cleanup;
-
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
0, &oldDef)))
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index af52466421..dc032aa60c 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -79,7 +79,7 @@ qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
* stored and we can remove directories and files in case of domain XML
* changes.
*/
-int
+static int
qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 5a53c79f38..039b3e60dd 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -54,6 +54,3 @@ bool qemuExtDevicesHasDevice(virDomainDefPtr def);
int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
virDomainDefPtr def,
virCgroupPtr cgroup);
-
-int qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
- virDomainDefPtr def);
@@ -0,0 +1,36 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 20 Dec 2017 12:58:36 +0100
Subject: [PATCH] util: probe: Add quiet versions of the "PROBE" macro
PROBE macro adds a logging entry, when used in places seeing a lot of
traffic this can cause a significant slowdown.
(cherry picked from commit f06e488d5484031a76e7ed231c8fef8fa1181d2c)
---
src/util/virprobe.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/util/virprobe.h b/src/util/virprobe.h
index 7565954af..bd8c32964 100644
--- a/src/util/virprobe.h
+++ b/src/util/virprobe.h
@@ -90,11 +90,19 @@
PROBE_EXPAND(LIBVIRT_ ## NAME, \
VIR_ADD_CASTS(__VA_ARGS__)); \
}
+
+# define PROBE_QUIET(NAME, FMT, ...) \
+ if (LIBVIRT_ ## NAME ## _ENABLED()) { \
+ PROBE_EXPAND(LIBVIRT_ ## NAME, \
+ VIR_ADD_CASTS(__VA_ARGS__)); \
+ }
# else
# define PROBE(NAME, FMT, ...) \
VIR_INFO_INT(&virLogSelf, \
__FILE__, __LINE__, __func__, \
#NAME ": " FMT, __VA_ARGS__);
+
+# define PROBE_QUIET(NAME, FMT, ...)
# endif
#endif /* __VIR_PROBE_H__ */
@@ -1,59 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 15 Aug 2019 21:52:28 -0400
Subject: [PATCH] network: fix crash during cleanup from failure to allocate
port
During networkPortCreateXML, if networkAllocatePort() failed,
networkReleasePort() would be called, which would (in the case of
network pools of macvtap passthrough devices) attempt to find the
allocated device by comparing port->plug.direct.linkdev to each device
in the pool. Since port->plug.direct.linkdev was still NULL, the
attempted strcmp would result in a SEGV.
Calling networkReleasePort() during error cleanup is something that
should only be done if networkAllocatePort() has already succeeded. It
turns out there is one other possible error exit from
networkPortCreateXML() that happens after networkAllocatePort() has
succeeded, so the code to call networkReleasePort() was just moved
down to there.
Resolves: https://bugzilla.redhat.com/1741390
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit dac697e8d7d6d9a607e61caeeec06b259edf513f)
---
src/network/bridge_driver.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 41fa89a4af..bc0f781c56 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5592,20 +5592,20 @@ networkPortCreateXML(virNetworkPtr net,
rc = networkNotifyPort(obj, portdef);
else
rc = networkAllocatePort(obj, portdef);
- if (rc < 0) {
+ if (rc < 0)
+ goto cleanup;
+
+ if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
virErrorPtr saved;
+
saved = virSaveLastError();
ignore_value(networkReleasePort(obj, portdef));
+ virNetworkPortDefFree(portdef);
virSetError(saved);
virFreeError(saved);
goto cleanup;
}
- if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
- virNetworkPortDefFree(portdef);
- goto cleanup;
- }
-
ret = virGetNetworkPort(net, portdef->uuid);
cleanup:
virNetworkObjEndAPI(&obj);
@@ -0,0 +1,49 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 20 Dec 2017 13:09:07 +0100
Subject: [PATCH] qemu: monitor: Decrease logging verbosity
The PROBE macro used in qemuMonitorIOProcess and the VIR_DEBUG message
in qemuMonitorJSONIOProcess create a lot of logging churn when debug
logging is enabled during monitor communication.
The messages logged from the PROBE macro are rather useless since they
are reporting the partial state of receiving the reply from qemu. The
actual full reply is still logged in qemuMonitorJSONIOProcessLine once
the full message is received.
(cherry picked from commit f10bb3347b43d900ff361cda5fe1996782284991)
---
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_monitor_json.c | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19082d8bf..3def28852 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -434,8 +434,8 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
# endif
#endif
- PROBE(QEMU_MONITOR_IO_PROCESS,
- "mon=%p buf=%s len=%zu", mon, mon->buffer, mon->bufferOffset);
+ PROBE_QUIET(QEMU_MONITOR_IO_PROCESS, "mon=%p buf=%s len=%zu",
+ mon, mon->buffer, mon->bufferOffset);
if (mon->json)
len = qemuMonitorJSONIOProcess(mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index df5fb7c8f..461aae089 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -259,7 +259,10 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon,
}
}
+#if DEBUG_IO
VIR_DEBUG("Total used %d bytes out of %zd available in buffer", used, len);
+#endif
+
return used;
}
@@ -1,57 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 15 Aug 2019 22:28:27 -0400
Subject: [PATCH] network: replace virSaveLastError() with
virErrorPreserveLast()
virErrorPreserveLast()/virErrorRestore() (added in commit 8333e7455
back in 2017), do a better better job of saving and restoring the last
libvirt error than virSaveLastError()/virErrorRestore() (they're
simpler, and they also save/restore the system errno).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 39de732aa728ae9b8a9414ad08b8d0ee7ed02732)
---
src/network/bridge_driver.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index bc0f781c56..c7e5151d09 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2987,13 +2987,12 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
cleanup:
if (ret < 0) {
+ virErrorPtr save_err;
+
+ virErrorPreserveLast(&save_err);
virNetworkObjUnsetDefTransient(obj);
- virErrorPtr save_err = virSaveLastError();
- int save_errno = errno;
networkShutdownNetwork(driver, obj);
- virSetError(save_err);
- virFreeError(save_err);
- errno = save_errno;
+ virErrorRestore(&save_err);
}
return ret;
}
@@ -5596,13 +5595,13 @@ networkPortCreateXML(virNetworkPtr net,
goto cleanup;
if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
- virErrorPtr saved;
+ virErrorPtr save_err;
- saved = virSaveLastError();
+ virErrorPreserveLast(&save_err);
ignore_value(networkReleasePort(obj, portdef));
virNetworkPortDefFree(portdef);
- virSetError(saved);
- virFreeError(saved);
+ virErrorRestore(&save_err);
+
goto cleanup;
}
@@ -0,0 +1,63 @@
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sat, 27 Jan 2018 23:43:58 +0100
Subject: [PATCH] virlog: determine the hostname on startup CVE-2018-6764
At later point it might not be possible or even safe to use getaddrinfo(). It
can in turn result in a load of NSS module.
Notably, on a LXC container startup we may find ourselves with the guest
filesystem already having replaced the host one. Loading a NSS module
from the guest tree would allow a malicous guest to escape the
confinement of its container environment because libvirt will not yet
have locked it down.
(cherry picked from commit 759b4d1b0fe5f4d84d98b99153dfa7ac289dd167)
---
src/util/virlog.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index d45a451a7..05e0e199e 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -64,6 +64,7 @@
VIR_LOG_INIT("util.log");
static regex_t *virLogRegex;
+static char *virLogHostname;
#define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -271,6 +272,12 @@ virLogOnceInit(void)
VIR_FREE(virLogRegex);
}
+ /* We get and remember the hostname early, because at later time
+ * it might not be possible to load NSS modules via getaddrinfo()
+ * (e.g. at container startup the host filesystem will not be
+ * accessible anymore. */
+ virLogHostname = virGetHostnameQuiet();
+
virLogUnlock();
return 0;
}
@@ -466,17 +473,14 @@ static int
virLogHostnameString(char **rawmsg,
char **msg)
{
- char *hostname = virGetHostnameQuiet();
char *hoststr;
- if (!hostname)
+ if (!virLogHostname)
return -1;
- if (virAsprintfQuiet(&hoststr, "hostname: %s", hostname) < 0) {
- VIR_FREE(hostname);
+ if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0) {
return -1;
}
- VIR_FREE(hostname);
if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
VIR_FREE(hoststr);
@@ -1,53 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 15 Aug 2019 16:34:21 -0400
Subject: [PATCH] access: fix incorrect addition to virAccessPermNetwork
Commit e69444e17 (first appeared in libvirt-5.5.0) added the new value
"VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS" to the virAccessPerNetwork
enum, and also the string "search_ports" to the VIR_ENUM_IMPL() macro
for that enum. Unfortunately, the enum value was added in the middle
of the list, while the string was added to the end of the
VIR_ENUM_IMPL().
This patch corrects that error by moving the new value to the end of
the enum definition, so that the order matches that of the string
list.
Resolves: https://bugzilla.redhat.com/1741428
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 8d6eaf5e099dab8400aa76bcc9a0ac74ff6f46e1)
---
src/access/viraccessperm.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index a42512d5e0..7480ee8c2f 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -410,18 +410,18 @@ typedef enum {
*/
VIR_ACCESS_PERM_NETWORK_START,
- /**
- * @desc: List network ports
- * @message: Listing network ports requires authorization
- */
- VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS,
-
/**
* @desc: Stop network
* @message: Stopping network requires authorization
*/
VIR_ACCESS_PERM_NETWORK_STOP,
+ /**
+ * @desc: List network ports
+ * @message: Listing network ports requires authorization
+ */
+ VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS,
+
VIR_ACCESS_PERM_NETWORK_LAST
} virAccessPermNetwork;
+27
View File
@@ -0,0 +1,27 @@
From: Andrea Bolognani <abologna@redhat.com>
Date: Wed, 7 Feb 2018 14:39:18 +0100
Subject: [PATCH] util: Fix syntax-check
Broken by 759b4d1b0fe5f4d84d98b99153dfa7ac289dd167.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
(cherry picked from commit 6ce3acc129bfdbe7fd02bcb8bbe8af6d13903684)
---
src/util/virlog.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 05e0e199e..056b53cda 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -478,9 +478,8 @@ virLogHostnameString(char **rawmsg,
if (!virLogHostname)
return -1;
- if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0) {
+ if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0)
return -1;
- }
if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
VIR_FREE(hoststr);
@@ -0,0 +1,121 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 12 Feb 2018 10:03:08 +0000
Subject: [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The fix for CVE-2018-6764 introduced a potential deadlock scenario
that gets triggered by the NSS module when virGetHostname() calls
getaddrinfo to resolve the hostname:
#0 0x00007f6e714b57e7 in futex_wait
#1 futex_wait_simple
#2 __pthread_once_slow
#3 0x00007f6e71d16e7d in virOnce
#4 0x00007f6e71d0997c in virLogInitialize
#5 0x00007f6e71d0a09a in virLogVMessage
#6 0x00007f6e71d09ffd in virLogMessage
#7 0x00007f6e71d0db22 in virObjectNew
#8 0x00007f6e71d0dbf1 in virObjectLockableNew
#9 0x00007f6e71d0d3e5 in virMacMapNew
#10 0x00007f6e71cdc50a in findLease
#11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r
#12 0x00007f6e724631fc in gaih_inet
#13 0x00007f6e72464697 in __GI_getaddrinfo
#14 0x00007f6e71d19e81 in virGetHostnameImpl
#15 0x00007f6e71d1a057 in virGetHostnameQuiet
#16 0x00007f6e71d09936 in virLogOnceInit
#17 0x00007f6e71d09952 in virLogOnce
#18 0x00007f6e714b5829 in __pthread_once_slow
#19 0x00007f6e71d16e7d in virOnce
#20 0x00007f6e71d0997c in virLogInitialize
#21 0x00007f6e71d0a09a in virLogVMessage
#22 0x00007f6e71d09ffd in virLogMessage
#23 0x00007f6e71d0db22 in virObjectNew
#24 0x00007f6e71d0dbf1 in virObjectLockableNew
#25 0x00007f6e71d0d3e5 in virMacMapNew
#26 0x00007f6e71cdc50a in findLease
#27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
#28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
#29 0x00007f6e7248f72f in __gethostbyname2_r
#30 0x00007f6e7248f494 in gethostbyname2
#31 0x000056348c30c36d in hosts_keys
#32 0x000056348c30b7d2 in main
Fortunately the extra stuff virGetHostname does is totally irrelevant to
the needs of the logging code, so we can just inline a call to the
native hostname() syscall directly.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit c2dc6698c88fb591639e542c8ecb0076c54f3dfb)
---
cfg.mk | 2 +-
src/util/virlog.c | 20 ++++++++++++++------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 56cb14bd9..a4131592c 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1158,7 +1158,7 @@ _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon|loggin
exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
(^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$)
-exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/virutil\.c$$
+exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/vir(util|log)\.c$$
exclude_file_name_regexp--sc_prohibit_internal_functions = \
^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 056b53cda..f76fc2caf 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -64,7 +64,7 @@
VIR_LOG_INIT("util.log");
static regex_t *virLogRegex;
-static char *virLogHostname;
+static char virLogHostname[HOST_NAME_MAX+1];
#define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -261,6 +261,8 @@ virLogPriorityString(virLogPriority lvl)
static int
virLogOnceInit(void)
{
+ int r;
+
if (virMutexInit(&virLogMutex) < 0)
return -1;
@@ -275,8 +277,17 @@ virLogOnceInit(void)
/* We get and remember the hostname early, because at later time
* it might not be possible to load NSS modules via getaddrinfo()
* (e.g. at container startup the host filesystem will not be
- * accessible anymore. */
- virLogHostname = virGetHostnameQuiet();
+ * accessible anymore.
+ * Must not use virGetHostname though as that causes re-entrancy
+ * problems if it triggers logging codepaths
+ */
+ r = gethostname(virLogHostname, sizeof(virLogHostname));
+ if (r == -1) {
+ ignore_value(virStrcpy(virLogHostname,
+ "(unknown)", sizeof(virLogHostname)));
+ } else {
+ NUL_TERMINATE(virLogHostname);
+ }
virLogUnlock();
return 0;
@@ -475,9 +486,6 @@ virLogHostnameString(char **rawmsg,
{
char *hoststr;
- if (!virLogHostname)
- return -1;
-
if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0)
return -1;
@@ -1,36 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 8 Aug 2019 13:42:24 +0100
Subject: [PATCH] network: fix ability to use openvswitch with vlans
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Use the correct enum constant when validating vlan usage.
This fixes a merge error in
commit 6cb0ec48bd95c95489a987e05a88e8bcf1f9109c
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Sep 3 17:34:22 2018 +0100
network: convert networkAllocateActualDevice to virNetworkPortDef
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 93c1d5fe7bb7a62ef884eb41b505b2809d1704b6)
---
src/network/bridge_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c7e5151d09..acaeb5c9a2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4813,7 +4813,7 @@ networkAllocatePort(virNetworkObjPtr obj,
if (!(port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI ||
(port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_DIRECT &&
port->plug.direct.mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) ||
- (port->plugtype == VIR_DOMAIN_NET_TYPE_BRIDGE &&
+ (port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE &&
port->virtPortProfile &&
port->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -0,0 +1,59 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 4 Jan 2018 11:11:53 +0100
Subject: [PATCH] qemuDomainAttachDeviceMknodHelper: Remove symlink before
creating it
https://bugzilla.redhat.com/show_bug.cgi?id=1528502
So imagine you have /dev/blah symlink which points to /dev/sda.
You attach /dev/blah as disk to your domain. Libvirt correctly
creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
However, then you detach the disk, change the symlink so that it
points to /dev/sdb and tries to attach the disk again. This time,
however, the attach fails (well, qemu attaches wrong disk)
because the code assumes that symlinks don't change. Well they
do.
This is inspired by test fix written by Eduardo Habkost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
(cherry picked from commit db98e7f67ea0d7699410f514f01947cef5128a6c)
---
src/qemu/qemu_domain.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 42d17c1b0..e0f4aaafa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8864,13 +8864,23 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
if (isLink) {
VIR_DEBUG("Creating symlink %s -> %s", data->file, data->target);
+
+ /* First, unlink the symlink target. Symlinks change and
+ * therefore we have no guarantees that pre-existing
+ * symlink is still valid. */
+ if (unlink(data->file) < 0 &&
+ errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Unable to remove symlink %s"),
+ data->file);
+ goto cleanup;
+ }
+
if (symlink(data->target, data->file) < 0) {
- if (errno != EEXIST) {
- virReportSystemError(errno,
- _("Unable to create symlink %s"),
- data->target);
- goto cleanup;
- }
+ virReportSystemError(errno,
+ _("Unable to create symlink %s (pointing to %s)"),
+ data->file, data->target);
+ goto cleanup;
} else {
delDevice = true;
}
@@ -1,127 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Sun, 11 Aug 2019 16:21:42 -0400
Subject: [PATCH] util: allow tap-based guest interfaces to have MAC address
prefix 0xFE
Back in July 2010, commit 6ea90b84 (meant to resolve
https://bugzilla.redhat.com/571991 ) added code to set the MAC address
of any tap device to the associated guest interface's MAC, but with
the first byte replaced with 0xFE. This was done in order to assure
that
1) the tap MAC and guest interface MAC were different (otherwise L2
forwarding through the tap would not work, and the kernel would
repeatedly issue a warning stating as much).
2) any bridge device that had one of these taps attached would *not*
take on the MAC of the tap (leading to network instability as
guests started and stopped)
A couple years later, https://bugzilla.redhat.com/798467 was filed,
complaining that a user could configure a tap-based guest interface to
have a MAC address that itself had a first byte of 0xFE, silently
(other than the kernel warning messages) resulting in a non-working
configuration. This was fixed by commit 5d571045, which logged an
error and failed the guest start / interface attach if the MAC's first
byte was 0xFE.
Although this restriction only reduces the potential pool of MAC
addresses from 2^46 (last two bits of byte 1 must be set to 10) by
2^32 (still 4 orders of magnitude larger than the entire IPv4 address
space), it also means that management software that autogenerates MAC
addresses must have special code to avoid an 0xFE prefix. Now after 7
years, someone has noticed this restriction and requested that we
remove it.
So instead of failing when 0xFE is found as the first byte, this patch
removes the restriction by just replacing the first byte in the tap
device MAC with 0xFA if the first byte in the guest interface is
0xFE. 0xFA is the next-highest value that still has 10 as the lowest
two bits, and still
2) meets the requirement of "tap MAC must be different from guest
interface MAC", and
3) is high enough that there should never be an issue of the attached
bridge device taking on the MAC of the tap.
The result is that *any* MAC can be chosen by management software
(although it would still not work correctly if a multicast MAC (lowest
bit of first byte set to 1) was chosen), but that's a different
issue).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com
(cherry picked from commit a60ee914009aa7f1a27fc0563337ded08b09247f)
---
src/qemu/qemu_interface.c | 11 ++++++++++-
src/util/virnetdevtap.c | 26 ++++++++++++--------------
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index c8effa68f4..72ed51cb1f 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -444,8 +444,17 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
}
virDomainAuditNetDevice(def, net, tunpath, true);
+
+ /* The tap device's MAC address cannot match the MAC address
+ * used by the guest. This results in "received packet on
+ * vnetX with own address as source address" error logs from
+ * the kernel.
+ */
virMacAddrSet(&tapmac, &net->mac);
- tapmac.addr[0] = 0xFE;
+ if (tapmac.addr[0] == 0xFE)
+ tapmac.addr[0] = 0xFA;
+ else
+ tapmac.addr[0] = 0xFE;
if (virNetDevSetMAC(net->ifname, &tapmac) < 0)
goto cleanup;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index b65c26bee1..4548b51b5b 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -668,7 +668,6 @@ int virNetDevTapCreateInBridgePort(const char *brname,
unsigned int flags)
{
virMacAddr tapmac;
- char macaddrstr[VIR_MAC_STRING_BUFLEN];
size_t i;
if (virNetDevTapCreate(ifname, tunpath, tapfd, tapfdSize, flags) < 0)
@@ -682,19 +681,18 @@ int virNetDevTapCreateInBridgePort(const char *brname,
*/
virMacAddrSet(&tapmac, macaddr);
if (!(flags & VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE)) {
- if (macaddr->addr[0] == 0xFE) {
- /* For normal use, the tap device's MAC address cannot
- * match the MAC address used by the guest. This results
- * in "received packet on vnetX with own address as source
- * address" error logs from the kernel.
- */
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unable to use MAC address starting with "
- "reserved value 0xFE - '%s' - "),
- virMacAddrFormat(macaddr, macaddrstr));
- goto error;
- }
- tapmac.addr[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
+ /* The tap device's MAC address cannot match the MAC address
+ * used by the guest. This results in "received packet on
+ * vnetX with own address as source address" error logs from
+ * the kernel. Making the tap address as high as possible
+ * discourages the bridge from using this tap's MAC as its own
+ * (a Linux host bridge will take on the lowest numbered MAC
+ * of all devices attached to it).
+ */
+ if (tapmac.addr[0] == 0xFE)
+ tapmac.addr[0] = 0xFA;
+ else
+ tapmac.addr[0] = 0xFE;
}
if (virNetDevSetMAC(*ifname, &tapmac) < 0)
@@ -0,0 +1,110 @@
From ce5aebeacd10a1c15cb3ee46a59c8b5ff235589e Mon Sep 17 00:00:00 2001
Message-Id: <ce5aebeacd10a1c15cb3ee46a59c8b5ff235589e.1530632895.git.crobinso@redhat.com>
From: Laine Stump <laine@laine.org>
Date: Wed, 25 Apr 2018 17:12:03 -0400
Subject: [PATCH] nwfilter: increase pcap buffer size to be compatible with
TPACKET_V3
When an nwfilter rule sets the parameter CTRL_IP_LEARNING to "dhcp",
this turns on the "dhcpsnoop" thread, which uses libpcap to monitor
traffic on the domain's tap device and extract the IP address from the
DHCP response.
If libpcap on the host is built with HAVE_TPACKET3 defined (to enable
support for TPACKET_V3), the dhcpsnoop code's initialization of the
libpcap socket would fail with the following error:
virNWFilterSnoopDHCPOpen:1134 : internal error: pcap_setfilter: can't remove kernel filter: Bad file descriptor
It turns out that this was because TPACKET_V3 requires a larger buffer
size than libvirt was setting (we were setting it to 128k). Changing
the buffer size to 256k eliminates the error, and the dhcpsnoop thread
once again works properly.
A fuller explanation of why TPACKET_V3 requires such a large buffer,
for future git spelunkers:
libpcap calls setsockopt(... SOL_PACKET, PACKET_RX_RING...) to setup a
ring buffer for receiving packets; two of the attributes sent to this
API are called tp_frame_size, and tp_frame_nr. If libpcap was built
with HAVE_TPACKET3 defined, tp_trame_size is set to MAXIMUM_SNAPLEN
(defined in libpcap sources as 262144) and tp_frame_nr is set to:
[the buffer size we set, i.e. PCAP_BUFFERSIZE i.e. 262144] / tp_frame_size.
So if PCAP_BUFFERSIZE < MAXIMUM_SNAPLEN, then tp_frame_nr (the number
of frames in the ring buffer) is 0, which is nonsensical. This same
value is later used as a multiplier to determine the size for a call
to malloc() (which would also fail).
(NB: if HAVE_TPACKET3 is *not* defined, then tp_frame_size is set to
the snaplen set by the user (in our case 576) plus a small amount to
account for ethernet headers, so 256k is far more than adequate)
Since the TPACKET_V3 code in libpcap actually reads multiple packets
into each frame, it's not a problem to have only a single frame
(especially when we are monitoring such infrequent traffic), so it's
okay to set this relatively small buffer size (in comparison to the
default, which is 2MB), which is important since every guest using
dhcp snooping in a nwfilter rule will hold 2 of these buffers for the
entire life of the guest.
Thanks to Christian Ehrhardt for discovering that buffer size was the
problem (this was not at all obvious from the error that was logged!)
Resolves: https://bugzilla.redhat.com/1547237
Fixes: https://bugs.launchpad.net/libvirt/+bug/1758037
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> (V1)
Reviewed-by: John Ferlan <jferlan@redhat.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/nwfilter/nwfilter_dhcpsnoop.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 6069e70460..50cfb944a2 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -256,10 +256,21 @@ struct _virNWFilterDHCPDecodeJob {
# define DHCP_BURST_INTERVAL_S 10 /* sec */
/*
- * libpcap 1.5 requires a 128kb buffer
- * 128 kb is bigger than (DHCP_PKT_BURST * PCAP_PBUFSIZE / 2)
+ * NB: Any libpcap built with HAVE_TPACKET3 will require
+ * PCAP_BUFFERSIZE to be at least 262144 (although
+ * pcap_set_buffer_size() with a lower value will succeed, and the
+ * error will only show up later when pcap_setfilter() is called).
+ *
+ * It is possible that in the future libpcap could increase the
+ * minimum size even further, but due to the fact that each guest
+ * using dhcp snooping keeps 2 pcap sockets open (and thus 2 buffers
+ * allocated) for the life of the guest, we want to minimize the
+ * length of the buffer, so instead of leaving it at the default size
+ * (2MB), we are setting it to the minimum viable size and including
+ * this clue in the source to help quickly resolve the problem when/if
+ * it reoccurs.
*/
-# define PCAP_BUFFERSIZE (128 * 1024)
+# define PCAP_BUFFERSIZE (256 * 1024)
# define MAX_QUEUED_JOBS (DHCP_PKT_BURST + 2 * DHCP_PKT_RATE)
@@ -1114,6 +1125,11 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac,
goto cleanup_nohandle;
}
+ /* IMPORTANT: If there is any failure of *any* pcap_* function
+ * during setup of the socket, look to the comment where
+ * PCAP_BUFFERSIZE is defined. It may be too small, even if the
+ * generated error doesn't imply that.
+ */
if (pcap_set_snaplen(handle, PCAP_PBUFSIZE) < 0 ||
pcap_set_buffer_size(handle, PCAP_BUFFERSIZE) < 0 ||
pcap_activate(handle) < 0) {
--
2.17.1
@@ -1,39 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 22 Aug 2019 14:48:46 +0100
Subject: [PATCH] remote: use Wants instead of Requires for libvirtd sockets
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To facilitate upgrades from earlier versions of libvirt which did not
use socket activation for libvirtd, we want to allow the libvirtd socket
units to be disabled (masked). This can only be supported if we use the
weaker Wants statement instead of Requires.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit ec7e31ed32065837e201cd3d8a3fb882e43b7fdb)
---
src/remote/libvirtd.service.in | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 3ddf0e229b..914bcbd5c4 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -2,9 +2,12 @@
Description=Virtualization daemon
Requires=virtlogd.socket
Requires=virtlockd.socket
-Requires=libvirtd.socket
-Requires=libvirtd-ro.socket
-Requires=libvirtd-admin.socket
+# Use Wants instead of Requires so that users
+# can disable these three .socket units to revert
+# to a traditional non-activation deployment setup
+Wants=libvirtd.socket
+Wants=libvirtd-ro.socket
+Wants=libvirtd-admin.socket
Wants=systemd-machined.service
Before=libvirt-guests.service
After=network.target
@@ -1,67 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 29 Aug 2019 12:04:42 +0100
Subject: [PATCH] remote: move timeout arg into sysconf file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We need to give users the ability to customize the length of the
shutdown timeout, or even disable timeouts entirely. Thus we must move
the timeout arg into the sysconf file, instead of the service unit.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 581767a98ab5f674ac335d6c270efa8576bfdfbf)
Conflicts:
src/remote/libvirtd.service.in - due to not having
5b816e16968ba02def56f067774ecd9a8c8d44d7 in rhel8
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741403
Message-Id: <20190829110444.12163-3-berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/remote/libvirtd.service.in | 6 +-----
src/remote/libvirtd.sysconf | 12 +++++++++---
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 914bcbd5c4..d5d98f0c12 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -24,11 +24,7 @@ Documentation=https://libvirt.org
[Service]
Type=notify
EnvironmentFile=-/etc/sysconfig/libvirtd
-# libvirtd.service is set to run on boot so that autostart of
-# VMs can be performed. We don't want it to stick around if
-# unused though, so we set a timeout. The socket activation
-# then ensures it gets started again if anything needs it
-ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS
+ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
ExecReload=/bin/kill -HUP $MAINPID
KillMode=process
Restart=on-failure
diff --git a/src/remote/libvirtd.sysconf b/src/remote/libvirtd.sysconf
index 5969518bf2..ee9db22bab 100644
--- a/src/remote/libvirtd.sysconf
+++ b/src/remote/libvirtd.sysconf
@@ -1,8 +1,14 @@
# Customizations for the libvirtd.service systemd unit
-# Listen for TCP/IP connections. This is not required if using systemd
-# socket activation.
-# NB. must setup TLS/SSL keys prior to using this
+# Default behaviour is for libvirtd.service to start on boot
+# so that VM autostart can be performed. We then want it to
+# shutdown again if nothing was started and rely on systemd
+# socket activation to start it again when some client app
+# connects.
+LIBVIRTD_ARGS="--timeout 120"
+
+# If systemd socket activation is disabled, then the following
+# can be used to listen on TCP/TLS sockets
#LIBVIRTD_ARGS="--listen"
# Override Kerberos service keytab for SASL/GSSAPI
@@ -1,99 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 29 Aug 2019 12:04:43 +0100
Subject: [PATCH] remote: forbid the --listen arg when systemd socket
activation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When using systemd socket activation the --listen arg has no
effect. This is confusing to users upgrading from previous versions of
libvirt as their config is silently ignored. Turn use of --listen into a
fatal error when sockets are passed from systemd.
This helps the admin discover the change in behaviour and thus decide
whether to stick with socket activation or revert to previous behaviour.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 3a6a725b8f575890ee6c151ad1f46ea0ceea1f3b)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741403
Message-Id: <20190829110444.12163-4-berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/remote/libvirtd.pod | 33 ++++++++++++++++++++++++++++++++-
src/remote/remote_daemon.c | 8 ++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/remote/libvirtd.pod b/src/remote/libvirtd.pod
index 4721e0f4ec..fa30d6a37a 100644
--- a/src/remote/libvirtd.pod
+++ b/src/remote/libvirtd.pod
@@ -30,6 +30,35 @@ and will be picked up automatically if their XML configuration has been
defined. Any guests whose XML configuration has not been defined will be lost
from the configuration.
+=head1 SYSTEM SOCKET ACTIVATION
+
+The B<libvirtd> daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+If the B<--listen> parameter is given, it will also listen on TCP/IP socket(s),
+according to the B<listen_tcp> and B<listen_tls> options in
+B</etc/libvirt/libvirtd.conf>
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened
+file descriptors. In this mode, it is not permitted to pass the B<--listen>
+parameter, and most of the socket related config options in
+B</etc/libvirt/libvirtd.conf> will no longer have any effect. To enable
+TCP or TLS sockets use either
+
+B<$ systemctl start libvirtd-tls.socket>
+
+Or
+
+B<$ systemctl start libvirtd-tcp.socket>
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+B<$ systemctl mask libvirtd.socket libvirtd-ro.socket \
+ libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket>
+
=head1 OPTIONS
=over
@@ -48,7 +77,9 @@ Use this configuration file, overriding the default value.
=item B<-l, --listen>
-Listen for TCP/IP connections.
+Listen for TCP/IP connections. This should not be set if using systemd
+socket activation. Instead activate the libvirtd-tls.socket or
+libvirtd-tcp.socket unit files.
=item B<-p, --pid-file> I<FILE>
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index d887b7abfb..b7ee1cf49b 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -391,6 +391,14 @@ daemonSetupNetworking(virNetServerPtr srv,
if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0)
return -1;
+#ifdef WITH_IP
+ if (act && ipsock) {
+ VIR_ERROR(_("--listen parameter not permitted with systemd activation "
+ "sockets, see 'man libvirtd' for further guidance"));
+ return -1;
+ }
+#endif /* ! WITH_IP */
+
if (config->unix_sock_group) {
if (virGetGroupID(config->unix_sock_group, &unix_sock_gid) < 0)
return ret;
@@ -1,86 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 29 Aug 2019 12:04:44 +0100
Subject: [PATCH] rpm: don't enable socket activation in upgrade if --listen
present
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Currently during RPM upgrade we restart libvirtd and unconditionally
enable use of systemd socket activation for the UNIX sockets.
If the user had previously given the --listen arg to libvirtd though,
this will no longer be honoured if socket activation is used.
We could start libvirtd-tcp.socket or libvirtd-tls.socket for this,
but mgmt tools like puppet/ansible might not be expecting this.
So for now we silently disable socket activation if we see --listen
was previously set on the host.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 66d04312d03c24c62bcf3b1dc2706f8775d4e2d3)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741403
Message-Id: <20190829110444.12163-5-berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
libvirt.spec.in | 44 +++++++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 045c0fed1a..58f7cae092 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1378,19 +1378,37 @@ fi
%posttrans daemon
if [ -f %{_localstatedir}/lib/rpm-state/libvirt/restart ]; then
- # Old libvirtd owns the sockets and will delete them on
- # shutdown. Can't use a try-restart as libvirtd will simply
- # own the sockets again when it comes back up. Thus we must
- # do this particular ordering
- /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1
- if test $? = 0 ; then
- /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
-
- /bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
- /bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
- /bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 || :
-
- /bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+ # See if user has previously modified their install to
+ # tell libvirtd to use --listen
+ grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1
+ if test $? = 0
+ then
+ # Then lets keep honouring --listen and *not* use
+ # systemd socket activation, because switching things
+ # might confuse mgmt tool like puppet/ansible that
+ # expect the old style libvirtd
+ /bin/systemctl mask libvirtd.socket >/dev/null 2>&1 || :
+ /bin/systemctl mask libvirtd-ro.socket >/dev/null 2>&1 || :
+ /bin/systemctl mask libvirtd-admin.socket >/dev/null 2>&1 || :
+ /bin/systemctl mask libvirtd-tls.socket >/dev/null 2>&1 || :
+ /bin/systemctl mask libvirtd-tcp.socket >/dev/null 2>&1 || :
+ else
+ # Old libvirtd owns the sockets and will delete them on
+ # shutdown. Can't use a try-restart as libvirtd will simply
+ # own the sockets again when it comes back up. Thus we must
+ # do this particular ordering, so that we get libvirtd
+ # running with socket activation in use
+ /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1
+ if test $? = 0
+ then
+ /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
+
+ /bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
+ /bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
+ /bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 || :
+
+ /bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+ fi
fi
fi
rm -rf %{_localstatedir}/lib/rpm-state/libvirt || :
@@ -1,27 +0,0 @@
From: Michael Chapman <mike@very.puzzling.org>
Date: Tue, 17 Sep 2019 17:03:57 +1000
Subject: [PATCH] remote: fix registration of TLS socket
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
(cherry picked from commit 522b3d2b24d0f7ac78dad442c990d4e34db0eaf2)
---
src/remote/remote_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index b7ee1cf49b..6b5a6c1523 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -473,7 +473,7 @@ daemonSetupNetworking(virNetServerPtr srv,
config->max_client_requests) < 0)
goto cleanup;
- if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "ip-tls")))) {
+ if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "libvirtd-tls.socket")))) {
virNetTLSContextPtr ctxt = NULL;
if (config->ca_file ||
@@ -1,34 +0,0 @@
From: Pavel Hrdina <phrdina@redhat.com>
Date: Thu, 5 Sep 2019 11:22:11 +0200
Subject: [PATCH] vircgroupv2: fix setting cpu.max period
When we set cpu.max period we need to parse the cpu.max file first as
it contains both quota and period values separated by space. When only
a single number is written to that file it will set quota. However,
in order to change period we need to write both values.
The code was prepared for that but mistakenly used new line to end the
string with the first value.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1749227
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
(cherry picked from commit 0bd4ad193d8ba7f0104f4739f19f2731e7cf9f56)
---
src/util/vircgroupv2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index e36c36685b..d8abe18943 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -1507,7 +1507,7 @@ virCgroupV2SetCpuCfsPeriod(virCgroupPtr group,
_("Invalid 'cpu.max' data."));
return -1;
}
- *tmp = '\n';
+ *tmp = '\0';
if (virAsprintf(&value, "%s %llu", str, cfs_period) < 0)
return -1;
@@ -1,55 +0,0 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 26 Sep 2019 15:00:55 -0400
Subject: [PATCH] vircgroupv2: Fix VM startup when legacy cgroups are defined
On Fedora 31, starting a 'mock' build alters /proc/$pid/cgroup,
probably due to usage of systemd-nspawn.
Before:
$ cat /proc/self/cgroup
0::/user.slice/user-1000.slice/...
After:
$ cat /proc/self/cgroup
1:name=systemd:/
0::/user.slice/user-1000.slice/...
The cgroupv2 code mishandles that first line in the second case, which
causes VM startup to fail with: Unable to read from
'/sys/fs/cgroup/machine/cgroup.controllers': No such file or directory
The kernel docs[1] say that the cgroupv2 path will always start with
'0::', which in the code here controllers="". Only set the v2 placement
path when we see that cgroup file entry.
[1] https://www.kernel.org/doc/html/v5.3/admin-guide/cgroup-v2.html#processes
https://bugzilla.redhat.com/show_bug.cgi?id=1751120
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/util/vircgroupv2.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index d8abe18943..acd48d1259 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -194,12 +194,16 @@ virCgroupV2DetectMounts(virCgroupPtr group,
static int
virCgroupV2DetectPlacement(virCgroupPtr group,
const char *path,
- const char *controllers ATTRIBUTE_UNUSED,
+ const char *controllers,
const char *selfpath)
{
if (group->unified.placement)
return 0;
+ /* controllers="" indicates the cgroupv2 controller path */
+ if (STRNEQ_NULLABLE(controllers, ""))
+ return 0;
+
/*
* selfpath == "/" + path="" -> "/"
* selfpath == "/libvirt.service" + path == "" -> "/libvirt.service"
@@ -1,72 +0,0 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 26 Sep 2019 15:25:52 -0400
Subject: [PATCH] vircgroup: Add some VIR_DEBUG statements
These helped with debugging
https://bugzilla.redhat.com/show_bug.cgi?id=1612383
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/util/vircgroup.c | 3 ++-
src/util/vircgroupv2.c | 9 +++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 825f62a97b..4f9d80666d 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1157,7 +1157,8 @@ virCgroupNewMachineSystemd(const char *name,
virCgroupFree(&init);
if (!path || STREQ(path, "/") || path[0] != '/') {
- VIR_DEBUG("Systemd didn't setup its controller");
+ VIR_DEBUG("Systemd didn't setup its controller, path=%s",
+ NULLSTR(path));
return -2;
}
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index acd48d1259..c536be0cd9 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -155,10 +155,14 @@ virCgroupV2CopyPlacement(virCgroupPtr group,
const char *path,
virCgroupPtr parent)
{
+ VIR_DEBUG("group=%p path=%s parent=%p", group, path, parent);
+
if (path[0] == '/') {
if (VIR_STRDUP(group->unified.placement, path) < 0)
return -1;
} else {
+ VIR_DEBUG("parent->unified.placement=%s", parent->unified.placement);
+
/*
* parent == "/" + path="" => "/"
* parent == "/libvirt.service" + path == "" => "/libvirt.service"
@@ -172,6 +176,7 @@ virCgroupV2CopyPlacement(virCgroupPtr group,
return -1;
}
+ VIR_DEBUG("set group->unified.placement=%s", group->unified.placement);
return 0;
}
@@ -200,6 +205,9 @@ virCgroupV2DetectPlacement(virCgroupPtr group,
if (group->unified.placement)
return 0;
+ VIR_DEBUG("group=%p path=%s controllers=%s selfpath=%s",
+ group, path, controllers, selfpath);
+
/* controllers="" indicates the cgroupv2 controller path */
if (STRNEQ_NULLABLE(controllers, ""))
return 0;
@@ -216,6 +224,7 @@ virCgroupV2DetectPlacement(virCgroupPtr group,
path) < 0)
return -1;
+ VIR_DEBUG("set group->unified.placement=%s", group->unified.placement);
return 0;
}
@@ -1,30 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 4 Oct 2019 16:33:37 +0200
Subject: [PATCH] qemu_driver: Fix comment of qemuStateCleanup()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The comment says that the function kills domains and networks.
This is obviously not the case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit e0b90162c94c45a9b0cf197e45b71f26036725d8)
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ba6def419..69416d6d1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1190,7 +1190,7 @@ qemuStateStop(void)
/**
* qemuStateCleanup:
*
- * Shutdown the QEMU daemon, it will stop all active domains and networks
+ * Release resources allocated by QEMU driver (no domain is shut off though)
*/
static int
qemuStateCleanup(void)
@@ -1,123 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 4 Oct 2019 16:57:04 +0200
Subject: [PATCH] driver: Introduce virDriverShouldAutostart()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Some of objects we manage can be autostarted on libvirtd startup
(e.g. domains, network, storage pools). The idea was that when
the host is started up these objects are started too without need
of user intervention. However, with the latest daemon split and
switch to socket activated, short lived daemons (we put --timeout
120 onto each daemon's command line) this doesn't do what we want
it to. The problem is not new though, we already had the session
daemon come and go and we circumvented this problem by
documenting it (see v4.10.0-92-g61b4e8aaf1). But now that we meet
the same problem at all fronts it's time to deal with it.
The solution implemented in this commit is to have a file (one
per each driver) that:
1) if doesn't exist, is created and autostart is allowed for
given driver,
2) if it does exist, then autostart is suppressed for given
driver.
All the files live in a location that doesn't survive host
reboots (/var/run/ for instance) and thus the file is
automatically not there on fresh host boot.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit ee16a195d97315ba3610b3640d347d3f4a358b55)
---
src/driver.c | 39 +++++++++++++++++++++++++++++++++++++++
src/driver.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 43 insertions(+)
diff --git a/src/driver.c b/src/driver.c
index 5e8f68f6df..471053686f 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -29,6 +29,7 @@
#include "virfile.h"
#include "virlog.h"
#include "virmodule.h"
+#include "virstring.h"
#include "virthread.h"
#include "configmake.h"
@@ -69,6 +70,44 @@ virDriverLoadModule(const char *name,
/* XXX unload modules, but we can't until we can unregister libvirt drivers */
+/**
+ * virDriverShouldAutostart:
+ * @dir: driver's run state directory (usually /var/run/libvirt/$driver)
+ * @autostart: whether driver should initiate autostart
+ *
+ * Automatic starting of libvirt's objects (e.g. domains, networks, storage
+ * pools, etc.) doesn't play nice with using '--timeout' on daemon's command
+ * line because the objects are attempted to autostart on every start of
+ * corresponding driver/daemon. To resolve this problem, a file is created in
+ * driver's private directory (which doesn't survive host's reboot) and thus
+ * autostart is attempted only once.
+ */
+int
+virDriverShouldAutostart(const char *dir,
+ bool *autostart)
+{
+ VIR_AUTOFREE(char *) path = NULL;
+
+ *autostart = false;
+
+ if (virAsprintf(&path, "%s/autostarted", dir) < 0)
+ return -1;
+
+ if (virFileExists(path)) {
+ VIR_DEBUG("Autostart file %s exists, skipping autostart", path);
+ return 0;
+ }
+
+ VIR_DEBUG("Autostart file %s does not exist, do autostart", path);
+ *autostart = true;
+
+ if (virFileTouch(path, 0600) < 0)
+ return -1;
+
+ return 0;
+}
+
+
virThreadLocal connectInterface;
virThreadLocal connectNetwork;
virThreadLocal connectNWFilter;
diff --git a/src/driver.h b/src/driver.h
index 898fb96df4..c52284498e 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -112,6 +112,9 @@ int virDriverLoadModule(const char *name,
const char *regfunc,
bool required);
+int virDriverShouldAutostart(const char *name,
+ bool *autostart);
+
virConnectPtr virGetConnectInterface(void);
virConnectPtr virGetConnectNetwork(void);
virConnectPtr virGetConnectNWFilter(void);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bbc5144aa2..c1c3974133 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1337,6 +1337,7 @@ virStreamClass;
# driver.h
+virDriverShouldAutostart;
virGetConnectInterface;
virGetConnectNetwork;
virGetConnectNodeDev;
@@ -1,187 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Sat, 5 Oct 2019 09:15:24 +0200
Subject: [PATCH] lib: autostart objects exactly once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
https://bugzilla.redhat.com/show_bug.cgi?id=1755303
With the recent work in daemon split and socket activation
daemons can come and go. They can and will be started many times
during a session which results in objects being autostarted
multiple times. This is not optimal. Use
virDriverShouldAutostart() to determine if autostart should be
done or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit bab464f8ea54d177e163f32c7c3476220694665c)
---
src/bhyve/bhyve_driver.c | 8 +++++++-
src/libxl/libxl_driver.c | 10 ++++++++--
src/lxc/lxc_driver.c | 7 ++++++-
src/network/bridge_driver.c | 12 +++++++++---
src/qemu/qemu_driver.c | 7 ++++++-
src/storage/storage_driver.c | 7 ++++++-
6 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 5387ac5570..12788155a8 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1218,6 +1218,8 @@ bhyveStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
+ bool autostart = true;
+
if (!privileged) {
VIR_INFO("Not running privileged, disabling driver");
return 0;
@@ -1301,7 +1303,11 @@ bhyveStateInitialize(bool privileged,
virBhyveProcessReconnectAll(bhyve_driver);
- bhyveAutostartDomains(bhyve_driver);
+ if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0)
+ goto cleanup;
+
+ if (autostart)
+ bhyveAutostartDomains(bhyve_driver);
return 0;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 492028c487..c1c94767c3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -655,6 +655,7 @@ libxlStateInitialize(bool privileged,
libxlDriverConfigPtr cfg;
char *driverConf = NULL;
char ebuf[1024];
+ bool autostart = true;
if (!libxlDriverShouldLoad(privileged))
return 0;
@@ -800,8 +801,13 @@ libxlStateInitialize(bool privileged,
NULL, NULL) < 0)
goto error;
- virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
- libxl_driver);
+ if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0)
+ goto error;
+
+ if (autostart) {
+ virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
+ libxl_driver);
+ }
virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
libxl_driver);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d0b6703101..24e6773f09 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1541,6 +1541,7 @@ static int lxcStateInitialize(bool privileged,
{
virCapsPtr caps = NULL;
virLXCDriverConfigPtr cfg = NULL;
+ bool autostart = true;
/* Check that the user is root, silently disable if not */
if (!privileged) {
@@ -1630,7 +1631,11 @@ static int lxcStateInitialize(bool privileged,
NULL, NULL) < 0)
goto cleanup;
- virLXCProcessAutostartAll(lxc_driver);
+ if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0)
+ goto cleanup;
+
+ if (autostart)
+ virLXCProcessAutostartAll(lxc_driver);
virObjectUnref(caps);
return 0;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index acaeb5c9a2..b830cdba42 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -716,6 +716,7 @@ networkStateInitialize(bool privileged,
int ret = -1;
char *configdir = NULL;
char *rundir = NULL;
+ bool autostart = true;
#ifdef WITH_FIREWALLD
DBusConnection *sysbus = NULL;
#endif
@@ -816,9 +817,14 @@ networkStateInitialize(bool privileged,
networkReloadFirewallRules(network_driver, true);
networkRefreshDaemons(network_driver);
- virNetworkObjListForEach(network_driver->networks,
- networkAutostartConfig,
- network_driver);
+ if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0)
+ goto error;
+
+ if (autostart) {
+ virNetworkObjListForEach(network_driver->networks,
+ networkAutostartConfig,
+ network_driver);
+ }
network_driver->networkEventState = virObjectEventStateNew();
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 69416d6d1a..5dd2616c7b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -726,6 +726,7 @@ qemuStateInitialize(bool privileged,
gid_t run_gid = -1;
char *hugepagePath = NULL;
char *memoryBackingPath = NULL;
+ bool autostart = true;
size_t i;
if (VIR_ALLOC(qemu_driver) < 0)
@@ -1071,7 +1072,11 @@ qemuStateInitialize(bool privileged,
qemuProcessReconnectAll(qemu_driver);
- qemuAutostartDomains(qemu_driver);
+ if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0)
+ goto error;
+
+ if (autostart)
+ qemuAutostartDomains(qemu_driver);
return 0;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 03ac6a6845..05192027d6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -253,6 +253,7 @@ storageStateInitialize(bool privileged,
{
VIR_AUTOFREE(char *) configdir = NULL;
VIR_AUTOFREE(char *) rundir = NULL;
+ bool autostart = true;
if (VIR_ALLOC(driver) < 0)
return -1;
@@ -314,7 +315,11 @@ storageStateInitialize(bool privileged,
storagePoolUpdateAllState();
- storageDriverAutostart();
+ if (virDriverShouldAutostart(driver->stateDir, &autostart) < 0)
+ goto error;
+
+ if (autostart)
+ storageDriverAutostart();
driver->storageEventState = virObjectEventStateNew();
@@ -1,34 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Sat, 5 Oct 2019 09:22:15 +0200
Subject: [PATCH] Revert "src: Document autostart for session demon"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 61b4e8aaf1bce07f282c152de556c3d6aa8d65be.
After previous commits this is no longer needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 897d8b34c8577d6e663b7b9bdb17ea4f5f1b83b9)
---
src/libvirt-domain.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2fe9bb8e91..d01fef358f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -6693,11 +6693,6 @@ virDomainCreateWithFiles(virDomainPtr domain, unsigned int nfiles,
* configured to be automatically started when the host
* machine boots.
*
- * Please note that this might result in unexpected behaviour if
- * used for some session URIs. Since the session daemon is started
- * with --timeout it comes and goes and as it does so it
- * autostarts domains which might have been shut off recently.
- *
* Returns -1 in case of error, 0 in case of success
*/
int
@@ -1,39 +0,0 @@
From: Laine Stump <laine@laine.org>
Date: Wed, 16 Oct 2019 14:06:54 -0400
Subject: [PATCH] util: allow sending mac addr to virNetNewLink without ifindex
Although until now, any use of the extra_args argument (a pointer to a
struct containing extra attributes to add the the RTM_NEWLINK message)
would always have the ifindex and mac set, so the code could assume it
was safe to add both to the message if extra_args != NULL. There is
now a use for setting a MAC address in the RTM_NEWLINK without setting
the ifindex, so we should check each of these separately.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit b596d6c106cd87d6d452c5dd4ad923a034544fbf)
---
src/util/virnetlink.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 7b3c6bf05f..394495ee2b 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -548,9 +548,14 @@ virNetlinkNewLink(const char *ifname,
NETLINK_MSG_NEST_END(nl_msg, linkinfo);
if (extra_args) {
- NETLINK_MSG_PUT(nl_msg, IFLA_LINK,
+ if (extra_args->ifindex) {
+ NETLINK_MSG_PUT(nl_msg, IFLA_LINK,
sizeof(uint32_t), extra_args->ifindex);
- NETLINK_MSG_PUT(nl_msg, IFLA_ADDRESS, VIR_MAC_BUFLEN, extra_args->mac);
+ }
+ if (extra_args->mac) {
+ NETLINK_MSG_PUT(nl_msg, IFLA_ADDRESS,
+ VIR_MAC_BUFLEN, extra_args->mac);
+ }
}
if (virNetlinkCommand(nl_msg, &resp, &buflen, 0, 0, NETLINK_ROUTE, 0) < 0)
@@ -1,213 +0,0 @@
From: Laine Stump <laine@laine.org>
Date: Thu, 17 Oct 2019 21:12:30 -0400
Subject: [PATCH] util: set bridge device MAC address explicitly during
virNetDevBridgeCreate
When libvirt first implemented a stable and configurable MAC address
for the bridges created for libvirt virtual networks (commit
5754dbd56d, in libvirt v0.8.8) most distro stable releases didn't
support explicitly setting the MAC address of a bridge; the bridge
just always assumed the lowest numbered MAC of all attached
interfaces. Because of this, we stabilized the bridge MAC address by
creating a "dummy" tap interface with a MAC address guaranteed to be
lower than any of the guest tap devices' MACs (which all started with
0xFE, so it's not difficult to do) and attached it to the bridge -
this was the inception of the "virbr0-nic" device that has confused so
many people over the years.
Even though the linux kernel had recently gained support for
explicitly setting a bridge MAC, we deemed it unnecessary to set the
MAC that way, because the other (indirect) method worked everywhere.
But recently there have been reports that the bridge MAC address was
not following the setting in the network config, and mismatched the
MAC of the dummy tap device (which was still correct). It turns out
that this is due to a change in systemd-242 that persists whatever MAC
address is set for a bridge when it's initially started. According to
the systemd NEWS file entry for version 242
(https://github.com/systemd/systemd/blob/master/NEWS):
"if a bridge interface is created without any slaves, and gains
a slave later, then now the bridge does not inherit slave's MAC."
This change was the result of:
https://github.com/systemd/systemd/issues/3374
(apparently if there is no MAC saved for a bridge by the name of a
bridge being created, the random MAC generated during creation is
saved, and then that same MAC is used to explicitly set the MAC each
time it is created). Once a bridge has an explicitly set MAC, the "use
the lowest numbered MAC of attached devices" rule is ignored, so our
dummy tap device is like the goggles - it does nothing! (well, almost).
We could whine about changes in default behavior, etc. etc., but
because the change was in response to actual user problems, that seems
likely a fruitless task. Fortunately, time has marched on, and even
distro releases that are old enough that they are no longer supported
by upstream libvirt (e.g. RHEL6) have support for explicitly setting a
bridge device MAC address, either during creation or with a separate
ioctl after creation, so we can now do that.
To enable explicitly setting the mac during bridge creation, we add a
mac arg to virNetDevBridgeCreate(). In the case of platforms where
the bridge is created with a netlink RTM_NEWLINK message, we just add
that mac to the message. For platforms that still use an ioctl (either
SIOCBRADDBR or SIOCIFCREATE2), we make a separate call to
virNetDevSetMAC() after creating the bridge.
(NB: I was unable to test the calling of virNetDevSetMAC() from the
SIOCIFCREATE2 (BSD) version of virNetDevBridgeCreate(); even though I
managed to get a FreeBSD system setup and libvirt built there, when I
tried to start the default network the SIOCIFCREATE2 ioctl itself
failed, so it never even got to the virNetDevSetMAC(). That leaves the
FreeBSD implementation untested.)
This makes the dummy tap pointless for purposes of setting the MAC
address, but it is still useful for IPv6 DAD initialization (which
apparently requires at least one interface to be attached to the
bridge and online), as well as for setting an initial MTU for the
bridge, so it hasn't been removed.
(NB: we can safely *always* call virNetDevBridgeCreate() with
&def->mac from the network driver because, in spite of the existence
of a "mac_specified" bool in the config suggesting that it may not
always be present, in reality a mac address will always be added to
any network that doesn't have one - this is guaranteed in all cases by
commit a47ae7c004)
https://bugzilla.redhat.com/show_bug.cgi?id=1760851
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 13ec827052fcd79a4350f499aab5f4aa20ea83fa)
---
src/network/bridge_driver.c | 2 +-
src/util/virnetdevbridge.c | 44 ++++++++++++++++++++++++++++++-------
src/util/virnetdevbridge.h | 3 ++-
3 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b830cdba42..d5c544f556 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2510,7 +2510,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
def->name);
return -1;
}
- if (virNetDevBridgeCreate(def->bridge) < 0)
+ if (virNetDevBridgeCreate(def->bridge, &def->mac) < 0)
return -1;
if (def->mac_specified) {
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 4f21a82fb7..368ce912d1 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -379,7 +379,8 @@ virNetDevBridgePortSetUnicastFlood(const char *brname ATTRIBUTE_UNUSED,
*/
#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
static int
-virNetDevBridgeCreateWithIoctl(const char *brname)
+virNetDevBridgeCreateWithIoctl(const char *brname,
+ const virMacAddr *mac)
{
VIR_AUTOCLOSE fd = -1;
@@ -392,22 +393,36 @@ virNetDevBridgeCreateWithIoctl(const char *brname)
return -1;
}
+ if (virNetDevSetMAC(brname, mac) < 0) {
+ virErrorPtr savederr;
+
+ virErrorPreserveLast(&savederr);
+ ignore_value(ioctl(fd, SIOCBRDELBR, brname));
+ virErrorRestore(&savederr);
+ return -1;
+ }
+
return 0;
}
#endif
#if defined(__linux__) && defined(HAVE_LIBNL)
int
-virNetDevBridgeCreate(const char *brname)
+virNetDevBridgeCreate(const char *brname,
+ const virMacAddr *mac)
{
/* use a netlink RTM_NEWLINK message to create the bridge */
int error = 0;
+ virNetlinkNewLinkData data = {
+ .mac = mac,
+ };
+
- if (virNetlinkNewLink(brname, "bridge", NULL, &error) < 0) {
+ if (virNetlinkNewLink(brname, "bridge", &data, &error) < 0) {
# if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
if (error == -EOPNOTSUPP) {
/* fallback to ioctl if netlink doesn't support creating bridges */
- return virNetDevBridgeCreateWithIoctl(brname);
+ return virNetDevBridgeCreateWithIoctl(brname, mac);
}
# endif
if (error < 0)
@@ -423,15 +438,17 @@ virNetDevBridgeCreate(const char *brname)
#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
int
-virNetDevBridgeCreate(const char *brname)
+virNetDevBridgeCreate(const char *brname,
+ const virMacAddr *mac)
{
- return virNetDevBridgeCreateWithIoctl(brname);
+ return virNetDevBridgeCreateWithIoctl(brname, mac);
}
#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFCREATE2)
int
-virNetDevBridgeCreate(const char *brname)
+virNetDevBridgeCreate(const char *brname,
+ const virMacAddr *mac)
{
struct ifreq ifr;
VIR_AUTOCLOSE s = -1;
@@ -448,10 +465,21 @@ virNetDevBridgeCreate(const char *brname)
if (virNetDevSetName(ifr.ifr_name, brname) == -1)
return -1;
+ if (virNetDevSetMAC(brname, mac) < 0) {
+ virErrorPtr savederr;
+
+ virErrorPreserveLast(&savederr);
+ ignore_value(virNetDevBridgeDelete(brname));
+ virErrorRestore(&savederr);
+ return -1;
+ }
+
return 0;
}
#else
-int virNetDevBridgeCreate(const char *brname)
+int
+virNetDevBridgeCreate(const char *brname,
+ const virMacAddr *mac G_GNUC_UNUSED)
{
virReportSystemError(ENOSYS,
_("Unable to create bridge %s"), brname);
diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h
index 8df5e51c2e..2a2bde8d0e 100644
--- a/src/util/virnetdevbridge.h
+++ b/src/util/virnetdevbridge.h
@@ -21,7 +21,8 @@
#include "internal.h"
#include "virmacaddr.h"
-int virNetDevBridgeCreate(const char *brname)
+int virNetDevBridgeCreate(const char *brname,
+ const virMacAddr *mac)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virNetDevBridgeDelete(const char *brname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
@@ -1,122 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 14 Oct 2019 16:37:03 +0200
Subject: [PATCH] virhostuptime: Add linux stub for musl
When we want to know the boot timestamp of the host, we can call
virHostGetBootTime(). Under the hood, it uses getutxid() which is
defined by POSIX and properly check for in configure. However,
musl took a path where it declares the function but instead of
providing any useful implementation it returns NULL meaning "no
record found". If that's the case, use our second best option -
/proc/uptime and a bit of maths.
https://bugzilla.redhat.com/show_bug.cgi?id=1760885
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
(cherry picked from commit 070d6969fe912b8f2e9aadee88c6bdda00a65f4f)
Signed-off-by: rpm-build <rpm-build>
Conflicts:
po/POTFILES.in
---
src/util/virhostuptime.c | 62 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 3 deletions(-)
diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
index 62b781acd5..a203961996 100644
--- a/src/util/virhostuptime.c
+++ b/src/util/virhostuptime.c
@@ -25,16 +25,68 @@
#endif
#include "virhostuptime.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
#include "virthread.h"
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("util.virhostuptime");
+
static unsigned long long bootTime;
static int bootTimeErrno;
static virOnceControl virHostGetBootTimeOnce = VIR_ONCE_CONTROL_INITIALIZER;
-#ifdef HAVE_GETUTXID
+#if defined(__linux__)
+# define UPTIME_FILE "/proc/uptime"
+static int
+virHostGetBootTimeProcfs(unsigned long long *btime)
+{
+ unsigned long long now;
+ double up;
+ VIR_AUTOFREE(char *) buf = NULL;
+ char *tmp;
+
+ if (virTimeMillisNow(&now) < 0)
+ return -errno;
+
+ /* 1KiB limit is more than enough. */
+ if (virFileReadAll(UPTIME_FILE, 1024, &buf) < 0)
+ return -errno;
+
+ /* buf contains two doubles now:
+ * $uptime $idle_time
+ * We're interested only in the first one */
+ if (!(tmp = strchr(buf, ' '))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("uptime file has unexpected format '%s'"),
+ buf);
+ return -EINVAL;
+ }
+
+ *tmp = '\0';
+
+ if (virStrToDouble(buf, NULL, &up) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse uptime value '%s'"),
+ buf);
+ return -EINVAL;
+ }
+
+ *btime = now / 1000 - up + 0.5;
+
+ return 0;
+}
+#endif /* defined(__linux__) */
+
+#if defined(HAVE_GETUTXID) || defined(__linux__)
static void
virHostGetBootTimeOnceInit(void)
{
+# ifdef HAVE_GETUTXID
struct utmpx id = {.ut_type = BOOT_TIME};
struct utmpx *res = NULL;
@@ -45,16 +97,20 @@ virHostGetBootTimeOnceInit(void)
}
endutxent();
+# endif /* HAVE_GETUTXID */
+
+ if (bootTimeErrno != 0 || bootTime == 0)
+ bootTimeErrno = -virHostGetBootTimeProcfs(&bootTime);
}
-#else /* !HAVE_GETUTXID */
+#else /* !defined(HAVE_GETUTXID) && !defined(__linux__) */
static void
virHostGetBootTimeOnceInit(void)
{
bootTimeErrno = ENOSYS;
}
-#endif /* HAVE_GETUTXID */
+#endif /* !defined(HAVE_GETUTXID) && !defined(__linux__) */
/**
* virHostGetBootTime:
@@ -1,31 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 14 Nov 2019 16:42:51 +0100
Subject: [PATCH] virhostuptime: Wrap virHostGetBootTimeProcfs() call in an
ifdef
The virHostGetBootTimeProcfs() function is defined only for Linux
and therefore it's only call should also be done if we're on
Linux.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 1b0de07f41bbe0d23ab96b604428ae55fd22aec0)
Signed-off-by: rpm-build <rpm-build>
---
src/util/virhostuptime.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
index a203961996..94004d05db 100644
--- a/src/util/virhostuptime.c
+++ b/src/util/virhostuptime.c
@@ -99,8 +99,10 @@ virHostGetBootTimeOnceInit(void)
endutxent();
# endif /* HAVE_GETUTXID */
+# ifdef __linux__
if (bootTimeErrno != 0 || bootTime == 0)
bootTimeErrno = -virHostGetBootTimeProcfs(&bootTime);
+# endif /* __linux__ */
}
#else /* !defined(HAVE_GETUTXID) && !defined(__linux__) */
@@ -1,71 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 19 Dec 2019 10:11:04 +0100
Subject: [PATCH] virhostuptime: Introduce virHostBootTimeInit()
The idea is to offer callers an init function that they can call
independently to ensure that the global variables get
initialized.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
(cherry picked from commit 9d69bc19b94544b06838b2e2895826991d107178)
Signed-off-by: rpm-build <rpm-build>
Conflicts:
src/util/virhostuptime.h
---
src/libvirt_private.syms | 1 +
src/util/virhostuptime.c | 12 +++++++++++-
src/util/virhostuptime.h | 3 +++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c1c3974133..fbd1c25597 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2136,6 +2136,7 @@ virHostMemSetParameters;
# util/virhostuptime.h
+virHostBootTimeInit;
virHostGetBootTime;
diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
index 94004d05db..d82e268264 100644
--- a/src/util/virhostuptime.c
+++ b/src/util/virhostuptime.c
@@ -126,7 +126,7 @@ virHostGetBootTimeOnceInit(void)
int
virHostGetBootTime(unsigned long long *when)
{
- if (virOnce(&virHostGetBootTimeOnce, virHostGetBootTimeOnceInit) < 0)
+ if (virHostBootTimeInit() < 0)
return -1;
if (bootTimeErrno) {
@@ -137,3 +137,13 @@ virHostGetBootTime(unsigned long long *when)
*when = bootTime;
return 0;
}
+
+
+int
+virHostBootTimeInit(void)
+{
+ if (virOnce(&virHostGetBootTimeOnce, virHostGetBootTimeOnceInit) < 0)
+ return -1;
+
+ return 0;
+}
diff --git a/src/util/virhostuptime.h b/src/util/virhostuptime.h
index 03c1517a64..0886b03767 100644
--- a/src/util/virhostuptime.h
+++ b/src/util/virhostuptime.h
@@ -25,3 +25,6 @@
int
virHostGetBootTime(unsigned long long *when)
ATTRIBUTE_NOINLINE;
+
+int
+virHostBootTimeInit(void);
@@ -1,54 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Tue, 17 Dec 2019 14:08:42 +0100
Subject: [PATCH] remote_daemon: Initialize host boot time global variable
This is not strictly needed, but it makes sure we initialize the
@bootTime global variable. Thing is, in order to validate XATTRs
and prune those set in some previous runs of the host, a
timestamp is recorded in XATTRs. The host boot time was unique
enough so it was chosen as the timestamp value. And to avoid
querying and parsing /proc/uptime every time, the query function
does that only once and stores the boot time in a global
variable. However, the only time the query function is called is
in a child process that does lock files and changes seclabels. So
effectively, we are doing exactly what we wanted to prevent from
happening.
The fix is simple, call the virHostBootTimeInit() function which
sets the global variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
(cherry picked from commit 35d603d5ae0abe91b8b48203c5b7051b1800682b)
Signed-off-by: rpm-build <rpm-build>
---
src/remote/remote_daemon.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 6b5a6c1523..43f9527405 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -57,6 +57,7 @@
#include "virgettext.h"
#include "util/virnetdevopenvswitch.h"
#include "virsystemd.h"
+#include "virhostuptime.h"
#include "driver.h"
@@ -1088,6 +1089,14 @@ int main(int argc, char **argv) {
exit(EXIT_FAILURE);
}
+ /* Let's try to initialize global variable that holds the host's boot time. */
+ if (virHostBootTimeInit() < 0) {
+ /* This is acceptable failure. Maybe we won't need the boot time
+ * anyway, and if we do, then virHostGetBootTime() returns an
+ * appropriate error. */
+ VIR_DEBUG("Ignoring failed boot time init");
+ }
+
daemonSetupNetDevOpenvswitch(config);
if (daemonSetupAccessManager(config) < 0) {
@@ -1,55 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 7 May 2020 22:32:59 -0400
Subject: [PATCH] network: make it safe to call networkSetupPrivateChains()
multiple times
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
networkSetupPrivateChains() is currently called only once per run of
libvirtd, so it can assume that errInitV4 and errInitV6 are empty/null
when it is called. In preparation for potentially calling this
function multiple times during one run, this patch moves the reset of
errInitV[46] to the top of the function, to assure no memory is
leaked.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit de110f110fb917a31b9f33ad8e4b3c1d3284766a)
---
src/network/bridge_driver_linux.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 35459c10d1..f4e4e2df76 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -48,6 +48,10 @@ static void networkSetupPrivateChains(void)
VIR_DEBUG("Setting up global firewall chains");
createdChains = false;
+ virFreeError(errInitV4);
+ errInitV4 = NULL;
+ virFreeError(errInitV6);
+ errInitV6 = NULL;
rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
if (rc < 0) {
@@ -56,8 +60,6 @@ static void networkSetupPrivateChains(void)
errInitV4 = virSaveLastError();
virResetLastError();
} else {
- virFreeError(errInitV4);
- errInitV4 = NULL;
if (rc) {
VIR_DEBUG("Created global IPv4 chains");
createdChains = true;
@@ -73,8 +75,6 @@ static void networkSetupPrivateChains(void)
errInitV6 = virSaveLastError();
virResetLastError();
} else {
- virFreeError(errInitV6);
- errInitV6 = NULL;
if (rc) {
VIR_DEBUG("Created global IPv6 chains");
createdChains = true;
@@ -1,265 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 7 May 2020 21:54:39 -0400
Subject: [PATCH] network: force re-creation of iptables private chains on
firewalld restart
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When firewalld is stopped, it removes *all* iptables rules and chains,
including those added by libvirt. Since restarting firewalld means
stopping and then starting it, any time it is restarted, libvirt needs
to recreate all the private iptables chains it uses, along with all
the rules it adds.
We already have code in place to call networkReloadFirewallRules() any
time we're notified of a firewalld start, and
networkReloadFirewallRules() will call
networkPreReloadFirewallRules(), which calls
networkSetupPrivateChains(); unfortunately that last call is called
using virOnce(), meaning that it will only be called the first time
through networkPreReloadFirewallRules() after libvirtd starts - so of
course when firewalld is later restarted, the call to
networkSetupPrivateChains() is skipped.
The neat and tidy way to fix this would be if there was a standard way
to reset a pthread_once_t object so that the next time virOnce was
called, it would think the function hadn't been called, and call it
again. Unfortunately, there isn't any official way of doing that (we
*could* just fill it with 0 and hope for the best, but that doesn't
seem very safe.
So instead, this patch just adds a static variable called
chainInitDone, which is set to true after networkSetupPrivateChains()
is called for the first time, and then during calls to
networkPreReloadFirewallRules(), if chainInitDone is set, we call
networkSetupPrivateChains() directly instead of via virOnce().
It may seem unsafe to directly call a function that is meant to be
called only once, but I think in this case we're safe - there's
nothing in the function that is inherently "once only" - it doesn't
initialize anything that can't safely be re-initialized (as long as
two threads don't try to do it at the same time), and it only happens
when responding to a dbus message that firewalld has been started (and
I don't think it's possible for us to be processing two of those at
once), and even then only if the initial call to the function has
already been completed (so we're safe if we receive a firewalld
restart call at a time when we haven't yet called it, or even if
another thread is already in the process of executing it. The only
problematic bit I can think of is if another thread is in the process
of adding an iptable rule at the time we're executing this function,
but 1) none of those threads will be trying to add chains, and 2) if
there was a concurrency problem with other threads adding iptables
rules while firewalld was being restarted, it would still be a problem
even without this change.
This is yet another patch that fixes an occurrence of this error:
COMMAND_FAILED: '/usr/sbin/iptables -w10 -w --table filter --insert LIBVIRT_INP --in-interface virbr0 --protocol tcp --destination-port 67 --jump ACCEPT' failed: iptables: No chain/target/match by that name.
In particular, this resolves: https://bugzilla.redhat.com/1813830
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit f5418b427e7d2f26803880309478de9103680826)
---
src/network/bridge_driver.c | 16 ++++---
src/network/bridge_driver_linux.c | 69 ++++++++++++++++++----------
src/network/bridge_driver_nop.c | 3 +-
src/network/bridge_driver_platform.h | 2 +-
4 files changed, 58 insertions(+), 32 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d5c544f556..981b4e4578 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -289,7 +289,9 @@ static int
networkShutdownNetworkExternal(virNetworkObjPtr obj);
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force);
static void
networkRefreshDaemons(virNetworkDriverStatePtr driver);
@@ -695,7 +697,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
if (reload) {
VIR_DEBUG("Reload in bridge_driver because of firewalld.");
- networkReloadFirewallRules(driver, false);
+ networkReloadFirewallRules(driver, false, true);
}
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -814,7 +816,7 @@ networkStateInitialize(bool privileged,
virNetworkObjListPrune(network_driver->networks,
VIR_CONNECT_LIST_NETWORKS_INACTIVE |
VIR_CONNECT_LIST_NETWORKS_TRANSIENT);
- networkReloadFirewallRules(network_driver, true);
+ networkReloadFirewallRules(network_driver, true, false);
networkRefreshDaemons(network_driver);
if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0)
@@ -884,7 +886,7 @@ networkStateReload(void)
network_driver->networkConfigDir,
network_driver->networkAutostartDir,
network_driver->xmlopt);
- networkReloadFirewallRules(network_driver, false);
+ networkReloadFirewallRules(network_driver, false, false);
networkRefreshDaemons(network_driver);
virNetworkObjListForEach(network_driver->networks,
networkAutostartConfig,
@@ -2267,14 +2269,16 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force)
{
VIR_INFO("Reloading iptables rules");
/* Ideally we'd not even register the driver when unprivilegd
* but until we untangle the virt driver that's not viable */
if (!driver->privileged)
return;
- networkPreReloadFirewallRules(driver, startup);
+ networkPreReloadFirewallRules(driver, startup, force);
virNetworkObjListForEach(driver->networks,
networkReloadFirewallRulesHelper,
NULL);
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index f4e4e2df76..f65928e556 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -36,11 +36,14 @@ VIR_LOG_INIT("network.bridge_driver_linux");
#define PROC_NET_ROUTE "/proc/net/route"
static virOnceControl createdOnce;
-static bool createdChains;
+static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */
+static bool createdChains; /* true iff networkSetupPrivateChains created chains during most recent call */
static virErrorPtr errInitV4;
static virErrorPtr errInitV6;
-/* Only call via virOnce */
+/* Usually only called via virOnce, but can also be called directly in
+ * response to firewalld reload (if chainInitDone == true)
+ */
static void networkSetupPrivateChains(void)
{
int rc;
@@ -82,6 +85,8 @@ static void networkSetupPrivateChains(void)
VIR_DEBUG("Global IPv6 chains already exist");
}
}
+
+ chainInitDone = true;
}
@@ -111,7 +116,10 @@ networkHasRunningNetworks(virNetworkDriverStatePtr driver)
}
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
+void
+networkPreReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force)
{
/*
* If there are any running networks, we need to
@@ -130,29 +138,42 @@ void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup
* of starting the network though as that makes them
* more likely to be seen by a human
*/
- if (!networkHasRunningNetworks(driver)) {
- VIR_DEBUG("Delayed global rule setup as no networks are running");
- return;
- }
+ if (chainInitDone && force) {
+ /* The Private chains have already been initialized once
+ * during this run of libvirtd, so 1) we can't do it again via
+ * virOnce(), and 2) we need to re-add the private chains even
+ * if there are currently no running networks, because the
+ * next time a network is started, libvirt will expect that
+ * the chains have already been added. So we call directly
+ * instead of via virOnce().
+ */
+ networkSetupPrivateChains();
- ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
+ } else {
+ if (!networkHasRunningNetworks(driver)) {
+ VIR_DEBUG("Delayed global rule setup as no networks are running");
+ return;
+ }
- /*
- * If this is initial startup, and we just created the
- * top level private chains we either
- *
- * - upgraded from old libvirt
- * - freshly booted from clean state
- *
- * In the first case we must delete the old rules from
- * the built-in chains, instead of our new private chains.
- * In the second case it doesn't matter, since no existing
- * rules will be present. Thus we can safely just tell it
- * to always delete from the builin chain
- */
- if (startup && createdChains) {
- VIR_DEBUG("Requesting cleanup of legacy firewall rules");
- iptablesSetDeletePrivate(false);
+ ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
+
+ /*
+ * If this is initial startup, and we just created the
+ * top level private chains we either
+ *
+ * - upgraded from old libvirt
+ * - freshly booted from clean state
+ *
+ * In the first case we must delete the old rules from
+ * the built-in chains, instead of our new private chains.
+ * In the second case it doesn't matter, since no existing
+ * rules will be present. Thus we can safely just tell it
+ * to always delete from the builin chain
+ */
+ if (startup && createdChains) {
+ VIR_DEBUG("Requesting cleanup of legacy firewall rules");
+ iptablesSetDeletePrivate(false);
+ }
}
}
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index ec7b1ed8b7..74a9b87158 100644
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -20,7 +20,8 @@
#include <config.h>
void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED,
- bool startup ATTRIBUTE_UNUSED)
+ bool startup ATTRIBUTE_UNUSED,
+ bool force ATTRIBUTE_UNUSED)
{
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 169417a6c0..48ab52c160 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -62,7 +62,7 @@ struct _virNetworkDriverState {
typedef struct _virNetworkDriverState virNetworkDriverState;
typedef virNetworkDriverState *virNetworkDriverStatePtr;
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
+void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup, bool force);
void networkPostReloadFirewallRules(bool startup);
int networkCheckRouteCollision(virNetworkDefPtr def);
@@ -1,85 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Fri, 1 May 2020 00:05:50 -0400
Subject: [PATCH] systemd: start libvirtd after firewalld/iptables services
When a system has enabled the iptables/ip6tables services rather than
firewalld, there is no explicit ordering of the start of those
services vs. libvirtd. This creates a problem when libvirtd.service is
started before ip[6]tables, as the latter, when it finally is started,
will remove all of the iptables rules that had previously been added
by libvirt, including the custom chains where libvirt's rules are
kept. This results in an error message similar to the following when a
user subsequently tries to start a new libvirt network:
"Error while activating network: Call to virNetworkCreate failed:
internal error: Failed to apply firewall rules
/usr/sbin/ip6tables -w --table filter --insert LIBVIRT_FWO \
--in-interface virbr2 --jump REJECT:
ip6tables: No chain/target/match by that name."
(Prior to logging this error, it also would have caused failure to
forward (or block) traffic in some cases, e.g. for guests on a NATed
network, since libvirt's rules to forward/block had all been deleted
and libvirt didn't know about it, so it couldn't fix the problem)
When this happens, the problem can be remedied by simply restarting
libvirtd.service (which has the side-effect of reloading all
libvirt-generated firewall rules)
Instead, we can just explicitly stating in the libvirtd.service file
that libvirtd.service should start after ip6tables.service and
ip6tables.service, eliminating the race condition that leads to the
error.
There is also nothing (that I can see) in the systemd .service files
to guarantee that firewalld.service will be started (if enabled) prior
to libvirtd.service. The same error scenario given above would occur
if libvirtd.service started before firewalld.service. Even before
that, though libvirtd would have detected that firewalld.service was
disabled, and then turn off all firewalld support. So, for example,
firewalld's libvirt zone wouldn't be used, and most likely traffic
from guests would therefore be blocked (all with no external
indication of the source of the problem other than a debug-level log
when libvirtd was started saying that firewalld wasn't in use); also
libvirtd wouldn't notice when firewalld reloaded its rules (which also
simultaneously deletes all of libvirt's rules).
I'm not aware of any reports that have been traced back to
libvirtd.service starting before firewalld.service, but have seen that
error reported multiple times, and also don't see an existing
dependency that would guarantee firewalld.service starts before
libvirtd.service, so it's possible it's been happening and we just
haven't gotten to the bottom of it.
This patch adds an After= line to the libvirtd.service file for each
of iptables.service, ip6tables.service, and firewalld.servicee, which
should guarantee that libvirtd.service isn't started until systemd has
started whichever of the others is enabled.
This race was diagnosed, and patch proposed, by Jason Montleon in
https://bugzilla.redhat.com/1723698 . At the time (April 2019) danpb
agreed with him that this change to libvirtd.service was a reasonable
thing to do, but I guess everyone thought someone else was going to
post a patch, so in the end nobody did.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 0756415f147dda15a417bd79eef9a62027d176e6)
---
src/remote/libvirtd.service.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index d5d98f0c12..ada5be62e6 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -11,6 +11,9 @@ Wants=libvirtd-admin.socket
Wants=systemd-machined.service
Before=libvirt-guests.service
After=network.target
+After=firewalld.service
+After=iptables.service
+After=ip6tables.service
After=dbus.service
After=iscsid.service
After=apparmor.service
@@ -1,41 +0,0 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 19 Feb 2020 08:40:59 +0100
Subject: [PATCH] qemuDomainGetStatsIOThread: Don't leak array with 0 iothreads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
qemuMonitorGetIOThreads returns a NULL-terminated list even when 0
iothreads are present. The caller didn't perform cleanup if there were 0
iothreads leaking the array.
https://bugzilla.redhat.com/show_bug.cgi?id=1804548
Fixes: d1eac92784573559b6fd56836e33b215c89308e3
Reported-by: Jing Yan <jiyan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
(cherry picked from commit 9bf9e0ae6af38c806f4672ca7b12a6b38d5a9581)
---
src/qemu/qemu_driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5dd2616c7b..331e53b971 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21632,8 +21632,12 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver,
if ((niothreads = qemuDomainGetIOThreadsMon(driver, dom, &iothreads)) < 0)
return -1;
- if (niothreads == 0)
- return 0;
+ /* qemuDomainGetIOThreadsMon returns a NULL-terminated list, so we must free
+ * it even if it returns 0 */
+ if (niothreads == 0) {
+ ret = 0;
+ goto cleanup;
+ }
QEMU_ADD_COUNT_PARAM(record, maxparams, "iothread", niothreads);
@@ -1,47 +0,0 @@
From: Yi Li <yili@winhong.com>
Date: Sat, 21 Dec 2019 08:33:33 +0800
Subject: [PATCH] storage: Fix daemon crash on lookup storagepool by targetpath
Causing a crash when storagePoolLookupByTargetPath beacuse of
Some types of storage pool have no target elements.
Use STREQ_NULLABLE instead of STREQ
Avoids segfaults when using NULL arguments.
Core was generated by `/usr/sbin/libvirtd'.
Program terminated with signal 11, Segmentation fault.
(gdb) bt
0 0x0000ffff9e951388 in strcmp () from /lib64/libc.so.6
1 0x0000ffff92103e9c in storagePoolLookupByTargetPathCallback (
obj=0xffff7009aab0, opaque=0xffff801058b0) at storage/storage_driver.c:1649
2 0x0000ffff9f2c52a4 in virStoragePoolObjListSearchCb (
payload=0xffff801058b0, name=<optimized out>, opaque=<optimized out>)
at conf/virstorageobj.c:476
3 0x0000ffff9f1f2f7c in virHashSearch (ctable=0xffff800f4f60,
iter=iter@entry=0xffff9f2c5278 <virStoragePoolObjListSearchCb>,
data=data@entry=0xffff95af7488, name=name@entry=0x0) at util/virhash.c:696
4 0x0000ffff9f2c64f0 in virStoragePoolObjListSearch (pools=0xffff800f2ce0,
searcher=searcher@entry=0xffff92103e68 <storagePoolLookupByTargetPathCallback>,
opaque=<optimized out>) at conf/virstorageobj.c:505
5 0x0000ffff92101f54 in storagePoolLookupByTargetPath (conn=0xffff5c0009f0,
path=0xffff7009a850 "/vms/images") at storage/storage_driver.c:1672
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Yi Li <yili@winhong.com>
(cherry picked from commit dfff16a7c261f8d28e3abe60a47165f845fa952f)
---
src/storage/storage_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 05192027d6..d878b0be92 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1675,7 +1675,7 @@ storagePoolLookupByTargetPathCallback(virStoragePoolObjPtr obj,
return false;
def = virStoragePoolObjGetDef(obj);
- return STREQ(path, def->target.path);
+ return STREQ_NULLABLE(path, def->target.path);
}
@@ -1,168 +0,0 @@
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Thu, 5 Dec 2019 10:08:52 -0600
Subject: [PATCH] qemu: don't hold both jobs for suspend
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding a
monitor job while we're querying the agent, we open ourselves up to a
DoS.
So split the function up a bit to only hold the monitor job while
querying qemu for whether the domain supports suspend. Then acquire only
an agent job while issuing the agent suspend command.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit a663a860819287e041c3de672aad1d8543098ecc)
---
src/qemu/qemu_driver.c | 94 ++++++++++++++++++++++++++----------------
1 file changed, 59 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 331e53b971..96c0b3505a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19867,6 +19867,59 @@ qemuDomainProbeQMPCurrentMachine(virQEMUDriverPtr driver,
}
+/* returns -1 on error, or if query is not supported, 0 if query was successful */
+static int
+qemuDomainQueryWakeupSuspendSupport(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ bool *wakeupSupported)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret = -1;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
+ return -1;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ return -1;
+
+ if ((ret = virDomainObjCheckActive(vm)) < 0)
+ goto endjob;
+
+ ret = qemuDomainProbeQMPCurrentMachine(driver, vm, wakeupSupported);
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+ return ret;
+}
+
+
+static int
+qemuDomainPMSuspendAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int target)
+{
+ qemuAgentPtr agent;
+ int ret = -1;
+
+ if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0)
+ return -1;
+
+ if ((ret = virDomainObjCheckActive(vm)) < 0)
+ goto endjob;
+
+ if (!qemuDomainAgentAvailable(vm, true))
+ goto endjob;
+
+ agent = qemuDomainObjEnterAgent(vm);
+ ret = qemuAgentSuspend(agent, target);
+ qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+ qemuDomainObjEndAgentJob(vm);
+ return ret;
+}
+
+
static int
qemuDomainPMSuspendForDuration(virDomainPtr dom,
unsigned int target,
@@ -19874,11 +19927,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
unsigned int flags)
{
virQEMUDriverPtr driver = dom->conn->privateData;
- qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
- qemuAgentPtr agent;
- qemuDomainJob job = QEMU_JOB_NONE;
int ret = -1;
+ bool wakeupSupported;
virCheckFlags(0, -1);
@@ -19903,17 +19954,6 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- priv = vm->privateData;
-
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
- job = QEMU_JOB_MODIFY;
-
- if (qemuDomainObjBeginJobWithAgent(driver, vm, job, QEMU_AGENT_JOB_MODIFY) < 0)
- goto cleanup;
-
- if (virDomainObjCheckActive(vm) < 0)
- goto endjob;
-
/*
* The case we want to handle here is when QEMU has the API (i.e.
* QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere
@@ -19921,16 +19961,11 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
* that don't know about this cap, will keep their old behavior of
* suspending 'in the dark'.
*/
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) {
- bool wakeupSupported;
-
- if (qemuDomainProbeQMPCurrentMachine(driver, vm, &wakeupSupported) < 0)
- goto endjob;
-
+ if (qemuDomainQueryWakeupSuspendSupport(driver, vm, &wakeupSupported) == 0) {
if (!wakeupSupported) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("Domain does not have suspend support"));
- goto endjob;
+ goto cleanup;
}
}
@@ -19940,29 +19975,18 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
target == VIR_NODE_SUSPEND_TARGET_HYBRID)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("S3 state is disabled for this domain"));
- goto endjob;
+ goto cleanup;
}
if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO &&
target == VIR_NODE_SUSPEND_TARGET_DISK) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("S4 state is disabled for this domain"));
- goto endjob;
+ goto cleanup;
}
}
- if (!qemuDomainAgentAvailable(vm, true))
- goto endjob;
-
- agent = qemuDomainObjEnterAgent(vm);
- ret = qemuAgentSuspend(agent, target);
- qemuDomainObjExitAgent(vm, agent);
-
- endjob:
- if (job)
- qemuDomainObjEndJobWithAgent(driver, vm);
- else
- qemuDomainObjEndAgentJob(vm);
+ ret = qemuDomainPMSuspendAgent(driver, vm, target);
cleanup:
virDomainObjEndAPI(&vm);
@@ -0,0 +1,74 @@
From e18672ce9a5fff383992fd6e842d1cbe85c141ea Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 12 Dec 2017 16:23:40 +0100
Subject: [PATCH 10/19] util: add virFileReadHeaderQuiet wrapper around
virFileReadHeaderFD
CVE-2017-5715
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 19 +++++++++++++++++++
src/util/virfile.h | 2 ++
3 files changed, 22 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f30a04b145..29b73fa046 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1703,6 +1703,7 @@ virFileReadAll;
virFileReadAllQuiet;
virFileReadBufQuiet;
virFileReadHeaderFD;
+virFileReadHeaderQuiet;
virFileReadLimFD;
virFileReadLink;
virFileReadValueBitmap;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 2f28e83f44..269db995ff 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1356,6 +1356,25 @@ virFileReadHeaderFD(int fd, int maxlen, char **buf)
}
+int
+virFileReadHeaderQuiet(const char *path,
+ int maxlen,
+ char **buf)
+{
+ int fd;
+ int len;
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ return -1;
+
+ len = virFileReadHeaderFD(fd, maxlen, buf);
+ VIR_FORCE_CLOSE(fd);
+
+ return len;
+}
+
+
/* A wrapper around saferead_lim that maps a failure due to
exceeding the maximum size limitation to EOVERFLOW. */
int
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 57ceb80721..657e7216fb 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -129,6 +129,8 @@ int virFileDeleteTree(const char *dir);
int virFileReadHeaderFD(int fd, int maxlen, char **buf)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);
+int virFileReadHeaderQuiet(const char *path, int maxlen, char **buf)
+ ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
int virFileReadLimFD(int fd, int maxlen, char **buf)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);
int virFileReadAll(const char *path, int maxlen, char **buf)
--
2.17.0
@@ -0,0 +1,36 @@
From a84e70ad247da5d3ad13615efd70b91951392aa1 Mon Sep 17 00:00:00 2001
From: Jiri Denemark <jdenemar@redhat.com>
Date: Fri, 5 Jan 2018 17:43:03 +0100
Subject: [PATCH 12/19] cpu_x86: Copy CPU signature from ancestor
When specifying a new CPU model in cpu_map.xml as an extension to an
existing model, we forgot to copy the signature (family + model) from
the original CPU model.
We don't use this way of specifying CPU models, but it's still supported
and it becomes useful when someone wants to quickly hack up a CPU model
for testing or when creating additional variants of existing models to
help with fixing some spectral issues.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
(cherry picked from commit b427cf4831d0ea7aac9dd1a3aa7682478356a483)
---
src/cpu/cpu_x86.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 2864454211..3b7a6f95fe 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1206,6 +1206,7 @@ x86ModelParse(xmlXPathContextPtr ctxt,
VIR_FREE(name);
model->vendor = ancestor->vendor;
+ model->signature = ancestor->signature;
if (x86DataCopy(&model->data, &ancestor->data) < 0)
goto error;
}
--
2.17.0
@@ -0,0 +1,97 @@
From de12d97c029d6644bb42afaa38410c4263bef41f Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 12 Dec 2017 16:23:41 +0100
Subject: [PATCH 13/19] util: introduce virHostCPUGetMicrocodeVersion
This new API reads host's CPU microcode version from /proc/cpuinfo.
Unfortunately, there is no other way of reading microcode version which
would be usable from both system and session daemon.
CVE-2017-5715
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virhostcpu.c | 43 ++++++++++++++++++++++++++++++++++++++++
src/util/virhostcpu.h | 2 ++
3 files changed, 46 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 29b73fa046..0ecd58a12c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1811,6 +1811,7 @@ virHostCPUGetCount;
virHostCPUGetInfo;
virHostCPUGetKVMMaxVCPUs;
virHostCPUGetMap;
+virHostCPUGetMicrocodeVersion;
virHostCPUGetOnline;
virHostCPUGetOnlineBitmap;
virHostCPUGetPresentBitmap;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c485a97211..713fdec553 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void)
return -1;
}
#endif /* HAVE_LINUX_KVM_H */
+
+
+#ifdef __linux__
+
+unsigned int
+virHostCPUGetMicrocodeVersion(void)
+{
+ char *outbuf = NULL;
+ char *cur;
+ unsigned int version = 0;
+
+ if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {
+ char ebuf[1024];
+ VIR_DEBUG("Failed to read microcode version from %s: %s",
+ CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
+ return 0;
+ }
+
+ /* Account for format 'microcode : XXXX'*/
+ if (!(cur = strstr(outbuf, "microcode")) ||
+ !(cur = strchr(cur, ':')))
+ goto cleanup;
+ cur++;
+
+ /* Linux places the microcode revision in a 32-bit integer, so
+ * ui is fine for us too. */
+ if (virStrToLong_ui(cur, &cur, 0, &version) < 0)
+ goto cleanup;
+
+ cleanup:
+ VIR_FREE(outbuf);
+ return version;
+}
+
+#else
+
+unsigned int
+virHostCPUGetMicrocodeVersion(void)
+{
+ return 0;
+}
+
+#endif
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index 67033de842..f9f3359288 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -66,4 +66,6 @@ virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu);
int virHostCPUGetOnline(unsigned int cpu, bool *online);
+unsigned int virHostCPUGetMicrocodeVersion(void);
+
#endif /* __VIR_HOSTCPU_H__*/
--
2.17.0
@@ -0,0 +1,51 @@
From a0ad8c160ed81417e4d5b46adf3118df1b6b1b77 Mon Sep 17 00:00:00 2001
From: Jiri Denemark <jdenemar@redhat.com>
Date: Wed, 13 Dec 2017 22:30:31 +0100
Subject: [PATCH 14/19] cpu_x86: Rename virCPUx86MapInitialize
The function will be used to initialize internal data of the x86 CPU
driver (including the CPU map).
CVE-2017-5715
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/cpu/cpu_x86.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 3b7a6f95fe..0cb0dcacb3 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -153,8 +153,8 @@ struct _virCPUx86Map {
};
static virCPUx86MapPtr cpuMap;
-int virCPUx86MapOnceInit(void);
-VIR_ONCE_GLOBAL_INIT(virCPUx86Map);
+int virCPUx86DriverOnceInit(void);
+VIR_ONCE_GLOBAL_INIT(virCPUx86Driver);
typedef enum {
@@ -1387,7 +1387,7 @@ virCPUx86LoadMap(void)
int
-virCPUx86MapOnceInit(void)
+virCPUx86DriverOnceInit(void)
{
if (!(cpuMap = virCPUx86LoadMap()))
return -1;
@@ -1399,7 +1399,7 @@ virCPUx86MapOnceInit(void)
static virCPUx86MapPtr
virCPUx86GetMap(void)
{
- if (virCPUx86MapInitialize() < 0)
+ if (virCPUx86DriverInitialize() < 0)
return NULL;
return cpuMap;
--
2.17.0
@@ -0,0 +1,133 @@
From c628c42493170bfd70f30d9fb56d0067e6e4828a Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 19 Jun 2018 16:47:20 +0100
Subject: [PATCH 15/19] conf: include x86 microcode version in virsh
capabiltiies
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell and
Broadwell machines.
In order to track the x86 microcode version in the QEMU capabilities,
we have to fetch it and store it in the host CPU. This also makes the
version visible in "virsh capabilities", which is a nice side effect.
CVE-2017-5715
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/conf/cpu_conf.c | 14 ++++++++++++++
src/conf/cpu_conf.h | 1 +
src/cpu/cpu_x86.c | 9 +++++++++
3 files changed, 24 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index c21d11d244..3f3c25320e 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -127,6 +127,7 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
VIR_STRDUP(dst->vendor_id, src->vendor_id) < 0 ||
VIR_ALLOC_N(dst->features, src->nfeatures) < 0)
return -1;
+ dst->microcodeVersion = src->microcodeVersion;
dst->nfeatures_max = src->nfeatures;
dst->nfeatures = 0;
@@ -178,6 +179,7 @@ virCPUDefStealModel(virCPUDefPtr dst,
VIR_STEAL_PTR(dst->model, src->model);
VIR_STEAL_PTR(dst->features, src->features);
+ dst->microcodeVersion = src->microcodeVersion;
dst->nfeatures_max = src->nfeatures_max;
src->nfeatures_max = 0;
dst->nfeatures = src->nfeatures;
@@ -379,6 +381,14 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
goto cleanup;
}
VIR_FREE(arch);
+
+ if (virXPathBoolean("boolean(./microcode[1]/@version)", ctxt) > 0 &&
+ virXPathUInt("string(./microcode[1]/@version)", ctxt,
+ &def->microcodeVersion) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("invalid microcode version"));
+ goto cleanup;
+ }
}
if (!(def->model = virXPathString("string(./model[1])", ctxt)) &&
@@ -723,6 +733,10 @@ virCPUDefFormatBuf(virBufferPtr buf,
if (formatModel && def->vendor)
virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor);
+ if (def->type == VIR_CPU_TYPE_HOST && def->microcodeVersion)
+ virBufferAsprintf(buf, "<microcode version='%u'/>\n",
+ def->microcodeVersion);
+
if (def->sockets && def->cores && def->threads) {
virBufferAddLit(buf, "<topology");
virBufferAsprintf(buf, " sockets='%u'", def->sockets);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index b44974f47e..a30ecf8681 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -133,6 +133,7 @@ struct _virCPUDef {
char *vendor_id; /* vendor id returned by CPUID in the guest */
int fallback; /* enum virCPUFallback */
char *vendor;
+ unsigned int microcodeVersion;
unsigned int sockets;
unsigned int cores;
unsigned int threads;
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 0cb0dcacb3..41aaa61c35 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -33,6 +33,7 @@
#include "virbuffer.h"
#include "virendian.h"
#include "virstring.h"
+#include "virhostcpu.h"
#define VIR_FROM_THIS VIR_FROM_CPU
@@ -153,6 +154,8 @@ struct _virCPUx86Map {
};
static virCPUx86MapPtr cpuMap;
+static unsigned int microcodeVersion;
+
int virCPUx86DriverOnceInit(void);
VIR_ONCE_GLOBAL_INIT(virCPUx86Driver);
@@ -1392,6 +1395,8 @@ virCPUx86DriverOnceInit(void)
if (!(cpuMap = virCPUx86LoadMap()))
return -1;
+ microcodeVersion = virHostCPUGetMicrocodeVersion();
+
return 0;
}
@@ -2409,6 +2414,9 @@ virCPUx86GetHost(virCPUDefPtr cpu,
virCPUDataPtr cpuData = NULL;
int ret = -1;
+ if (virCPUx86DriverInitialize() < 0)
+ goto cleanup;
+
if (!(cpuData = virCPUDataNew(archs[0])))
goto cleanup;
@@ -2417,6 +2425,7 @@ virCPUx86GetHost(virCPUDefPtr cpu,
goto cleanup;
ret = x86DecodeCPUData(cpu, cpuData, models, nmodels, NULL);
+ cpu->microcodeVersion = microcodeVersion;
cleanup:
virCPUx86DataFree(cpuData);
--
2.17.0
@@ -0,0 +1,535 @@
From a31edb693bb79f1ad8931db284f1dbceae178f27 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 19 Jun 2018 16:50:02 +0100
Subject: [PATCH 16/19] qemu: capabilities: force update if the microcode
version does not match
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell
and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and
XML, and rebuild the cache if the versions do not match.
CVE-2017-5715
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/qemu/qemu_capabilities.c | 40 ++++++++++++++++++-
src/qemu/qemu_capabilities.h | 6 ++-
src/qemu/qemu_capspriv.h | 6 +++
src/qemu/qemu_driver.c | 9 ++++-
.../caps_1.2.2.x86_64.xml | 1 +
.../caps_1.3.1.x86_64.xml | 1 +
.../caps_1.4.2.x86_64.xml | 1 +
.../caps_1.5.3.x86_64.xml | 1 +
.../caps_1.6.0.x86_64.xml | 1 +
.../caps_1.7.0.x86_64.xml | 1 +
.../caps_2.1.1.x86_64.xml | 1 +
.../caps_2.4.0.x86_64.xml | 1 +
.../caps_2.5.0.x86_64.xml | 1 +
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
.../caps_2.6.0.ppc64le.xml | 1 +
.../caps_2.6.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
.../caps_2.7.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
.../caps_2.8.0.x86_64.xml | 1 +
.../caps_2.9.0.ppc64le.xml | 1 +
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
.../caps_2.9.0.x86_64.xml | 1 +
tests/qemucapabilitiestest.c | 14 +++++--
tests/qemucapsprobe.c | 2 +-
tests/testutilsqemu.c | 2 +-
27 files changed, 89 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2de84715ea..72b70ce750 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -500,6 +500,7 @@ struct _virQEMUCaps {
unsigned int version;
unsigned int kvmVersion;
unsigned int libvirtVersion;
+ unsigned int microcodeVersion;
char *package;
virArch arch;
@@ -2304,6 +2305,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
ret->version = qemuCaps->version;
ret->kvmVersion = qemuCaps->kvmVersion;
+ ret->microcodeVersion = qemuCaps->microcodeVersion;
if (VIR_STRDUP(ret->package, qemuCaps->package) < 0)
goto error;
@@ -3809,6 +3811,7 @@ struct _virQEMUCapsCachePriv {
uid_t runUid;
gid_t runGid;
virArch hostArch;
+ unsigned int microcodeVersion;
};
typedef struct _virQEMUCapsCachePriv virQEMUCapsCachePriv;
typedef virQEMUCapsCachePriv *virQEMUCapsCachePrivPtr;
@@ -3931,6 +3934,13 @@ virQEMUCapsLoadCache(virArch hostArch,
goto cleanup;
}
+ if (virXPathUInt("string(./microcodeVersion)", ctxt,
+ &qemuCaps->microcodeVersion) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing microcode version in QEMU capabilities cache"));
+ goto cleanup;
+ }
+
if (virXPathBoolean("boolean(./package)", ctxt) > 0) {
qemuCaps->package = virXPathString("string(./package)", ctxt);
if (!qemuCaps->package &&
@@ -4195,6 +4205,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
virBufferAsprintf(&buf, "<kvmVersion>%d</kvmVersion>\n",
qemuCaps->kvmVersion);
+ virBufferAsprintf(&buf, "<microcodeVersion>%u</microcodeVersion>\n",
+ qemuCaps->microcodeVersion);
+
if (qemuCaps->package)
virBufferAsprintf(&buf, "<package>%s</package>\n",
qemuCaps->package);
@@ -4336,6 +4349,16 @@ virQEMUCapsIsValid(void *data,
return false;
}
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
+ priv->microcodeVersion != qemuCaps->microcodeVersion) {
+ VIR_DEBUG("Outdated capabilities for '%s': microcode version changed "
+ "(%u vs %u)",
+ qemuCaps->binary,
+ priv->microcodeVersion,
+ qemuCaps->microcodeVersion);
+ return false;
+ }
+
return true;
}
@@ -5151,6 +5174,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
const char *libDir,
uid_t runUid,
gid_t runGid,
+ unsigned int microcodeVersion,
bool qmpOnly)
{
virQEMUCapsPtr qemuCaps;
@@ -5207,6 +5231,9 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+ qemuCaps->microcodeVersion = microcodeVersion;
+
cleanup:
VIR_FREE(qmperr);
return qemuCaps;
@@ -5228,6 +5255,7 @@ virQEMUCapsNewData(const char *binary,
priv->libDir,
priv->runUid,
priv->runGid,
+ priv->microcodeVersion,
false);
}
@@ -5310,7 +5338,8 @@ virFileCachePtr
virQEMUCapsCacheNew(const char *libDir,
const char *cacheDir,
uid_t runUid,
- gid_t runGid)
+ gid_t runGid,
+ unsigned int microcodeVersion)
{
char *capsCacheDir = NULL;
virFileCachePtr cache = NULL;
@@ -5333,6 +5362,7 @@ virQEMUCapsCacheNew(const char *libDir,
priv->runUid = runUid;
priv->runGid = runGid;
+ priv->microcodeVersion = microcodeVersion;
cleanup:
VIR_FREE(capsCacheDir);
@@ -5810,3 +5840,11 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
return -1;
return 0;
}
+
+
+void
+virQEMUCapsSetMicrocodeVersion(virQEMUCapsPtr qemuCaps,
+ unsigned int microcodeVersion)
+{
+ qemuCaps->microcodeVersion = microcodeVersion;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9c92d6b469..eea296c9c3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -514,8 +514,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps,
const char *machineType);
virFileCachePtr virQEMUCapsCacheNew(const char *libDir,
- const char *cacheDir,
- uid_t uid, gid_t gid);
+ const char *cacheDir,
+ uid_t uid,
+ gid_t gid,
+ unsigned int microcodeVersion);
virQEMUCapsPtr virQEMUCapsCacheLookup(virFileCachePtr cache,
const char *binary);
virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virFileCachePtr cache,
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index d05256bd35..38c14ffa01 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -36,6 +36,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
const char *libDir,
uid_t runUid,
gid_t runGid,
+ unsigned int microcodeVersion,
bool qmpOnly);
int virQEMUCapsLoadCache(virArch hostArch,
@@ -101,4 +102,9 @@ virQEMUCapsParseHelpStr(const char *qemu,
int
virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps,
const char *str);
+
+void
+virQEMUCapsSetMicrocodeVersion(virQEMUCapsPtr qemuCaps,
+ unsigned int microcodeVersion);
+
#endif
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 208ccc9bc3..d8dc5388ea 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -631,6 +631,8 @@ qemuStateInitialize(bool privileged,
gid_t run_gid = -1;
char *hugepagePath = NULL;
size_t i;
+ virCPUDefPtr hostCPU = NULL;
+ unsigned int microcodeVersion = 0;
if (VIR_ALLOC(qemu_driver) < 0)
return -1;
@@ -853,10 +855,15 @@ qemuStateInitialize(bool privileged,
run_gid = cfg->group;
}
+ if ((hostCPU = virCPUProbeHost(virArchFromHost())))
+ microcodeVersion = hostCPU->microcodeVersion;
+ virCPUDefFree(hostCPU);
+
qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
cfg->cacheDir,
run_uid,
- run_gid);
+ run_gid,
+ microcodeVersion);
if (!qemu_driver->qemuCapsCache)
goto error;
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml
index 956284d5d3..f3f66cd8f5 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml
@@ -111,6 +111,7 @@
<flag name='query-cpu-definitions'/>
<version>1002002</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>26900</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='qemu64'/>
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml
index 99384ce5e6..1c4d5ff4a4 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml
@@ -129,6 +129,7 @@
<flag name='query-cpu-definitions'/>
<version>1003001</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>30198</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='qemu64'/>
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml
index aea043c57d..a50383c259 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml
@@ -130,6 +130,7 @@
<flag name='query-cpu-definitions'/>
<version>1004002</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>30915</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
index 6f860e4f25..ad3e122775 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
@@ -142,6 +142,7 @@
<flag name='kernel-irqchip'/>
<version>1005003</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>47019</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
index e5dc8360de..7b2324d697 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
@@ -147,6 +147,7 @@
<flag name='kernel-irqchip'/>
<version>1006000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>45248</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
index 86d87eaf0c..4ba509a753 100644
--- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
@@ -149,6 +149,7 @@
<flag name='kernel-irqchip'/>
<version>1007000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>50692</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
index 2fa551b1a0..416703ac89 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
@@ -165,6 +165,7 @@
<flag name='kernel-irqchip'/>
<version>2001001</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>59488</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index f97e4cb813..4550139e0c 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -190,6 +190,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2004000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>75653</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
index 2ba40fc494..6072438688 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -196,6 +196,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2005000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>216775</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index 0b34fa30d4..6fc0ab25e0 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -174,6 +174,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>228838</microcodeVersion>
<package></package>
<arch>aarch64</arch>
<cpu type='kvm' name='pxa262'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index d41d578c7e..1846bf6a7c 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -174,6 +174,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>228838</microcodeVersion>
<package></package>
<arch>aarch64</arch>
<cpu type='kvm' name='pxa262'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index f1c9fc98a4..199fc2cd22 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -169,6 +169,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>263602</microcodeVersion>
<package></package>
<arch>ppc64</arch>
<cpu type='kvm' name='default'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index bdf006f6be..5897fbc0c9 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -206,6 +206,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>227579</microcodeVersion>
<package></package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
index fe7bca93b9..4c208008be 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
@@ -136,6 +136,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2007000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>217559</microcodeVersion>
<package></package>
<arch>s390x</arch>
<cpu type='kvm' name='host'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index 3fd28f09fe..e3a154806c 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -209,6 +209,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2007000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>239276</microcodeVersion>
<package> (v2.7.0)</package>
<arch>x86_64</arch>
<cpu type='kvm' name='Opteron_G5'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 21bbb820d0..f13c783d44 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -138,6 +138,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>242460</microcodeVersion>
<package></package>
<arch>s390x</arch>
<hostCPU type='kvm' model='zEC12.2-base' migratability='no'>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
index 761f9d1415..f5bd1d7272 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
@@ -211,6 +211,7 @@
<flag name='virtio-gpu.max_outputs'/>
<version>2008000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>255931</microcodeVersion>
<package> (v2.8.0)</package>
<arch>x86_64</arch>
<cpu type='kvm' name='host' usable='yes'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
index 9551907c66..2d1d0f9a89 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml
@@ -175,6 +175,7 @@
<flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>347135</microcodeVersion>
<package> (v2.9.0)</package>
<arch>ppc64</arch>
<cpu type='kvm' name='default'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index 0a6fbd0776..3b733801f8 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -140,6 +140,7 @@
<flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>265878</microcodeVersion>
<package></package>
<arch>s390x</arch>
<hostCPU type='kvm' model='z13.2-base' migratability='no'>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
index 1294ebdb31..086594def5 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
@@ -223,6 +223,7 @@
<flag name='disk-share-rw'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
+ <microcodeVersion>321194</microcodeVersion>
<package> (v2.9.0)</package>
<arch>x86_64</arch>
<hostCPU type='kvm' model='base' migratability='yes'>
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 3ae55fc62f..4608fffbb2 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -61,10 +61,16 @@ testQemuCaps(const void *opaque)
qemuMonitorTestGetMonitor(mon)) < 0)
goto cleanup;
- if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM) &&
- virQEMUCapsInitQMPMonitorTCG(capsActual,
- qemuMonitorTestGetMonitor(mon)) < 0)
- goto cleanup;
+ if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) {
+ if (virQEMUCapsInitQMPMonitorTCG(capsActual,
+ qemuMonitorTestGetMonitor(mon)) < 0)
+ goto cleanup;
+
+ /* Fill microcodeVersion with a "random" value which is the file
+ * length to provide a reproducible number for testing.
+ */
+ virQEMUCapsSetMicrocodeVersion(capsActual, virFileLength(repliesFile, -1));
+ }
if (!(actual = virQEMUCapsFormatCache(capsActual)))
goto cleanup;
diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c
index 4b8d6229b4..a5f5a38b16 100644
--- a/tests/qemucapsprobe.c
+++ b/tests/qemucapsprobe.c
@@ -72,7 +72,7 @@ main(int argc, char **argv)
return EXIT_FAILURE;
if (!(caps = virQEMUCapsNewForBinaryInternal(VIR_ARCH_NONE, argv[1], "/tmp",
- -1, -1, true)))
+ -1, -1, 0, true)))
return EXIT_FAILURE;
virObjectUnref(caps);
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 2c7124bf26..f8182033fc 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -603,7 +603,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
/* Using /dev/null for libDir and cacheDir automatically produces errors
* upon attempt to use any of them */
- driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0);
+ driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0, 0);
if (!driver->qemuCapsCache)
goto error;
--
2.17.0
@@ -0,0 +1,142 @@
From ac0e85360cd8f25160b67ee9fb45663d20f82c1d Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 19 Jun 2018 16:51:13 +0100
Subject: [PATCH 17/19] cpu: add CPU features and model for indirect branch
prediction protection
CVE-2017-5715
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/cpu/cpu_map.xml | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 8e7ac4973d..c31e7ce36a 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -283,6 +283,9 @@
<feature name='avx512-4fmaps'>
<cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000008'/>
</feature>
+ <feature name='spec-ctrl'>
+ <cpuid eax_in='0x07' ecx_in='0x00' edx='0x04000000'/>
+ </feature>
<!-- Processor Extended State Enumeration sub leaf 1 -->
<feature name='xsaveopt'>
@@ -411,6 +414,11 @@
<cpuid eax_in='0x80000007' edx='0x00000100'/>
</feature>
+ <!-- More AMD-specific features -->
+ <feature name='ibpb'>
+ <cpuid eax_in='0x80000008' ebx='0x00001000'/>
+ </feature>
+
<!-- models -->
<model name='486'>
<feature name='fpu'/>
@@ -857,6 +865,10 @@
<feature name='syscall'/>
<feature name='tsc'/>
</model>
+ <model name='Nehalem-IBRS'>
+ <model name='Nehalem'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Westmere'>
<signature family='6' model='44'/>
@@ -894,6 +906,10 @@
<feature name='syscall'/>
<feature name='tsc'/>
</model>
+ <model name='Westmere-IBRS'>
+ <model name='Westmere'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='SandyBridge'>
<signature family='6' model='42'/>
@@ -937,6 +953,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='SandyBridge-IBRS'>
+ <model name='SandyBridge'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='IvyBridge'>
<signature family='6' model='58'/>
@@ -986,6 +1006,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='IvyBridge-IBRS'>
+ <model name='IvyBridge'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Haswell-noTSX'>
<signature family='6' model='60'/>
@@ -1039,6 +1063,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='Haswell-noTSX-IBRS'>
+ <model name='Haswell-noTSX'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Haswell'>
<signature family='6' model='60'/>
@@ -1094,6 +1122,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='Haswell-IBRS'>
+ <model name='Haswell'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Broadwell-noTSX'>
<signature family='6' model='61'/>
@@ -1151,6 +1183,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='Broadwell-noTSX-IBRS'>
+ <model name='Broadwell-noTSX'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Broadwell'>
<signature family='6' model='61'/>
@@ -1210,6 +1246,10 @@
<feature name='x2apic'/>
<feature name='xsave'/>
</model>
+ <model name='Broadwell-IBRS'>
+ <model name='Broadwell'/>
+ <feature name='spec-ctrl'/>
+ </model>
<model name='Skylake-Client'>
<signature family='6' model='94'/>
@@ -1278,6 +1318,10 @@
<feature name='xsavec'/>
<feature name='xsaveopt'/>
</model>
+ <model name='Skylake-Client-IBRS'>
+ <model name='Skylake-Client'/>
+ <feature name='spec-ctrl'/>
+ </model>
<!-- AMD CPUs -->
<model name='athlon'>
--
2.17.0
@@ -0,0 +1,37 @@
From 9a252992aa81b4873b22f174de9d345f4289051c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 21 May 2018 23:05:07 +0100
Subject: [PATCH 18/19] cpu: define the 'ssbd' CPUID feature bit
(CVE-2018-3639)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
New microcode introduces the "Speculative Store Bypass Disable"
CPUID feature bit. This needs to be exposed to guest OS to allow
them to protect against CVE-2018-3639.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 1dbca2eccad58d91a5fd33962854f1a653638182)
---
src/cpu/cpu_map.xml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index c31e7ce36a..87301dc0ef 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -286,6 +286,9 @@
<feature name='spec-ctrl'>
<cpuid eax_in='0x07' ecx_in='0x00' edx='0x04000000'/>
</feature>
+ <feature name='ssbd'>
+ <cpuid eax_in='0x07' ecx_in='0x00' edx='0x80000000'/>
+ </feature>
<!-- Processor Extended State Enumeration sub leaf 1 -->
<feature name='xsaveopt'>
--
2.17.0
@@ -0,0 +1,46 @@
From 7774fbbda1c886633eaf0015d6211fc0ad703bc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 21 May 2018 23:05:08 +0100
Subject: [PATCH 19/19] cpu: define the 'virt-ssbd' CPUID feature bit
(CVE-2018-3639)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Some AMD processors only support a non-architectural means of
enabling Speculative Store Bypass Disable. To allow simplified
handling in virtual environments, hypervisors will expose an
architectural definition through CPUID bit 0x80000008_EBX[25].
This needs to be exposed to guest OS running on AMD x86 hosts to
allow them to protect against CVE-2018-3639.
Note that since this CPUID bit won't be present in the host CPUID
results on physical hosts, it will not be enabled automatically
in guests configured with "host-model" CPU unless using QEMU
version >= 2.9.0. Thus for older versions of QEMU, this feature
must be manually enabled using policy=force. Guests using the
"host-passthrough" CPU mode do not need special handling.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
src/cpu/cpu_map.xml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 87301dc0ef..e31c9ae86c 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -421,6 +421,9 @@
<feature name='ibpb'>
<cpuid eax_in='0x80000008' ebx='0x00001000'/>
</feature>
+ <feature name='virt-ssbd'>
+ <cpuid eax_in='0x80000008' ebx='0x02000000'/>
+ </feature>
<!-- models -->
<model name='486'>
--
2.17.0
+21
View File
@@ -0,0 +1,21 @@
# Makefile for source rpm: libvirt
# $Id$
NAME := libvirt
SPECFILE = $(firstword $(wildcard *.spec))
define find-makefile-common
for d in common ../common ../../common ; do if [ -f $$d/Makefile.common ] ; then if [ -f $$d/CVS/Root -a -w $$d/Makefile.common ] ; then cd $$d ; cvs -Q update ; fi ; echo "$$d/Makefile.common" ; break ; fi ; done
endef
MAKEFILE_COMMON := $(shell $(find-makefile-common))
ifeq ($(MAKEFILE_COMMON),)
# attempt a checkout
define checkout-makefile-common
test -f CVS/Root && { cvs -Q -d $$(cat CVS/Root) checkout common && echo "common/Makefile.common" ; } || { echo "ERROR: I can't figure out how to checkout the 'common' module." ; exit -1 ; } >&2
endef
MAKEFILE_COMMON := $(shell $(checkout-makefile-common))
endif
include $(MAKEFILE_COMMON)
+751 -497
View File
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -1 +1 @@
SHA512 (libvirt-5.6.0.tar.xz) = 95fe931394fb31288faf73349bb298f08f63cf062f851b9935303145f8166f69128be9360757f0e1845256c14f4d7672843dba0dc6c086b1c3c8bfc035cc8986
SHA512 (libvirt-3.7.0.tar.xz) = b3f7021ef4c6954430f8fa503f0c49e3df4f662b228cb631ba2c2139ecec2307dde6cec05037cc28663e82ab1001296c20c5c68acd183cd364dd484a7746f498