VMware prereqs and privileges#913
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 48 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extracts VMware privilege requirements into a new reference module, restructures VMware prerequisite content into Limitations/Prerequisites/Considerations, updates assembly includes and procedure cross-references, and parameterizes hard-coded product/provider names across multiple documentation files. ChangesVMware Documentation Privileges Separation and Parameterization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
documentation/modules/ref_vmware-privileges.adoc (1)
20-20: 💤 Low valueConsider rewording for clarity.
The phrase "by the {vmw} Virtual Infrastructure eXtension (VIX) API" embeds the
{vmw}attribute within the sentence, which may resolve to "VMware" or "vSphere" depending on attribute definitions. Consider whether this should read "by the VMware Virtual Infrastructure eXtension (VIX) API" (with VMware hard-coded since VIX is specifically a VMware technology) or adjust the sentence structure for clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@documentation/modules/ref_vmware-privileges.adoc` at line 20, The table row containing "`Virtual machine.Guest operating system management by VIX API`" embeds the {vmw} attribute and is ambiguous; update the phrasing to explicitly reference VMware's VIX (e.g., "Allows managing a virtual machine using the VMware Virtual Infrastructure eXtension (VIX) API") or restructure to remove the inline {vmw} so VIX is clearly tied to VMware, and ensure the modified sentence replaces the existing table cell text.documentation/modules/ref_vmware-prerequisites.adoc (1)
32-32: ⚡ Quick winConsider breaking up the complex sentence for readability.
The Considerations paragraph on warm migrations of Microsoft Windows VMs is a single very long sentence (100+ words) that covers multiple concepts: VSS usage, manual startup requirement, failure symptoms, and the automatic startup behavior. Consider breaking this into 2-3 shorter sentences for improved readability.
📝 Suggested revision for clarity
Considerations:: -* *Warm migrations of Microsoft Windows VMs:* For virtual machines (VMs) running Microsoft Windows, Volume Shadow Copy Service (VSS) inside the guest VM is used to quiesce the file system and applications. When performing a warm migration of a Microsoft Windows virtual machine from {vmw}, you must start VSS on the Windows guest operating system in order for the snapshot and `Quiesce guest file system` to succeed. If you do not start VSS, the snapshot creation during the warm migration fails with the following error: `An error occurred while taking a snapshot: Failed to restart the virtual machine.` If you set the VSS service to `Manual` and start a snapshot creation with `Quiesce guest file system = yes`, the {vmw} Snapshot provider service requests VSS to start the shadow copy in the background. +* *Warm migrations of Microsoft Windows VMs:* For virtual machines (VMs) running Microsoft Windows, Volume Shadow Copy Service (VSS) inside the guest VM is used to quiesce the file system and applications. When performing a warm migration of a Microsoft Windows virtual machine from {vmw}, you must start VSS on the Windows guest operating system in order for the snapshot and `Quiesce guest file system` to succeed. If you do not start VSS, the snapshot creation during the warm migration fails with the following error: `An error occurred while taking a snapshot: Failed to restart the virtual machine.` ++ +If you set the VSS service to `Manual` and start a snapshot creation with `Quiesce guest file system = yes`, the {vmw} Snapshot provider service requests VSS to start the shadow copy in the background.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@documentation/modules/ref_vmware-prerequisites.adoc` at line 32, The long "Warm migrations of Microsoft Windows VMs" sentence should be split into 2–3 shorter sentences: 1) state that VSS is used to quiesce the guest file system and applications during warm migration from {vmw} and that VSS must be started in the Windows guest for snapshots and "Quiesce guest file system" to succeed, 2) describe the failure symptom and include the exact error text (“An error occurred while taking a snapshot: Failed to restart the virtual machine.”), and 3) note that if the VSS service is set to Manual, starting a snapshot with Quiesce guest file system = yes triggers the {vmw} Snapshot provider to request VSS to start in the background; update the paragraph in documentation/modules/ref_vmware-prerequisites.adoc accordingly for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@documentation/modules/con_migrating-vms-by-using-mtv-ui.adoc`:
- Around line 32-34: The three broken link targets use the wrong suffix `_mtv`;
update the link targets in the additional resources list so each anchor ends
with `_{context}` instead of `_mtv`: change
`assembly_software-requirements-for-migration_mtv` to
`assembly_software-requirements-for-migration_{context}`,
`ref_vmware-privileges_mtv` to `ref_vmware-privileges_{context}`, and
`proc_creating-vddk-image_mtv` to `proc_creating-vddk-image_{context}` so the
links resolve correctly with the existing `{mtv-plan}` attribute.
---
Nitpick comments:
In `@documentation/modules/ref_vmware-prerequisites.adoc`:
- Line 32: The long "Warm migrations of Microsoft Windows VMs" sentence should
be split into 2–3 shorter sentences: 1) state that VSS is used to quiesce the
guest file system and applications during warm migration from {vmw} and that VSS
must be started in the Windows guest for snapshots and "Quiesce guest file
system" to succeed, 2) describe the failure symptom and include the exact error
text (“An error occurred while taking a snapshot: Failed to restart the virtual
machine.”), and 3) note that if the VSS service is set to Manual, starting a
snapshot with Quiesce guest file system = yes triggers the {vmw} Snapshot
provider to request VSS to start in the background; update the paragraph in
documentation/modules/ref_vmware-prerequisites.adoc accordingly for clarity.
In `@documentation/modules/ref_vmware-privileges.adoc`:
- Line 20: The table row containing "`Virtual machine.Guest operating system
management by VIX API`" embeds the {vmw} attribute and is ambiguous; update the
phrasing to explicitly reference VMware's VIX (e.g., "Allows managing a virtual
machine using the VMware Virtual Infrastructure eXtension (VIX) API") or
restructure to remove the inline {vmw} so VIX is clearly tied to VMware, and
ensure the modified sentence replaces the existing table cell text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6548656-bc31-4447-9841-e413a233024c
📒 Files selected for processing (6)
documentation/doc-Planning_your_migration/assemblies/assembly_migrating-vms-web-console.adocdocumentation/doc-Planning_your_migration/assemblies/assembly_provider-specific-requirements-for-migration.adocdocumentation/modules/con_migrating-vms-by-using-mtv-ui.adocdocumentation/modules/proc_creating-vmware-role-mtv-permissions.adocdocumentation/modules/ref_vmware-prerequisites.adocdocumentation/modules/ref_vmware-privileges.adoc
Jenny-Anne
force-pushed
the
MTV-3278-vmware-tools-warm-migrations
branch
from
3afe933 to
29852e2
Compare
|
@Jenny-Anne A few minor points, which are probably all legacy. Otherwise, LGTM. |
- Explicitly list both required Windows services (VSS and VMware Snapshot Provider) - Explain what each service does and how they work together - Document failure behavior when services are disabled - Consolidate VMware Tools prerequisites into single bullet - Improve clarity and flow throughout prerequisites - Apply technical writing improvements (active voice, concise phrasing) Addresses Jira requirement to document Windows services for warm migration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Jenny-Anne
force-pushed
the
MTV-3278-vmware-tools-warm-migrations
branch
from
e07667c to
c0f5f73
Compare
Resolved conflicts in:
- documentation/doc-Planning_your_migration/assemblies/assembly_migrating-vms-web-console.adoc
- documentation/modules/con_migrating-vms-by-using-mtv-ui.adoc
Accepted main branch changes for link structure updates and ensured consistent use of {vmw} variable throughout.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
solenoci
left a comment
solenoci
left a comment
There was a problem hiding this comment.
I have one NP, otherwise the requirements lgtm (although vSan + VDDK statement seems weird to me but I will leave that to another reviewer)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
documentation/modules/ref_vmware-prerequisites.adoc (2)
12-13: ⚖️ Poor tradeoffClarify VDDK requirement scope and consider moving into Prerequisites.
The standalone paragraph stating "Create a VDDK image to accelerate migrations" (line 12) appears to position VDDK as optional for acceleration, but line 21 in Prerequisites states "You must use a VDDK image if the source VMs are backed by {vmw} vSAN," which makes it mandatory for vSAN. This creates potential confusion about when VDDK is required versus recommended.
Additionally, this content sits outside the definition list structure (Limitations/Prerequisites/Considerations), making the document organization inconsistent.
Consider either:
- Moving this VDDK guidance into the Prerequisites section with clarified scope, or
- Revising the wording to distinguish between "recommended for acceleration" and "required for vSAN-backed VMs"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@documentation/modules/ref_vmware-prerequisites.adoc` around lines 12 - 13, The standalone sentence "Create a VDDK image to accelerate migrations" conflicts with the Prerequisites note requiring VDDK for vSAN and is outside the document's definition list; either move this guidance into the existing Prerequisites item and explicitly state scope (e.g., "Required for source VMs backed by {vmw} vSAN; recommended otherwise to accelerate migrations") or keep it in place but change the wording to clearly distinguish "Recommended to accelerate migrations" vs "Required for vSAN-backed VMs" and place it inside the same Definition List structure under Prerequisites/Limitations (referencing the exact phrase "Create a VDDK image to accelerate migrations" and the "Prerequisites" section so the editor can locate and update the text).
24-24: 💤 Low valueConsider clarifying the hibernation trade-off.
The statement "When hibernation is disabled, VM data might be lost if there is a power outage" correctly warns about the consequence, but the causal relationship could be clearer. Consider rewording to emphasize the trade-off:
"Disable hibernation on the source VMs. Note that without hibernation enabled, VM data might be lost during a power outage."
This minor adjustment helps readers understand they're removing protection, not introducing a new risk.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@documentation/modules/ref_vmware-prerequisites.adoc` at line 24, Update the sentence in documentation/modules/ref_vmware-prerequisites.adoc that currently reads "Disable hibernation on the source VMs. When hibernation is disabled, VM data might be lost if there is a power outage." to clarify the trade-off; reword it to something like: "Disable hibernation on the source VMs. Note that without hibernation enabled, VM data might be lost during a power outage." ensuring the text emphasizes that disabling hibernation removes a protection rather than creating a new risk.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@documentation/modules/ref_source-vm-migration-considerations.adoc`:
- Line 22: Update the grammatical error in the sentence that reads "{vmw} Tools
automatically triggers VSS to start" by changing the verb to plural form
"trigger" so it reads "{vmw} Tools automatically trigger VSS to start"; locate
and edit that exact string in ref_source-vm-migration-considerations.adoc (the
sentence near the VSS/snapshot explanation) to apply the correction.
---
Nitpick comments:
In `@documentation/modules/ref_vmware-prerequisites.adoc`:
- Around line 12-13: The standalone sentence "Create a VDDK image to accelerate
migrations" conflicts with the Prerequisites note requiring VDDK for vSAN and is
outside the document's definition list; either move this guidance into the
existing Prerequisites item and explicitly state scope (e.g., "Required for
source VMs backed by {vmw} vSAN; recommended otherwise to accelerate
migrations") or keep it in place but change the wording to clearly distinguish
"Recommended to accelerate migrations" vs "Required for vSAN-backed VMs" and
place it inside the same Definition List structure under
Prerequisites/Limitations (referencing the exact phrase "Create a VDDK image to
accelerate migrations" and the "Prerequisites" section so the editor can locate
and update the text).
- Line 24: Update the sentence in
documentation/modules/ref_vmware-prerequisites.adoc that currently reads
"Disable hibernation on the source VMs. When hibernation is disabled, VM data
might be lost if there is a power outage." to clarify the trade-off; reword it
to something like: "Disable hibernation on the source VMs. Note that without
hibernation enabled, VM data might be lost during a power outage." ensuring the
text emphasizes that disabling hibernation removes a protection rather than
creating a new risk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 704f08c8-93fd-4194-b9f0-c57923fd872b
📒 Files selected for processing (8)
documentation/doc-Planning_your_migration/assemblies/assembly_migrating-vms-web-console.adocdocumentation/doc-Planning_your_migration/assemblies/assembly_provider-specific-requirements-for-migration.adocdocumentation/modules/con_migrating-vms-by-using-mtv-ui.adocdocumentation/modules/proc_adding-source-provider.adocdocumentation/modules/proc_creating-vmware-role-mtv-permissions.adocdocumentation/modules/ref_source-vm-migration-considerations.adocdocumentation/modules/ref_vmware-prerequisites.adocdocumentation/modules/ref_vmware-privileges.adoc
✅ Files skipped from review due to trivial changes (4)
- documentation/modules/proc_adding-source-provider.adoc
- documentation/doc-Planning_your_migration/assemblies/assembly_migrating-vms-web-console.adoc
- documentation/modules/proc_creating-vmware-role-mtv-permissions.adoc
- documentation/modules/ref_vmware-privileges.adoc
🚧 Files skipped from review as they are similar to previous changes (2)
- documentation/doc-Planning_your_migration/assemblies/assembly_provider-specific-requirements-for-migration.adoc
- documentation/modules/con_migrating-vms-by-using-mtv-ui.adoc
Thanks for reviewing @solenoci. Updated the vSAN and VDDK statement to "You must use a VDDK image if the source VMs are backed by VMware vSAN." |
|
@Jenny-Anne Hi, what I meant by VDDK + vSan is that I am not sure that's even supported (I think not but I could be wrong) |
Hi @solenoci Thanks. A warning about vSAN is in the legacy (currently published) doc twice. It wasn't part of this PR. Can anyone confirm if it's supported or not and I can update as needed?
|
3 participants
Addresses three docs bugs.
Jira:
https://redhat.atlassian.net/browse/MTV-2872
https://redhat.atlassian.net/browse/MTV-3278
https://redhat.atlassian.net/browse/MTV-3922
Clarified Windows services requirements and consolidated VMware Tools prerequisites for warm migrations
Updated wording to clarify that users don't need to manually start VSS on hundreds of VMs - VSS starts automatically when triggered by VMware Tools during snapshot operations.
Moved VMware privileges to a separate reference topic and separated the table into several tables for each section
Previews:
Summary by CodeRabbit