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 > [email protected] > http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
