cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Wilder Rodrigues <WRodrig...@schubergphilis.com>
Subject Contrail plugin - VirtualMchineModel
Date Tue, 11 Feb 2014 08:00:52 GMT
Hi guys,

We are working with FindBugs and trying to get Scariest and Scary bugs fixed on the master
branch.

FindBugs has reported a bug on the contrail plugin, in the VirtualMachineModel class. If you
check the Checking the commit id cc2b1c4961244d9c3d8b452f1dcaa6614e56d11a, you will see the
following:

        ServiceInstance siObj;
       try {
            siObj = (ServiceInstance) api.findById(ServiceInstance.class, serviceUuid);
        } catch (IOException ex) {
            s_logger.warn("service-instance read", ex);
            throw new CloudRuntimeException("Unable to read service-instance object", ex);
        }

        ServiceInstanceModel siModel;
        if (siObj == null) {
            siModel = new ServiceInstanceModel(serviceUuid);
            siModel.build(controller, siObj);
            manager.getDatabase().getServiceInstances().add(siModel);
        } else {
            String fqn = StringUtils.join(siObj.getQualifiedName(), ':');
            siModel = manager.getDatabase().lookupServiceInstance(fqn);
            if (siModel == null) {
                siModel = new ServiceInstanceModel(serviceUuid);
                siModel.build(controller, siObj);
                manager.getDatabase().getServiceInstances().add(siModel);
            }
        }

If the ServiceInstance object (siObj) is null after the call to api.findById, the following
call to siModel.buil() will break really bad:

::: ServiceInstanceModel:::build() method

    public void build(ModelController controller, ServiceInstance siObj) {
        ApiConnector api = controller.getApiAccessor();
        _serviceInstance = siObj;
        _fqName = StringUtils.join(siObj.getQualifiedName(), ':');
        ServiceInstanceType props = siObj.getProperties();
        // TODO: read management network names and cache network objects.
        ObjectReference ref = siObj.getServiceTemplate().get(0);

As you can see in the snippet above, the ServiceInstance siObj is used in 4 places, whereas
we will face a NPE in the getQualifiedName() method.

I would suggest that in the VirtualMachineModel.build() method we check for empty/valid UUID
before calling the private method (buildServiceInstance). At this moment the build() method
only checks if the UUID is not null.

Looking forward to your response.

With kind regards,
Wilder

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message