Skip to content

convert: avoid duplicate sd targets for Windows guests on q35*#122

Open
ssahani wants to merge 1 commit intolibguestfs:masterfrom
ssahani:disk-collision
Open

convert: avoid duplicate sd targets for Windows guests on q35*#122
ssahani wants to merge 1 commit intolibguestfs:masterfrom
ssahani:disk-collision

Conversation

@ssahani
Copy link
Copy Markdown
Contributor

@ssahani ssahani commented Dec 10, 2025

Windows guests commonly include IDE CD-ROM devices. When converting such guests to q35 (required for UEFI), virt-v2v maps legacy IDE devices to SATA, causing both SATA and SCSI devices to share the sd* naming namespace.

virt-v2v previously generated target device names using per-bus slot indices, which could result in duplicate entries (e.g. main disk on SCSI and CD-ROM on SATA). Libvirt rejects such domains with a duplicate target device error.

Fix this by allocating target device names from a shared per-prefix namespace (sd, vd, hd, fd) so that all devices using the same guest-visible naming scheme receive unique target names, matching virt-install behavior and libvirt expectations.

Windows guests commonly include IDE CD-ROM devices. When converting such
guests to q35 (required for UEFI), virt-v2v maps legacy IDE devices to
SATA, causing both SATA and SCSI devices to share the sd* naming
namespace.

virt-v2v previously generated target device names using per-bus slot
indices, which could result in duplicate <target dev='sda'> entries
(e.g. main disk on SCSI and CD-ROM on SATA). Libvirt rejects such domains
with a duplicate target device error.

Fix this by allocating target device names from a shared per-prefix
namespace (sd, vd, hd, fd) so that all devices using the same
guest-visible naming scheme receive unique target names, matching
virt-install behavior and libvirt expectations.

Signed-off-by: Susant Sahani <ssahani@redhat.com>
@rwmjones
Copy link
Copy Markdown
Member

Unfortunately I don't think this approach can work, since it will end up moving disks between slots, which will end up breaking /dev/sdX device names inside the guest /etc files.

Unfortunately # 2, the actual fix for this is going to be extremely difficult (there's a reason why this bug has sat on the backlog for a long time).

It involves changing how we do device renaming so that we never use names like /dev/sdX inside the guest, but instead rewrite them as necessary to UUIDs. After such a change, we could do something like this as we wouldn't be so sensitive to the order that disks are added to the libvirt XML. This is a very complicated and invasive change.

@crobinso
Copy link
Copy Markdown
Collaborator

crobinso commented Dec 12, 2025

Hmm why does this re-order disks from guest perspective? IIRC libvirt qemu target dev= names do not affect how disks are exposed to the guest, only explicit <address> config affects that (which means device XML ordering also affects that when no <address> is manually set). I believe once upon a time target dev= impacted device ordering on qemu command line but that was not been the case for a long time.

This is my understanding but I haven't dug super into libvirt code. But for example, there is no (direct) touching of 'disk->dst' in libvirt qemu_domain_address.c

I think nowadays for qemu driver, target dev= is just a unique identifier for the device in the XML and nothing more.

(unless libguestfs uses target dev name to adjust device ordering or <address> ?)

@rwmjones
Copy link
Copy Markdown
Member

Because it could possibly insert the CD-ROM into the list of /dev/sdX devices visible in the guest, ie. the fact that this fails now is good.

The real issue however is that we need to stop using /dev/sdX names when we rewrite device names inside the guest, because (with modern kernels) those names aren't stable, and because then it won't matter as much in what order we add disks to the guest (boot order would still matter, although probably only for BIOS).

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.

3 participants