Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18025 )
Change subject: [security] KUDU-3316 Add encrypted file keys ...................................................................... Patch Set 8: (1 comment) > (15 comments) > > > Patch Set 3: > > > > (7 comments) > > > > I took a quick look and since I see the size of the encryption is > still floating around, may be it makes sense to re-evaluate the > approach I already pointed at > https://gerrit.cloudera.org/#/c/18025/3/src/kudu/util/env.h@194 > ? > > > > What do you think? > > What do you mean the by the size floating around and how would your > suggestion address that? There are more details at https://gerrit.cloudera.org/#/c/18025/8/src/kudu/util/file_cache-test.cc@107 http://gerrit.cloudera.org:8080/#/c/18025/8/src/kudu/consensus/log_index.cc File src/kudu/consensus/log_index.cc: http://gerrit.cloudera.org:8080/#/c/18025/8/src/kudu/consensus/log_index.cc@124 PS8, Line 124: Read(file_->GetEncryptionHeaderSize() > Answered this on https://gerrit.cloudera.org/c/18025/8/src/kudu/util/file_c I don't think I got a proper answer there. You mentioned that you started with that approach, but after a while for the reason you didn't not exactly specify you decided to switch to passing around the offset and the header size all over at the higher levels. It would be great if you provide the exact reason. -- To view, visit http://gerrit.cloudera.org:8080/18025 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb1282c117271fda63a8cc54c00add7cc96dcffd Gerrit-Change-Number: 18025 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Wed, 12 Jan 2022 17:56:54 +0000 Gerrit-HasComments: Yes
