core: Ignore isValid() == false, if migration succeeded

[smelamud]

In MaintenanceVdsCommand, if migrateAllVms() returned successful return
value, do not check isValid() and do not copy validation messages from
it.

Change-Id: I9c38423c88b7a4e8a84d6187835abe490278def5
Bug-Url: https://bugzilla.redhat.com/2108000
Signed-off-by: Shmuel Melamud smelamud@redhat.com

[ahadas]

/ost

[ahadas]

this change is alright - indeed we need to take the validation result(s) only in case the command didn't succeed. I wonder though how come that we end up in this state in which we get isValid==false and succeeded=true - maybe we have an issue in MigrateVmToServer..

[ahadas]

this change is alright - indeed we need to take the validation result only in case the command didn't succeed. I wonder though how come that we end up in this state in which we get isValid==false and succeeded=true - maybe we have an issue in MigrateVmToServer..

ah no, I think I get it now - it's probably not MigrateVmToServer that fools us but MigrateMultipleVmsCommand which adds the validation messages for vms that the scheduler failed on (in the validation phase), yet proceeds with those that it managed to schedule, if exist (and in the execute phase it surprisingly fails if it is not able to migrate all the vms..) - @smelamud does it match the scenario you saw?

[ahadas]

@smelamud I'm getting this in so we'll get feedback from our automation on this but please reply to the question above

[smelamud]

@ahadas Adding validation messages does not set valid = true by itself, so it couldn't cause the error. As it seems, the return value that caused the error is the value created and returned when there is no VMs running on the host. I've posted one more PR to fix this: #536

[ahadas]

ack, so that happens only when moving a host with no running vm to maintenance then. I don't think we need #536, this change would be enough