From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 71FF91FF348 for ; Wed, 17 Apr 2024 15:16:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 220107774; Wed, 17 Apr 2024 15:16:23 +0200 (CEST) Message-ID: Date: Wed, 17 Apr 2024 15:16:19 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Fiona Ebner , Proxmox VE development discussion References: <20240416131909.2867605-1-d.csapak@proxmox.com> <20240416131909.2867605-8-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL -0.036 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH storage 7/9] ovf: implement parsing nics X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 4/17/24 14:09, Fiona Ebner wrote: > Am 16.04.24 um 15:19 schrieb Dominik Csapak: >> by iterating over the relevant parts and trying to parse out the >> 'ResourceSubType'. The content of that is not standardized, but I only >> ever found examples that are compatible with vmware, meaning it's >> either 'e1000', 'e1000e' or 'vmxnet3' (in various capitalizations; thus >> the `lc()`) >> >> As a fallback i used vmxnet3, since i guess most OVAs are tuned for >> vmware. > > I'm not familiar enough with the OVA/OVF ecosystem, but is this really > the best default. I'd kinda expect e1000(e) to cause less issues in case > we were not able to get the type from the OVA/OVF. And people coming > from VMWare are likely going to use the dedicated importer. i did choose that, since from what i saw looking for ovas, they are mostly tailored for vmware consumption, so i thought it'd make sense to use that as default. not opposed to use e1000 though. i think in practice it won't make much difference > >> >> Signed-off-by: Dominik Csapak >> --- >> src/PVE/Storage/DirPlugin.pm | 2 +- >> src/PVE/Storage/OVF.pm | 20 +++++++++++++++++++- >> src/test/run_ovf_tests.pl | 5 +++++ >> 3 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm >> index 8a248c7..21c8350 100644 >> --- a/src/PVE/Storage/DirPlugin.pm >> +++ b/src/PVE/Storage/DirPlugin.pm >> @@ -294,7 +294,7 @@ sub get_import_metadata { >> 'create-args' => $res->{qm}, >> 'disks' => $disks, >> warnings => $warnings, >> - net => [], >> + net => $res->{net}, >> }; >> } >> >> diff --git a/src/PVE/Storage/OVF.pm b/src/PVE/Storage/OVF.pm >> index f438de2..c3e7ed9 100644 >> --- a/src/PVE/Storage/OVF.pm >> +++ b/src/PVE/Storage/OVF.pm >> @@ -120,6 +120,12 @@ sub get_ostype { >> return $ostype_ids->{$id} // 'other'; >> } >> >> +my $allowed_nic_models = [ >> + 'e1000', >> + 'e1000e', >> + 'vmxnet3', >> +]; >> + >> sub find_by { >> my ($key, $param) = @_; >> foreach my $resource (@resources) { >> @@ -355,9 +361,21 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); >> >> $qm->{boot} = "order=" . join(';', @$boot); >> >> + my $nic_id = dtmf_name_to_id('Ethernet Adapter'); >> + my $xpath_find_nics = "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${nic_id}]"; >> + my @nic_items = $xpc->findnodes($xpath_find_nics); >> + >> + my $net = {}; >> + >> + my $net_count = 0; >> + foreach my $item_node (@nic_items) { > > Style nit: please use for instead of foreach > >> + my $model = $xpc->findvalue('rasd:ResourceSubType', $item_node); >> + $model = lc($model); >> + $model = 'vmxnet3' if ! grep $model, @$allowed_nic_models; > > >> + $net->{"net${net_count}"} = { model => $model }; >> } > > $net_count is never increased. > >> >> - return {qm => $qm, disks => \@disks}; >> + return {qm => $qm, disks => \@disks, net => $net}; >> } >> >> 1; >> diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl >> index 8cf5662..d9a7b4b 100755 >> --- a/src/test/run_ovf_tests.pl >> +++ b/src/test/run_ovf_tests.pl >> @@ -54,6 +54,11 @@ is($win10noNs->{disks}->[0]->{disk_address}, 'scsi0', 'single disk vm (no defaul >> is($win10noNs->{disks}->[0]->{backing_file}, "$test_manifests/Win10-Liz-disk1.vmdk", 'single disk vm (no default rasd NS) has the correct disk backing device'); >> is($win10noNs->{disks}->[0]->{virtual_size}, 2048, 'single disk vm (no default rasd NS) has the correct size'); >> >> +print "testing nics\n"; >> +is($win2008->{net}->{net0}->{model}, 'e1000', 'win2008 has correct nic model'); >> +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 "\ntesting vm.conf extraction\n"; >> >> is($win2008->{qm}->{boot}, 'order=scsi0;scsi1', 'win2008 VM boot is correct'); _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel