| Message ID | DM5PR05MB319313D651AF41646F61C85AA2410@DM5PR05MB3193.namprd05.prod.outlook.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Shashank,
Thanks for reviewing the patch!
The main problem is we have two switch names defined currently in the documentation. I wanted to change it to have only one.
Sorry I wasn't clear in the commit message.
The idea is we instruct to create a switch named `OVS-Extended-Switch`. For a sysadmin typing that name later on would be annoying when they want to do operation on the switch. I.e.: if they want to check if the extension is enabled they would have to issue the following command:
Get-VMSwitchExtension -VMSwitchName "OVS-Extended-Switch"
I usually create a switch with the name = switch type so it is easy to distinguish which does what, but point taken, some can find that confusing.
"ovsext-switch" is still long IMO and "ovsext" is the name of the running driver.
What about "ovs_switch" ?
Response to the other comment inlined.
Thanks,
Alin.
From: Shashank Ram [mailto:rams@vmware.com]
Sent: Sunday, February 5, 2017 9:21 AM
To: Alin Serdean <aserdean@cloudbasesolutions.com<mailto:aserdean@cloudbasesolutions.com>>; dev@openvswitch.org<mailto:dev@openvswitch.org>
Subject: Re: [PATCH 4/4] doc-windows: Consolidate switch names in documentation
Hi Alin, I don't see a big value from this patch if I understand it correctly. Firstly, your commit is not clear about what it means when it says "typing it later on". Are you referring to typing it later on in the documentation? Secondly, since Hyper-V has different switch types such as "internal", "external", "private", replacing the switch name in the documentation with "external" might confuse readers unnecessarily, since its also a type. If you want to rename it, you could just call it something like "ovsext-switch" or just "ovsext".
Please find the other comments inline.
Thanks,
Shashank
- PS > New-VMSwitch "OVS-Extended-Switch" -NetAdapterName "Ethernet0" `
+ PS > New-VMSwitch external -NetAdapterName "Ethernet0" `
-AllowManagementOS $false
[SR]: The commit msgh does not mention anything about the "AllowManagementOS" flag being added. What's the reason for this?
[Alin Serdean]: Sorry that was intended to be included in patch 1
Thanks Alin for clarifying. Like Nithin already mentioned in his response, I think we should keep it as is to be more explicit in the documentation. The documentation in the context of this patch is only providing an example, and if someone felt it were too long to type, they would just use copy-paste. You could instead make the documentation clearer by mentioning that the name here is just an example, by saying something along the lines - To create a new Hyper-V switch with the name "OVS-Extended-Switch", ........ Thanks, Shashank
diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst index ece207d..2341b5c 100644 --- a/Documentation/intro/install/windows.rst +++ b/Documentation/intro/install/windows.rst @@ -274,12 +274,12 @@ Enforcement' during boot. The following commands can be used: In the Virtual Switch Manager configuration you can enable the Open vSwitch Extension on an existing switch or create a new switch. -The command to create a new switch named 'OVS-Extended-Switch' using a physical -NIC named 'Ethernet0' is: +The command to create a new switch named 'external' using a physical NIC named +'Ethernet0' is: .. code-block:: ps1con - PS > New-VMSwitch "OVS-Extended-Switch" -NetAdapterName "Ethernet0" ` + PS > New-VMSwitch external -NetAdapterName "Ethernet0" ` -AllowManagementOS $false [SR]: The commit msgh does not mention anything about the "AllowManagementOS" flag being added. What's the reason for this?