In these cases the check that is removed has been done a few lines above already (as can even be seen in the context). Drop them. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- examples/dommigrate/dommigrate.c | 3 +-- src/conf/nwfilter_conf.c | 4 +--- src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_hotplug.c | 20 +++++++++----------- src/util/virresctrl.c | 2 +- tools/virsh-snapshot.c | 5 ++--- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/examples/dommigrate/dommigrate.c b/examples/dommigrate/dommigrate.c index 1b6072d138..60cfb3fb83 100644 --- a/examples/dommigrate/dommigrate.c +++ b/examples/dommigrate/dommigrate.c @@ -37,51 +37,50 @@ int main(int argc, char *argv[]) { char *src_uri, *dst_uri, *domname; int ret = 0; virConnectPtr conn = NULL; virDomainPtr dom = NULL; if (argc < 4) { ret = usage(argv[0], 1); goto out; } src_uri = argv[1]; dst_uri = argv[2]; domname = argv[3]; printf("Attempting to connect to the source hypervisor...\n"); conn = virConnectOpenAuth(src_uri, virConnectAuthPtrDefault, 0); if (!conn) { ret = 1; fprintf(stderr, "No connection to the source hypervisor: %s.\n", virGetLastErrorMessage()); goto out; } printf("Attempting to retrieve domain %s...\n", domname); dom = virDomainLookupByName(conn, domname); if (!dom) { fprintf(stderr, "Failed to find domain %s.\n", domname); goto cleanup; } printf("Attempting to migrate %s to %s...\n", domname, dst_uri); if ((ret = virDomainMigrateToURI(dom, dst_uri, VIR_MIGRATE_PEER2PEER, NULL, 0)) != 0) { fprintf(stderr, "Failed to migrate domain %s.\n", domname); goto cleanup; } printf("Migration finished with success.\n"); cleanup: if (dom != NULL) virDomainFree(dom); - if (conn != NULL) - virConnectClose(conn); + virConnectClose(conn); out: return ret; } diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 7cad3ccc57..3f28c7b451 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2610,128 +2610,126 @@ static virNWFilterDefPtr virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { virNWFilterDefPtr ret; xmlNodePtr curr = ctxt->node; char *uuid = NULL; char *chain = NULL; char *chain_pri_s = NULL; virNWFilterEntryPtr entry; int chain_priority; const char *name_prefix; if (VIR_ALLOC(ret) < 0) return NULL; ret->name = virXPathString("string(./@name)", ctxt); if (!ret->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("filter has no name")); goto cleanup; } chain_pri_s = virXPathString("string(./@priority)", ctxt); if (chain_pri_s) { if (virStrToLong_i(chain_pri_s, NULL, 10, &chain_priority) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("Could not parse chain priority '%s'"), chain_pri_s); goto cleanup; } if (chain_priority < NWFILTER_MIN_FILTER_PRIORITY || chain_priority > NWFILTER_MAX_FILTER_PRIORITY) { virReportError(VIR_ERR_INVALID_ARG, _("Priority '%d' is outside valid " "range of [%d,%d]"), chain_priority, NWFILTER_MIN_FILTER_PRIORITY, NWFILTER_MAX_FILTER_PRIORITY); goto cleanup; } } chain = virXPathString("string(./@chain)", ctxt); if (chain) { name_prefix = virNWFilterIsAllowedChain(chain); if (name_prefix == NULL) goto cleanup; ret->chainsuffix = chain; if (chain_pri_s) { ret->chainPriority = chain_priority; } else { /* assign default priority if none can be found via lookup */ - if (!name_prefix || - intMapGetByString(chain_priorities, name_prefix, 0, - &ret->chainPriority) < 0) { + if (intMapGetByString(chain_priorities, name_prefix, 0, &ret->chainPriority) < 0) { /* assign default chain priority */ ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + NWFILTER_MIN_FILTER_PRIORITY) / 2; } } chain = NULL; } else { if (VIR_STRDUP(ret->chainsuffix, virNWFilterChainSuffixTypeToString(VIR_NWFILTER_CHAINSUFFIX_ROOT)) < 0) goto cleanup; } uuid = virXPathString("string(./uuid)", ctxt); ret->uuid_specified = (uuid != NULL); if (uuid == NULL) { if (virUUIDGenerate(ret->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to generate uuid")); goto cleanup; } } else { if (virUUIDParse(uuid, ret->uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed uuid element")); goto cleanup; } VIR_FREE(uuid); } curr = curr->children; while (curr != NULL) { if (curr->type == XML_ELEMENT_NODE) { if (VIR_ALLOC(entry) < 0) goto cleanup; if (virXMLNodeNameEqual(curr, "rule")) { if (!(entry->rule = virNWFilterRuleParse(curr))) { virNWFilterEntryFree(entry); goto cleanup; } } else if (virXMLNodeNameEqual(curr, "filterref")) { if (!(entry->include = virNWFilterIncludeParse(curr))) { virNWFilterEntryFree(entry); goto cleanup; } } if (entry->rule || entry->include) { if (VIR_APPEND_ELEMENT_COPY(ret->filterEntries, ret->nentries, entry) < 0) { virNWFilterEntryFree(entry); goto cleanup; } } else { virNWFilterEntryFree(entry); } } curr = curr->next; } VIR_FREE(chain); VIR_FREE(chain_pri_s); return ret; cleanup: virNWFilterDefFree(ret); VIR_FREE(chain); VIR_FREE(uuid); VIR_FREE(chain_pri_s); return NULL; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 74f34af292..ee70444bd4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4201,31 +4201,30 @@ static int qemuBuildNVRAMCommandLine(virCommandPtr cmd, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { if (!def->nvram) return 0; if (qemuDomainIsPSeries(def)) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvram device is not supported by " "this QEMU binary")); return -1; } char *optstr; virCommandAddArg(cmd, "-global"); optstr = qemuBuildNVRAMDevStr(def->nvram); if (!optstr) return -1; - if (optstr) - virCommandAddArg(cmd, optstr); + virCommandAddArg(cmd, optstr); VIR_FREE(optstr); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvram device is only supported for PPC64")); return -1; } return 0; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 00dbff6b2a..19b7e6e8ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3502,52 +3502,50 @@ static int qemuDomainChangeNetBridge(virDomainObjPtr vm, virDomainNetDefPtr olddev, virDomainNetDefPtr newdev) { int ret = -1; const char *oldbridge = virDomainNetGetActualBridgeName(olddev); const char *newbridge = virDomainNetGetActualBridgeName(newdev); if (!oldbridge || !newbridge) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name")); goto cleanup; } VIR_DEBUG("Change bridge for interface %s: %s -> %s", olddev->ifname, oldbridge, newbridge); if (virNetDevExists(newbridge) != 1) { virReportError(VIR_ERR_OPERATION_FAILED, _("bridge %s doesn't exist"), newbridge); goto cleanup; } - if (oldbridge) { - ret = virNetDevBridgeRemovePort(oldbridge, olddev->ifname); - virDomainAuditNet(vm, olddev, NULL, "detach", ret == 0); - if (ret < 0) { - /* warn but continue - possibly the old network - * had been destroyed and reconstructed, leaving the - * tap device orphaned. - */ - VIR_WARN("Unable to detach device %s from bridge %s", - olddev->ifname, oldbridge); - } + ret = virNetDevBridgeRemovePort(oldbridge, olddev->ifname); + virDomainAuditNet(vm, olddev, NULL, "detach", ret == 0); + if (ret < 0) { + /* warn but continue - possibly the old network + * had been destroyed and reconstructed, leaving the + * tap device orphaned. + */ + VIR_WARN("Unable to detach device %s from bridge %s", + olddev->ifname, oldbridge); } ret = virNetDevBridgeAddPort(newbridge, olddev->ifname); virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0); if (ret < 0) { ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname); virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0); if (ret < 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("unable to recover former state by adding port " "to bridge %s"), oldbridge); } goto cleanup; } /* caller will replace entire olddev with newdev in domain nets list */ ret = 0; cleanup: return ret; } diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 97891b2f8f..0857d4e882 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2111,57 +2111,57 @@ static int virResctrlAllocMemoryBandwidth(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, virResctrlAllocPtr free) { size_t i; virResctrlAllocMemBWPtr mem_bw_alloc = alloc->mem_bw; virResctrlAllocMemBWPtr mem_bw_free = free->mem_bw; virResctrlInfoMemBWPtr mem_bw_info = resctrl->membw_info; if (!mem_bw_alloc) return 0; - if (mem_bw_alloc && !mem_bw_info) { + if (!mem_bw_info) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("RDT Memory Bandwidth allocation unsupported")); return -1; } for (i = 0; i < mem_bw_alloc->nbandwidths; i++) { if (!mem_bw_alloc->bandwidths[i]) continue; if (*(mem_bw_alloc->bandwidths[i]) % mem_bw_info->bandwidth_granularity) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Memory Bandwidth allocation of size " "%u is not divisible by granularity %u"), *(mem_bw_alloc->bandwidths[i]), mem_bw_info->bandwidth_granularity); return -1; } if (*(mem_bw_alloc->bandwidths[i]) < mem_bw_info->min_bandwidth) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Memory Bandwidth allocation of size " "%u is smaller than the minimum " "allowed allocation %u"), *(mem_bw_alloc->bandwidths[i]), mem_bw_info->min_bandwidth); return -1; } if (i > mem_bw_info->max_id) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("bandwidth controller id %zd does not " "exist, max controller id %u"), i, mem_bw_info->max_id); return -1; } if (*(mem_bw_alloc->bandwidths[i]) > *(mem_bw_free->bandwidths[i])) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Not enough room for allocation of %u%% " "bandwidth on node %zd, available bandwidth %u%%"), *(mem_bw_alloc->bandwidths[i]), i, *(mem_bw_free->bandwidths[i])); return -1; } } return 0; } diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index e38ebb1f28..025321c58e 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1046,323 +1046,322 @@ static virshSnapshotListPtr virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, virDomainSnapshotPtr from, unsigned int orig_flags, bool tree) { size_t i; char **names = NULL; int count = -1; bool descendants = false; bool roots = false; virDomainSnapshotPtr *snaps; virshSnapshotListPtr snaplist = vshMalloc(ctl, sizeof(*snaplist)); virshSnapshotListPtr ret = NULL; const char *fromname = NULL; int start_index = -1; int deleted = 0; bool filter_fallback = false; unsigned int flags = orig_flags; virshControlPtr priv = ctl->privData; /* Try the interface available in 0.9.13 and newer. */ if (!priv->useSnapshotOld) { if (from) count = virDomainSnapshotListAllChildren(from, &snaps, flags); else count = virDomainListAllSnapshots(dom, &snaps, flags); /* If we failed because of flags added in 1.0.1, we can do * fallback filtering. */ if (count < 0 && last_error->code == VIR_ERR_INVALID_ARG && flags & (VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS | VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)) { flags &= ~(VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS | VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION); vshResetLibvirtError(); filter_fallback = true; if (from) count = virDomainSnapshotListAllChildren(from, &snaps, flags); else count = virDomainListAllSnapshots(dom, &snaps, flags); } } if (count >= 0) { /* When mixing --from and --tree, we also want a copy of from * in the list, but with no parent for that one entry. */ snaplist->snaps = vshCalloc(ctl, count + (tree && from), sizeof(*snaplist->snaps)); snaplist->nsnaps = count; for (i = 0; i < count; i++) snaplist->snaps[i].snap = snaps[i]; VIR_FREE(snaps); if (tree) { for (i = 0; i < count; i++) { if (virshGetSnapshotParent(ctl, snaplist->snaps[i].snap, &snaplist->snaps[i].parent) < 0) goto cleanup; } if (from) { snaplist->snaps[snaplist->nsnaps++].snap = from; virDomainSnapshotRef(from); } } goto success; } /* Assume that if we got this far, then the --no-leaves and * --no-metadata flags were not supported. Disable groups that * have no impact. */ /* XXX should we emulate --no-leaves? */ if (flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES && flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES | VIR_DOMAIN_SNAPSHOT_LIST_LEAVES); if (flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA && flags & VIR_DOMAIN_SNAPSHOT_LIST_METADATA) flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA | VIR_DOMAIN_SNAPSHOT_LIST_METADATA); if (flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) { /* We can emulate --no-metadata if --metadata was supported, * since it was an all-or-none attribute on old servers. */ count = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_LIST_METADATA); if (count < 0) goto cleanup; if (count > 0) return snaplist; flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA; } if (flags & (VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS | VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)) { flags &= ~(VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS | VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION); filter_fallback = true; } /* This uses the interfaces available in 0.8.0-0.9.6 * (virDomainSnapshotListNames, global list only) and in * 0.9.7-0.9.12 (addition of virDomainSnapshotListChildrenNames * for child listing, and new flags), as follows, with [*] by the * combinations that need parent info (either for filtering * purposes or for the resulting tree listing): * old new * list global as-is global as-is * list --roots *global + filter global + flags * list --from *global + filter child as-is * list --from --descendants *global + filter child + flags * list --tree *global as-is *global as-is * list --tree --from *global + filter *child + flags * * Additionally, when --tree and --from are both used, from is * added to the final list as the only element without a parent. * Otherwise, --from does not appear in the final list. */ if (from) { fromname = virDomainSnapshotGetName(from); if (!fromname) { vshError(ctl, "%s", _("Could not get snapshot name")); goto cleanup; } descendants = (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) || tree; if (tree) flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; /* Determine if we can use the new child listing API. */ if (priv->useSnapshotOld || ((count = virDomainSnapshotNumChildren(from, flags)) < 0 && last_error->code == VIR_ERR_NO_SUPPORT)) { /* We can emulate --from. */ /* XXX can we also emulate --leaves? */ vshResetLibvirtError(); priv->useSnapshotOld = true; flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; goto global; } if (tree && count >= 0) count++; } else { global: /* Global listing (including fallback when --from failed with * child listing). */ count = virDomainSnapshotNum(dom, flags); /* Fall back to simulation if --roots was unsupported. */ /* XXX can we also emulate --leaves? */ if (!from && count < 0 && last_error->code == VIR_ERR_INVALID_ARG && (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) { vshResetLibvirtError(); roots = true; flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; count = virDomainSnapshotNum(dom, flags); } } if (count < 0) { if (!last_error) vshError(ctl, _("failed to collect snapshot list")); goto cleanup; } if (!count) goto success; names = vshCalloc(ctl, sizeof(*names), count); /* Now that we have a count, collect the list. */ if (from && !priv->useSnapshotOld) { if (tree) { - if (count) - count = virDomainSnapshotListChildrenNames(from, names + 1, - count - 1, flags); + count = virDomainSnapshotListChildrenNames(from, names + 1, + count - 1, flags); if (count >= 0) { count++; names[0] = vshStrdup(ctl, fromname); } } else { count = virDomainSnapshotListChildrenNames(from, names, count, flags); } } else { count = virDomainSnapshotListNames(dom, names, count, flags); } if (count < 0) goto cleanup; snaplist->snaps = vshCalloc(ctl, sizeof(*snaplist->snaps), count); snaplist->nsnaps = count; for (i = 0; i < count; i++) { snaplist->snaps[i].snap = virDomainSnapshotLookupByName(dom, names[i], 0); if (!snaplist->snaps[i].snap) goto cleanup; } /* Collect parents when needed. With the new API, --tree and * --from together put from as the first element without a parent; * with the old API we still need to do a post-process filtering * based on all parent information. */ if (tree || (from && priv->useSnapshotOld) || roots) { for (i = (from && !priv->useSnapshotOld); i < count; i++) { if (from && priv->useSnapshotOld && STREQ(names[i], fromname)) { start_index = i; if (tree) continue; } if (virshGetSnapshotParent(ctl, snaplist->snaps[i].snap, &snaplist->snaps[i].parent) < 0) goto cleanup; if ((from && ((tree && !snaplist->snaps[i].parent) || (!descendants && STRNEQ_NULLABLE(fromname, snaplist->snaps[i].parent)))) || (roots && snaplist->snaps[i].parent)) { virshDomainSnapshotFree(snaplist->snaps[i].snap); snaplist->snaps[i].snap = NULL; VIR_FREE(snaplist->snaps[i].parent); deleted++; } } } if (tree) goto success; if (priv->useSnapshotOld && descendants) { bool changed = false; bool remaining = false; /* Make multiple passes over the list - first pass finds * direct children and NULLs out all roots and from, remaining * passes NULL out any undecided entry whose parent is not * still in list. We mark known descendants by clearing * snaps[i].parents. Sorry, this is O(n^3) - hope your * hierarchy isn't huge. XXX Is it worth making O(n^2 log n) * by using qsort and bsearch? */ if (start_index < 0) { vshError(ctl, _("snapshot %s disappeared from list"), fromname); goto cleanup; } for (i = 0; i < count; i++) { if (i == start_index || !snaplist->snaps[i].parent) { VIR_FREE(names[i]); virshDomainSnapshotFree(snaplist->snaps[i].snap); snaplist->snaps[i].snap = NULL; VIR_FREE(snaplist->snaps[i].parent); deleted++; } else if (STREQ(snaplist->snaps[i].parent, fromname)) { VIR_FREE(snaplist->snaps[i].parent); changed = true; } else { remaining = true; } } if (!changed) { ret = vshMalloc(ctl, sizeof(*snaplist)); goto cleanup; } while (changed && remaining) { changed = remaining = false; for (i = 0; i < count; i++) { bool found_parent = false; size_t j; if (!names[i] || !snaplist->snaps[i].parent) continue; for (j = 0; j < count; j++) { if (!names[j] || i == j) continue; if (STREQ(snaplist->snaps[i].parent, names[j])) { found_parent = true; if (!snaplist->snaps[j].parent) VIR_FREE(snaplist->snaps[i].parent); else remaining = true; break; } } if (!found_parent) { changed = true; VIR_FREE(names[i]); virshDomainSnapshotFree(snaplist->snaps[i].snap); snaplist->snaps[i].snap = NULL; VIR_FREE(snaplist->snaps[i].parent); deleted++; } } } } success: if (filter_fallback) { /* Older API didn't filter on status or location, but the * information is available in domain XML. */ if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS)) orig_flags |= VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS; if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)) orig_flags |= VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION; for (i = 0; i < snaplist->nsnaps; i++) { switch (virshSnapshotFilter(ctl, snaplist->snaps[i].snap, orig_flags)) { case 1: break; case 0: virshDomainSnapshotFree(snaplist->snaps[i].snap); snaplist->snaps[i].snap = NULL; VIR_FREE(snaplist->snaps[i].parent); deleted++; break; default: goto cleanup; } } } qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps), virshSnapSorter); snaplist->nsnaps -= deleted; VIR_STEAL_PTR(ret, snaplist); cleanup: virshSnapshotListFree(snaplist); if (names && count > 0) for (i = 0; i < count; i++) VIR_FREE(names[i]); VIR_FREE(names); return ret; } -- 2.19.2 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list