neutron

Code review comment for lp:~salvatore-orlando/neutron/quantum-api-alignment

quantum-api-alignment
Merge into diablo

Revision history for this message

dan wendlandt (danwent) wrote on 2011-08-08:

Hi salvatore, haven't run it yet and only skimmed the test code, but looks good overall.

I do think though that we need to get to the bottom of the "state + status" discussion before pushing this branch, as even this branch had some things that surprised me, particularly that one could not make an attachment to a port in the down state. This seems at odds to my understanding from the email thread that "state" was really akin to "admin state" on a traditional switch. When a switch is "admin down", my understanding is that the common behavior one can still configure the port, its just that no packets will pass.

Also, the spec still seems somewhat unclear on the issue of port "state". For example, the following section, as I interpret it, seems to imply that the state is either "down" or "active" based on whether a VIF is plugged in (i.e., something similar to the operational "link status" we discussed in the last email):

"The VIF will be connected to the network via a logical port. The logical port begins in a “down” state. Logic ports are transient when in the “down” state, by this we mean they don’t have any binding to a physical implementation. The plugin must create this binding as a part of the attachment process. When a VIF is attached to a logical port the plugin creates the binding to the physical hardware required to allow operation of the VIF. This is indicated by the change in port state from “down” to “active”. As a result of the attachment operation, quantum can now knows the capabilities of the VIF as dictated by the physical binding. As a result the logical port may advertise capabilities once it moves to the “active” state."

If you'd prefer to push this code first to avoid merge conflicts and then continue having the discussion that's fine with me, but I want to make sure we clear up this point before finalizing 1.0 (An alternative would be to drop the state/status notion all together for v1.0, and revisit it for a later version).

Hi salvatore, haven't run it yet and only skimmed the test code, but looks good overall.

I do think though that we need to get to the bottom of the "state + status" discussion before pushing this branch, as even this branch had some things that surprised me, particularly that one could not make an attachment to a port in the down state.  This seems at odds to my understanding from the email thread that "state" was really akin to "admin state" on a traditional switch.  When a switch is "admin down", my understanding is that the common behavior one can still configure the port, its just that no packets will pass.

Also, the spec still seems somewhat unclear on the issue of port "state".  For example, the following section, as I interpret it, seems to imply that the state is either "down" or "active" based on whether a VIF is plugged in (i.e., something similar to the operational "link status" we discussed in the last email):

« Back to merge proposal