weizhouapache commented on a change in pull request #4800:
URL: https://github.com/apache/cloudstack/pull/4800#discussion_r593171611
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -4266,7 +4266,8 @@ public void setBackingFileFormat(String volPath) {
Map<String, String> info = qemu.info(file);
String backingFilePath = info.get(new String("backing_file"));
String backingFileFormat = info.get(new
String("backing_file_format"));
- if
(org.apache.commons.lang.StringUtils.isEmpty(backingFileFormat)) {
+ if (org.apache.commons.lang.StringUtils.isNotBlank(backingFilePath)
+ &&
org.apache.commons.lang.StringUtils.isEmpty(backingFileFormat)) {
Review comment:
> Maybe some log messages for debug/warn purposes would be interesting.
Or do you think that it is something that system admins should not be bothered
with?
>
> I am thinking especially on cases such as `backingFileFormat` not empty
and blank/empty `backingFilePath`, or _vice versa_.
>
> Additionally, what do you think of replacing
`StringUtils.isEmpty(backingFileFormat)` with
`StringUtils.isBlank(backingFileFormat)`? In such case it would then check if
`backingFileFormat` is whitespace, empty ("") or null.
@GabrielBrascher there is a info next to these lines
```
s_logger.info("Setting backing file format of " + volPath);
```
I will add a line as comment to explain why we need this change.
https://libvirt.org/kbase/backing_chains.html#vm-refuses-to-start-due-to-misconfigured-backing-store-format
StringUtils.isBlank(backingFileFormat) and
StringUtils.isEmpty(backingFileFormat) have no difference I think. there are
only 2 possible values: empty and 'qcow2'. I will change to isBlank
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
