Back to success stories

Sample of Defect

Project Name CID Checker Category Developer Description
ovirt-engine 1323817 NULL_RETURNS Null pointer dereferences Possible Null Pointer exception was found in a feature in which I'm the feature owner. That exception could have rendered the entire feature non operational (And many log messages that the user wouldn't understand).
File: /backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/arem/AffinityRulesEnforcer.java
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
    private static final Logger log = LoggerFactory.getLogger(AffinityRulesEnforcer.class);

    @Inject
    private AffinityGroupDao affinityGroupDao;
    @Inject
    private VmDao vmDao;
    @Inject
    private SchedulingManager schedulingManager;
    private final Random random = new Random();

    protected enum FailMode {
        IMMEDIATELY, // Fail when first violation is detected
        GET_ALL // Collect all violations
    }

    public VM chooseNextVmToMigrate(VDSGroup vdsGroup) {
        List<AffinityGroup> allHardAffinityGroups = getAllHardAffinityGroups(vdsGroup);

        Set<Set<Guid>> unifiedPositiveAffinityGroups = AffinityRulesUtils.getUnifiedPositiveAffinityGroups(
                allHardAffinityGroups);
        List<AffinityGroup> unifiedAffinityGroups = AffinityRulesUtils.setsToAffinityGroups(
                unifiedPositiveAffinityGroups);

        // Add negative affinity groups
 < 1. Iterating over another element of "allHardAffinityGroups"
 < 4. No elements left in "allHardAffinityGroups", leaving loop
61
        for (AffinityGroup ag : allHardAffinityGroups) {
 < 2. Condition "!ag.isPositive()", taking true branch
62
63
64
            if (!ag.isPositive()) {
                unifiedAffinityGroups.add(ag);
            }
 < 3. Jumping back to the beginning of the loop
65
66
67
68
        }

        // Create a set of all VMs in affinity groups
        Set<Guid> allVms = new HashSet<>();
 < 5. Iterating over another element of "unifiedAffinityGroups"
 < 7. No elements left in "unifiedAffinityGroups", leaving loop
69
70
        for (AffinityGroup group : unifiedAffinityGroups) {
            allVms.addAll(group.getEntityIds());
 < 6. Jumping back to the beginning of the loop
71
72
73
74
75
76
77
        }

        Map<Guid, Guid> vmToHost = createMapOfVmToHost(allVms);

        // There is no need to migrate when no collision was detected
        Set<AffinityGroup> violatedAffinityGroups =
                checkForAffinityGroupViolations(unifiedAffinityGroups, vmToHost, FailMode.GET_ALL);
 < 8. Condition "violatedAffinityGroups.isEmpty()", taking false branch
78
79
80
81
82
83
84
85
86
        if (violatedAffinityGroups.isEmpty()) {
            log.debug("No affinity group collision detected for cluster {}. Standing by.", vdsGroup.getId());
            return null;
        }

        // Find a VM that is breaking the affinityGroup and can be theoretically migrated
        // - start with bigger Affinity Groups
        List<AffinityGroup> affGroupsBySize = new ArrayList<>(violatedAffinityGroups);
        Collections.sort(affGroupsBySize, Collections.reverseOrder(new AffinityGroupComparator()));
 < 9. Iterating over another element of "affGroupsBySize"
 < 14. Iterating over another element of "affGroupsBySize"
88
89
        for (AffinityGroup affinityGroup : affGroupsBySize) {
            final List<VM> candidateVms;
 < 10. Condition "affinityGroup.isPositive()", taking true branch
 < 15. Condition "affinityGroup.isPositive()", taking true branch
91
            if (affinityGroup.isPositive()) {
 <<< CID 1323817: Null pointer dereferences NULL_RETURNS
 <<< 16. "findVmViolatingPositiveAg" returns null (checked 0 out of 1 times).
 <<< CID 1323817: Null pointer dereferences NULL_RETURNS
 <<< 17. Dereferencing a pointer that might be null "findVmViolatingPositiveAg(affinityGroup, vmToHost)" when calling "getVmsByIds". (The virtual call resolves to "org.ovirt.engine.core.dao.VmDaoImpl.getVmsByIds".)
92
93
                candidateVms = vmDao.getVmsByIds(findVmViolatingPositiveAg(affinityGroup, vmToHost));
                log.info("Positive affinity group violation detected");
 < 11. Falling through to end of if statement
94
95
96
97
            } else {
                candidateVms = vmDao.getVmsByIds(findVmViolatingNegativeAg(affinityGroup, vmToHost));
                log.info("Negative affinity group violation detected");
            }
 < 12. Condition "!candidateVms.isEmpty()", taking false branch
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
            while (!candidateVms.isEmpty()) {
                final int index = random.nextInt(candidateVms.size());
                final VM candidateVm = candidateVms.get(index);

                // Test whether any migration is possible, this uses current AffinityGroup settings
                // and so won't allow more breakage
                boolean canMove = schedulingManager.canSchedule(vdsGroup, candidateVm,
                        new ArrayList<Guid>(), new ArrayList<Guid>(),
                        null, new ArrayList<String>());

                if (canMove) {
                    log.debug("VM {} is a viable candidate for solving affinity group violation situation.",
                            candidateVm.getId());
                    return candidateVm;
                }
                log.debug("VM {} is NOT a viable candidate for solving affinity group violation situation.",
                        candidateVm.getId());
                candidateVms.remove(index);
            }
 < 13. Jumping back to the beginning of the loop
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
        }

        // No possible migration..
        return null;
    }

    private Map<Guid, Guid> createMapOfVmToHost(Set<Guid> allVms) {
        Map<Guid, Guid> outputMap = new HashMap<>();

        for (VM vm : vmDao.getVmsByIds(new ArrayList<>(allVms))) {
            Guid hostId = vm.getRunOnVds();

            if (hostId != null) {
                outputMap.put(vm.getId(), hostId);
            }
        }

        return outputMap;
    }

    /**
     * Select VMs from the broken affinity group that are running on the same host.
     *
     * @param affinityGroup broken affinity rule
     * @param vmToHost      vm to host assignments
     * @return a list of vms which are candidates for migration
     */
Events:
16. returned_null AffinityRulesEnforcer.java:92
17. dereference AffinityRulesEnforcer.java:92