Juan Hernandez has posted comments on this change. Change subject: restapi: openstack_volume_type on Disk entity ......................................................................
Patch Set 2: (6 comments) https://gerrit.ovirt.org/#/c/41966/2/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 605: disk.wipe_after_delete: xs:boolean Line 606: disk.quota.id: xs:string Line 607: disk.disk_profile.id: xs:string Line 608: disk.storage_domains.storage_domain--COLLECTION: {storage_domain.id|name: 'xs:string'} Line 609: disk.openstack_volume_type: xs:string Should be "disk.openstack_volume_type.name". Line 610: description: add a new disk to the virtual machine allocating space from the storage domain Line 611: #the signature below is for direct-LUN disk, which doesn't require size, but requires the lun id, the lun type, and the lun connection-details. Line 612: - mandatoryArguments: {disk.interface: 'xs:string', disk.lun_storage.type: 'xs:string', Line 613: disk.lun_storage.logical_unit--COLLECTION: {logical_unit.id: 'xs:string', logical_unit.address: 'xs:string', logical_unit.port: 'xs:int', logical_unit.target: 'xs:string'}} Line 882: disk.wipe_after_delete: xs:boolean Line 883: disk.quota.id: xs:string Line 884: disk.disk_profile.id: xs:string Line 885: disk.storage_domains.storage_domain--COLLECTION: {storage_domain.id|name: 'xs:string'} Line 886: disk.openstack_volume_type: xs:string Same. Line 887: description: add a new disk to the system with specified size, space will be allocated from the storage domain for the disk Line 888: #the signature below is for direct-LUN disk, which doesn't require size, but requires the lun id, the lun type, and the lun connection-details. Line 889: - mandatoryArguments: {disk.interface: 'xs:string', disk.lun_storage.type: 'xs:string', Line 890: disk.lun_storage.logical_unit--COLLECTION: {logical_unit.id: 'xs:string', logical_unit.address: 'xs:string', logical_unit.port: 'xs:int', logical_unit.target: 'xs:string'}} Line 2998: disk.propagate_errors: xs:boolean Line 2999: disk.wipe_after_delete: xs:boolean Line 3000: disk.quota.id: xs:string Line 3001: disk.disk_profile.id: xs:string Line 3002: disk.openstack_volume_type: xs:string Same. Line 3003: description: add a new disk to the storage domain with the specified size allocating space from the storage domain Line 3004: #the signature below is for direct-LUN disk, which doesn't require size, but requires the lun id, the lun type, and the lun connection-details. Line 3005: - mandatoryArguments: {disk.interface: 'xs:string', disk.format: 'xs:string', disk.lun_storage.type: 'xs:string', Line 3006: disk.lun_storage.logical_unit--COLLECTION: {logical_unit.id: 'xs:string', logical_unit.address: 'xs:string', logical_unit.port: 'xs:int', logical_unit.target: 'xs:string'}} Line 3065: disk.propagate_errors: xs:boolean Line 3066: disk.wipe_after_delete: xs:boolean Line 3067: disk.quota.id: xs:string Line 3068: disk.disk_profile.id: xs:string Line 3069: disk.openstack_volume_type: xs:string Same. Line 3070: description: add a new disk with the specified size to the storage domain in the data center, allocating space from the storage domain Line 3071: #the signature below is for direct-LUN disk, which doesn't require size, but requires the lun id, the lun type, and the lun connection-details. Line 3072: - mandatoryArguments: {disk.interface: 'xs:string', disk.format: 'xs:string', disk.lun_storage.type: 'xs:string', Line 3073: disk.lun_storage.logical_unit--COLLECTION: {logical_unit.id: 'xs:string', logical_unit.address: 'xs:string', logical_unit.port: 'xs:int', logical_unit.target: 'xs:string'}} https://gerrit.ovirt.org/#/c/41966/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DiskMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DiskMapper.java: Line 161: if (disk.isSetDiskProfile() && disk.getDiskProfile().isSetId()) { Line 162: diskImage.setDiskProfileId(GuidUtils.asGuid(disk.getDiskProfile().getId())); Line 163: } Line 164: if (disk.isSetOpenstackVolumeType()) { Line 165: diskImage.setCinderVolumeType(disk.getOpenstackVolumeType().getName()); This should check if "name" has a value before calling "getName". If no name is provided by the user then the "setCinderVolumeType" shouldn't be called. Otherwise we would be calling it with "null" as value, and that may have undesired effects. Line 166: } Line 167: } Line 168: Line 169: @Mapping(from = org.ovirt.engine.core.common.businessentities.storage.Disk.class, to = Disk.class) Line 240: DiskProfile diskProfile = new DiskProfile(); Line 241: diskProfile.setId(entity.getDiskProfileId().toString()); Line 242: model.setDiskProfile(diskProfile); Line 243: } Line 244: if (entity.getCinderVolumeType() != null) { The "openstack_volume_type" element my have already been populated, and this is replacing it completely. Instead of doing this try to check it: OpenStackVolumeType volumeType = model.getOpenStackVolumeType(); if (volumeType == null) { volumeType = new OpenStackVolumeType(); model.setOpenStackVolumeType(volumeType); } volumeType.setWhatever(...); ... I know that this won't be a problem in this case, but better to be prepared for potential changse in the future. Line 245: OpenStackVolumeType volumeType = new OpenStackVolumeType(); Line 246: volumeType.setName(entity.getCinderVolumeType()); Line 247: model.setOpenstackVolumeType(volumeType); Line 248: } -- To view, visit https://gerrit.ovirt.org/41966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I952c431b8f7e7d0508855892b518c43bef9c7b7f Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
