PrepareForMigrationCommand subtype of StartCommand

[Prasanna Santhanam <Pr...@citrix.com>]

In the Command type hierarchy the PrepareForMigrationCommand extends a
StartCommand. 

In my mind, StartCommand is intended for VMs to be started where as
PrepareForMigrationCommand is for hosts. Most of the ServerResources
implement PrepareForMigrationCommand before StartCommand and so we
don't seem to be hitting any issues. 

Is this intended or a bug? If intended - can someone explain why?

Thanks,

-- 
Prasanna.,

RE: PrepareForMigrationCommand subtype of StartCommand

[Nitin Mehta <Ni...@citrix.com>]

Prasanna - What you raise is a very interesting point. But this would be an issue only in some of the ServerResources where implementation is like below code. Here there is no exact match of the class and hence it can get into the implementation of its parent unless the order is maintained.
So if someone sends RebootRouterCommand which lets say hypothetically is subclass of StopCommand then the execute((StopCommand) cmd gets executed here. It's important to maintain order just like we catch exceptions keeping the most generalized classes in the end.

			if (cmd instanceof StopCommand) {
				return execute((StopCommand) cmd);
			} else if (cmd instanceof GetVmStatsCommand) {
				return execute((GetVmStatsCommand) cmd);
			} else if (cmd instanceof RebootRouterCommand) {
				return execute((RebootRouterCommand) cmd);

Ideally the implementations of ServerResources should be like this. Example - CitrixResourceBase.java so that there is an exact match of the command.
		Class<? extends Command> clazz = cmd.getClass();
       		 if (clazz == CreateCommand.class) {
           			 return execute((CreateCommand) cmd);
    		    } else if (clazz == SetPortForwardingRulesCommand.class) {
		               return execute((SetPortForwardingRulesCommand) cmd);


I think it's a bug.


Thanks,
-Nitin

-----Original Message-----
From: Prasanna Santhanam [mailto:Prasanna.Santhanam@citrix.com] 
Sent: Thursday, August 02, 2012 4:28 PM
To: CloudStack Dev
Subject: PrepareForMigrationCommand subtype of StartCommand

In the Command type hierarchy the PrepareForMigrationCommand extends a StartCommand. 

In my mind, StartCommand is intended for VMs to be started where as PrepareForMigrationCommand is for hosts. Most of the ServerResources implement PrepareForMigrationCommand before StartCommand and so we don't seem to be hitting any issues. 

Is this intended or a bug? If intended - can someone explain why?

Thanks,

--
Prasanna.,

RE: PrepareForMigrationCommand subtype of StartCommand

[Edison Su <Ed...@citrix.com>]

> -----Original Message-----
> From: Prasanna Santhanam [mailto:Prasanna.Santhanam@citrix.com]
> Sent: Thursday, August 02, 2012 3:58 AM
> To: CloudStack Dev
> Subject: PrepareForMigrationCommand subtype of StartCommand
> 
> In the Command type hierarchy the PrepareForMigrationCommand extends a
> StartCommand.
> 
> In my mind, StartCommand is intended for VMs to be started where as
> PrepareForMigrationCommand is for hosts. Most of the ServerResources
> implement PrepareForMigrationCommand before StartCommand and so we
> don't seem to be hitting any issues.
> 
> Is this intended or a bug? If intended - can someone explain why?

It should not extend StartCommand, PrepareForMigrationCommand only needs a VirtualMachineTO.
Could you help to fix it?

> 
> Thanks,
> 
> --
> Prasanna.,