greenwich commented on PR #10380:
URL: https://github.com/apache/ozone/pull/10380#issuecomment-4581669907

   > > Thanks! LGTM with one note though:
   > > It's not directly related to this PR: 
`StorageTypeUtils.getStorageTypeProto` throws an unchecked 
`IllegalArgumentException` for any `org.apache.hadoop.fs.StorageType` it 
doesn't map. With Hadoop `3.4.3` that enum includes `NVDIMM`, which 
`StorageTypeProto` doesn't cover, and `StorageLocation.parse` accepts an 
`[NVDIMM]/...` entry in h`dds.datanode.dir` with no Ozone-side filtering. So a 
datanode with such a volume would throw here on every 
`buildContainerReplicaProto`, breaking that DN's container report.
   > 
   > @greenwich
   > 
   > Thank you for your feedback. This is a vulnerability that needs to be 
fixed.
   > 
   > I'm considering whether to use Hadoop's `org.apache.hadoop.fs.StorageType` 
in Ozone. Perhaps we should define a custom `org.apache.ozone.fs.StorageType`. 
Currently, using `org.apache.hadoop.fs.StorageType` doesn't seem to offer any 
benefits. Hadoop's `org.apache.hadoop.fs.FileSystem` only uses `StorageType` in 
`setQuotaByStorageType`, and Ozone may not be compatible with this interface. 
If we define a custom `StorageType`, we can set the enum content more flexibly.
   
   I agree, having `org.apache.ozone.fs.StorageType` is a much better choice. 
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to