Few more (here and inline):
1. Add can-do-action for network name with name 'none'. This is not must
- if we change none with N/A in the network drop-down list.
2. In "plugged --> plugged" - updateVmInteface should be replaced with
updateVmDevice.
On 11/20/2012 09:40 AM, Livnat Peer wrote:
Hi Alona,
I reviewed the DD of wired/unwired and I have a few questions/comments -
1. update vnic while the VM is running should be limited to cluster 3.2
(except for plug/unplug)
2. RunVM - why do we need to pass the 'linkState' property to VDSM? I
think that if the link is down we should pass none as the network and if
the link is up we should pass the network name, like we did so far. the
link-state is internal implementation detail in the engine.
Isn't there any consideration from current 3.2 VDSM versions built from
source ? I agree that stable/formal VDSM which supports 3.2 should be
capable of handling empty network name. If decided to pass the linkState
- it must be sent with a default value (true), same goes for createVM.
3. looking on the VDSM API -
* why do we need 'type' ?
* why do we need 'device'?
* why do we need 'alias'?
* As I wrote I think we don't need 'linkState'
4. I am missing the error handling description when the user performs -
Update Vnic while changing the type for example. What is the behavior if
we succeed to unplug but not plug after that, etc.
5. Updated APIs section -This section is based on the fact that we pass
linkstate to vdsm which is not needed IMO, so if we change that this
section should be updated accordingly.
6. The EVENT section is empty, what events should be issued to audit log?
I think we should have an even for update vnic and if plug/unplug is
required a second event should be issued.
If we keep using internally the ActivateDeactivateVmNic command by the
UpdateVmNic command - each command should create the relevant event-log.
7. why does link state is varchar in DB? you expect to support other
option in addition to up/Down (which can be represented by 1 bit), maybe
n/a status?
+1
8. About the open issues-
* About deprecation of ActivateDeactivateVmNic command - I think that
we should remove this command from the backend to avoid duplicate flows
and bugs etc. for Backwards compatibility we should update the clients
to use the new UpdateVmNetworkInterface. the down side for that is we
have to make the can do action validation more complicated (according to
cluster level etc.)
It will make the command execution more complex as well - since the
ActivateDeactivateVmNic command is non-transactive - so the internal
implementation should be copied to certain flows of the
UpdateVmNetworkInterface (which involves VDSM calls).
In any case - UpdateVmNetworkInterface should be marked as non-transactive.
* If we remove ActivateDeactivateVmNic there is no need to rename it ;)
In case we decide to keep ActivateDeactivateVmNic - according to the
design changes it will pass also the linkState to VDSM, therefore i
won't modify its name, as it performs wire/unwire in addition to
plug/unplug.
Thanks, Livnat
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel