hi Eitan,
Thanks for the change. I had a few minor comments. I should be able to ACK a v3.
> + /* Update properties only for NETDEV ports*/
> + if (vport == NULL || vport->ovsType != OVS_VPORT_TYPE_NETDEV) {
> + status = STATUS_DATA_NOT_ACCEPTED;
> + goto update_port_done;
> + }
For the HV-internal port that 'ovsType' is going to be OVS_VPORT_TYPE_INTERNAL.
So, we'll have to accept that port as well. A better variable to check might be:
if (vport == NULL || vport->hvDeleted) {
status = STATUS_DATA_NOT_ACCEPTED;
...
I was thinking of renaming 'hvDeleted' to something like 'presentOnHv'. If you
want to do it as part of this change, that is fine.
> +
> + /* Store the nic and the OVS states as Nic Create won't be called */
> + ovsState = vport->ovsState;
> + nicState = vport->nicState;
> + /* Currently only the port friendly name is being updated */
> + OvsInitVportWithPortParam(vport, portParam);
Can we ascertain that some of the key data members have not changed? Eg.
vport->portId, vport->portType, and vport->portState. I have a feeling that we
should white-list the properties that are allowed to change and then copy them
rather than calling OvsInitVportWithPortParam().
Also, like Ankur pointed out, what does it mean if the Hyper-V name of a port
changes while there's a OVS port attached to it? While the vport changes were
being done, I was of the opinion that we should not allow the HV friendly name
to change while OVS port is attached. Practically it may not matter, but can
lead to confusion.
That said, I am ok with allowing the friendly name to change, provided it is
well-documented. So, perhaps, you can document this somewhere.
thanks,
Nithin
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
