From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 29D7D1FF0A8 for ; Sat, 03 Oct 2026 15:28:17 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B74E121724; Sat, 03 Oct 2026 15:28:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dualfroz.com; s=dkim; t=1791034088; h=from:subject:date:message-id:to:mime-version: content-transfer-encoding; bh=OWi44mR3UlXaO4sT7Lsyxxt2Q0VYlwbVDJziehzMDHQ=; b=CQfr45I9XkJQexpQwvR5hUIP9B2wz3AsLiP4thmsVQdc86H7EfSNAYMwbLrF4Xdl4XqL12 EmMdJZL+la/L/nm0an4IheR4akYZtGY5MGNKfMPUoGHv4epYK7jv7mnB+SjG9uYx1sN2jg f2PXPEqYkqXlcI0FtfCVBhYz+yXUfK6y9ALuoj9QjQkDO0hD1JRGOWVUVcdysVd8sO22zQ CerewIuAA2UaPTovtNamRLKiLIsJs0Bv+/ENXWYm9NzeVLhhZkkNiAu6YTe+OzLeToo/Dd LNleTUi02qmizYx/Jcfdd545WTiM0lPINXRfZgpsUetVR3QAqC+Exk60xYFKvQ== From: Michal Fox To: pve-devel@lists.proxmox.com Subject: [PATCH storage] fix #6184: import: ovf: add CD drives Date: Sat, 3 Oct 2026 13:28:07 +0000 Message-ID: <20261003132807.7-1-me@dualfroz.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.253 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy KAM_SHORT 0.001 Use of a URL Shortener for very short URL SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: UKEQIQLH4M575W3CM3SLCWBPTHGEOEJX X-Message-ID-Hash: UKEQIQLH4M575W3CM3SLCWBPTHGEOEJX X-MailFrom: me@dualfroz.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The OVF parser only looks at disk drives, so the CD drives of a guest are dropped on import. Appliances shipped as OVA sometimes come with an ISO image in a CD drive, for example to install the guest on first boot. Such a guest then does not boot after the import, and the user is not told about the image at all. Add the CD drives of the manifest as empty drives on their controller, after the disks in the boot order, like the ESXi import does. As CD images cannot be imported, report a referenced image with the existing 'cdrom-image-ignored' warning, so that the import wizard tells the user to configure the drive in the 'Advanced' tab. As the address only takes the position on the controller into account, a CD drive can end up with the address of a disk, for example with a disk on the first IDE controller and the CD drive on the second one. Such a CD drive gets the next free address on its bus. Signed-off-by: Michal Fox --- Tested with the OVF tests, extended by a manifest based on the one attached to the bug report, and with a script that calls get_import_metadata() for an OVF and an OVA with that manifest: the CD drive is now part of the create arguments and the boot order, and the image is reported as 'cdrom-image-ignored'. Without the fix, the new checks fail. The pve-storage tests pass, except for the ZFS and LVM plugin tests, which I could not run here. 'qm importovf' only takes the name, memory and cores from the parsed configuration, so it is not affected. This touches the same files as my patch for #5998 ("import: avoid duplicate disk keys with multiple controllers"), which is not applied yet. Both apply on their own, but applied together, the test hunk for the boot order conflicts, and the expected boot order of the manifest added there needs ';sata1' at the end, as it has a CD drive too. src/PVE/GuestImport/OVF.pm | 43 ++++++++- src/PVE/Storage/DirPlugin.pm | 10 +++ src/test/ovf_manifests/cdrom-image.ovf | 116 +++++++++++++++++++++++++ src/test/run_ovf_tests.pl | 37 +++++++- 4 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 src/test/ovf_manifests/cdrom-image.ovf diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm index a228cd2..f00c36a 100644 --- a/src/PVE/GuestImport/OVF.pm +++ b/src/PVE/GuestImport/OVF.pm @@ -385,7 +385,46 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id, $net_count++; } - return { qm => $qm, disks => \@disks, net => $net }; + # CD drives are added empty, as images cannot be imported, but the referenced image is kept + # so the user can be told about it + my $cdrom_id = dtmf_name_to_id('CD Drive'); + my $xpath_find_cdroms = + "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${cdrom_id}]"; + my @cdrom_items = $xpc->findnodes($xpath_find_cdroms); + + my $addresses_in_use = { map { $_->{disk_address} => 1 } @disks }; + my @cdroms; + for my $item_node (@cdrom_items) { + my $controller_id = $xpc->findvalue('rasd:Parent', $item_node); + my $xpath_find_parent_type = sprintf( + "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\ +ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id, + ); + my $bus = id_to_pve($xpc->findvalue($xpath_find_parent_type)); + if (!$bus) { + warn "invalid or missing controller for CD drive, skipping\n"; + next; + } + + my $index = $xpc->findvalue('rasd:AddressOnParent', $item_node); + $index = 0 if $index !~ m/^\d+$/; + $index++ while $addresses_in_use->{"$bus$index"}; + my $address = "$bus$index"; + $addresses_in_use->{$address} = 1; + + my $image; + my $host_resource = $xpc->findvalue('rasd:HostResource', $item_node); + if ($host_resource =~ m|^(?:ovf:)?/file/([[:alnum:]\-\._~]+)$|) { + my $xpath_find_image = + sprintf("/ovf:Envelope/ovf:References/ovf:File[\@ovf:id='%s']/\@ovf:href", $1); + $image = $xpc->findvalue($xpath_find_image) || undef; + } + + $qm->{$address} = 'none,media=cdrom'; + push @cdroms, { disk_address => $address, image => $image }; + } + + return { qm => $qm, disks => \@disks, net => $net, cdroms => \@cdroms }; } my sub resolve_disks { @@ -436,6 +475,8 @@ my sub resolve_disks { push @$boot_order, $pve_disk_address; } + push @$boot_order, map { $_->{disk_address} } $qm->{cdroms}->@*; + $qm->{qm}->{boot} = "order=" . join(';', @$boot_order) if scalar(@$boot_order) > 0; $qm->{disks} = \@disks; diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index 80c4a03..723a59a 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -299,6 +299,16 @@ sub get_import_metadata { }; } + for my $cdrom ($res->{cdroms}->@*) { + next if !defined($cdrom->{image}); + push @$warnings, + { + type => 'cdrom-image-ignored', + key => $cdrom->{disk_address}, + value => $cdrom->{image}, + }; + } + if (defined($res->{qm}->{bios}) && $res->{qm}->{bios} eq 'ovmf') { $disks->{efidisk0} = 1; push @$warnings, { type => 'efi-state-lost', key => 'bios', value => 'ovmf' }; diff --git a/src/test/ovf_manifests/cdrom-image.ovf b/src/test/ovf_manifests/cdrom-image.ovf new file mode 100644 index 0000000..665461c --- /dev/null +++ b/src/test/ovf_manifests/cdrom-image.ovf @@ -0,0 +1,116 @@ + + + + + + + + + Virtual disk information + + + + + The list of logical networks + + Service Port + + + + A virtual machine + cdrom-image + + The kind of installed guest operating system + + + Virtual hardware requirements + + Virtual Hardware Family + 0 + cdrom-image + vmx-07 + + + hertz * 10^6 + Number of Virtual CPUs + 1 virtual CPU(s) + 1 + 3 + 1 + + + byte * 2^20 + Memory Size + 2048MB of memory + 2 + 4 + 2048 + + + 0 + SCSI Controller + SCSI Controller 0 + 3 + lsilogic + 6 + + + 1 + IDE Controller + VirtualIDEController 1 + 4 + 5 + + + 0 + IDE Controller + VirtualIDEController 0 + 5 + 5 + + + 0 + Hard Disk 1 + ovf:/disk/vmdisk1 + 6 + 3 + 17 + + + 0 + Hard Disk 2 + ovf:/disk/vmdisk2 + 7 + 5 + 17 + + + 0 + false + Floppy Drive + Floppy 1 + 8 + 14 + + + 0 + true + CD-ROM 1 + ovf:/file/file2 + 9 + 4 + 15 + + + 9 + true + VM Network + E1000 ethernet adapter on "VM Network" + Ethernet 0 + 10 + E1000 + 10 + + + + diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl index 23f0aa6..1215882 100755 --- a/src/test/run_ovf_tests.pl +++ b/src/test/run_ovf_tests.pl @@ -45,6 +45,14 @@ if (my $err = $@) { fail('parsing XEE.ovf should have failed!'); } +my $cdromImage = eval { PVE::GuestImport::OVF::parse_ovf("$test_manifests/cdrom-image.ovf") }; +if (my $err = $@) { + fail('parse cdrom image'); + warn("error: $err\n"); +} else { + ok('parse cdrom image'); +} + print "testing disks\n"; is( @@ -101,22 +109,43 @@ is($win10->{net}->{net0}->{model}, 'e1000e', 'win10 has correct nic model'); is($win10noNs->{net}->{net0}->{model}, 'e1000e', 'win10 (no default rasd NS) has correct nic model'); +print "testing cdroms\n"; +is_deeply( + $win2008->{cdroms}, + [{ disk_address => 'sata1', image => undef }], + 'win2008 has an empty cdrom', +); +is($win2008->{qm}->{sata1}, 'none,media=cdrom', 'win2008 cdrom is added to the config'); +is_deeply( + $cdromImage->{cdroms}, + [{ disk_address => 'ide1', image => 'installer.iso' }], + 'cdrom with image gets the next free address on its bus', +); +is($cdromImage->{qm}->{ide1}, 'none,media=cdrom', 'cdrom with image is added empty to the config'); +is_deeply( + [map { $_->{disk_address} } $cdromImage->{disks}->@*], + ['scsi0', 'ide0'], + 'cdrom with image vm has the correct disks', +); + print "\ntesting vm.conf extraction\n"; -is($win2008->{qm}->{boot}, 'order=scsi0;scsi1', 'win2008 VM boot is correct'); +is($win2008->{qm}->{boot}, 'order=scsi0;scsi1;sata1', 'win2008 VM boot is correct'); is($win2008->{qm}->{name}, 'Win2008-R2x64', 'win2008 VM name is correct'); is($win2008->{qm}->{memory}, '2048', 'win2008 VM memory is correct'); is($win2008->{qm}->{cores}, '1', 'win2008 VM cores are correct'); is($win2008->{qm}->{ostype}, 'win7', 'win2008 VM ostype is correcty'); -is($win10->{qm}->{boot}, 'order=scsi0', 'win10 VM boot is correct'); +is($win10->{qm}->{boot}, 'order=scsi0;sata1', 'win10 VM boot is correct'); is($win10->{qm}->{name}, 'Win10-Liz', 'win10 VM name is correct'); is($win10->{qm}->{memory}, '6144', 'win10 VM memory is correct'); is($win10->{qm}->{cores}, '4', 'win10 VM cores are correct'); # older esxi/ovf standard used 'other' for windows10 is($win10->{qm}->{ostype}, 'other', 'win10 VM ostype is correct'); -is($win10noNs->{qm}->{boot}, 'order=scsi0', 'win10 VM (no default rasd NS) boot is correct'); +is( + $win10noNs->{qm}->{boot}, 'order=scsi0;sata1', 'win10 VM (no default rasd NS) boot is correct', +); is($win10noNs->{qm}->{name}, 'Win10-Liz', 'win10 VM (no default rasd NS) name is correct'); is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default rasd NS) memory is correct'); is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are correct'); @@ -124,4 +153,6 @@ is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are corr is($win10noNs->{qm}->{ostype}, 'other', 'win10 VM (no default rasd NS) ostype is correct'); is($win10noNs->{qm}->{bios}, 'ovmf', 'win10 VM (no default rasd NS) bios is correct'); +is($cdromImage->{qm}->{boot}, 'order=scsi0;ide0;ide1', 'cdrom with image VM boot is correct'); + done_testing(); -- 2.43.0