> On June 14, 2013, 3:24 p.m., John Burwell wrote: > > api/src/com/cloud/storage/template/TemplateInfo.java, line 32 > > <https://reviews.apache.org/r/11861/diff/2/?file=305010#file305010line32> > > > > Null is not a sufficient check for blank. Use StringUtils#isBlank to > > catch both null and instances containing only spaces.
empty or spaces are not harmfull in this location > On June 14, 2013, 3:24 p.m., John Burwell wrote: > > core/src/com/cloud/storage/template/TemplateLocation.java, line 168 > > <https://reviews.apache.org/r/11861/diff/2/?file=305013#file305013line168> > > > > Why are is the path being manipulated in addition to the work being > > down by the TemplateInfo class? These rules should be completely > > encapsulated in the TemplateInfo class. The two manipulations are not exactly the same. File.separator is not always "/". > On June 14, 2013, 3:24 p.m., John Burwell wrote: > > api/src/com/cloud/storage/template/TemplateInfo.java, line 38 > > <https://reviews.apache.org/r/11861/diff/2/?file=305010#file305010line38> > > > > WARN should be used to let an operator know there is a condition in the > > system that could lead to instability. A data cleanup will not result in > > eventual system instability. I recommend converting to debug or trace. The point is that a mixed posix-paths/UNC system triggered this fix. A double slash has double meaning in such an environment. However the error, be it human or system generated, may or may not destabalize cloudstack, so I will stick with the info. It is certainly not debug in my opinion. It is not a bug that needs debugging. quoting Hiroaki KAWAI: If something went wrong and double slash was passed to Winfows based NFS, the reason may A) there was another code that generates double slash B) cloudstack configuration or something user input was bad C) some path components became empty string because of database error or something unexpeceted D) cloudstack is really being attacked etc. - daan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11861/#review21907 ----------------------------------------------------------- On June 15, 2013, 12:41 p.m., daan Hoogland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11861/ > ----------------------------------------------------------- > > (Updated June 15, 2013, 12:41 p.m.) > > > Review request for cloudstack. > > > Description > ------- > > double slash breaks windows based nfs servers [CLOUDSTACK-2968] > > > This addresses bug CLOUDSTACK-2968. > > > Diffs > ----- > > api/src/com/cloud/storage/template/TemplateInfo.java 6559d73 > core/src/com/cloud/agent/api/storage/CreateEntityDownloadURLCommand.java > 98a957f > core/src/com/cloud/agent/api/storage/DownloadAnswer.java bb7b8a9 > core/src/com/cloud/storage/template/TemplateLocation.java 58d023a > engine/schema/src/com/cloud/storage/VMTemplateHostVO.java b8dfc41 > > engine/storage/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java > a6880c3 > server/src/com/cloud/storage/download/DownloadListener.java 1d48803 > server/src/com/cloud/storage/download/DownloadMonitorImpl.java f72a563 > server/src/com/cloud/template/HypervisorTemplateAdapter.java 322f32e > server/src/com/cloud/template/TemplateManagerImpl.java 517d4ba > utils/src/com/cloud/utils/FileUtil.java 74f4088 > utils/test/com/cloud/utils/FileUtilTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/11861/diff/ > > > Testing > ------- > > database analysis > > > Thanks, > > daan Hoogland > >
