hi Eitan,
I applied your patch, and ran some tests on how validation is performed:
1)
If "" is provided for vmName argument, then powershell throws an exception, esp
since it is declared as mandatory. So, having the if ($vmName) check is not
necessary, and if so, setting $vnic to 0, is also not necessary. There were my
original comments.
One additional comment I had was to to rename 'vmName' to "VMName" in
accordance to Windows naming conventions of using upper case for first letter.
If you think having that checks serves a purpose. I am fine with it.
Acked-by: Nithin Raju <nit...@vmware.com>
Validation checks:
--- Eg 1 ---
PS C:\ovs\openvswitch-review> Set-VMNetworkAdapterOVSPortDirect "" ovs-port-11
Set-VMNetworkAdapterOVSPortDirect : Cannot bind argument to parameter 'vmName'
because it is an empty string.
At line:1 char:36
+ Set-VMNetworkAdapterOVSPortDirect "" ovs-port-11
+ ~~
+ CategoryInfo : InvalidData: (:)
[Set-VMNetworkAdapterOVSPortDirect], ParameterBindingValidationExceptio
n
+ FullyQualifiedErrorId :
ParameterArgumentValidationErrorEmptyStringNotAllowed,Set-VMNetworkAdapterOVSPortDirect
--- Eg 2 ---
Supply values for the following parameters:
OVSPortName: PS C:\ovs\openvswitch-review> Set-VMNetworkAdapterOVSPortDirect
-vmName "" ovs-port-11
Set-VMNetworkAdapterOVSPortDirect : Cannot bind argument to parameter 'vmName'
because it is an empty string.
At line:1 char:44
+ Set-VMNetworkAdapterOVSPortDirect -vmName "" ovs-port-11
+ ~~
+ CategoryInfo : InvalidData: (:)
[Set-VMNetworkAdapterOVSPortDirect], ParameterBindingValidationExceptio
n
+ FullyQualifiedErrorId :
ParameterArgumentValidationErrorEmptyStringNotAllowed,Set-VMNetworkAdapterOVSPortDirect
--- Eg 3 ---
PS C:\ovs\openvswitch-review> Set-VMNetworkAdapterOVSPortDirect -OVSPortName
ovs-port-11
cmdlet Set-VMNetworkAdapterOVSPortDirect at command pipeline position 1
Supply values for the following parameters:
-------------
thanks,
-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
