Dear guys

I found a bug about listUsageRecords API in 4.3 related to a issue
CLOUDSTACK-6472 and made a patch. Would you review the patch and
correct me if I am wrong?

Sam Schmit push a commit about NPEs/Null UUIDs at May 5th but I think
his commit is not enough.

    commit e8047e11db7ef2bdc44bbedfdc47e63c55f080c5
    Author: Sam Schmit <sam.sch...@appcore.com>
    Date:   Mon May 5 16:38:23 2014 -0500

        CLOUDSTACK-6472 (4.3 specific) listUsageRecords: Pull
information from removed items as well, fixing NPEs/Null UUIDs with
usage API calls.

        Signed-off-by: Sebastien Goasguen <run...@gmail.com>

When I called listUsageRecords against CloudStack 4.3 management
server that was build at latest 4.3 branch, usage records about
destroyed instances of usage type 1 and 2 lacked 'virtualmachineid' as
below.

    {
      "account": "sgcadm",
      "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
      "description": "mycluster-front allocated (ServiceOffering: 13)
(Template: 203)",
      "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
      "enddate": "2014-05-20'T'08:59:59+09:00",
      "name": "mycluster-front",
      "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
      "rawusage": "0.223611",
      "startdate": "2014-05-19'T'09:00:00+09:00",
      "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
      "type": "KVM",
      "usage": "0.223611 Hrs",
      "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
      "usagetype": 2,
      "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
    },

Hence, it is unable to refer to any destroyed instance id. I patched
createUsageResponse method like this.

    diff --git a/server/src/com/cloud/api/ApiResponseHelper.java
b/server/src/com/cloud/api/ApiResponseHelper.java
    index 7718fc1..cbf3914 100755
    --- a/server/src/com/cloud/api/ApiResponseHelper.java
    +++ b/server/src/com/cloud/api/ApiResponseHelper.java
    @@ -3304,7 +3304,7 @@ public class ApiResponseHelper implements
ResponseGenerator {
             usageRecResponse.setUsage(usageRecord.getUsageDisplay());
             usageRecResponse.setUsageType(usageRecord.getUsageType());
             if (usageRecord.getVmInstanceId() != null) {
    -            VMInstanceVO vm =
_entityMgr.findById(VMInstanceVO.class,
usageRecord.getVmInstanceId());
    +            VMInstanceVO vm =
_entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
usageRecord.getVmInstanceId());
                 if (vm != null) {
                     usageRecResponse.setVirtualMachineId(vm.getUuid());
                 }

After that, listUsageRecords API contains 'virtualmachineid' whether
or not instances are destroyed. Here is a result.

    {
      "account": "sgcadm",
      "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
      "description": "mycluster-front allocated (ServiceOffering: 13)
(Template: 203)",
      "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
      "enddate": "2014-05-20'T'08:59:59+09:00",
      "name": "mycluster-front",
      "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
      "rawusage": "0.223611",
      "startdate": "2014-05-19'T'09:00:00+09:00",
      "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
      "type": "KVM",
      "usage": "0.223611 Hrs",
      "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
      "usagetype": 2,
      "virtualmachineid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
      "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
    },

I think this is an expectecd behaviour, so please review and test this
patch.


Best Regards

--
Hiroki Ohashi

Reply via email to