You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "iishitahere (via GitHub)" <gi...@apache.org> on 2024/12/08 16:32:36 UTC
[PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
iishitahere opened a new pull request, #10060:
URL: https://github.com/apache/cloudstack/pull/10060
This PR fixes the traffic shaping issue for secondary NICs in VmwareTrafficLabel. Previously, traffic shaping was not applied correctly to secondary NICs, causing inconsistencies. This fix ensures that traffic limits and guarantees are properly enforced for both primary and secondary NICs, adhering to VMware's intended behavior.
Changes:
Fixed the traffic shaping functionality for secondary NICs in VmwareTrafficLabel.
Updated configurations to ensure traffic shaping is applied consistently across all NICs.
Testing:
Verified the fix by testing traffic shaping on VMware setups with multiple NICs, ensuring secondary NICs are properly shaped without affecting primary NICs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2713619914
@iishitahere , please consider [this comment](https://github.com/apache/cloudstack/pull/10060#discussion_r1874994355)
> > > > @iishitahere , are you still persuing this?
> > >
> > >
> > > Hi @DaanHoogland, since all checks have passed, I just wanted to ask if there's anything specific that I need to work on for this PR. If there are any adjustments or improvements required, I'll be happy to pursue them. Thank you!
> >
> >
> > @iishitahere , as is this PR does nothing. What is your intention with this change?
>
> @DaanHoogland, the intent was to ensure traffic shaping consistency across secondary NICs. I’ll review the impact of this change and get back to you soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2571691146
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/10060?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 4.02%. Comparing base [(`546ef31`)](https://app.codecov.io/gh/apache/cloudstack/commit/546ef31aedb7c30a52f9fef0c505fa9e6968dd21?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`7388a9e`)](https://app.codecov.io/gh/apache/cloudstack/commit/7388a9ed6ea3cdab2cd876f471c4108a48ed4514?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 90 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (546ef31) and HEAD (7388a9e). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (546ef31) | HEAD (7388a9e) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #10060 +/- ##
=============================================
- Coverage 15.80% 4.02% -11.79%
=============================================
Files 5627 394 -5233
Lines 492363 32361 -460002
Branches 59696 5713 -53983
=============================================
- Hits 77828 1301 -76527
+ Misses 406012 30912 -375100
+ Partials 8523 148 -8375
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/10060/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/10060/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.02% <ø> (-0.03%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/10060/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/10060?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#discussion_r1874994355
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/network/VmwareTrafficLabel.java:
##########
@@ -120,4 +126,9 @@ public void setVirtualSwitchName(String vSwitchName) {
public void setVirtualSwitchType(VirtualSwitchType vSwitchType) {
_vSwitchType = vSwitchType;
}
+
+ // Getter to ensure traffic shaping consistency across all NICs
+ public boolean isTrafficShapingConsistent() {
Review Comment:
@iishitahere
this method is not used anywhere. did I miss something ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2703990809
@iishitahere , are you still persuing this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2713601083
> > > @iishitahere , are you still persuing this?
> >
> >
> > Hi @DaanHoogland, since all checks have passed, I just wanted to ask if there's anything specific that I need to work on for this PR. If there are any adjustments or improvements required, I'll be happy to pursue them. Thank you!
>
> @iishitahere , as is this PR does nothing. What is your intention with this change?
@DaanHoogland, the intent was to ensure traffic shaping consistency across secondary NICs. I’ll review the impact of this change and get back to you soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2573001436
> @iishitahere I see you've multiple PRs which are trying to solve the issue - #10007 The behaviour, though confusing is already mentioned in the code/documentation,
>
> https://github.com/apache/cloudstack/blob/db2e89a3a2c9e8df2d24ed3e6aeae280bf22f0ba/server/src/main/java/com/cloud/network/NetworkModelImpl.java#L1045-L1051
>
> If you're looking to simplify it maybe first you need to discuss your approach to prevent re-works. You may discuss with others on the issue itself or start a thread at dev mailing list.
>
> As I understand you're interested in contributing to the project and still getting started, I would suggest you to look into this guide to get started with setup and development, https://github.com/shapeblue/hackerbook/tree/main Maybe also start with a more simpler issue which may not require an actual hypervisor to develop/test and simulator can be used. I see one here, #10103. It has proper reproduction steps and which you can do on a simulator-based env. You've already added your interest on #10122. You can deploy your simulator dev environment for it and it may require changing some .vue file and adding/updating key in en.json translation file.
Hi @shwstppr,
I sincerely apologize for not discussing my approach before submitting the multiple PRs related to issue #10007. I understand the importance of having a clear and coordinated plan, and I realize this could have led to unnecessary rework. I will make sure to avoid this in the future and will discuss my approach before starting any new work.
Thank you for the helpful suggestion and for pointing me to the documentation and setup guide. I’ll make sure to review everything more thoroughly moving forward. I appreciate your support and feedback, and I’ll consider your advice to start with simpler issues like #10103.
Thanks again!
Best regards,
Ishita Jaiswal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2536032258
Hi @DaanHoogland,
Thank you for your valuable feedback and for highlighting the issues in my PR. I truly appreciate the guidance and patience.
Updates I Will Make:
Committing Missing Changes:
I will review my local repository to ensure all necessary changes, including updated configurations and related code, are committed.
This will include updates ensuring consistent traffic shaping across all NICs and any related testing code.
Branching Best Practices:
I acknowledge submitting the PR from the main branch was not ideal. Moving forward, I will create a dedicated branch for each feature or fix.
Addressing Unused Method:
I will verify the relevance of the isTrafficShapingConsistent() method and remove it if unnecessary.
I plan to update the PR with these changes by 13/12/2024 for sure. Could you confirm if there are specific configurations or tests you'd like me to prioritize? Additionally, should I include specific documentation or test cases to validate these updates?
Thank you again for your guidance, and I’ll ensure future contributions adhere to project standards.
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2536036721
Hi @DaanHoogland,
Thank you for your valuable feedback and for highlighting the issues in my PR. I truly appreciate the guidance and patience.
Updates I Will Make:
Committing Missing Changes:
I will review my local repository to ensure all necessary changes, including updated configurations and related code, are committed.
This will include updates ensuring consistent traffic shaping across all NICs and any related testing code.
Branching Best Practices:
I acknowledge submitting the PR from the main branch was not ideal. Moving forward, I will create a dedicated branch for each feature or fix.
Addressing Unused Method:
I will verify the relevance of the isTrafficShapingConsistent() method and remove it if unnecessary.
I plan to update the PR with these changes by 13/12/2024. Could you confirm if there are specific configurations or tests you'd like me to prioritize? Additionally, should I include specific documentation or test cases to validate these updates?
Thank you again for your guidance, and I’ll ensure future contributions adhere to project standards.
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2571394105
Hi @DaanHoogland ,
Apologies for the delay. I’ve implemented enhancements to improve compatibility with traffic shaping and VMware network configurations, particularly focusing on default NIC IP handling, command execution, and process management. Could you please review the changes and provide feedback?
Thank you for the opportunity and for trusting me with this task. Your input is appreciated!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2566309302
> @iishitahere , I welcome your enthausiasm in contributing but you now have three PRs open, this, #10134 and #10109. All of them seem to be missing code. I think you need to look at your git repos to see how and where changes got lost. In addition to that, can you reference the issue you solve in the PRs? There is a placeholder in the default description text for that. You can uncomment it and add the respective issue numbers.
Hi @DaanHoogland,
Thank you for your feedback, and I apologize for the confusion caused by the missing code.
I’ve now fixed the missing code and pushed the latest changes, ensuring consistent traffic shaping across all NICs.
I’ve also removed the unused isTrafficShapingConsistent() method to simplify the code.
The issue reference has been added to the PR description: Fixes #10007.
Please review the changes and let me know if there are any further concerns or adjustments needed!
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2526209687
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:
- In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
- Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
- Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
- Please follow [ASF Code of Conduct](https://github.com/apache/.github/blob/main/.github/CODE_OF_CONDUCT.md) for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
- Be sure to read the [CloudStack Coding Conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions).
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2549280740
> @iishitahere , thanks for your update. I still don't see the method `isTrafficShapingConsistent()` being used anywhere, as @weizhouapache pointed out in [#10060 (comment)](https://github.com/apache/cloudstack/pull/10060#discussion_r1874994355) .
>
> Can you have another look at my remarks in [#10060 (comment)](https://github.com/apache/cloudstack/pull/10060#issuecomment-2527215140)? especially the second part.
Hi @DaanHoogland ,
Thanks for pointing this out. I revisited your remarks in #10060 (comment), particularly the second part, and you're right—isTrafficShapingConsistent() doesn’t appear to be used.
I’ll look into it further and share an update soon. Let me know if there’s anything else I should consider.
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2572915763
> @iishitahere , this PR still is missing code. There's only a traffic shaping consistent flag, which is never used. Ass it looks now, it will do nothing.
Hi @DaanHoogland,
Thank you for pointing this out. Could you please guide me on how I can help to address this? I would appreciate any specific suggestions or insights on what needs to be added or modified to ensure the traffic shaping consistent flag is fully implemented and functional.
Looking forward to your feedback!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2572814817
@iishitahere , this PR still is missing code. There's only a traffic shaping consistent flag, which is never used. Ass it looks now, it will do nothing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2566253369
@iishitahere , I welcome your enthausiasm in contributing but you now have three PRs open, this, #10134 and #10109. All of them seem to be missing code. I think you need to look at your git repos to see how and where changes got lost.
In addition to that, can you reference the issue you solve in the PRs? There is a placeholder in the default description text for that. You can uncomment it and add the respective issue numbers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2572944012
@iishitahere I see you've multiple PRs which are trying to solve the issue - https://github.com/apache/cloudstack/issues/10007
The behaviour, though confusing is already mentioned in the code/documentation, https://github.com/apache/cloudstack/blob/db2e89a3a2c9e8df2d24ed3e6aeae280bf22f0ba/server/src/main/java/com/cloud/network/NetworkModelImpl.java#L1045-L1051
If you're looking to simplify it maybe first you need to discuss your approach to prevent re-works. You may discuss with others on the issue itself or start a thread at dev mailing list.
As I understand you're interested in contributing to the project and still getting started, I would suggest you to look into this guide to get started with setup and development, https://github.com/shapeblue/hackerbook/tree/main
Maybe also start with a more simpler issue which may not require an actual hypervisor to develop/test and simulator can be used. I see one here, https://github.com/apache/cloudstack/issues/10103. It has proper reproduction steps and which you can do on a simulator-based env.
You've already added your interest on https://github.com/apache/cloudstack/issues/10122. You can deploy your simulator dev environment for it and it may require changing some .vue file and adding/updating key in en.json translation file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-3056977386
Hi @iishitahere is this PR ready for review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #10060: Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel
URL: https://github.com/apache/cloudstack/pull/10060
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-3065104431
> Hi @iishitahere is this PR ready for review?
looking at the code, this does nothing yet @nvazquez . I think we can’t consider this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2544840807
@iishitahere , thanks for your update. I still don't see the method `isTrafficShapingConsistent()` being used anywhere, as @weizhouapache pointed out in https://github.com/apache/cloudstack/pull/10060#discussion_r1874994355 .
Can you have another look at my remarks in https://github.com/apache/cloudstack/pull/10060#issuecomment-2527215140? especially the second part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2713627025
> @iishitahere , please consider [this comment](https://github.com/apache/cloudstack/pull/10060#discussion_r1874994355)
>
> > > > > @iishitahere , are you still persuing this?
> > > >
> > > >
> > > > Hi @DaanHoogland, since all checks have passed, I just wanted to ask if there's anything specific that I need to work on for this PR. If there are any adjustments or improvements required, I'll be happy to pursue them. Thank you!
> > >
> > >
> > > @iishitahere , as is this PR does nothing. What is your intention with this change?
> >
> >
> > @DaanHoogland, the intent was to ensure traffic shaping consistency across secondary NICs. I’ll review the impact of this change and get back to you soon.
Got it, I will work on it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2543343189
Hi @DaanHoogland ,
I have updated the PR with the following change:
Added the vm.network.throttling.rate configuration with a default value of 200 (data transfer rate in megabits per second for user VM's default network).
I haven’t made any other modifications regarding the removal of previous configurations as they were not required.
Could you please review the changes and let me know if any further adjustments are needed?
Thank you for your time and feedback!
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2527215140
@iishitahere , it looks like you did not commit all your changes, as there is only one file submitted. Also it looks like you are new to using git/github as you submitted a PR from your main branch instead of a new branch. Please reach out if you need more help.
> Changes: Fixed the traffic shaping functionality for secondary NICs in VmwareTrafficLabel.
This ^ is the only change that seems to be there.
> Updated configurations to ensure traffic shaping is applied consistently across all NICs. Testing: Verified the fix by testing traffic shaping on VMware setups with multiple NICs, ensuring secondary NICs are properly shaped without affecting primary NICs.
These ^ changes are missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2571685236
Hi @DaanHoogland,
Dear Maintainer,
I hope this message finds you well. I am working on a pull request, but I am encountering issues with five failing checks during the build process. Below are the checks that are failing:
Build / build (pull_request) - Failing after 2 minutes.
Coverage Check / codecov (pull_request) - Failing after 2 minutes.
License Check / build (pull_request) - Failing after 1 minute.
Lint / Run pre-commit (pull_request) - Failing after 58 seconds.
UI Build / build (pull_request) - Failing after 4 minutes.
I have tried reviewing the logs, but I am unable to resolve the issues. Could you please guide me on the potential cause of these failures and suggest any steps to resolve them?
Looking forward to your assistance!
Best regards,
Ishita Jaiswal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2572554669
> I hope this message finds you well. I am working on a pull request, but I am encountering issues with five failing checks during the build process. Below are the checks that are failing:
>
> Build / build (pull_request) - Failing after 2 minutes. Coverage Check / codecov (pull_request) - Failing after 2 minutes. License Check / build (pull_request) - Failing after 1 minute. Lint / Run pre-commit (pull_request) - Failing after 58 seconds. UI Build / build (pull_request) - Failing after 4 minutes. I have tried reviewing the logs, but I am unable to resolve the issues. Could you please guide me on the potential cause of these failures and suggest any steps to resolve them?
@iishitahere , where do you see these errors? On this PR I see
```
All checks have passed
1 skipped and 24 successful checks
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2527510679
> @iishitahere , it looks like you did not commit all your changes, as there is only one file submitted. Also it looks like you are new to using git/github as you submitted a PR from your main branch instead of a new branch. Please reach out if you need more help.
>
> > Changes: Fixed the traffic shaping functionality for secondary NICs in VmwareTrafficLabel.
>
> This ^ is the only change that seems to be there.
>
> > Updated configurations to ensure traffic shaping is applied consistently across all NICs. Testing: Verified the fix by testing traffic shaping on VMware setups with multiple NICs, ensuring secondary NICs are properly shaped without affecting primary NICs.
>
> These ^ changes are missing.
Hi @DaanHoogland,
Thank you for your feedback and for pointing out the issues.
About the missing changes: I understand the concern. It seems I may have missed committing all the necessary files or changes. I will review my local repository, commit the remaining updates, and push them to this PR shortly.
Branching: You are correct that I used the main branch for this PR. I appreciate the guidance on best practices and will create a dedicated branch for any future contributions.
Thank you again for your help! I'll address these issues promptly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-3645898588
@iishitahere, closing this one is old. please update and reopen if you think it is still relevant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2572647923
> > I hope this message finds you well. I am working on a pull request, but I am encountering issues with five failing checks during the build process. Below are the checks that are failing:
> > Build / build (pull_request) - Failing after 2 minutes. Coverage Check / codecov (pull_request) - Failing after 2 minutes. License Check / build (pull_request) - Failing after 1 minute. Lint / Run pre-commit (pull_request) - Failing after 58 seconds. UI Build / build (pull_request) - Failing after 4 minutes. I have tried reviewing the logs, but I am unable to resolve the issues. Could you please guide me on the potential cause of these failures and suggest any steps to resolve them?
>
> @iishitahere , where do you see these errors? On this PR I see
>
> ```
> All checks have passed
> 1 skipped and 24 successful checks
> ```
Dear @DaanHoogland ,
I hope this email finds you well.
Thank you for reviewing my pull request. I recently encountered issues with several failing checks during the build process, as mentioned earlier. However, upon reviewing the current status, I see that all checks have now passed successfully, with one skipped and 24 marked as successful.
Given the current status, I kindly request you to proceed with merging the pull request if everything looks good from your side. Please let me know if there are any additional changes or actions required on my part.
Looking forward to your feedback.
Best regards,
Ishita Jaiswal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2573010531
Dear @DaanHoogland,
I hope this message finds you well. I have completed my proposal for the GSOC 2025 project titled "Fixing VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel". I would greatly appreciate it if you could take the time to review my proposal and provide any feedback or suggestions that could help improve it.
You can view the proposal through the following link:
[Review My GSOC Proposal](https://1drv.ms/w/c/067f1044f863904d/EdNyqcGK15dCl-Ut49hXSqwBsC2cSIn8s7ScSYMEGZJEbA?e=kn0oaj)
Your input would be invaluable to me, and I look forward to hearing your thoughts.
Thank you in advance for your time and feedback.
Best regards,
Ishita Jaiswal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2566330085
> @iishitahere , I welcome your enthausiasm in contributing but you now have three PRs open, this, #10134 and #10109. All of them seem to be missing code. I think you need to look at your git repos to see how and where changes got lost. In addition to that, can you reference the issue you solve in the PRs? There is a placeholder in the default description text for that. You can uncomment it and add the respective issue numbers.
Hi @DaanHoogland,
I hope you're doing well. After reviewing the feedback on my PRs, I realized that there were multiple issues with the branches, and I’ve encountered some difficulties with pushing the changes and referencing the issues correctly. As a result, I’ve decided to close the current PRs and start fresh with a new branch.
I have already cloned the repository, made the necessary changes, and will be pushing the updates shortly. I’ll ensure that the new PR includes all required changes and references the relevant issues properly.
Thank you for your patience and understanding. I’ll keep you updated on the progress, and I hope the new PR will meet the expectations.
Best regards,
Ishita
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2709039196
> > @iishitahere , are you still persuing this?
>
> Hi @DaanHoogland, since all checks have passed, I just wanted to ask if there's anything specific that I need to work on for this PR. If there are any adjustments or improvements required, I'll be happy to pursue them. Thank you!
@iishitahere , as is this PR does nothing. What is your intention with this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix VMware Traffic Shaping for Secondary NICs in VmwareTrafficLabel [cloudstack]
Posted by "iishitahere (via GitHub)" <gi...@apache.org>.
iishitahere commented on PR #10060:
URL: https://github.com/apache/cloudstack/pull/10060#issuecomment-2707019172
> @iishitahere , are you still persuing this?
Hi @DaanHoogland, since all checks have passed, I just wanted to ask if there's anything specific that I need to work on for this PR. If there are any adjustments or improvements required, I'll be happy to pursue them. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org