harikrishna-patnala commented on code in PR #6589:
URL: https://github.com/apache/cloudstack/pull/6589#discussion_r1250404467
##########
engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql:
##########
Review Comment:
These changes need to be moved to schema-41810to41900.sql.
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/DetachVolumeCmd.java:
##########
Review Comment:
any reason for these setters in Cmd file ? if not can you please remove them.
##########
engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java:
##########
@@ -214,7 +214,7 @@ public NicVO findByNetworkIdAndMacAddress(long networkId,
String mac) {
SearchCriteria<NicVO> sc = AllFieldsSearch.create();
sc.setParameters("network", networkId);
sc.setParameters("macAddress", mac);
- return findOneBy(sc);
+ return findOneIncludingRemovedBy(sc);
Review Comment:
Is this an intended change for this PR? can you please help me understand
why we are including removed entries.
Also there are other many usages of this method, so I see a chance of
regression here.
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -538,13 +543,33 @@ private Long getPoolIdFromDatastoreUuid(String
datastoreUuid) {
/**
* Get pool ID for disk
*/
- private Long getPoolId(VirtualDisk disk) {
+ private Long getPoolId(VirtualDisk disk, long datacenterId, long
clusterId) {
VirtualDeviceBackingInfo backing = disk.getBacking();
checkBackingInfo(backing);
VirtualDiskFlatVer2BackingInfo info =
(VirtualDiskFlatVer2BackingInfo)backing;
String[] fileNameParts = info.getFileName().split(" ");
- String datastoreUuid = StringUtils.substringBetween(fileNameParts[0],
"[", "]");
- return getPoolIdFromDatastoreUuid(datastoreUuid);
+ String datastore = StringUtils.substringBetween(fileNameParts[0], "[",
"]");
+ if (UuidUtils.isUuid(datastore)) {
+ return getPoolIdFromDatastoreUuid(datastore);
+ }
+ return getPoolIdFromDatastoreNameOrPath(datastore, datacenterId,
clusterId);
+ }
+
+ protected Long getPoolIdFromDatastoreNameOrPath(String datastore, long
datacenterId, long clusterId) {
Review Comment:
just to confirm, are we fixing any known issue here with respect to pool ID
? I don't see this related to the backups
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
