Bug 872506

Summary: Importing a VM from an OVF without the diskAlias property with copyCollapse=false will not auto-generate disk aliases
Product: Red Hat Enterprise Virtualization Manager Reporter: Geyang Kong <gkong>
Component: ovirt-engineAssignee: Allon Mureinik <amureini>
Status: CLOSED ERRATA QA Contact: Dafna Ron <dron>
Severity: medium Docs Contact:
Priority: urgent    
Version: 3.2.0CC: abaron, acathrow, amureini, chetan, cpelland, dron, dyasny, iheim, italkohe, lcui, lpeer, lyarwood, mbooth, mhuth, mjenner, mzhan, Rhev-m-bugs, rhodain, rwu, sgrinber, tzheng, yeylon, ykaul
Target Milestone: ---Keywords: ZStream
Target Release: 3.2.0   
Hardware: x86_64   
OS: Linux   
Whiteboard: storage
Fixed In Version: sf3 Doc Type: Release Note
Doc Text:
Disk alias are mandatory on Red Hat Enterprise Virtualization Manager in order to run virtual machines, but they are not mandatory in the OVF format. Now, aliases are auto-generatesd for disks imported from an OVF that does not contain aliases, so virtual machines imported from other sources can successfully run on Red Hat Enterprise Virtualization Manager.
Story Points: ---
Clone Of:
: 888309 (view as bug list) Environment:
Last Closed: 2013-06-10 21:16:27 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Storage RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 880742    
Bug Blocks: 888309, 915537    
Attachments:
Description Flags
meta file
none
ovf file
none
engine's log
none
full engine's log none

Description Geyang Kong 2012-11-02 09:21:09 UTC
Description of problem:
  Guest based on iscsi storage with 2nd disk on local storage cannot be started on rhev after conversion.

Version-Release number of selected component (if applicable):
virt-v2v-0.8.9-2.el6.x86_64

Reproduce steps:
1. Have a ESXi5.1 host, and add an iscsi storage to it.
2. Install a guest on ESXi host, make sure install it on iscsi storage.
3. Add a disk to the guest, locate the disk file on ESXi local storage.
4. Run v2v

Actual results:
1. Guest cannot be started with following error.

2012-11-02 16:15:15,837 ERROR [org.ovirt.engine.core.vdsbroker.vdsbroker.VdsBrokerCommand] (pool-4-thread-41) [48ddc720] Failed in CreateVDS method, for vds: dell-op780-04.qe.lab.eng.nay.redhat.com; host: 10.66.72.26
2012-11-02 16:15:15,837 ERROR [org.ovirt.engine.core.vdsbroker.VDSCommandBase] (pool-4-thread-41) [48ddc720] Command CreateVDS execution failed. Exception: NullPointerException: 
2012-11-02 16:15:15,837 INFO  [org.ovirt.engine.core.vdsbroker.vdsbroker.CreateVDSCommand] (pool-4-thread-41) [48ddc720] FINISH, CreateVDSCommand, log id: 36a0c29a
2012-11-02 16:15:15,837 ERROR [org.ovirt.engine.core.vdsbroker.CreateVmVDSCommand] (pool-4-thread-41) [48ddc720] Error in excuting CreateVmVDSCommand: java.lang.NullPointerException
        at org.ovirt.engine.core.dal.comparators.DiskImageByDiskAliasComparator.compare(DiskImageByDiskAliasComparator.java:11) [engine-dal.jar:]
        at org.ovirt.engine.core.dal.comparators.DiskImageByDiskAliasComparator.compare(DiskImageByDiskAliasComparator.java:7) [engine-dal.jar:]
        at java.util.TimSort.countRunAndMakeAscending(TimSort.java:324) [rt.jar:1.7.0_03-icedtea]
        at java.util.TimSort.sort(TimSort.java:189) [rt.jar:1.7.0_03-icedtea]
        at java.util.TimSort.sort(TimSort.java:173) [rt.jar:1.7.0_03-icedtea]
        at java.util.Arrays.sort(Arrays.java:659) [rt.jar:1.7.0_03-icedtea]
        at java.util.Collections.sort(Collections.java:217) [rt.jar:1.7.0_03-icedtea]
        at org.ovirt.engine.core.vdsbroker.vdsbroker.VmInfoBuilderBase.getSortedDisks(VmInfoBuilderBase.java:187) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.vdsbroker.VmOldInfoBuilder.buildVmDrives(VmOldInfoBuilder.java:57) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.vdsbroker.CreateVDSCommand.buildVmData(CreateVDSCommand.java:64) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.vdsbroker.CreateVDSCommand.ExecuteVdsBrokerCommand(CreateVDSCommand.java:24) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.vdsbroker.VdsBrokerCommand.ExecuteVDSCommand(VdsBrokerCommand.java:86) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.VDSCommandBase.ExecuteCommand(VDSCommandBase.java:63) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.dal.VdcCommandBase.Execute(VdcCommandBase.java:41) [engine-dal.jar:]
        at org.ovirt.engine.core.vdsbroker.CreateVmVDSCommand.ExecuteVdsIdCommand(CreateVmVDSCommand.java:65) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.VdsIdVDSCommandBase.ExecuteVDSCommand(VdsIdVDSCommandBase.java:49) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.vdsbroker.VDSCommandBase.ExecuteCommand(VDSCommandBase.java:63) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.dal.VdcCommandBase.Execute(VdcCommandBase.java:41) [engine-dal.jar:]
        at org.ovirt.engine.core.vdsbroker.ResourceManager.runVdsCommand(ResourceManager.java:388) [engine-vdsbroker.jar:]
        at org.ovirt.engine.core.bll.VDSBrokerFrontendImpl.RunVdsCommand(VDSBrokerFrontendImpl.java:33) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.VDSBrokerFrontendImpl.RunAsyncVdsCommand(VDSBrokerFrontendImpl.java:48) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.RunVmCommand.CreateVm(RunVmCommand.java:407) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.RunVmCommand.RunVm(RunVmCommand.java:201) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.RunVmCommand.ExecuteVmCommand(RunVmCommand.java:255) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.VmCommand.executeCommand(VmCommand.java:78) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.CommandBase.executeWithoutTransaction(CommandBase.java:825) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.CommandBase.executeActionInTransactionScope(CommandBase.java:916) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.CommandBase.runInTransaction(CommandBase.java:1300) [engine-bll.jar:]
        at org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInSuppressed(TransactionSupport.java:168) [engine-utils.jar:]
        at org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInScope(TransactionSupport.java:107) [engine-utils.jar:]
        at org.ovirt.engine.core.bll.CommandBase.execute(CommandBase.java:931) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.CommandBase.executeAction(CommandBase.java:285) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.MultipleActionsRunner.executeValidatedCommands(MultipleActionsRunner.java:182) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.MultipleActionsRunner.RunCommands(MultipleActionsRunner.java:162) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.SortedMultipleActionsRunnerBase.RunCommands(SortedMultipleActionsRunnerBase.java:16) [engine-bll.jar:]
        at org.ovirt.engine.core.bll.MultipleActionsRunner$1.run(MultipleActionsRunner.java:84) [engine-bll.jar:]
        at org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil$InternalWrapperRunnable.run(ThreadPoolUtil.java:64) [engine-utils.jar:]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) [rt.jar:1.7.0_03-icedtea]
        at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334) [rt.jar:1.7.0_03-icedtea]
        at java.util.concurrent.FutureTask.run(FutureTask.java:166) [rt.jar:1.7.0_03-icedtea]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) [rt.jar:1.7.0_03-icedtea]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) [rt.jar:1.7.0_03-icedtea]
        at java.lang.Thread.run(Thread.java:722) [rt.jar:1.7.0_03-icedtea]

2012-11-02 16:15:15,839 ERROR [org.ovirt.engine.core.vdsbroker.VDSCommandBase] (pool-4-thread-41) [48ddc720] Command CreateVmVDS execution failed. Exception: RuntimeException: java.lang.NullPointerException
2012-11-02 16:15:15,839 INFO  [org.ovirt.engine.core.vdsbroker.CreateVmVDSCommand] (pool-4-thread-41) [48ddc720] FINISH, CreateVmVDSCommand, log id: 1693cc22
2012-11-02 16:15:15,840 INFO  [org.ovirt.engine.core.bll.RunVmCommand] (pool-4-thread-41) [48ddc720] Lock freed to object EngineLock [exclusiveLocks= key: 65aa8011-a52a-4fb5-9fb1-2b8753abb1ae value: VM
, sharedLocks= ]
2012-11-02 16:15:15,840 ERROR [org.ovirt.engine.core.bll.RunVmCommand] (pool-4-thread-41) [48ddc720] Command org.ovirt.engine.core.bll.RunVmCommand throw Vdc Bll exception. With error message VdcBLLException: java.lang.RuntimeException: java.lang.NullPointerException


Expected results:
1. Guest can be started normally and have 2 disks.

Additional info:

Comment 2 Geyang Kong 2012-11-02 09:37:55 UTC
Created attachment 637005 [details]
meta file

Comment 3 Geyang Kong 2012-11-02 09:46:15 UTC
Created attachment 637006 [details]
ovf file

Comment 4 Matthew Booth 2012-11-02 13:49:57 UTC
This appears to be a bug in ovirt-engine. DiskImageByDiskAliasComparator does not handle the case where either of the disks it is passed do not have an alias. This would appear to be a regression introduced by upstream patch 80583b2c4c76f390f87102bf514ec41eca6dd153.

As RHEV must continue to be able to import guests created before this attribute was added, I'm moving this to RHEV.

Comment 5 Allon Mureinik 2012-11-15 10:37:19 UTC
By now, there are several flows in oVirt/RHEV-M engine that implicitly assume that disk alias is not null.

IMHO, the correct approach would not be to support such disk in the said comperator, but to add a default alias (e.g., VmName_Disk1) to the imported vm if it did not have the alias.

Comment 6 Itamar Heim 2012-11-15 19:07:52 UTC
temp workaround is to add alias to the ovf

Comment 7 Allon Mureinik 2012-11-18 13:38:49 UTC
Posted I38ad9c56f9b97e174807d3fcbe0b3e43355803ee, pending review.

Comment 8 Allon Mureinik 2012-11-21 10:50:48 UTC
This analysis here is not correct.

I've manually removed disk aliases from an OVF, and was able to import it, and run the VM.
This bug requires further analysis.

Comment 9 Allon Mureinik 2012-11-22 09:15:08 UTC
Geyang, can you add the engine's log(s) please?

Comment 10 Allon Mureinik 2012-11-22 17:27:39 UTC
Geyang, also please share the exact engine build/version

Comment 11 Geyang Kong 2012-11-23 02:36:58 UTC
Created attachment 650163 [details]
engine's log

Comment 12 Geyang Kong 2012-11-23 02:38:39 UTC
RHEVM's build is rhevm3.1 si24.4
Package is rhevm-3.1.0-31.el6ev.noarch

Comment 13 Allon Mureinik 2012-11-25 12:58:25 UTC
Geyang, thanks, but if you have the FULL log (from the begining of the import till this failure), it would be a great help.

Comment 14 Geyang Kong 2012-11-26 04:27:22 UTC
Created attachment 651738 [details]
full engine's log

Comment 15 Allon Mureinik 2012-11-27 17:45:44 UTC
Further analysis:

If the import is performed with copyCollapse=false and the original OVF lacks diskAliases (e.g., was created with v2v as described by Geyang, was created with RHEV-M 3.0, etc.), the disks in the imported VM will NOT have auto-generated aliases.

Technical insight:
Take a look at org.ovirt.engine.core.bll.ImportVmCommand.addVmImagesAndSnapshots(). The function starts with if (getParameters().getCopyCollapse()) (line 599 in the current HEAD).
The positive branch eventually sets a disk alias, while the negative branch does not.

Implication:
Once the VM is created without a disk alias, it cannot be run. Running it will result in an NPE, as described by Geyang in comment #1.

Workaround:
As Itamar suggested in comment #6, editing the OVF and adding a disk alias will solve the issue.
AN alternative workaround (and easier, IMHO) is to edit the disks after the VM is improted (via the UI), and add the said aliases.

Relationship to bug 880742:
Due to bug 880742, setCopyCollapse will frequently be forced to true (wrongfully!), which made this bug hard to reproduce. This may be considered as a mitigating factor, but IMHO, 880742 is a much worse bug from the customer's perspective, especially since it has no work-around.

Now that the bug is properly diagnozed and it's scope is understood, I'm reseting all flags (except devel-ack, which I'm giving for adding an auto-generated alias even if copyColapse=false).
PM/QA - please ack/nack your respective flags, and suggest which 3.1.z (3.2.0?) version this bug should be solved in.

Comment 17 Allon Mureinik 2012-12-02 14:36:56 UTC
Patch sent upstream:
Change-Id I4f03699ece4d20f2d20beb791c3cb1518e39c4d1
http://gerrit.ovirt.org/#/c/9631/

Comment 18 Lee Yarwood 2012-12-14 12:15:31 UTC
*** Bug 887091 has been marked as a duplicate of this bug. ***

Comment 19 Mark Huth 2012-12-16 23:40:13 UTC
Just wondering if DiskImageByDiskAliasComparator.java should also be modified to include the possibility of diskAlias of being null?

In BaseDisk.java, the hashCode and equals methods include logic to handle null values for diskAlias:

    186         result = prime * result + ((diskAlias == null) ? 0 : diskAlias.hashCode());
...
    206         if (diskAlias == null) {
    207             if (other.diskAlias != null)
    208                 return false;
    209         } else if (!diskAlias.equals(other.diskAlias))
    210             return false;

Wouldn't it be a good idea for the comparator also do the same - just in case?  At the moment if the comparator is passed a Disk with a null diskAlias value (for whatever reason) it will cause a NPE.  Or has this already been addressed and I missed it?

Comment 20 Allon Mureinik 2012-12-17 12:51:14 UTC
Mark - I adressed this approach in comment 5:

> By now, there are several flows in oVirt/RHEV-M engine that implicitly
> assume that disk alias is not null.
> 
> IMHO, the correct approach would not be to support such disk in the said
> comperator, but to add a default alias (e.g., VmName_Disk1) to the imported
> vm if it did not have the alias.

The comperator could easily be fixed, but that would be just one usecase that would fail. We probably have a dozen or so flows with this assumption, which will be a hell just to map.

Comment 21 Allon Mureinik 2012-12-18 11:37:00 UTC
Merged upstream.

Comment 23 Allon Mureinik 2012-12-20 06:27:03 UTC
*** Bug 887085 has been marked as a duplicate of this bug. ***

Comment 26 Dafna Ron 2013-01-30 10:14:52 UTC
verified on sf4

reproduced by removing the disk alias from the ovf in the export domain and importing the vm to the setup. 

we auto generated a disk alias during import.

Comment 28 errata-xmlrpc 2013-06-10 21:16:27 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-0888.html