July 2020 - Devel - libvirt List Archives

[PATCH v4 0/3] tpm: Fix default choices for CRB and SPAPR dev models
by Stefan Berger 08 Jul '20

08 Jul '20

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

3 5

[PATCH v3 0/2] tpm: Prevent choice of TPM 1.2 backend for SPAPR
by Stefan Berger 08 Jul '20

08 Jul '20

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 8

[PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
by Stefan Berger 08 Jul '20

08 Jul '20

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

3 5

[PATCH] domain_conf: Replace the name string with 'vcpu' if it is 'vcpus'
by Yi Wang 08 Jul '20

08 Jul '20

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

3 2

[PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
by Stefan Berger 08 Jul '20

08 Jul '20

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

3 3

[PATCH] qemuBuildNumaCommandLine: Fix @masterInitiator check
by Michal Privoznik 08 Jul '20

08 Jul '20

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

1 0

[PATCH 00/32] always use g_auto() for virBuffer and virFirewall objects
by Laine Stump 08 Jul '20

08 Jul '20

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

2 65

[PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
by Peter Krempa 08 Jul '20

08 Jul '20

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

2 2

[libvirt PATCH v2 0/2] Use the default capabilities cache lookup rather than an arch based lookup
by Erik Skultety 08 Jul '20

08 Jul '20

https://bugzilla.redhat.com/show_bug.cgi?id=1852311 Since v1: - dropped virQEMUCapsCacheLookupByArch since after patch 1 there was no further usage - dropped virQEMUCapsCompareArch since virQEMUCapsCacheLookupByArch was the only caller pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/164239584 Erik Skultety (2): qemu: Use virQEMUCapsCacheLookupDefault instead of lookup by arch qemu: capabilities: Drop the virQEMUCapsCacheLookupByArch function src/qemu/qemu_capabilities.c | 63 ------------------------------------ src/qemu/qemu_capabilities.h | 2 -- src/qemu/qemu_driver.c | 6 ++-- 3 files changed, 4 insertions(+), 67 deletions(-) -- 2.26.2

2 3

[PATCH] docs/index.html.in: Add knowledge base link and description on index page
by Jianan Gao 08 Jul '20

08 Jul '20

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

2 1