[PATCH v4 0/3] tpm: Fix default choices for CRB and SPAPR dev models
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.ibm.com>
This series of patches adds an additional check for the SPAPR device model
that prevents the choice of a TPM 1.2 backend and chooses a TPM 2 as default.
Also CRB now chooses a TPM 2 as default since TPM 1.2 wouldn't work with it,
either.
Stefan
Stefan Berger (3):
qemu: Move setting of TPM default to post parse function
qemu: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
qemu: Choose TPM 2 for backend as default for CRB interface
src/qemu/qemu_domain.c | 12 +++++++++---
src/qemu/qemu_validate.c | 10 ++++++----
2 files changed, 15 insertions(+), 7 deletions(-)
--
2.17.1
4 years, 11 months
[PATCH v3 0/2] tpm: Prevent choice of TPM 1.2 backend for SPAPR
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.ibm.com>
This series of patches adds an additional check for the SPAPR device model
that prevents the choice of a TPM 1.2 backend.
Stefan
Stefan Berger (2):
qemu: Move setting of TPM default to post parse function
conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
src/qemu/qemu_domain.c | 11 ++++++++---
src/qemu/qemu_validate.c | 10 ++++++----
2 files changed, 14 insertions(+), 7 deletions(-)
--
2.17.1
4 years, 11 months
[PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.ibm.com>
The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so
prevent the choice of TPM 1.2 when the SPAPR device model is chosen
and use a default of '2.0' (TPM 2) for the backend.
Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com>
---
src/qemu/qemu_validate.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index bd7590a00a..f4d71aebf5 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3645,8 +3645,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
virQEMUCapsFlags flag;
/* TPM 1.2 and 2 are not compatible, so we choose a specific version here */
- if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT)
- tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
+ if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR)
+ tpm->version = VIR_DOMAIN_TPM_VERSION_2_0;
+ else
+ tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
+ }
switch (tpm->version) {
case VIR_DOMAIN_TPM_VERSION_1_2:
@@ -3658,6 +3662,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
virDomainTPMModelTypeToString(tpm->model));
return -1;
}
+ /* TPM 1.2 + SPAPR do not work with any 'type' (backend) */
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TPM 1.2 is not supported with the SPAPR device model"));
+ return -1;
+ }
break;
case VIR_DOMAIN_TPM_VERSION_2_0:
case VIR_DOMAIN_TPM_VERSION_DEFAULT:
--
2.17.1
4 years, 11 months
[PATCH] domain_conf: Replace the name string with 'vcpu' if it is 'vcpus'
by Yi Wang
From: Liao Pingfang <liao.pingfang(a)zte.com.cn>
If the name is 'vcpus', we will get 'vcpussched' instead of 'vcpusched'
in the error message as following:
... 19155 : vcpussched attributes 'vcpus' must not overlap
So we use 'vcpu' to replace 'vcpus'.
Signed-off-by: Liao Pingfang <liao.pingfang(a)zte.com.cn>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c883cd..5dcc836 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20050,7 +20050,7 @@ virDomainThreadSchedParseHelper(xmlNodePtr node,
if (sched->policy != VIR_PROC_POLICY_NONE) {
virReportError(VIR_ERR_XML_DETAIL,
_("%ssched attributes 'vcpus' must not overlap"),
- name);
+ STREQ(name, "vcpus") ? "vcpu" : name);
return -1;
}
--
2.9.5
4 years, 11 months
[PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.ibm.com>
The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so
prevent the choice of TPM 1.2 when the SPAPR device model is chosen
and use a default of '2.0' (TPM 2) for the emulator.
Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com>
---
src/conf/domain_conf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2f4528d336..749d135f96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13633,7 +13633,10 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
version = virXMLPropString(backends[0], "version");
if (!version) {
- def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
+ if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR)
+ def->version = VIR_DOMAIN_TPM_VERSION_2_0;
+ else
+ def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
} else {
if ((def->version = virDomainTPMVersionTypeFromString(version)) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -13641,6 +13644,12 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
version);
goto error;
}
+ if (def->version == VIR_DOMAIN_TPM_VERSION_1_2 &&
+ def->model == VIR_DOMAIN_TPM_MODEL_SPAPR) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TPM 1.2 is not supported with the SPAPR device model"));
+ goto error;
+ }
}
switch (def->type) {
--
2.17.1
4 years, 11 months
[PATCH] qemuBuildNumaCommandLine: Fix @masterInitiator check
by Michal Privoznik
A few commits ago, in aeecbc87b73, I've implemented command line
generation for ACPI HMAT. For this, we need to know if at least
one guest NUMA node has vCPUs. This is tracked in
@masterInitiator variable, which is initialized to -1, then we
iterate through guest NUMA nodes and break the loop if we find a
node with a vCPU. After the loop, if masterInitiator is still
negative then no NUMA node has a vCPU and we error out. But this
exact check was missing comparison for negativeness.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Pushed under oops and trivial rules.
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7f215b4cc6..f06a2f2754 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7312,7 +7312,7 @@ qemuBuildNumaCommandLine(virQEMUDriverConfigPtr cfg,
}
}
- if (masterInitiator) {
+ if (masterInitiator < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("At least one NUMA node has to have CPUs"));
goto cleanup;
--
2.26.2
4 years, 11 months
[PATCH 00/32] always use g_auto() for virBuffer and virFirewall objects
by Laine Stump
Like most other things I do, this started as a distraction from
something else. I noticed that a function that took a virBufferPtr as
an argument was clearing that buffer on error, but most (but *not
all*) of its callers (which were the creators/owners of said
virBuffer) were already clearing the buffer object on error
anyway. Here's the original patch resulting from my seeing this in one
place:
https://www.redhat.com/archives/libvir-list/2020-June/msg01163.html
In the example I saw, eliminating the extra clearing of the virBuffer
in the subordinate function would simplify the error-cleanup code for
that function, but it turned out the calling function *wasn't*
clearing the virBuffer when an error occurred.
Looking further, I saw this same pattern occurred in other places in
the code - a function would create a new buffer (with "virBuffer buf =
VIR_BUFFER_INITIALIZER;"), and clear/free that buffer when it was
finished *unless there was an error*, in which case some functions
would properly clear the buffer, and some would just return, I guess
assuming the caller that generated the error had cleared the buffer.
This not only makes the error cleanup logic in subordinate functions
messier, it seems philosophically wrong to me, it also sounds like a
memory leak just waiting to happen.
So I decided that the way it should more properly work is this:
1) all virBuffers should be declared with g_auto(virBuffer), so that
they are automatically cleared (with virBufferFreeAndReset()) when
that toplevel function declaring/initializing the buffer returns to
its caller.
2) subordinate functions called with the virBuffer object as an arg
should just leave the buffer in whatever state it was when the error
occurred. Since those functions don't use the virBuffer after the
error happens, and the caller doesn't look at anything in the
virBuffer to determine an error has occurred, this is completely safe.
(obvious exceptions to (2) are of course those functions whose main
intent is in fact to consume the virBuffer,
e.g. virBufferContentAndReset(), and virCommandAddArgBuffer())
Patches 01 - 15 handle part (1) - *all* declarations of virBuffer as
an automatic are changed to g_auto(virBuffer), and any
virBufferFreeAndReset() calls in those same functions are removed.
(I have it split into so many patches because virBuffer is used all
over the place, and I figured it would make it easier to backport just
one part of the entire set if needed when backported some unrelated
bugfix that only touched one of the many directories represented
here. I would happily squash together any group of patches anyone
wants, however).
Patches 16 and 17 fix a couple "one-off" anomolies in the code related
to virBuffers.
Patch 18 then takes care of point (2) above, by removing any
extraneous virBufferFreeAndReset() calls in subordinate functions that
are no longer necessary due to the guarantee that the toplevel will
cleanup after error.
Patches 19-30 just eliminate any labels (and associated "ret"
variables and "goto's) that have been rendered pointless by removal of
virBufferFreeAndReset().
Finally Patches 31 and 32 convert all usages of virFirewall to use
g_autoptr(). They are included in this same set because virFirewall
objects are often declared right next to virBuffer objects, so doing
those patches in a different order would have caused merge conflicts..
NB: In many of the cases where "virBuffer" was changed to
"g_auto(virBuffer)", it doesn't actually eliminate any code from the
function, due to the function *always* calling
virBufferContentAndReset() anyway. I considered not changing those
cases, but in the ended decided it was better to add g_auto() even in
those cases for two reasons:
1) consistency makes verification easier, and means someone a year
from now won't come up with the same idea and waste time verifying all
those cases of virBuffer don't need g_auto().
2) if those functions ever change to have an alternate return that
doesn't explicitly call virBufferContentAndReset(), they will still be
safe.
3) the extra overhead is very minimal; a small price to pay for (1)
and (2)
NB2: these patches aren't just academic code churning; they have fixed
some actual (well, theoretically actual) memory leaks, I just haven't
taken the time to try and track all of them down or document them,
because they only occur in error cases which will likely never
happen. One example from my notes:
virStoragePoolSaveState calls
virStoragePoolDefFormatBuf which calls
virStoragePoolSourceFormat, which errors, returns
virStoragePoolDefFormatBuf, returns
virStoragePoolSaveState returns without freeing virBuffer
Laine Stump (32):
bhyve: use g_auto() for all virBuffers
esx: use g_auto() for all virBuffers
hyperv: use g_auto() for all virBuffers
libxl: use g_auto() for all virBuffers
lxc: use g_auto() for all virBuffers
qemu: use g_auto() for all virBuffers
tests: use g_auto for all virBuffers
tools: use g_auto() for all virBuffers
conf: use g_auto() for all virBuffers
util: use g_auto() for all virBuffers
cpu: use g_auto() for all virBuffers
rpc: use g_auto() for all virBuffers
nwfilter: use g_auto() for all virBuffers
network: use g_auto() for all virBuffers
use g_auto() for all remaining non-g_auto() virBuffers
qemu: remove unnecessary virBufferFreeAndReset() after
virCommandAddArgBuffer()
conf: consistently check for error when calling virSysinfoFormat()
remove redundant calls to virBufferFreeAndReset()
libxml: eliminate extra copy of string
bhyve: eliminate unnecessary labels
conf: eliminate unnecessary labels
libxl: eliminate unnecessary labels
util: eliminate unnecessary labels
tests: eliminate unnecessary labels
tools: eliminate unnecessary labels
network: eliminate unnecessary labels
lxc: eliminate unnecessary labels
esx: eliminate unnecessary labels
nwfilter: eliminate unnecessary labels
storage: eliminate unnecessary labels
use g_autoptr() for all usages of virFirewallNew/Free
eliminate unnecessary labels and ret variables
src/bhyve/bhyve_command.c | 40 +++---
src/bhyve/bhyve_driver.c | 4 +-
src/conf/capabilities.c | 13 +-
src/conf/checkpoint_conf.c | 11 +-
src/conf/cpu_conf.c | 27 ++--
src/conf/domain_addr.c | 2 +-
src/conf/domain_capabilities.c | 2 +-
src/conf/domain_conf.c | 105 +++++++-------
src/conf/interface_conf.c | 7 +-
src/conf/network_conf.c | 8 +-
src/conf/node_device_conf.c | 2 +-
src/conf/nwfilter_conf.c | 12 +-
src/conf/secret_conf.c | 8 +-
src/conf/snapshot_conf.c | 14 +-
src/conf/storage_capabilities.c | 6 +-
src/conf/storage_conf.c | 28 ++--
src/conf/virnetworkobj.c | 10 +-
src/conf/virnetworkportdef.c | 6 +-
src/conf/virnwfilterbindingdef.c | 6 +-
src/conf/virnwfilterbindingobj.c | 6 +-
src/conf/virsavecookie.c | 8 +-
src/cpu/cpu_map.c | 2 +-
src/cpu/cpu_x86.c | 6 +-
src/esx/esx_driver.c | 19 +--
src/esx/esx_util.c | 4 +-
src/esx/esx_vi.c | 28 ++--
src/esx/esx_vi_methods.c | 6 +-
src/hyperv/hyperv_driver.c | 34 +++--
src/hyperv/hyperv_wmi.c | 11 +-
src/hypervisor/domain_driver.c | 7 +-
src/libxl/libxl_conf.c | 19 +--
src/libxl/libxl_driver.c | 2 +-
src/libxl/libxl_migration.c | 2 +-
src/libxl/xen_common.c | 28 ++--
src/libxl/xen_xl.c | 45 ++----
src/libxl/xen_xm.c | 10 +-
src/locking/lock_driver_sanlock.c | 2 +-
src/lxc/lxc_container.c | 4 +-
src/lxc/lxc_controller.c | 12 +-
src/lxc/lxc_driver.c | 2 +-
src/lxc/lxc_fuse.c | 3 +-
src/network/bridge_driver.c | 25 ++--
src/network/bridge_driver_linux.c | 36 ++---
src/node_device/node_device_udev.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 160 +++++++++-------------
src/nwfilter/nwfilter_gentech_driver.c | 6 +-
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/openvz/openvz_driver.c | 5 +-
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_block.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_command.c | 5 +-
src/qemu/qemu_dbus.c | 2 +-
src/qemu/qemu_domain.c | 4 +-
src/qemu/qemu_driver.c | 5 +-
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_migration_cookie.c | 6 +-
src/qemu/qemu_monitor.c | 2 +-
src/rpc/virnetclient.c | 4 +-
src/rpc/virnetlibsshsession.c | 7 +-
src/rpc/virnetsocket.c | 2 +-
src/rpc/virnetsshsession.c | 2 +-
src/security/virt-aa-helper.c | 4 +-
src/storage/storage_backend_rbd.c | 7 +-
src/storage/storage_util.c | 14 +-
src/util/virbitmap.c | 4 +-
src/util/vircommand.c | 3 +-
src/util/virconf.c | 5 +-
src/util/virdnsmasq.c | 6 +-
src/util/virebtables.c | 24 +---
src/util/virfile.c | 2 +-
src/util/virfilecache.c | 2 +-
src/util/virfirewall.c | 2 +-
src/util/viriptables.c | 14 +-
src/util/virlog.c | 5 +-
src/util/virnetdevip.c | 3 +-
src/util/virpidfile.c | 2 +-
src/util/virqemu.c | 11 +-
src/util/virresctrl.c | 10 +-
src/util/virsocketaddr.c | 25 ++--
src/util/virstoragefile.c | 2 +-
src/util/virstring.c | 4 +-
src/util/virsysinfo.c | 5 +-
src/util/virsystemd.c | 4 +-
src/util/viruri.c | 2 +-
src/util/virxml.c | 12 +-
src/vmx/vmx.c | 5 +-
src/vz/vz_driver.c | 4 +-
tests/commandtest.c | 3 +-
tests/cputest.c | 2 +-
tests/networkxml2firewalltest.c | 3 +-
tests/nodedevmdevctltest.c | 6 +-
tests/nwfilterebiptablestest.c | 21 +--
tests/nwfilterxml2firewalltest.c | 3 +-
tests/qemublocktest.c | 2 +-
tests/qemucommandutiltest.c | 2 +-
tests/qemumigparamstest.c | 6 +-
tests/qemumonitorjsontest.c | 9 +-
tests/qemumonitortestutils.c | 2 +-
tests/testutils.c | 2 +-
tests/vboxsnapshotxmltest.c | 3 +-
tests/virbuftest.c | 58 ++++----
tests/vircgrouptest.c | 3 +-
tests/virfirewalltest.c | 80 +++--------
tests/virhostcputest.c | 3 +-
tests/virkmodtest.c | 4 +-
tests/virnetdevbandwidthtest.c | 3 +-
tools/virsh-checkpoint.c | 3 +-
tools/virsh-domain-monitor.c | 3 +-
tools/virsh-domain.c | 58 ++++----
tools/virsh-pool.c | 19 ++-
tools/virsh-secret.c | 2 +-
tools/virsh-snapshot.c | 3 +-
tools/virsh-volume.c | 3 +-
tools/vsh-table.c | 2 +-
tools/vsh.c | 15 +-
116 files changed, 505 insertions(+), 853 deletions(-)
--
2.25.4
4 years, 11 months
[PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
by Peter Krempa
Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
This is okay for the command line visitor, but the property is declared
as boolean in qemu and thus it will not work when using QMP.
Modify the type to boolean. This changes the command line, but
fortunately the command line visitor in qemu parses both 'yes' and 'on'
as true for the property.
https://bugzilla.redhat.com/show_bug.cgi?id=1854684
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
.../memory-hotplug-nvdimm-pmem.x86_64-latest.args | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0c4c77cf8c..c32db06e34 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3187,7 +3187,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
"with this QEMU binary"));
return -1;
}
- if (virJSONValueObjectAdd(props, "s:pmem", "on", NULL) < 0)
+ if (virJSONValueObjectAdd(props, "b:pmem", true, NULL) < 0)
return -1;
}
diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
index 5dfba9b50a..00a78baa92 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
@@ -19,7 +19,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-smp 2,sockets=2,dies=1,cores=1,threads=1 \
-numa node,nodeid=0,cpus=0-1,mem=214 \
-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
-share=no,size=536870912,pmem=on \
+share=no,size=536870912,pmem=yes \
-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
-display none \
--
2.26.2
4 years, 11 months
[PATCH] docs/index.html.in: Add knowledge base link and description on index page
by Jianan Gao
Add link and description of libvirt knowledge base to make it
easier for users and testers to understand libvirt.
Signed-off-by: Jianan Gao <jgao(a)redhat.com>
---
docs/index.html.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/docs/index.html.in b/docs/index.html.in
index 26e8406917..586defff54 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -63,6 +63,9 @@
<a href="formatbackup.html">backup jobs</a></dd>
<dt><a href="http://wiki.libvirt.org">Wiki</a></dt>
<dd>Read further community contributed content</dd>
+
+ <dt><a href="https://libvirt.org/kbase.html">Knowledge base</a></dt>
+ <dd>Learn more about libvirt through knowledge base</dd>
</dl>
</div>
--
2.21.3
4 years, 11 months
