Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3268083702 > Hey @weizhouapache , I see that this PR is in the milestone of 4.20.2 but it fixed a bug reported on 4.21.0. It is not intuitive for me. Could you please clarify how this development process works? @daviftorres all 4.20 commits will be merged into 4.21 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
daviftorres commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3285593134 It works like a charm! Good job!! https://github.com/user-attachments/assets/5cd5343d-c171-421a-9c93-acc1b380ab35"; /> -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3284301555 For those who face the issue after upgrading to 4.21.0.0 - backup /usr/share/cloudstack-agent/lib/cloud-plugin-hypervisor-kvm-4.21.0.0.jar - download file [cloud-plugin-hypervisor-kvm-4.21.0.0.zip](https://github.com/user-attachments/files/22293537/cloud-plugin-hypervisor-kvm-4.21.0.0.zip) - move to /usr/share/cloudstack-agent/lib/cloud-plugin-hypervisor-kvm-4.21.0.0.jar - systemctl restart cloudstack-agent -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
daviftorres commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3267615102 Hey @weizhouapache , I see that this PR is in the milestone of 4.20.2 but it fixed a bug reported on 4.21.0. It is not intuitive for me. Could you please clarify how this development process works? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3252249711 @shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
shwstppr commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3252266172 The [change](https://github.com/apache/cloudstack/pull/11245) that is causing issue #11552 went into 4.19, @weizhouapache are you okay if I rebase this to 4.19 or is that not needed considering 4.19 is EOL? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3261917343 tested by https://lists.apache.org/thread/8gd4179nq43lzd05h6gf6g23fq3k5yxo -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache merged PR #11557: URL: https://github.com/apache/cloudstack/pull/11557 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3261741549 [SF] Trillian test result (tid-14227) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 54531 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11557-t14227-kvm-ol8.zip Smoke tests completed. 140 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_10_reboot_cpvm_forced | `Failure` | 963.30 | test_ssvm.py -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3259304147 @weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3259303127 @blueorangutan test -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3252374440 > The [change](https://github.com/apache/cloudstack/pull/11245) that is causing issue #11552 went into 4.19, @weizhouapache are you okay if I rebase this to 4.19 or is that not needed considering 4.19 is EOL? @shwstppr I think 4.20 is good for this PR -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3252589104 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14842 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
shwstppr commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3252243371 Refactored and added tests @blueorangutan package -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3246003097 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3248115441 [SF] Trillian test result (tid-14194) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 53178 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11557-t14194-kvm-ol8.zip Smoke tests completed. 141 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3245617277 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14819 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
DaanHoogland commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3245995906 @blueorangutan test -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
DaanHoogland commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3245327289 @blueorangutan package -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
blueorangutan commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3245330899 @DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
codecov[bot] commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3244380305 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/11557?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 3.63%. Comparing base ([`973b333`](https://app.codecov.io/gh/apache/cloudstack/commit/973b333e4019981b5ff49537157856ab3ddb3d61?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3680ff4`](https://app.codecov.io/gh/apache/cloudstack/commit/3680ff432b10b3d5d96a64c955761eb880cfbb5d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 290 commits behind head on main. > :exclamation: There is a different number of reports uploaded between BASE (973b333) and HEAD (3680ff4). Click for more details. > > HEAD has 1 upload less than BASE > >| Flag | BASE (973b333) | HEAD (3680ff4) | >|--|--|--| >|unittests|1|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #11557 +/- ## = - Coverage 16.17%3.63% -12.54% = Files 5656 441 -5215 Lines49800837019 -460989 Branches 60404 6785-53619 = - Hits 80541 1345-79196 + Misses 40850535513 -372992 + Partials 8962 161 -8801 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/11557/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/11557/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.63% <ø> (-0.38%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/11557/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. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/11557?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). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
[PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
shwstppr opened a new pull request, #11557:
URL: https://github.com/apache/cloudstack/pull/11557
### Description
Fixes #11552
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
- [ ] test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
How did you try to break this feature and the system 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
weizhouapache commented on PR #11557: URL: https://github.com/apache/cloudstack/pull/11557#issuecomment-3244650385 @shwstppr I modified the target branch to 4.20 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
DaanHoogland commented on code in PR #11557:
URL: https://github.com/apache/cloudstack/pull/11557#discussion_r2315556912
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java:
##
@@ -250,7 +250,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String
guestOsType, String nicA
intf.defBridgeNet(_bridges.get("private"), null, nic.getMac(),
getGuestNicModel(guestOsType, nicAdapter));
} else if (nic.getType() == Networks.TrafficType.Storage) {
String storageBrName = nic.getName() == null ?
_bridges.get("private") : nic.getName();
-if (nic.getBroadcastType() ==
Networks.BroadcastDomainType.Storage) {
+if
(Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()) &&
nic.getBroadcastUri() != null) {
Review Comment:
`Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()` already
implies `nic.getBroadcastUri() != null`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] kvm: add ssvm storage nic null uri check during plug [cloudstack]
DaanHoogland commented on code in PR #11557:
URL: https://github.com/apache/cloudstack/pull/11557#discussion_r2315556912
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java:
##
@@ -250,7 +250,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String
guestOsType, String nicA
intf.defBridgeNet(_bridges.get("private"), null, nic.getMac(),
getGuestNicModel(guestOsType, nicAdapter));
} else if (nic.getType() == Networks.TrafficType.Storage) {
String storageBrName = nic.getName() == null ?
_bridges.get("private") : nic.getName();
-if (nic.getBroadcastType() ==
Networks.BroadcastDomainType.Storage) {
+if
(Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()) &&
nic.getBroadcastUri() != null) {
Review Comment:
`Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()` already
implies `nic.getBroadcastUri() != null`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
