Skip to content

Add xenserver.create.full.clone global setting#13114

Open
erma07 wants to merge 2 commits intoapache:mainfrom
wavecomee:feature/xen-full-clone
Open

Add xenserver.create.full.clone global setting#13114
erma07 wants to merge 2 commits intoapache:mainfrom
wavecomee:feature/xen-full-clone

Conversation

@erma07
Copy link
Copy Markdown

@erma07 erma07 commented May 7, 2026

Description

This PR adds a new global setting xenserver.create.full.clone that mirrors the existing vmware.create.full.clone behaviour for the XenServer/XCP-ng hypervisor.

Until now, XenServer-backed VMs created from a template have always been provisioned as linked clones (VDI.clone()), which on file-backed SRs (NFS/EXT/XFS) produces a copy-on-write disk that depends on the cached template VDI. This has two practical drawbacks:

  1. The cached template VDI cannot be removed while any linked-clone child still exists (XenServer enforces a VHD parent ref-count), so template cleanup is blocked.
  2. There is a hard ceiling of ~30 linked clones per VHD chain in XenServer.

When the new setting is enabled (per-pool or globally), the XenServer storage processor instead issues VDI.copy(conn, sr) for the template→volume operation, producing a fully-independent VDI with no parent VHD relationship. Deleting the template afterwards is harmless to existing VMs, and the per-chain clone ceiling no longer applies.

Default value is false, so existing deployments are unaffected unless an operator opts in.

How it's wired up:

  • New ConfigKey<Boolean> XenserverCreateCloneFull in StorageManager (StoragePool scope, dynamic, default "false"), registered via StorageManagerImpl.getConfigKeys().
  • AncientDataMotionStrategy keeps addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest byte-for-byte unchanged. A new sibling helper addFullCloneAndDiskprovisiongStrictnessFlagOnXenServerDest writes the per-pool XenServer value onto PrimaryDataStoreTO.fullCloneFlag, and a new dispatcher addFullCloneAndDiskprovisiongStrictnessFlagOnDest switches on dataTO.getHypervisorType() to call the right per-hypervisor helper. All seven existing call sites that used to call the VMware helper now call the dispatcher instead — VMware behaviour is preserved exactly.
  • PrimaryDataStoreTO.fullCloneFlag already exists and is hypervisor-agnostic; no TO change required.
  • On the agent, XenServerStorageProcessor.cloneVolumeFromBaseTemplate reads ((PrimaryDataStoreTO) destStore).isFullCloneFlag() and, when true, calls tmpltvdi.copy(conn, tmpltvdi.getSR(conn)) instead of tmpltvdi.createClone(conn, new HashMap<>()). Xenserver625StorageProcessor does not override the method, so all XenServer versions are covered with a single edit.

The legacy CitrixCreateCommandWrapper.CreateCommand path is intentionally left untouched — new CreateCommand(...) is only constructed by test code in this repository, so the wrapper is unreachable in production today.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • 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

Screenshots (if appropriate):

Global Settings UI showing the new xenserver.create.full.clone key — TODO: attach screenshot.

How Has This Been Tested?

Unit tests

  • Added AncientDataMotionStrategyTest.testAddFullCloneFlagOnXenServerDest, mirroring the existing VMware test, which overrides the XenserverCreateCloneFull default, sets dataTO.getHypervisorType() to HypervisorType.XenServer, calls the new XenServer helper, and verifies that setFullCloneFlag(true) is invoked on the destination PrimaryDataStoreTO.
  • Existing testAddFullCloneFlagOnVMwareDest and testAddFullCloneFlagOnNotVmwareDest continue to pass — the VMware helper was not modified.

Manual test environment (fill in)

  • CloudStack version: <version>
  • XenServer / XCP-ng version: <version>
  • Primary storage type tested: <NFS / EXT / LVM / iSCSI>
  • Template format / size: <format / size>

Manual scenarios (fill in results)

  1. With xenserver.create.full.clone=false (default), deploy a VM from a template and run on the XenServer host:

    xe vdi-list params=uuid,sm-config name-label=ROOT-<vmid>
    

    Expect sm-config:vhd-parent to be present (linked clone, today's behaviour). Result: ☐

  2. Set xenserver.create.full.clone=true on the pool (Infrastructure → Primary Storage → Settings) and deploy another VM from the same template. Re-run the command above and expect sm-config:vhd-parent to be absent, and physical-utilisation to be on the same order of magnitude as virtual-size. Result: ☐

  3. Delete the cached template VDI on the primary storage. The full-clone-backed VM keeps running normally; a linked-clone-backed VM would lose data. Restore the template afterwards. Result: ☐

  4. Repeat (1)–(3) on a VMware pool to confirm the vmware.create.full.clone path is unaffected by the helper refactor. Result: ☐

How did you try to break this feature and the system with this change?

  • Mixed hypervisors: verified the dispatch's switch only triggers when dataTO.getHypervisorType() is XenServer; pools with KVM, Hyperv, etc. fall through and fullCloneFlag is left untouched (linked-clone behaviour preserved).
  • No hypervisor type: if Volume.getHypervisorType() returns HypervisorType.None (storage pool not bound to a cluster with a known hypervisor), the dispatch falls through to the default branch and the volume is linked-cloned — safe default, no regression.
  • VMware regression: the original addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest is byte-for-byte unchanged; the seven call sites that previously called it now go through the dispatcher, which routes VMware traffic back into the same method.
  • Managed storage path (out of scope, documented): flows that route through StorageSystemDataMotionStrategy (e.g. SolidFire) bypass the dispatcher — same VMware-side limitation today, behaviour preserved.
  • Legacy CreateCommand path (out of scope, documented): CitrixCreateCommandWrapper.execute(CreateCommand) still always linked-clones, but new CreateCommand(...) has no production callers in this repository (only test code).
  • Default value: false, so the change is a no-op on upgrade until an operator opts in.

Adds a StoragePool-scoped boolean ConfigKey mirroring vmware.create.full.clone
so XenServer-backed VMs can be deployed as full VDI copies (VDI.copy) instead
of always using linked clones (VDI.clone). Default false preserves today's
behavior.

The per-pool flag flows into the existing PrimaryDataStoreTO.fullCloneFlag
through a new dispatch method addFullCloneAndDiskprovisiongStrictnessFlagOnDest
that switches on hypervisor type.
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented May 7, 2026

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:

@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.52%. Comparing base (96ca1b2) to head (1c47f93).
⚠️ Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (96ca1b2) and HEAD (1c47f93). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (96ca1b2) HEAD (1c47f93)
unittests 1 0
Additional details and impacted files
@@              Coverage Diff              @@
##               main   #13114       +/-   ##
=============================================
- Coverage     18.09%    3.52%   -14.57%     
=============================================
  Files          6037      464     -5573     
  Lines        542546    40151   -502395     
  Branches      66431     7557    -58874     
=============================================
- Hits          98148     1415    -96733     
+ Misses       433378    38548   -394830     
+ Partials      11020      188    -10832     
Flag Coverage Δ
uitests 3.52% <ø> (ø)
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

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

code lgtm

not tested

@DaanHoogland
Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17760

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants