cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "daan Hoogland" <daan.hoogl...@gmail.com>
Subject Re: Review Request: double slash fix for windows based nfs servers [CLOUDSTACK-2968]
Date Sat, 15 Jun 2013 12:42:56 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message