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

Document CPU limits and other limits of VMs in XCP-ng #296

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

thomas-dkmt
Copy link
Collaborator

I’ve updated the Requirements page of the XCP-ng documentation, so that it includes the VM limits (CPU, Network, vGPU, etc.), and I improved the overall style and language.

Before submitting the pull request, you must agree with the following statements by checking both boxes with a 'x'.

  • [x ] "I accept that my contribution is placed under the CC BY-SA 2.0 license [1]."
  • [x ] "My contribution complies with the Developer Certificate of Origin [2]."

[1] https://creativecommons.org/licenses/by-sa/2.0/
[2] https://docs.xcp-ng.org/project/contributing/#developer-certificate-of-origin-dco

@nathanael-h
Copy link

Looks good, only 2 questions/suggestions.

@stormi
Copy link
Member

stormi commented Nov 12, 2024

Adding @olivierlambert and @julien-f to the reviewers as the topic of what we say we support or not is a product management topic IMO.

@stormi
Copy link
Member

stormi commented Nov 12, 2024

Why is there a modified version of my already merged "Add doc related to Nested Virtualization, and update 8.3 release notes" commit in this PR?

@julien-f
Copy link
Collaborator

julien-f commented Nov 12, 2024

@thomas-dkmt Ensure your master is up-to-date, rebase your branch on top of it and re-push 🙂

@thomas-dkmt thomas-dkmt force-pushed the cpu-limits branch 3 times, most recently from b63429c to 877b586 Compare November 13, 2024 09:39
@julien-f julien-f removed their request for review November 13, 2024 16:42
@thomas-dkmt thomas-dkmt force-pushed the cpu-limits branch 2 times, most recently from c6940b8 to 156cfe1 Compare November 18, 2024 12:31
@stormi
Copy link
Member

stormi commented Nov 20, 2024

@thomas-dkmt Ensure your master is up-to-date, rebase your branch on top of it and re-push 🙂

There still is a problem with this branch. It looks like a rebase it still needed.

@stormi
Copy link
Member

stormi commented Nov 20, 2024

Nitpicking about the commit messages: the convention in commit messages is usually to use the imperative form rather than the past tense. For example "Improved the style and presentation across the whole page" would become "Improve the style and presentation across the whole page".

@thomas-dkmt
Copy link
Collaborator Author

@thomas-dkmt Ensure your master is up-to-date, rebase your branch on top of it and re-push 🙂

There still is a problem with this branch. It looks like a rebase it still needed.

Rebase done

- FreeBSD and related distributions (e.g., pfSense, TrueNAS)
- OpenBSD

## Virtual Machine Requirements
Copy link
Member

@olivierlambert olivierlambert Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure at all with this entire section:

  • we are very cautious and a quick overview isn't great for us by looking at the numbers
  • We tested more than 64 vCPU with some Linux VMs IIRC, and topology will be one important work next year. So if we provide a limit, we should always put that in perspective with actual work
  • it's even worse for the 2TiB limit, which isn't true for all VDI. Raw are not limited, SMAPIv3 current drivers neither and we aren't far to get qcow2 on tapdisk. So to me it's simply too negative to restrict too much by displaying those numbers without context.
  • I'm not aware of a CD drive limit per VM, I remember some XS doc explaining how to add another one
  • I'm not even sure about some limits (only one passed through GPU? never heard of it)

The rest of the PR is OK to me. Maybe we can start to merge everything except the VM limits which require more work to be suitable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to address all this in a new commit. We're not in a hurry to merge this PR so I think we can take the time to finalize it as a whole.

Mention how to go beyond some limits when some trade-offs
are acceptable.

Remove some not essential limits we are unsure about at the moment.

Signed-off-by: Samuel Verschelde <[email protected]>
@stormi
Copy link
Member

stormi commented Dec 2, 2024

Ping @olivierlambert whenever you find time

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.

5 participants