Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update github.com/opiproject/opi-api digest to ca2c25b #20

Merged
merged 3 commits into from
Nov 17, 2022

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Nov 17, 2022

Mend Renovate

This PR contains the following updates:

Package Type Update Change
github.com/opiproject/opi-api require digest ffe4aad -> ca2c25b

Configuration

πŸ“… Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

β™» Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

πŸ”• Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

@renovate renovate bot requested a review from a team as a code owner November 17, 2022 17:08
@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #20 (6f18655) into main (eb15131) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff          @@
##            main     #20   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          3       3           
  Lines        547     579   +32     
=====================================
- Misses       547     579   +32     
Impacted Files Coverage Ξ”
frontend.go 0.00% <0.00%> (ΓΈ)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb glimchb force-pushed the renovate/github.com-opiproject-opi-api-digest branch from b9cd7bd to 6f18655 Compare November 17, 2022 18:27
@glimchb glimchb merged commit b2ec889 into main Nov 17, 2022
@glimchb glimchb deleted the renovate/github.com-opiproject-opi-api-digest branch November 17, 2022 18:32
@benlwalker
Copy link

Technically, a user could do this sequence of steps:

  1. Create subsystem
  2. Create namespace
  3. Create controller

In this case, it needs to go add all the namespaces to the controller inside the CreateNvmeController call.

@glimchb
Copy link
Member

glimchb commented Nov 17, 2022

This is why I thought we should keep the API calls separate and mandate CTRL-id when you create NS and if not provided, it does the for-loop...

@benlwalker
Copy link

benlwalker commented Nov 17, 2022

A namespace can't be in just a single controller, so the namespace having a controller id isn't sufficient. Image this scenario:

  1. Create subsystem.
  2. Create controller.
  3. Create namespace (with controller id)
  4. Create a second controller

Where some time passes between step 3 and 4. That's a valid thing to do and it needs to be handled. Now the namespace should be in two controllers. That's the reason I removed controller_id from the namespace object - it doesn't model the relationship correctly.

@glimchb
Copy link
Member

glimchb commented Nov 17, 2022

I would imagine :
5. Create a namespace (with second controller id)

@benlwalker
Copy link

So now you have two namespace resources representing the same physical object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

missing mrvl_nvm_ctrlr_attach_ns and mrvl_nvm_ctrlr_detach_ns
2 participants