- Devel - libvirt List Archives

[libvirt] [PATCH] test: storage: Fill in default vol types for every pool
by Cole Robinson 08 Mar '19

08 Mar '19

Fill in a default volume type for every pool type, as reported by the VolGetInfo API. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- A more complete fix would take into account VolDef->type, so test driver users could set this value via volume XML. That would require adding a PostParse callback to the storage driver infrastructure. This is still an improvement in the interim and would be part of a complete fix anyways src/test/test_driver.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ce0df1f8e3..3f37b2ede7 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5164,13 +5164,26 @@ testStorageVolDelete(virStorageVolPtr vol, static int testStorageVolumeTypeForPool(int pooltype) { - switch (pooltype) { - case VIR_STORAGE_POOL_DIR: - case VIR_STORAGE_POOL_FS: - case VIR_STORAGE_POOL_NETFS: - return VIR_STORAGE_VOL_FILE; - default: - return VIR_STORAGE_VOL_BLOCK; + switch ((virStoragePoolType) pooltype) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_NETFS: + case VIR_STORAGE_POOL_VSTORAGE: + return VIR_STORAGE_VOL_FILE; + case VIR_STORAGE_POOL_SHEEPDOG: + case VIR_STORAGE_POOL_ISCSI_DIRECT: + case VIR_STORAGE_POOL_GLUSTER: + case VIR_STORAGE_POOL_RBD: + return VIR_STORAGE_VOL_NETWORK; + case VIR_STORAGE_POOL_LOGICAL: + case VIR_STORAGE_POOL_DISK: + case VIR_STORAGE_POOL_MPATH: + case VIR_STORAGE_POOL_ISCSI: + case VIR_STORAGE_POOL_SCSI: + case VIR_STORAGE_POOL_ZFS: + case VIR_STORAGE_POOL_LAST: + default: + return VIR_STORAGE_VOL_BLOCK; } } -- 2.20.1

3 3

[libvirt] [PATCH] util: skip RDMA detection for non-PCI network devices
by Pavel Hrdina 08 Mar '19

08 Mar '19

Only PCI devices have '/sys/class/net/<ifname>/device/resource' so we need to skip this check for all other network devices. Without this patch and RDMA enabled libvirt will not detect any network device that doesn't have the path above which includes 'lo', 'virbr', 'tun', etc. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1639258 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/util/virnetdev.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index aeb9caab2a..699f2a0acb 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2914,8 +2914,14 @@ virNetDevRDMAFeature(const char *ifname, if (virAsprintf(&eth_devpath, SYSFS_NET_DIR "%s/device/resource", ifname) < 0) goto cleanup; - if (!virFileExists(eth_devpath)) + + /* If /sys/class/net/<ifname>/device/resource doesn't exist it is not a PCI + * device and therefore it will not have RDMA. */ + if (!virFileExists(eth_devpath)) { + ret = 0; goto cleanup; + } + if (virFileReadAll(eth_devpath, RESOURCE_FILE_LEN, &eth_res_buf) < 0) goto cleanup; -- 2.20.1

2 1

[libvirt] [PATCH] conf: fix title and description for virDomainSetMetadata API
by Pavel Hrdina 08 Mar '19

08 Mar '19

If we pass XML to virDomainDefineXML API with these two elements: ... <title></title> <description></description> ... libvirt correctly ignores these two elements and they will not appear in the parsed XML. However, if we use virDomainSetMetadata API and with "" as value for title or description we will end up with the parsed XML that contains these empty elements. Let's fix the behavior of this API to behave the same as virDomainDefineXML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1518042 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/conf/domain_conf.c | 6 +++--- tests/metadatatest.c | 23 ++++++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 477deb777e..2fba58e5c4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30363,7 +30363,7 @@ virDomainDefSetMetadata(virDomainDefPtr def, xmlDocPtr doc = NULL; xmlNodePtr old; xmlNodePtr new = NULL; - char *tmp; + char *tmp = NULL; int ret = -1; if (type >= VIR_DOMAIN_METADATA_LAST) { @@ -30374,7 +30374,7 @@ virDomainDefSetMetadata(virDomainDefPtr def, switch ((virDomainMetadataType) type) { case VIR_DOMAIN_METADATA_DESCRIPTION: - if (VIR_STRDUP(tmp, metadata) < 0) + if (STRNEQ_NULLABLE(metadata, "") && VIR_STRDUP(tmp, metadata) < 0) goto cleanup; VIR_FREE(def->description); @@ -30382,7 +30382,7 @@ virDomainDefSetMetadata(virDomainDefPtr def, break; case VIR_DOMAIN_METADATA_TITLE: - if (VIR_STRDUP(tmp, metadata) < 0) + if (STRNEQ_NULLABLE(metadata, "") && VIR_STRDUP(tmp, metadata) < 0) goto cleanup; VIR_FREE(def->title); diff --git a/tests/metadatatest.c b/tests/metadatatest.c index 786c62e623..a9080b32d7 100644 --- a/tests/metadatatest.c +++ b/tests/metadatatest.c @@ -167,6 +167,7 @@ struct metadataTest { virDomainPtr dom; const char *data; + const char *expect; int type; bool fail; }; @@ -232,7 +233,7 @@ testTextMetadata(const void *data) actual = virDomainGetMetadata(test->dom, test->type, NULL, 0); - if (STRNEQ_NULLABLE(test->data, actual)) { + if (STRNEQ_NULLABLE(test->expect, actual)) { virReportError(VIR_ERR_INTERNAL_ERROR, "expected metadata doesn't match actual: " "expected:'%s'\ngot: '%s'", @@ -248,10 +249,11 @@ testTextMetadata(const void *data) return ret; } -#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, FAIL) \ +#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, EXPECT, FAIL) \ do { \ test.type = VIR_DOMAIN_METADATA_ ## TYPE; \ test.data = DATA; \ + test.expect = EXPECT; \ test.fail = FAIL; \ \ if (virTestRun("text metadata: " #TYPE " " INDEX " ", \ @@ -259,9 +261,16 @@ testTextMetadata(const void *data) ret = EXIT_FAILURE; \ } while (0) -#define TEST_TITLE(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, false) -#define TEST_TITLE_FAIL(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, true) -#define TEST_DESCR(INDEX, DATA) TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, false) +#define TEST_TITLE(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, false) +#define TEST_TITLE_EXPECT(INDEX, DATA, EXPECT) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, EXPECT, false) +#define TEST_TITLE_FAIL(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, true) +#define TEST_DESCR(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, DATA, false) +#define TEST_DESCR_EXPECT(INDEX, DATA, EXPECT) \ + TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, EXPECT, false) static int mymain(void) @@ -290,7 +299,7 @@ mymain(void) TEST_TITLE("2", NULL); TEST_TITLE("3", "blah"); TEST_TITLE_FAIL("4", "qwe\nrt"); - TEST_TITLE("5", ""); + TEST_TITLE_EXPECT("5", "", NULL); TEST_TITLE_FAIL("6", "qwert\n"); TEST_TITLE_FAIL("7", "\n"); @@ -298,7 +307,7 @@ mymain(void) TEST_DESCR("2", NULL); TEST_DESCR("3", "qwert"); TEST_DESCR("4", "\n"); - TEST_DESCR("5", ""); + TEST_DESCR_EXPECT("5", "", NULL); virDomainFree(test.dom); virConnectClose(test.conn); -- 2.20.1

2 1

[libvirt] [PATCH] qemu: Fix query-cpus-fast target architecture detection
by Boris Fiuczynski 08 Mar '19

08 Mar '19

From: Viktor Mihajlovski <mihajlov(a)linux.ibm.com> Since qemu 2.13 reports the target architecture in a property called 'target' additionally to the property 'arch', that has been used in qemu 2.12 in the response data of 'query-cpus-fast'. Libvirts monitor code prefers the 'target' property over 'arch'. At least for s390(x), target is reported as 's390x' while arch is 's390'. In a later step a comparison is performed against 's390' which fails for qemu 2.13 and later. In consequence the architecture specific data for s390 won't be extracted from the returned data, leading to incorrect values being reported by virsh domstats --vcpu. Changing to check explicitly for 's390' and 's390x'. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cf474eb6c8..0236323a51 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1772,7 +1772,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, goto cleanup; /* process optional architecture-specific data */ - if (STREQ_NULLABLE(arch, "s390")) + if (STREQ_NULLABLE(arch, "s390") || STREQ_NULLABLE(arch, "s390x")) qemuMonitorJSONExtractCPUS390Info(entry, cpus + i); } -- 2.17.0

2 3

[libvirt] [PATCH] news: document virtio-{non-}transitional feature
by Cole Robinson 08 Mar '19

08 Mar '19

Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- docs/news.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index f58c30ee22..81466c3d55 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,25 @@ virConnectGetStoragePoolCapabilites API. </description> </change> + <change> + <summary> + qemu: Support virtio-{non-}transitional device models + </summary> + <description> + <code>virtio-transitional</code> and + <code>virtio-non-transitional</code> <code>model</code> values + were added to the QEMU driver for the following devices: + <code>disk</code>, <code>interface</code>, <code>filesystem</code>, + <code>rng</code>, <code>vsock</code>, <code>memballoon</code>, + <code>controller</code> type <code>scsi</code>, + <code>controller</code> type <code>virtio-serial</code>, + <code>input</code> bus <code>virtio</code> + type <code>passthrough</code>, + <code>hostdev</code> type <code>scsi_host</code>. These new + models can be used to give fine grained control over what + virtio device version is presented to the guest. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.20.1

2 1

[libvirt] [PATCH v2] tools/virt-host-validate: Fix IOMMU check on s390x
by Thomas Huth 08 Mar '19

08 Mar '19

When running virt-host-validate on an s390x host, the tool currently warns that it is "Unknown if this platform has IOMMU support". We can use the common check for entries in /sys/kernel/iommu_groups here, too, but it only makes sense to check it if there are also PCI devices available. It's also common on s390x that there are no PCI devices assigned to the LPAR, and in that case there is no need for the PCI-related IOMMU, so without PCI devices we should simply skip this test. Signed-off-by: Thomas Huth <thuth(a)redhat.com> --- v2: - Use virDirOpen() + virDirRead() instead of stat() - this should hopefully work now as expected. tools/virt-host-validate-common.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 73e3bdb..e27b558 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -337,7 +337,12 @@ int virHostValidateIOMMU(const char *hvname, virBitmapPtr flags; struct stat sb; const char *bootarg = NULL; - bool isAMD = false, isIntel = false, isPPC = false; + bool isAMD = false, isIntel = false; + virArch arch = virArchFromHost(); + struct dirent *dent; + DIR *dir; + int rc; + flags = virHostValidateGetCPUFlags(); if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX)) @@ -347,8 +352,6 @@ int virHostValidateIOMMU(const char *hvname, virBitmapFree(flags); - isPPC = ARCH_IS_PPC64(virArchFromHost()); - if (isIntel) { virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support")); if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) { @@ -373,8 +376,17 @@ int virHostValidateIOMMU(const char *hvname, "hardware platform"); return -1; } - } else if (isPPC) { + } else if (ARCH_IS_PPC64(arch)) { /* Empty Block */ + } else if (ARCH_IS_S390(arch)) { + /* On s390x, we skip the IOMMU check if there are no PCI devices + * (which is quite usual on s390x) */ + if (!virDirOpen(&dir, "/sys/bus/pci/devices")) + return 0; + rc = virDirRead(dir, &dent, NULL); + VIR_DIR_CLOSE(dir); + if (rc <= 0) + return 0; } else { virHostMsgFail(level, "Unknown if this platform has IOMMU support"); @@ -391,7 +403,7 @@ int virHostValidateIOMMU(const char *hvname, virHostMsgCheck(hvname, "%s", _("if IOMMU is enabled by kernel")); if (sb.st_nlink <= 2) { - if (!isPPC) + if (bootarg) virHostMsgFail(level, "IOMMU appears to be disabled in kernel. " "Add %s to kernel cmdline arguments", bootarg); -- 1.8.3.1

4 5

[libvirt] [PATCH] snapshots: Trivial doc improvements
by Eric Blake 08 Mar '19

08 Mar '19

Fix an incorrect @xmlDesc comment, as well as adding more details about which XML element should be root. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing this one as trivial; noticed it while working on Jan's suggestion of a new API for bulk dumpxml/redefine. src/libvirt-domain-snapshot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c index a647a500d6..11d84289f8 100644 --- a/src/libvirt-domain-snapshot.c +++ b/src/libvirt-domain-snapshot.c @@ -98,11 +98,11 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) /** * virDomainSnapshotCreateXML: * @domain: a domain object - * @xmlDesc: string containing an XML description of the domain + * @xmlDesc: string containing an XML description of the domain snapshot * @flags: bitwise-OR of virDomainSnapshotCreateFlags * * Creates a new snapshot of a domain based on the snapshot xml - * contained in xmlDesc. + * contained in xmlDesc, with a top-level element <domainsnapshot>. * * If @flags is 0, the domain can be active, in which case the * snapshot will be a full system snapshot (capturing both disk state, @@ -247,7 +247,8 @@ virDomainSnapshotCreateXML(virDomainPtr domain, * @snapshot: a domain snapshot object * @flags: bitwise-OR of supported virDomainSnapshotXMLFlags * - * Provide an XML description of the domain snapshot. + * Provide an XML description of the domain snapshot, with a top-level + * element of <domainsnapshot>. * * No security-sensitive data will be included unless @flags contains * VIR_DOMAIN_SNAPSHOT_XML_SECURE; this flag is rejected on read-only -- 2.20.1

1 0

[libvirt] [PATCH v3 00/18] bulk snapshot list/redefine (incremental backup saga)
by Eric Blake 07 Mar '19

07 Mar '19

While looking at my work on incremental backups, Nir raised a good point: if we want to recreate a set of known checkpoints on one machine that will be taking over a domain from another machine, my initial proposal required making multiple API calls to list the XML for each checkpoint on the source, and then more API calls to redefine each checkpoint on the destination; it also had the drawback that the list has to be presented in topological order (the API won't let you define a child checkpoint if the parent is not defined first). He asked if I could instead add bulk APIs, for getting the XML for all checkpoints at once, and then for redefining all checkpoints at once. Since my checkpoint code borrows heavily from concepts in the snapshot code, I chose to tackle the problem by starting with this series, which does the same thing for snapshots as what I plan to do for checkpoints. That is, since this patch series adds virDomainGetXMLDesc(, VIR_DOMAIN_XML_SNAPSHOTS) and virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST), the checkpoint series will add virDOmainGetXMLDesc(, VIR_DOMAIN_XML_CHECKPOINTS) and virDomainCheckpointCreateXML(, VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE_LIST) with very similar code. Also available at: https://repo.or.cz/libvirt/ericb.git/shortlog/refs/tags/snapshot-bulk-v3 Changes since v2: - address lots of comments from John - split and rearrange some patches - incorporate remaining patches from my series fixing --redefine bugs - rebase to master 001/18:[down] 'qemu: Refactor snapshot check for _LIVE vs. _REDEFINE' 002/18:[down] 'snapshot: Rework virDomainSnapshotState enum' 003/18:[down] 'qemu: Use virDomainSnapshotState for switch statements' 004/18:[0017] [FC] 'snapshot: Give virDomainSnapshotDefFormat its own flags' 005/18:[0003] [FC] 'snapshot: Refactor virDomainSnapshotDefFormat' 006/18:[0029] [FC] 'snapshot: Add virDomainSnapshotObjListFormat' 007/18:[down] 'domain: Add struct for future domain format parameters' 008/18:[down] 'snapshot: Avoid latent use-after-free when cleaning snapshots' 009/18:[0071] [FC] 'domain: Expand virDomainDefFormatInternal with snapshots' 010/18:[down] 'snapshot: Split out virDomainSnapshotRedefineValidate helper' 011/18:[down] 'snapshot: Add virDomainSnapshotObjListParse' 012/18:[0005] [FC] 'domain: Add VIR_DOMAIN_XML_SNAPSHOTS flag' 013/18:[0002] [FC] 'snapshot: Add VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST flag' 014/18:[0018] [FC] 'virsh: Expose bulk snapshot dumpxml/redefine' 015/18:[0010] [FC] 'test: Implement bulk snapshot operations' 016/18:[down] 'qemu: Factor out qemuDomainSnapshotValidate() helper' 017/18:[down] 'qemu: Const-correct snapshot directory name' 018/18:[0033] [FC] 'qemu: Implement bulk snapshot operations' Eric Blake (18): qemu: Refactor snapshot check for _LIVE vs. _REDEFINE snapshot: Rework virDomainSnapshotState enum qemu: Use virDomainSnapshotState for switch statements snapshot: Give virDomainSnapshotDefFormat its own flags snapshot: Refactor virDomainSnapshotDefFormat snapshot: Add virDomainSnapshotObjListFormat domain: Add struct for future domain format parameters snapshot: Avoid latent use-after-free when cleaning snapshots domain: Expand virDomainDefFormatInternal with snapshots snapshot: Split out virDomainSnapshotRedefineValidate helper snapshot: Add virDomainSnapshotObjListParse domain: Add VIR_DOMAIN_XML_SNAPSHOTS flag snapshot: Add VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST flag virsh: Expose bulk snapshot dumpxml/redefine test: Implement bulk snapshot operations qemu: Factor out qemuDomainSnapshotValidate() helper qemu: Const-correct snapshot directory name qemu: Implement bulk snapshot operations include/libvirt/libvirt-domain-snapshot.h | 3 + include/libvirt/libvirt-domain.h | 1 + src/conf/domain_conf.h | 21 +- src/conf/snapshot_conf.h | 47 +- src/qemu/qemu_domain.h | 2 +- src/conf/domain_conf.c | 69 ++- src/conf/snapshot_conf.c | 513 ++++++++++++++++------ src/esx/esx_driver.c | 1 - src/libvirt-domain-snapshot.c | 23 +- src/libvirt-domain.c | 8 +- src/libvirt_private.syms | 3 + src/network/bridge_driver.c | 2 +- src/qemu/qemu_domain.c | 47 +- src/qemu/qemu_driver.c | 234 ++++++---- src/test/test_driver.c | 52 ++- src/vbox/vbox_common.c | 13 +- src/vz/vz_driver.c | 3 +- tests/domainsnapshotxml2xmltest.c | 16 +- tools/virsh-domain.c | 7 + tools/virsh-snapshot.c | 14 + tools/virsh.pod | 18 +- 21 files changed, 790 insertions(+), 307 deletions(-) -- 2.20.1

3 38

[libvirt] [PATCH] LXC containers don't stopped under some conditions
by Maxim Kozin 07 Mar '19

07 Mar '19

LXC containers can be started, but time to time can't be stopped. 1) virsh shutdown with option "--mode initctl" always stop container, but virsh report error: "Container does not provide an initctl pipe" In container present /dev/initctl 2) virsh shutdown with option "--mode signal" never stop lxc container, at least for centos 7.5/7.6 In container log: systemd: Received SIGTERM. systemd: Reexecuting. systemd: systemd 219 running in system mode. (+PAM +AUDIT +SELINUX +IMA -APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ +LZ4 -SECCOMP +BLKID +ELFUTILS +KMOD +IDN) systemd: Detected virtualization lxc-libvirt. systemd: Detected architecture x86-64. 3) virsh shutdown without option "--mode" can stop lxc container, but in 2 cases: - cointainer not under heavy load - in gdb when perform step-by-setp debug But most often not, with simptoms as with --mode signal. Patch tested only with host centos 7.6 and guests centos 7.5/7.6 Short comments to patch. lxcDomainShutdownFlags() return rc=0 and container begin stopped. We not go to endjob label, but later we pass check: if (rc == 0 && (flags == 0 || (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { And trying next shutdown method with sigterm to PID 1. If container heavy loaded, it not stopped. IF not or if you wait in gdb, then first method succefully perform shutdown. Signed-off-by: Maxim Kozin <kolomaxes(a)gmail.com> --- src/lxc/lxc_driver.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b1ef221..760f9f8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3269,7 +3269,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL | VIR_DOMAIN_SHUTDOWN_SIGNAL, -1); @@ -3298,19 +3298,17 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { if (kill(priv->initpid, SIGTERM) < 0 && @@ -3347,7 +3345,7 @@ lxcDomainReboot(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL | VIR_DOMAIN_REBOOT_SIGNAL, -1); @@ -3376,19 +3374,17 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_REBOOT_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_REBOOT_SIGNAL))) { if (kill(priv->initpid, SIGHUP) < 0 && -- 1.8.3.1

2 1

[libvirt] [PATCH] conf: Remove unnecessary @ctxt check in virSecurityLabelDefsParseXML
by John Ferlan 07 Mar '19

07 Mar '19

Failure would have occurred before in callers other virXPath calls. Found by Coverity due to commit 66a508d2 using VIR_XPATH_NODE_AUTORESTORE to access @ctxt before the if condition. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/domain_conf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb660f5764..dc57b23084 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8516,8 +8516,7 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def, virCapsHostPtr host = &caps->host; VIR_AUTOFREE(xmlNodePtr *) list = NULL; - /* Check args and save context */ - if (def == NULL || ctxt == NULL) + if (def == NULL) return 0; /* Allocate a security labels based on XML */ -- 2.20.1

2 1