----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22820/#review46317 -----------------------------------------------------------
Ship it! 736bf540e8ef759a101d221622c64f3b3c3ed425 on master - daan Hoogland On June 20, 2014, 5:37 p.m., Logan B wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22820/ > ----------------------------------------------------------- > > (Updated June 20, 2014, 5:37 p.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > This is a proposed fix for: > https://issues.apache.org/jira/browse/CLOUDSTACK-6938 > > Previously when attempting to create a template from a snapshot stored in S3 > the task would fail with "Unable to create directory." This was due to the > fact that Java's 'mkdirs()' directive returns 'false' if the directory > already exists. I just changed the logic to check for an existing directory > first, then attempt to create it if one doesn't already exist. The task will > still fail if it legitimately can't create a folder, but should work if one > already exists now. > > Original code: > if (!downloadDirectory.mkdirs()) { > final String errMsg = "Unable to create directory " + downloadPath + " to > copy from S3 to cache."; > s_logger.error(errMsg); > return new CopyCmdAnswer(errMsg); > } else { > s_logger.debug("Directory " + downloadPath + " already exists"); > } > > New code: > if (downloadDirectory.exists()) { > s_logger.debug("Directory " + downloadPath + " already exists"); > } else { > if (!downloadDirectory.mkdirs()) { > final String errMsg = "Unable to create directory " + downloadPath + > " to copy from S3 to cache."; > s_logger.error(errMsg); > return new CopyCmdAnswer(errMsg); > } > > > Diffs > ----- > > > services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java > 6927f02 > > Diff: https://reviews.apache.org/r/22820/diff/ > > > Testing > ------- > > Tested this on the CloudStack 4.4-SNAPSHOT. Original code produced a failure > with an existing directory. I replaced the > 'cloud-secondary-storage-4.4.0-SNAPSHOT.jar' on the SSVM, and it no longer > failed. > > The only 'bug' I can see is that the 's_logger.debug' message for the > .exists() check doesn't seem to appear in the SSVM logs. > > > Thanks, > > Logan B > >
