[pve-devel] [PATCH docs v2] fix #5429: network: override device names: include Type=ether

2024-04-29 Thread Friedrich Weber
Mention that the systemd link file should contain `Type=ether` in most
setup, to make sure it only applies to Ethernet devices and does not
ever apply to e.g. bridges or bonds which inherit the MAC address of
the Ethernet device. Mention that some setups may require other
options.

Reported in the forum [0] and in #5429 [1].

[0] https://forum.proxmox.com/threads/144557/post-656188
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5429

Fixes: 96c0261 ("fix #4847: network: extend section on interface naming scheme")
Signed-off-by: Friedrich Weber 
---

Notes:
Changes v1 -> v2:

- link #5429 which was opened in the meantime
- expand on why Type=ether is recommended
- mention that some setups may require other choices (thx Thomas)

v1: https://lists.proxmox.com/pipermail/pve-devel/2024-April/063659.html

 pve-network.adoc | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/pve-network.adoc b/pve-network.adoc
index ef586ec..be8d63c 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -173,16 +173,25 @@ configured, including their naming.
 To assign a name to a particular network device, you need a way to uniquely and
 permanently identify that device in the `[Match]` section. One possibility is
 to match the device's MAC address using the `MACAddress` option, as it is
-unlikely to change. Then, you can assign a name using the `Name` option in the
-`[Link]` section.
+unlikely to change.
 
-For example, to assign the name `enwan0` to the device with MAC address
-`aa:bb:cc:dd:ee:ff`, create a file `/etc/systemd/network/10-enwan0.link` with
-the following contents:
+The `[Match]` section should also contain a `Type` option to make sure it only
+matches the expected physical interface, and not bridge/bond/VLAN interfaces
+with the same MAC address. In most setups, `Type` should be set to `ether` to
+match only Ethernet devices, but some setups may require other choices. See the
+https://manpages.debian.org/stable/udev/systemd.link.5.en.html[systemd.link(5)
+manpage] for more details.
+
+Then, you can assign a name using the `Name` option in the `[Link]` section.
+
+For example, to assign the name `enwan0` to the Ethernet device with MAC
+address `aa:bb:cc:dd:ee:ff`, create a file
+`/etc/systemd/network/10-enwan0.link` with the following contents:
 
 
 [Match]
 MACAddress=aa:bb:cc:dd:ee:ff
+Type=ether
 
 [Link]
 Name=enwan0
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-29 Thread Fiona Ebner
Reported in the community forum:
https://forum.proxmox.com/threads/145848/#post-658694

Signed-off-by: Fiona Ebner 
---

Minimally tested, that an upgrade from an existing 23.04 container
works, there still is network and no obviously bad messages in the
container's journal.

 src/PVE/LXC/Setup/Ubuntu.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/LXC/Setup/Ubuntu.pm b/src/PVE/LXC/Setup/Ubuntu.pm
index 905cacb..cea8ef5 100644
--- a/src/PVE/LXC/Setup/Ubuntu.pm
+++ b/src/PVE/LXC/Setup/Ubuntu.pm
@@ -12,6 +12,7 @@ use PVE::LXC::Setup::Debian;
 use base qw(PVE::LXC::Setup::Debian);
 
 my $known_versions = {
+'24.04' => 1, # noble
 '23.10' => 1, # mantic
 '23.04' => 1, # lunar
 '22.10' => 1, # kinetic
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-29 Thread Fiona Ebner
Am 29.04.24 um 11:23 schrieb Fiona Ebner:
> Reported in the community forum:
> https://forum.proxmox.com/threads/145848/#post-658694
> 
> Signed-off-by: Fiona Ebner 
> ---
> 
> Minimally tested, that an upgrade from an existing 23.04 container
> works, there still is network and no obviously bad messages in the
> container's journal.
> 

Hmm, while the upgrade did work, starting from an Ubuntu 24.04 template
and setting a static IP does not seem to work, like described here:
https://forum.proxmox.com/threads/145848/post-658058


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-29 Thread Fiona Ebner
Am 29.04.24 um 11:36 schrieb Fiona Ebner:
> Am 29.04.24 um 11:23 schrieb Fiona Ebner:
>> Reported in the community forum:
>> https://forum.proxmox.com/threads/145848/#post-658694
>>
>> Signed-off-by: Fiona Ebner 
>> ---
>>
>> Minimally tested, that an upgrade from an existing 23.04 container
>> works, there still is network and no obviously bad messages in the
>> container's journal.
>>
> 
> Hmm, while the upgrade did work, starting from an Ubuntu 24.04 template
> and setting a static IP does not seem to work, like described here:
> https://forum.proxmox.com/threads/145848/post-658058

Seems like the ordering of the configuration files is the issue. The
following would fix it, but probably needs to be special-cased for new
Ubuntu (or new systemd, would still need to check where the change came
in exactly) not to mess up existing containers, right?

> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index 084b039..162498a 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -249,7 +249,7 @@ sub setup_systemd_networkd {
> my $d = PVE::LXC::Config->parse_lxc_network($conf->{$k});
> next if !$d->{name};
>  
> -   my $filename = "/etc/systemd/network/$d->{name}.network";
> +   my $filename = "/etc/systemd/network/10-$d->{name}.network";
>  
> my $data = <<"DATA";
>  [Match]


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH manager] ui: Remove pveACMEPluginView in favor of pmxACMEPluginView

2024-04-29 Thread Filip Schauer

bump

On 29/08/2023 13:00, Filip Schauer wrote:
Remove pveACMEPluginView and use the ACMEPluginView from the 
proxmox-widget-toolkit instead. This leaves pveACMEPluginEditor 
unused, so remove it as well. Signed-off-by: Filip Schauer 
--- www/manager6/dc/ACMEClusterView.js | 100 
+ www/manager6/dc/ACMEPluginEdit.js | 223 
- 2 files changed, 2 insertions(+), 321 
deletions(-) delete mode 100644 www/manager6/dc/ACMEPluginEdit.js



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 11:56 schrieb Fiona Ebner:
> Am 29.04.24 um 11:36 schrieb Fiona Ebner:
>> Am 29.04.24 um 11:23 schrieb Fiona Ebner:
>>> Reported in the community forum:
>>> https://forum.proxmox.com/threads/145848/#post-658694
>>>
>>> Signed-off-by: Fiona Ebner 
>>> ---
>>>
>>> Minimally tested, that an upgrade from an existing 23.04 container
>>> works, there still is network and no obviously bad messages in the
>>> container's journal.
>>>
>> Hmm, while the upgrade did work, starting from an Ubuntu 24.04 template
>> and setting a static IP does not seem to work, like described here:
>> https://forum.proxmox.com/threads/145848/post-658058
>
> Seems like the ordering of the configuration files is the issue. The
> following would fix it, but probably needs to be special-cased for new
> Ubuntu (or new systemd, would still need to check where the change came
> in exactly) not to mess up existing containers, right?

Yes, at least that would reduce regression potential of unknown issues.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH manager] ui: Remove pveACMEPluginView in favor of pmxACMEPluginView

2024-04-29 Thread Thomas Lamprecht
subject:

ui: acme: switch plugin view over to the one from widget-toolkit


(having internal xtypes in the subject already is not really that
useful)

Am 29/08/2023 um 13:00 schrieb Filip Schauer:
> Remove pveACMEPluginView and use the ACMEPluginView from the
> proxmox-widget-toolkit instead. This leaves pveACMEPluginEditor unused,
> so remove it as well.

when got this moved, would be good to have some references here, which would
it also make it easier to decide if we need a new bump of the verisoned
dependency in d/control.




___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage v3 04/10] ovf: implement parsing the ostype

2024-04-29 Thread Dominik Csapak
use the standards info about the ostypes to map to our own
(see comment for link to the relevant part of the dmtf schema)

every type that is not listed we map to 'other', so no need to have it
in a list.

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/OVF.pm | 69 ++
 src/test/run_ovf_tests.pl  |  5 +++
 2 files changed, 74 insertions(+)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 6b79078..cf08cb6 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -55,6 +55,71 @@ my @resources = (
 { id => 35, dtmf_name => 'Vendor Reserved'}
 );
 
+# see https://schemas.dmtf.org/wbem/cim-html/2.55.0+/CIM_OperatingSystem.html
+my $ostype_ids = {
+18 => 'winxp', # 'WINNT',
+29 => 'solaris', # 'Solaris',
+36 => 'l26', # 'LINUX',
+58 => 'w2k', # 'Windows 2000',
+67 => 'wxp', #'Windows XP',
+69 => 'w2k3', # 'Microsoft Windows Server 2003',
+70 => 'w2k3', # 'Microsoft Windows Server 2003 64-Bit',
+71 => 'wxp', # 'Windows XP 64-Bit',
+72 => 'wxp', # 'Windows XP Embedded',
+73 => 'wvista', # 'Windows Vista',
+74 => 'wvista', # 'Windows Vista 64-Bit',
+75 => 'wxp', # 'Windows Embedded for Point of Service', ??
+76 => 'w2k8', # 'Microsoft Windows Server 2008',
+77 => 'w2k8', # 'Microsoft Windows Server 2008 64-Bit',
+79 => 'l26', # 'RedHat Enterprise Linux',
+80 => 'l26', # 'RedHat Enterprise Linux 64-Bit',
+81 => 'solaris', #'Solaris 64-Bit',
+82 => 'l26', # 'SUSE',
+83 => 'l26', # 'SUSE 64-Bit',
+84 => 'l26', # 'SLES',
+85 => 'l26', # 'SLES 64-Bit',
+87 => 'l26', # 'Novell Linux Desktop',
+89 => 'l26', # 'Mandriva',
+90 => 'l26', # 'Mandriva 64-Bit',
+91 => 'l26', # 'TurboLinux',
+92 => 'l26', # 'TurboLinux 64-Bit',
+93 => 'l26', # 'Ubuntu',
+94 => 'l26', # 'Ubuntu 64-Bit',
+95 => 'l26', # 'Debian',
+96 => 'l26', # 'Debian 64-Bit',
+97 => 'l24', # 'Linux 2.4.x',
+98 => 'l24', # 'Linux 2.4.x 64-Bit',
+99 => 'l26', # 'Linux 2.6.x',
+100 => 'l26', # 'Linux 2.6.x 64-Bit',
+101 => 'l26', # 'Linux 64-Bit',
+103 => 'win7', # 'Microsoft Windows Server 2008 R2',
+105 => 'win7', # 'Microsoft Windows 7',
+106 => 'l26', # 'CentOS 32-bit',
+107 => 'l26', # 'CentOS 64-bit',
+108 => 'l26', # 'Oracle Linux 32-bit',
+109 => 'l26', # 'Oracle Linux 64-bit',
+111 => 'win8', # 'Microsoft Windows Server 2011', ??
+112 => 'win8', # 'Microsoft Windows Server 2012',
+113 => 'win8', # 'Microsoft Windows 8',
+114 => 'win8', # 'Microsoft Windows 8 64-bit',
+115 => 'win8', # 'Microsoft Windows Server 2012 R2',
+116 => 'win10', # 'Microsoft Windows Server 2016',
+117 => 'win8', # 'Microsoft Windows 8.1',
+118 => 'win8', # 'Microsoft Windows 8.1 64-bit',
+119 => 'win10', # 'Microsoft Windows 10',
+120 => 'win10', # 'Microsoft Windows 10 64-bit',
+121 => 'win10', # 'Microsoft Windows Server 2019',
+122 => 'win11', # 'Microsoft Windows 11 64-bit',
+123 => 'win11', # 'Microsoft Windows Server 2022',
+# others => 'other',
+};
+
+sub get_ostype {
+my ($id) = @_;
+
+return $ostype_ids->{$id} // 'other';
+}
+
 sub find_by {
 my ($key, $param) = @_;
 foreach my $resource (@resources) {
@@ -160,6 +225,10 @@ sub parse_ovf {
 my 
$xpath_find_disks="/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${disk_id}]";
 my @disk_items = $xpc->findnodes($xpath_find_disks);
 
+my $xpath_find_ostype_id = 
"/ovf:Envelope/ovf:VirtualSystem/ovf:OperatingSystemSection/\@ovf:id";
+my $ostype_id = $xpc->findvalue($xpath_find_ostype_id);
+$qm->{ostype} = get_ostype($ostype_id);
+
 # disks metadata is split in four different xml elements:
 # * as an Item node of type DiskDrive in the VirtualHardwareSection
 # * as an Disk node in the DiskSection
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index 5a80ab2..c433c9d 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -59,13 +59,18 @@ print "\ntesting vm.conf extraction\n";
 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}->{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}->{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

[pve-devel] [PATCH storage v3 02/10] plugin: dir: implement import content type

2024-04-29 Thread Dominik Csapak
in DirPlugin and not Plugin (because of cyclic dependency of
Plugin -> OVF -> Storage -> Plugin otherwise)

only ovf is currently supported (though ova will be shown in import
listing), expects the files to not be in a subdir, and adjacent to the
ovf file.

listed will be all ovf/qcow2/raw/vmdk files.
ovf because it can be imported, and the rest because they can be used
in the 'import-from' part of qemu-server.

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/OVF.pm |  3 +++
 src/PVE/Storage.pm |  8 +++
 src/PVE/Storage/DirPlugin.pm   | 36 +-
 src/PVE/Storage/Plugin.pm  | 11 -
 src/test/parse_volname_test.pm | 18 +++
 src/test/path_to_volume_id_test.pm | 21 +
 6 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 055ebf5..0eb5e9c 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -222,6 +222,8 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
}
 
($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
+   ($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$|; 
# untaint & check no sub/parent dirs
+   die "invalid path\n" if !$filepath;
 
my $virtual_size = PVE::Storage::file_size_info($backing_file_path);
die "error parsing $backing_file_path, cannot determine file size\n"
@@ -231,6 +233,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
disk_address => $pve_disk_address,
backing_file => $backing_file_path,
virtual_size => $virtual_size
+   relative_path => $filepath,
};
push @disks, $pve_disk;
 
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index f19a115..1ed91c2 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -114,6 +114,10 @@ our $VZTMPL_EXT_RE_1 = qr/\.tar\.(gz|xz|zst)/i;
 
 our $BACKUP_EXT_RE_2 = 
qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/;
 
+our $IMPORT_EXT_RE_1 = qr/\.(ovf|qcow2|raw|vmdk)/;
+
+our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
+
 # FIXME remove with PVE 8.0, add versioned breaks for pve-manager
 our $vztmpl_extension_re = $VZTMPL_EXT_RE_1;
 
@@ -612,6 +616,7 @@ sub path_to_volume_id {
my $backupdir = $plugin->get_subdir($scfg, 'backup');
my $privatedir = $plugin->get_subdir($scfg, 'rootdir');
my $snippetsdir = $plugin->get_subdir($scfg, 'snippets');
+   my $importdir = $plugin->get_subdir($scfg, 'import');
 
if ($path =~ m!^$imagedir/(\d+)/([^/\s]+)$!) {
my $vmid = $1;
@@ -640,6 +645,9 @@ sub path_to_volume_id {
} elsif ($path =~ m!^$snippetsdir/([^/]+)$!) {
my $name = $1;
return ('snippets', "$sid:snippets/$name");
+   } elsif ($path =~ 
m!^$importdir/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!) {
+   my $name = $1;
+   return ('import', "$sid:import/$name");
}
 }
 
diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm
index 2efa8d5..3e3b1e7 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -10,6 +10,7 @@ use IO::File;
 use POSIX;
 
 use PVE::Storage::Plugin;
+use PVE::GuestImport::OVF;
 use PVE::JSONSchema qw(get_standard_option);
 
 use base qw(PVE::Storage::Plugin);
@@ -22,7 +23,7 @@ sub type {
 
 sub plugindata {
 return {
-   content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup 
=> 1, snippets => 1, none => 1 },
+   content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup 
=> 1, snippets => 1, none => 1, import => 1 },
 { images => 1,  rootdir => 1 }],
format => [ { raw => 1, qcow2 => 1, vmdk => 1, subvol => 1 } , 'raw' ],
 };
@@ -247,4 +248,37 @@ sub check_config {
 return $opts;
 }
 
+sub get_import_metadata {
+my ($class, $scfg, $volname, $storeid) = @_;
+
+my ($vtype, $name, undef, undef, undef, undef, $fmt) = 
$class->parse_volname($volname);
+die "invalid content type '$vtype'\n" if $vtype ne 'import';
+die "invalid format\n" if $fmt ne 'ova' && $fmt ne 'ovf';
+
+# NOTE: all types of warnings must be added to the return schema of the 
import-metadata API endpoint
+my $warnings = [];
+
+my $path = $class->path($scfg, $volname, $storeid, undef);
+my $res = PVE::GuestImport::OVF::parse_ovf($path);
+my $disks = {};
+for my $disk ($res->{disks}->@*) {
+   my $id = $disk->{disk_address};
+   my $size = $disk->{virtual_size};
+   my $path = $disk->{relative_path};
+   $disks->{$id} = {
+   volid => "$storeid:import/$path",
+   defined($size) ? (size => $size) : (),
+   };
+}
+
+return {
+   type => 'vm',
+   source => $volname,
+   'create-args' => $res->{qm},
+   'disks' => $disks,
+   warnings => $warnings,

[pve-devel] [PATCH storage v3 06/10] ovf: implement rudimentary boot order

2024-04-29 Thread Dominik Csapak
simply add all parsed disks to the boot order in the order we encounter
them (similar to the esxi plugin).

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/OVF.pm | 6 +-
 src/test/run_ovf_tests.pl  | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 767590e..f0609de 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -245,6 +245,8 @@ sub parse_ovf {
 # when all the nodes has been found out, we copy the relevant information 
to
 # a $pve_disk hash ref, which we push to @disks;
 
+my $boot_order = [];
+
 foreach my $item_node (@disk_items) {
 
my $disk_node;
@@ -349,9 +351,11 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
};
$pve_disk->{virtual_size} = $virtual_size if defined($virtual_size);
push @disks, $pve_disk;
-
+   push @$boot_order, $pve_disk_address;
 }
 
+$qm->{boot} = "order=" . join(';', @$boot_order) if scalar(@$boot_order) > 
0;
+
 return {qm => $qm, disks => \@disks};
 }
 
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index e92258d..3b04100 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -56,17 +56,20 @@ is($win10noNs->{disks}->[0]->{virtual_size}, 2048, 'single 
disk vm (no default r
 
 print "\ntesting vm.conf extraction\n";
 
+is($win2008->{qm}->{boot}, 'order=scsi0;scsi1', '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}->{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}->{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');
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage v3 07/10] ovf: implement parsing nics

2024-04-29 Thread 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 e1000, since that is our default too, and should
work for most guest operating systems.

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/OVF.pm   | 23 ++-
 src/PVE/Storage/DirPlugin.pm |  2 +-
 src/test/run_ovf_tests.pl|  5 +
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index f0609de..d7e3ce4 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/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) {
@@ -356,7 +362,22 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
 
 $qm->{boot} = "order=" . join(';', @$boot_order) if scalar(@$boot_order) > 
0;
 
-return {qm => $qm, disks => \@disks};
+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;
+for my $item_node (@nic_items) {
+   my $model = $xpc->findvalue('rasd:ResourceSubType', $item_node);
+   $model = lc($model);
+   $model = 'e1000' if ! grep { $_ eq $model } @$allowed_nic_models;
+   $net->{"net${net_count}"} = { model => $model };
+   $net_count++;
+}
+
+return {qm => $qm, disks => \@disks, net => $net};
 }
 
 1;
diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm
index b98b603..6a6b5e9 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -293,7 +293,7 @@ sub get_import_metadata {
'create-args' => $res->{qm},
'disks' => $disks,
warnings => $warnings,
-   net => [],
+   net => $res->{net},
 };
 }
 
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index 3b04100..b8fa4b1 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');
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage v3 09/10] plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs

2024-04-29 Thread Dominik Csapak
and reuse the DirPlugin implementation

Signed-off-by: Dominik Csapak 
---
 src/PVE/Storage/BTRFSPlugin.pm | 5 +
 src/PVE/Storage/CIFSPlugin.pm  | 6 +-
 src/PVE/Storage/CephFSPlugin.pm| 6 +-
 src/PVE/Storage/GlusterfsPlugin.pm | 6 +-
 src/PVE/Storage/NFSPlugin.pm   | 6 +-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index 42815cb..b7e3f82 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -40,6 +40,7 @@ sub plugindata {
backup => 1,
snippets => 1,
none => 1,
+   import => 1,
},
{ images => 1, rootdir => 1 },
],
@@ -930,4 +931,8 @@ sub volume_import {
 return "$storeid:$volname";
 }
 
+sub get_import_metadata {
+return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1
diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
index 2184471..475065a 100644
--- a/src/PVE/Storage/CIFSPlugin.pm
+++ b/src/PVE/Storage/CIFSPlugin.pm
@@ -99,7 +99,7 @@ sub type {
 sub plugindata {
 return {
content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1,
-  backup => 1, snippets => 1}, { images => 1 }],
+  backup => 1, snippets => 1, import => 1}, { images => 1 }],
format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
 };
 }
@@ -314,4 +314,8 @@ sub update_volume_attribute {
 return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/CephFSPlugin.pm b/src/PVE/Storage/CephFSPlugin.pm
index 8aad518..36c64ea 100644
--- a/src/PVE/Storage/CephFSPlugin.pm
+++ b/src/PVE/Storage/CephFSPlugin.pm
@@ -116,7 +116,7 @@ sub type {
 
 sub plugindata {
 return {
-   content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1},
+   content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1, 
import => 1 },
 { backup => 1 }],
 };
 }
@@ -261,4 +261,8 @@ sub update_volume_attribute {
 return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/GlusterfsPlugin.pm 
b/src/PVE/Storage/GlusterfsPlugin.pm
index 2b7f9e1..9d17180 100644
--- a/src/PVE/Storage/GlusterfsPlugin.pm
+++ b/src/PVE/Storage/GlusterfsPlugin.pm
@@ -97,7 +97,7 @@ sub type {
 
 sub plugindata {
 return {
-   content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, 
snippets => 1},
+   content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, 
snippets => 1, import => 1},
 { images => 1 }],
format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
 };
@@ -352,4 +352,8 @@ sub check_connection {
 return defined($server) ? 1 : 0;
 }
 
+sub get_import_metadata {
+return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index f2e4c0d..72e9c6d 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -53,7 +53,7 @@ sub type {
 
 sub plugindata {
 return {
-   content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup 
=> 1, snippets => 1 },
+   content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup 
=> 1, snippets => 1, import => 1 },
 { images => 1 }],
format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
 };
@@ -223,4 +223,8 @@ sub update_volume_attribute {
 return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage v3 10/10] add 'import' content type to 'check_volume_access'

2024-04-29 Thread Dominik Csapak
in the same branch as 'vztmpl' and 'iso'

Signed-off-by: Dominik Csapak 
---
 src/PVE/Storage.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 31b2ad5..fe29842 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -540,7 +540,7 @@ sub check_volume_access {
 
return if $rpcenv->check($user, "/storage/$sid", 
['Datastore.Allocate'], 1);
 
-   if ($vtype eq 'iso' || $vtype eq 'vztmpl') {
+   if ($vtype eq 'iso' || $vtype eq 'vztmpl' || $vtype eq 'import') {
# require at least read access to storage, (custom) templates/ISOs 
could be sensitive
$rpcenv->check_any($user, "/storage/$sid", 
['Datastore.AllocateSpace', 'Datastore.Audit']);
} elsif (defined($ownervm) && defined($vmid) && ($ownervm == $vmid)) {
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 4/9] ui: enable upload/download/remove buttons for 'import' type storages

2024-04-29 Thread Dominik Csapak
but only for non esxi ones, since that does not allow
uploading/downloading there

Signed-off-by: Dominik Csapak 
---
 www/manager6/storage/Browser.js| 9 +++--
 www/manager6/window/UploadToStorage.js | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 2123141d..934ce706 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -28,7 +28,9 @@ Ext.define('PVE.storage.Browser', {
let res = storageInfo.data;
let plugin = res.plugintype;
 
-   me.items = plugin !== 'esxi' ? [
+   let isEsxi = plugin === 'esxi';
+
+   me.items = !isEsxi ? [
{
title: gettext('Summary'),
xtype: 'pveStorageSummary',
@@ -142,8 +144,11 @@ Ext.define('PVE.storage.Browser', {
iconCls: 'fa fa-desktop',
itemId: 'contentImport',
content: 'import',
-   useCustomRemoveButton: true, // hide default remove button
+   useCustomRemoveButton: isEsxi, // hide default remove 
button for esxi
showColumns: ['name', 'format'],
+   enableUploadButton: enableUpload && !isEsxi,
+   enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
+   useUploadButton: !isEsxi,
itemdblclick: (view, record) => 
createGuestImportWindow(record),
tbar: [
{
diff --git a/www/manager6/window/UploadToStorage.js 
b/www/manager6/window/UploadToStorage.js
index 3c5bba88..cdf548a8 100644
--- a/www/manager6/window/UploadToStorage.js
+++ b/www/manager6/window/UploadToStorage.js
@@ -9,6 +9,7 @@ Ext.define('PVE.window.UploadToStorage', {
 title: gettext('Upload'),
 
 acceptedExtensions: {
+   'import': ['.ova'],
iso: ['.img', '.iso'],
vztmpl: ['.tar.gz', '.tar.xz', '.tar.zst'],
 },
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 1/9] ui: fix special 'import' icon for non-esxi storages

2024-04-29 Thread Dominik Csapak
we only want to show that icon in the tree when the storage is solely
used for importing, not when it's just one of several content types.

Signed-off-by: Dominik Csapak 
---
 www/manager6/Utils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f5608944..1310b04d 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1244,7 +1244,7 @@ Ext.define('PVE.Utils', {
// templates
objType = 'template';
status = type;
-   } else if (type === 'storage' && record.content.indexOf('import') !== 
-1) {
+   } else if (type === 'storage' && record.content === 'import') {
return 'fa fa-cloud-download';
} else {
// everything else
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 2/9] ui: guest import: add ova-needs-extracting warning text

2024-04-29 Thread Dominik Csapak
Signed-off-by: Dominik Csapak 
---
 www/manager6/window/GuestImport.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/window/GuestImport.js 
b/www/manager6/window/GuestImport.js
index 4bedc211..76ba6dc8 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -937,6 +937,7 @@ Ext.define('PVE.window.GuestImport', {
gettext('EFI state cannot be imported, you may need to 
reconfigure the boot order (see {0})'),
'https://pve.proxmox.com/wiki/OVMF/UEFI_Boot_Entries";>OVMF/UEFI Boot 
Entries',
),
+   'ova-needs-extracting': gettext('Importing from an OVA requires 
extra space while extracting the contained disks into the import or selected 
storage.'),
};
 let message = warningsCatalogue[w.type];
if (!w.type || !message) {
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 3/9] ui: enable import content type for relevant storages

2024-04-29 Thread Dominik Csapak
Signed-off-by: Dominik Csapak 
---
 www/manager6/Utils.js| 1 +
 www/manager6/form/ContentTypeSelector.js | 2 +-
 www/manager6/storage/CephFSEdit.js   | 2 +-
 www/manager6/storage/GlusterFsEdit.js| 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 1310b04d..ff2fae25 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -690,6 +690,7 @@ Ext.define('PVE.Utils', {
'iso': gettext('ISO image'),
'rootdir': gettext('Container'),
'snippets': gettext('Snippets'),
+   'import': gettext('Import'),
 },
 
 volume_is_qemu_backup: function(volid, format) {
diff --git a/www/manager6/form/ContentTypeSelector.js 
b/www/manager6/form/ContentTypeSelector.js
index d0fa0b08..431bd948 100644
--- a/www/manager6/form/ContentTypeSelector.js
+++ b/www/manager6/form/ContentTypeSelector.js
@@ -10,7 +10,7 @@ Ext.define('PVE.form.ContentTypeSelector', {
me.comboItems = [];
 
if (me.cts === undefined) {
-   me.cts = ['images', 'iso', 'vztmpl', 'backup', 'rootdir', 
'snippets'];
+   me.cts = ['images', 'iso', 'vztmpl', 'backup', 'rootdir', 
'snippets', 'import'];
}
 
Ext.Array.each(me.cts, function(ct) {
diff --git a/www/manager6/storage/CephFSEdit.js 
b/www/manager6/storage/CephFSEdit.js
index 6a95a00a..2cdcf7cd 100644
--- a/www/manager6/storage/CephFSEdit.js
+++ b/www/manager6/storage/CephFSEdit.js
@@ -92,7 +92,7 @@ Ext.define('PVE.storage.CephFSInputPanel', {
me.column2 = [
{
xtype: 'pveContentTypeSelector',
-   cts: ['backup', 'iso', 'vztmpl', 'snippets'],
+   cts: ['backup', 'iso', 'vztmpl', 'snippets', 'import'],
fieldLabel: gettext('Content'),
name: 'content',
value: 'backup',
diff --git a/www/manager6/storage/GlusterFsEdit.js 
b/www/manager6/storage/GlusterFsEdit.js
index 8155d9c2..df7fe23f 100644
--- a/www/manager6/storage/GlusterFsEdit.js
+++ b/www/manager6/storage/GlusterFsEdit.js
@@ -99,7 +99,7 @@ Ext.define('PVE.storage.GlusterFsInputPanel', {
},
{
xtype: 'pveContentTypeSelector',
-   cts: ['images', 'iso', 'backup', 'vztmpl', 'snippets'],
+   cts: ['images', 'iso', 'backup', 'vztmpl', 'snippets', 
'import'],
name: 'content',
value: 'images',
multiSelect: true,
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 7/9] ui: guest import: add storage selector for ova extraction storage

2024-04-29 Thread Dominik Csapak
but only when we detect the 'ova-needs-extraction' warning.
This can be used to select the storage where the disks contained in an
OVA will be extracted to temporarily.

Signed-off-by: Dominik Csapak 
---
 www/manager6/window/GuestImport.js | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/www/manager6/window/GuestImport.js 
b/www/manager6/window/GuestImport.js
index 76ba6dc8..972f715b 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -303,6 +303,7 @@ Ext.define('PVE.window.GuestImport', {
os: 'l26',
maxCdDrives: false,
uniqueMACAdresses: false,
+   isOva: false,
warnings: [],
},
 
@@ -432,6 +433,10 @@ Ext.define('PVE.window.GuestImport', {
}
}
 
+   if (config['import-extraction-storage'] === '') {
+   delete config['import-extraction-storage'];
+   }
+
return config;
},
 
@@ -553,6 +558,22 @@ Ext.define('PVE.window.GuestImport', {
allowBlank: false,
fieldLabel: gettext('Default Bridge'),
},
+   {
+   xtype: 'pveStorageSelector',
+   reference: 'extractionStorage',
+   fieldLabel: gettext('Extraction Storage'),
+   storageContent: 'images',
+   emptyText: gettext('Import Storage'),
+   autoSelect: false,
+   name: 'import-extraction-storage',
+   disabled: true,
+   hidden: true,
+   allowBlank: true,
+   bind: {
+   disabled: '{!isOva}',
+   hidden: '{!isOva}',
+   },
+   },
],
 
columnB: [
@@ -925,6 +946,7 @@ Ext.define('PVE.window.GuestImport', {
 
me.lookup('defaultStorage').setNodename(me.nodename);
me.lookup('defaultBridge').setNodename(me.nodename);
+   me.lookup('extractionStorage').setNodename(me.nodename);
 
let renderWarning = w => {
const warningsCatalogue = {
@@ -1006,6 +1028,7 @@ Ext.define('PVE.window.GuestImport', {
}
 
me.getViewModel().set('warnings', data.warnings.map(w => 
renderWarning(w)));
+   me.getViewModel().set('isOva', data.warnings.map(w => 
w.type).indexOf('ova-needs-extracting') !== -1);
 
let osinfo = PVE.Utils.get_kvm_osinfo(me.vmConfig.ostype ?? '');
let prepareForVirtIO = (me.vmConfig.ostype ?? 
'').startsWith('w') && (me.vmConfig.bios ?? '').indexOf('ovmf') !== -1;
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH qemu-server v3 2/4] use OVF from Storage

2024-04-29 Thread Dominik Csapak
and delete it here (incl tests; they live in pve-storage now).

Signed-off-by: Dominik Csapak 
---
 PVE/CLI/qm.pm |   4 +-
 PVE/QemuServer/Makefile   |   1 -
 PVE/QemuServer/OVF.pm | 242 --
 test/Makefile |   5 +-
 test/ovf_manifests/Win10-Liz-disk1.vmdk   | Bin 65536 -> 0 bytes
 test/ovf_manifests/Win10-Liz.ovf  | 142 --
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 --
 test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 ---
 test/ovf_manifests/disk1.vmdk | Bin 65536 -> 0 bytes
 test/ovf_manifests/disk2.vmdk | Bin 65536 -> 0 bytes
 test/run_ovf_tests.pl |  71 -
 11 files changed, 3 insertions(+), 749 deletions(-)
 delete mode 100644 PVE/QemuServer/OVF.pm
 delete mode 100644 test/ovf_manifests/Win10-Liz-disk1.vmdk
 delete mode 100755 test/ovf_manifests/Win10-Liz.ovf
 delete mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf
 delete mode 100755 test/ovf_manifests/Win_2008_R2_two-disks.ovf
 delete mode 100644 test/ovf_manifests/disk1.vmdk
 delete mode 100644 test/ovf_manifests/disk2.vmdk
 delete mode 100755 test/run_ovf_tests.pl

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index b105830f..2b85d072 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -28,13 +28,13 @@ use PVE::Tools qw(extract_param file_get_contents);
 
 use PVE::API2::Qemu::Agent;
 use PVE::API2::Qemu;
+use PVE::GuestImport::OVF;
 use PVE::QemuConfig;
 use PVE::QemuServer::Drive;
 use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Agent qw(agent_available);
 use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
-use PVE::QemuServer::OVF;
 use PVE::QemuServer;
 
 use PVE::CLIHandler;
@@ -729,7 +729,7 @@ __PACKAGE__->register_method ({
my $storecfg = PVE::Storage::config();
PVE::Storage::storage_check_enabled($storecfg, $storeid);
 
-   my $parsed = PVE::QemuServer::OVF::parse_ovf($ovf_file);
+   my $parsed = PVE::GuestImport::OVF::parse_ovf($ovf_file);
 
if ($dryrun) {
print to_json($parsed, { pretty => 1, canonical => 1});
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index ac26e56f..89d12091 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -2,7 +2,6 @@ SOURCES=PCI.pm  \
USB.pm  \
Memory.pm   \
ImportDisk.pm   \
-   OVF.pm  \
Cloudinit.pm\
Agent.pm\
Helpers.pm  \
diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
deleted file mode 100644
index b97b0520..
--- a/PVE/QemuServer/OVF.pm
+++ /dev/null
@@ -1,242 +0,0 @@
-# Open Virtualization Format import routines
-# https://www.dmtf.org/standards/ovf
-package PVE::QemuServer::OVF;
-
-use strict;
-use warnings;
-
-use XML::LibXML;
-use File::Spec;
-use File::Basename;
-use Data::Dumper;
-use Cwd 'realpath';
-
-use PVE::Tools;
-use PVE::Storage;
-
-# map OVF resources types to descriptive strings
-# this will allow us to explore the xml tree without using magic numbers
-# 
http://schemas.dmtf.org/wbem/cim-html/2/CIM_ResourceAllocationSettingData.html
-my @resources = (
-{ id => 1, dtmf_name => 'Other' },
-{ id => 2, dtmf_name => 'Computer System' },
-{ id => 3, dtmf_name => 'Processor' },
-{ id => 4, dtmf_name => 'Memory' },
-{ id => 5, dtmf_name => 'IDE Controller', pve_type => 'ide' },
-{ id => 6, dtmf_name => 'Parallel SCSI HBA', pve_type => 'scsi' },
-{ id => 7, dtmf_name => 'FC HBA' },
-{ id => 8, dtmf_name => 'iSCSI HBA' },
-{ id => 9, dtmf_name => 'IB HCA' },
-{ id => 10, dtmf_name => 'Ethernet Adapter' },
-{ id => 11, dtmf_name => 'Other Network Adapter' },
-{ id => 12, dtmf_name => 'I/O Slot' },
-{ id => 13, dtmf_name => 'I/O Device' },
-{ id => 14, dtmf_name => 'Floppy Drive' },
-{ id => 15, dtmf_name => 'CD Drive' },
-{ id => 16, dtmf_name => 'DVD drive' },
-{ id => 17, dtmf_name => 'Disk Drive' },
-{ id => 18, dtmf_name => 'Tape Drive' },
-{ id => 19, dtmf_name => 'Storage Extent' },
-{ id => 20, dtmf_name => 'Other storage device', pve_type => 'sata'},
-{ id => 21, dtmf_name => 'Serial port' },
-{ id => 22, dtmf_name => 'Parallel port' },
-{ id => 23, dtmf_name => 'USB Controller' },
-{ id => 24, dtmf_name => 'Graphics controller' },
-{ id => 25, dtmf_name => 'IEEE 1394 Controller' },
-{ id => 26, dtmf_name => 'Partitionable Unit' },
-{ id => 27, dtmf_name => 'Base Partitionable Unit' },
-{ id => 28, dtmf_name => 'Power' },
-{ id => 29, dtmf_name => 'Cooling Capacity' },
-{ id => 30, dtmf_name => 'Ethernet Switch Port' },
-{ id => 31, dtmf_name => 'Logical Disk' },
-{ id => 32, dtmf_name => 'Storage Volume' },
-{ id => 33, dtmf_name => 'Ethernet Connection' },
-{ id => 34, dtmf_name => 'DMTF reserved' },
-   

[pve-devel] [PATCH storage/qemu-server/manager v3] implement ova/ovf import for file based storages

2024-04-29 Thread Dominik Csapak
This series enables importing ova/ovf from directory based storages,
inclusive upload/download via the webui (ova only).

It also improves the ovf importer by parsing the ostype, nics, bootorder
(and firmware from vmware exported files).

I opted to move the OVF.pm to pve-storage, since there is no
real other place where we could put it. I put it in a new module
'GuestImport'

We now extract the images into either a given target storage or in the
import storage in the 'images' dir so accidentally left over images
are discoverable by the ui/cli.

changes from v2:
* use better 'format' values for embedded images (e.g. ova+vmdk)
* use this format to decide if images should be extracted
* consistent use of the 'safe character' classes when listing
  and parsing
* also list vmdk/qcow2/raw images in content listing
  (this will be useful when we have a gui for the 'import-from'
  in the wizard/disk edit for vms)
* a few gui adaptions


changes from v1:
* move ovf code to GuestImport
* move extract/checking code to GuestImport
* don't return 'image' types from import volumes
* use allow 'safe' characters for filenames of ova/ovfs and inside
* check for non-regular files (e.g. symlinks) after extraction
* add new 'import-extraction-storage' for import
* rename panel in gui for directory storages
* typo fixes
* and probably more, see the individual patches for details

pve-storage:

Dominik Csapak (10):
  copy OVF.pm from qemu-server
  plugin: dir: implement import content type
  plugin: dir: handle ova files for import
  ovf: implement parsing the ostype
  ovf: implement parsing out firmware type
  ovf: implement rudimentary boot order
  ovf: implement parsing nics
  api: allow ova upload/download
  plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs
  add 'import' content type to 'check_volume_access'

 src/PVE/API2/Storage/Status.pm|  19 +-
 src/PVE/GuestImport.pm| 100 +
 src/PVE/GuestImport/Makefile  |   3 +
 src/PVE/GuestImport/OVF.pm| 383 ++
 src/PVE/Makefile  |   2 +
 src/PVE/Storage.pm|  21 +-
 src/PVE/Storage/BTRFSPlugin.pm|   5 +
 src/PVE/Storage/CIFSPlugin.pm |   6 +-
 src/PVE/Storage/CephFSPlugin.pm   |   6 +-
 src/PVE/Storage/DirPlugin.pm  |  52 ++-
 src/PVE/Storage/GlusterfsPlugin.pm|   6 +-
 src/PVE/Storage/Makefile  |   1 +
 src/PVE/Storage/NFSPlugin.pm  |   6 +-
 src/PVE/Storage/Plugin.pm |  16 +-
 src/test/Makefile |   5 +-
 src/test/ovf_manifests/Win10-Liz-disk1.vmdk   | Bin 0 -> 65536 bytes
 src/test/ovf_manifests/Win10-Liz.ovf  | 142 +++
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 143 +++
 .../ovf_manifests/Win_2008_R2_two-disks.ovf   | 145 +++
 src/test/ovf_manifests/disk1.vmdk | Bin 0 -> 65536 bytes
 src/test/ovf_manifests/disk2.vmdk | Bin 0 -> 65536 bytes
 src/test/parse_volname_test.pm|  33 ++
 src/test/path_to_volume_id_test.pm|  21 +
 src/test/run_ovf_tests.pl |  85 
 24 files changed, 1188 insertions(+), 12 deletions(-)
 create mode 100644 src/PVE/GuestImport.pm
 create mode 100644 src/PVE/GuestImport/Makefile
 create mode 100644 src/PVE/GuestImport/OVF.pm
 create mode 100644 src/test/ovf_manifests/Win10-Liz-disk1.vmdk
 create mode 100755 src/test/ovf_manifests/Win10-Liz.ovf
 create mode 100755 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
 create mode 100755 src/test/ovf_manifests/Win_2008_R2_two-disks.ovf
 create mode 100644 src/test/ovf_manifests/disk1.vmdk
 create mode 100644 src/test/ovf_manifests/disk2.vmdk
 create mode 100755 src/test/run_ovf_tests.pl

qemu-server:

Dominik Csapak (4):
  api: delete unused OVF.pm
  use OVF from Storage
  api: create: implement extracting disks when needed for import-from
  api: create: add 'import-extraction-storage' parameter

 PVE/API2/Qemu.pm  |  92 ++-
 PVE/API2/Qemu/Makefile|   2 +-
 PVE/API2/Qemu/OVF.pm  |  53 
 PVE/CLI/qm.pm |   4 +-
 PVE/QemuServer.pm |   5 +-
 PVE/QemuServer/Helpers.pm |  10 +
 PVE/QemuServer/Makefile   |   1 -
 PVE/QemuServer/OVF.pm | 242 --
 test/Makefile |   5 +-
 test/ovf_manifests/Win10-Liz-disk1.vmdk   | Bin 65536 -> 0 bytes
 test/ovf_manifests/Win10-Liz.ovf  | 142 --
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 --
 test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 ---
 test/ovf_manifests/disk1.vmdk | Bin 65536 -> 0 bytes
 test/ovf_manifests/disk2.vmdk | Bin 65536 -> 0 byt

[pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import

2024-04-29 Thread Dominik Csapak
since we want to handle ova files (which are only ovf+images bundled in
a tar file) for import, add code that handles that.

we introduce a valid volname for files contained in ovas like this:

 storage:import/archive.ova/disk-1.vmdk

by basically treating the last part of the path as the name for the
contained disk we want.

in that case we return 'import' as type with 'vmdk/qcow2/raw' as format
(we cannot use something like 'ova+vmdk' without extending the 'format'
parsing to that for all storages/formats. This is because it runs
though a verify format check at least once)

we then provide 3 functions to use for that:

* copy_needs_extraction: determines from the given volid (like above) if
  that needs extraction to copy it, currently only 'import' vtype + a
  volid with the above format returns true

* extract_disk_from_import_file: this actually extracts the file from
  the archive. Currently only ova is supported, so the extraction with
  'tar' is hardcoded, but again we can easily extend/modify that should
  we need to.

  we currently extract into the either the import storage or a given
  target storage in the images directory so if the cleanup does not
  happen, the user can still see and interact with the image via
  api/cli/gui

* cleanup_extracted_image: intended to cleanup the extracted images from
  above

we have to modify the `parse_ovf` a bit to handle the missing disk
images, and we parse the size out of the ovf part (since this is
informal only, it should be no problem if we cannot parse it sometimes)

Signed-off-by: Dominik Csapak 
---
 src/PVE/API2/Storage/Status.pm |   1 +
 src/PVE/GuestImport.pm | 100 +
 src/PVE/GuestImport/OVF.pm |  53 ++---
 src/PVE/Makefile   |   1 +
 src/PVE/Storage.pm |   2 +-
 src/PVE/Storage/DirPlugin.pm   |  15 -
 src/PVE/Storage/Plugin.pm  |   5 ++
 src/test/parse_volname_test.pm |  15 +
 8 files changed, 182 insertions(+), 10 deletions(-)
 create mode 100644 src/PVE/GuestImport.pm

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index dc6cc69..acde730 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -749,6 +749,7 @@ __PACKAGE__->register_method({
'efi-state-lost',
'guest-is-running',
'nvme-unsupported',
+   'ova-needs-extracting',
'ovmf-with-lsi-unsupported',
'serial-port-socket-only',
],
diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm
new file mode 100644
index 000..d405e30
--- /dev/null
+++ b/src/PVE/GuestImport.pm
@@ -0,0 +1,100 @@
+package PVE::GuestImport;
+
+use strict;
+use warnings;
+
+use File::Path;
+
+use PVE::Storage;
+use PVE::Tools qw(run_command);
+
+sub copy_needs_extraction {
+my ($volid) = @_;
+my $cfg = PVE::Storage::config();
+my ($vtype, $name, undef, undef, undef, undef, $fmt) = 
PVE::Storage::parse_volname($cfg, $volid);
+
+# only volumes inside ovas need extraction
+return $vtype eq 'import' && $fmt =~ m/^ova\+(.*)$/;
+}
+
+sub extract_disk_from_import_file {
+my ($volid, $vmid, $target_storeid) = @_;
+
+my ($source_storeid, $volname) = PVE::Storage::parse_volume_id($volid);
+$target_storeid //= $source_storeid;
+my $cfg = PVE::Storage::config();
+my $source_scfg = PVE::Storage::storage_config($cfg, $source_storeid);
+my $source_plugin = PVE::Storage::Plugin->lookup($source_scfg->{type});
+
+my ($vtype, $name, undef, undef, undef, undef, $fmt) =
+   $source_plugin->parse_volname($volname);
+
+die "only files with content type 'import' can be extracted\n"
+   if $vtype ne 'import' || $fmt !~ m/^ova\+/;
+
+# extract the inner file from the name
+my $archive;
+my $inner_file;
+if ($name =~ m!^(.*\.ova)/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$!) {
+   $archive = "import/$1";
+   $inner_file = $2;
+   ($fmt) = $fmt =~ /^ova\+(.*)$/;
+} else {
+   die "cannot extract $volid - invalid volname $volname\n";
+}
+
+my ($ova_path) = $source_plugin->path($source_scfg, $archive, 
$source_storeid);
+
+my $target_scfg = PVE::Storage::storage_config($cfg, $target_storeid);
+my $target_plugin = PVE::Storage::Plugin->lookup($target_scfg->{type});
+
+my $destdir = $target_plugin->get_subdir($target_scfg, 'images');
+
+my $pid = $$;
+$destdir .= "/tmp_${pid}_${vmid}";
+mkpath $destdir;
+
+($ova_path) = $ova_path =~ m|^(.*)$|; # untaint
+
+my $source_path = "$destdir/$inner_file";
+my $target_path;
+my $target_volname;
+eval {
+   run_command(['tar', '-x', '--force-local', '-C', $destdir, '-f', 
$ova_path, $inner_file]);
+
+   # check for symlinks and other non regular files
+   if (-l $source_path || ! -f $source_pa

[pve-devel] [PATCH storage v3 05/10] ovf: implement parsing out firmware type

2024-04-29 Thread Dominik Csapak
it seems there is no part of the ovf standard that handles which type of
bios there is (at least i could not find it). Every ovf/ova i tested
either has no info about it, or has it in a vmware specific property
which we parse here.

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/OVF.pm | 5 +
 src/PVE/Storage/DirPlugin.pm   | 5 +
 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf | 1 +
 src/test/run_ovf_tests.pl  | 1 +
 4 files changed, 12 insertions(+)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index cf08cb6..767590e 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -229,6 +229,11 @@ sub parse_ovf {
 my $ostype_id = $xpc->findvalue($xpath_find_ostype_id);
 $qm->{ostype} = get_ostype($ostype_id);
 
+# vmware specific firmware config, seems to not be standardized in ovf ?
+my $xpath_find_firmware = 
"/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/vmw:Config[\@vmw:key=\"firmware\"]/\@vmw:value";
+my $firmware = $xpc->findvalue($xpath_find_firmware) || 'seabios';
+$qm->{bios} = 'ovmf' if $firmware eq 'efi';
+
 # disks metadata is split in four different xml elements:
 # * as an Item node of type DiskDrive in the VirtualHardwareSection
 # * as an Disk node in the DiskSection
diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm
index ea89464..b98b603 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -282,6 +282,11 @@ sub get_import_metadata {
};
 }
 
+if (defined($res->{qm}->{bios}) && $res->{qm}->{bios} eq 'ovmf') {
+   $disks->{efidisk0} = 1;
+   push @$warnings, { type => 'efi-state-lost', key => 'bios', value => 
'ovmf' };
+}
+
 return {
type => 'vm',
source => $volname,
diff --git a/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf 
b/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
index b93540f..10ccaf1 100755
--- a/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
+++ b/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
@@ -137,6 +137,7 @@
   
   
   
+  
 
   
 
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index c433c9d..e92258d 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -72,5 +72,6 @@ is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default 
rasd NS) memory is
 is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are 
correct');
 # older esxi/ovf standard used 'other' for windows10
 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');
 
 done_testing();
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage v3 08/10] api: allow ova upload/download

2024-04-29 Thread Dominik Csapak
introducing a separate regex that only contains ova, since
upload/downloading ovfs does not make sense (since the disks are then
missing).

Signed-off-by: Dominik Csapak 
---
 src/PVE/API2/Storage/Status.pm | 18 ++
 src/PVE/Storage.pm | 11 +++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index acde730..6c0c1e5 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -369,7 +369,7 @@ __PACKAGE__->register_method ({
 name => 'upload',
 path => '{storage}/upload',
 method => 'POST',
-description => "Upload templates and ISO images.",
+description => "Upload templates, ISO images and OVAs.",
 permissions => {
check => ['perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
 },
@@ -382,7 +382,7 @@ __PACKAGE__->register_method ({
content => {
description => "Content type.",
type => 'string', format => 'pve-storage-content',
-   enum => ['iso', 'vztmpl'],
+   enum => ['iso', 'vztmpl', 'import'],
},
filename => {
description => "The name of the file to create. Caution: This 
will be normalized!",
@@ -448,6 +448,11 @@ __PACKAGE__->register_method ({
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
+   } elsif ($content eq 'import') {
+   if ($filename !~ m![^/]+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!) {
+   raise_param_exc({ filename => "wrong file extension" });
+   }
+   $path = PVE::Storage::get_import_dir($cfg, $param->{storage});
} else {
raise_param_exc({ content => "upload content type '$content' not 
allowed" });
}
@@ -544,7 +549,7 @@ __PACKAGE__->register_method({
 name => 'download_url',
 path => '{storage}/download-url',
 method => 'POST',
-description => "Download templates and ISO images by using an URL.",
+description => "Download templates, ISO images and OVAs by using an URL.",
 proxyto => 'node',
 permissions => {
description => 'Requires allocation access on the storage and as this 
allows one to probe'
@@ -572,7 +577,7 @@ __PACKAGE__->register_method({
content => {
description => "Content type.", # TODO: could be optional & 
detected in most cases
type => 'string', format => 'pve-storage-content',
-   enum => ['iso', 'vztmpl'],
+   enum => ['iso', 'vztmpl', 'import'],
},
filename => {
description => "The name of the file to create. Caution: This 
will be normalized!",
@@ -642,6 +647,11 @@ __PACKAGE__->register_method({
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
+   } elsif ($content eq 'import') {
+   if ($filename !~ m![^/]+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!) {
+   raise_param_exc({ filename => "wrong file extension" });
+   }
+   $path = PVE::Storage::get_import_dir($cfg, $param->{storage});
} else {
raise_param_exc({ content => "upload content-type '$content' is not 
allowed" });
}
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index adc1b45..31b2ad5 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -116,6 +116,8 @@ our $BACKUP_EXT_RE_2 = 
qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
 
 our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
 
+our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova)/;
+
 our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
 
 # FIXME remove with PVE 8.0, add versioned breaks for pve-manager
@@ -464,6 +466,15 @@ sub get_iso_dir {
 return $plugin->get_subdir($scfg, 'iso');
 }
 
+sub get_import_dir {
+my ($cfg, $storeid) = @_;
+
+my $scfg = storage_config($cfg, $storeid);
+my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+return $plugin->get_subdir($scfg, 'import');
+}
+
 sub get_vztmpl_dir {
 my ($cfg, $storeid) = @_;
 
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH qemu-server v3 1/4] api: delete unused OVF.pm

2024-04-29 Thread Dominik Csapak
the api part was never in use by anything

Signed-off-by: Dominik Csapak 
---
 PVE/API2/Qemu/Makefile |  2 +-
 PVE/API2/Qemu/OVF.pm   | 53 --
 2 files changed, 1 insertion(+), 54 deletions(-)
 delete mode 100644 PVE/API2/Qemu/OVF.pm

diff --git a/PVE/API2/Qemu/Makefile b/PVE/API2/Qemu/Makefile
index bdd4762b..5d4abda6 100644
--- a/PVE/API2/Qemu/Makefile
+++ b/PVE/API2/Qemu/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Agent.pm CPU.pm Machine.pm OVF.pm
+SOURCES=Agent.pm CPU.pm Machine.pm
 
 .PHONY: install
 install:
diff --git a/PVE/API2/Qemu/OVF.pm b/PVE/API2/Qemu/OVF.pm
deleted file mode 100644
index cc0ef2da..
--- a/PVE/API2/Qemu/OVF.pm
+++ /dev/null
@@ -1,53 +0,0 @@
-package PVE::API2::Qemu::OVF;
-
-use strict;
-use warnings;
-
-use PVE::JSONSchema qw(get_standard_option);
-use PVE::QemuServer::OVF;
-use PVE::RESTHandler;
-
-use base qw(PVE::RESTHandler);
-
-__PACKAGE__->register_method ({
-name => 'readovf',
-path => '',
-method => 'GET',
-proxyto => 'node',
-description => "Read an .ovf manifest.",
-protected => 1,
-parameters => {
-   additionalProperties => 0,
-   properties => {
-   node => get_standard_option('pve-node'),
-   manifest => {
-   description => "Path to .ovf manifest.",
-   type => 'string',
-   },
-   },
-},
-returns => {
-   type => 'object',
-   additionalProperties => 1,
-   properties => PVE::QemuServer::json_ovf_properties(),
-   description => "VM config according to .ovf manifest.",
-},
-code => sub {
-   my ($param) = @_;
-
-   my $manifest = $param->{manifest};
-   die "check for file $manifest failed - $!\n" if !-f $manifest;
-
-   my $parsed = PVE::QemuServer::OVF::parse_ovf($manifest);
-   my $result;
-   $result->{cores} = $parsed->{qm}->{cores};
-   $result->{name} =  $parsed->{qm}->{name};
-   $result->{memory} = $parsed->{qm}->{memory};
-   my $disks = $parsed->{disks};
-   for my $disk (@$disks) {
-   $result->{$disk->{disk_address}} = $disk->{backing_file};
-   }
-   return $result;
-}});
-
-1;
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 5/9] ui: disable 'import' button for non importable formats

2024-04-29 Thread Dominik Csapak
importable formats are currently ova/ovf/vmx

Signed-off-by: Dominik Csapak 
---
 www/manager6/storage/Browser.js | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 934ce706..822257e7 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -124,6 +124,7 @@ Ext.define('PVE.storage.Browser', {
});
}
if (contents.includes('import')) {
+   let isImportable = format => ['ova', 'ovf', 
'vmx'].indexOf(format) !== -1;
let createGuestImportWindow = (selection) => {
if (!selection) {
return;
@@ -149,13 +150,18 @@ Ext.define('PVE.storage.Browser', {
enableUploadButton: enableUpload && !isEsxi,
enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
useUploadButton: !isEsxi,
-   itemdblclick: (view, record) => 
createGuestImportWindow(record),
+   itemdblclick: (view, record) => {
+   if (isImportable(record.data.format)) {
+   createGuestImportWindow(record);
+   }
+   },
tbar: [
{
xtype: 'proxmoxButton',
disabled: true,
text: gettext('Import'),
iconCls: 'fa fa-cloud-download',
+   enableFn: rec => isImportable(rec.data.format),
handler: function() {
let grid = this.up('pveStorageContentView');
let selection = grid.getSelection()?.[0];
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 8/9] ui: guest import: change icon/text for non-esxi import storage

2024-04-29 Thread Dominik Csapak
since 'virtual guests' only make sense for a hypervisor, not e.g. a
directory for OVAs

also change the icon from 'desktop' to 'cloud-download' in the
non-esxi case

Signed-off-by: Dominik Csapak 
---
 www/manager6/storage/Browser.js | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 822257e7..763abc70 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -141,8 +141,10 @@ Ext.define('PVE.storage.Browser', {
};
me.items.push({
xtype: 'pveStorageContentView',
-   title: gettext('Virtual Guests'),
-   iconCls: 'fa fa-desktop',
+   // each gettext needs to be in a separate line
+   title: isEsxi ? gettext('Virtual Guests')
+   : gettext('Import'),
+   iconCls: isEsxi ? 'fa fa-desktop' : 'fa fa-cloud-download',
itemId: 'contentImport',
content: 'import',
useCustomRemoveButton: isEsxi, // hide default remove 
button for esxi
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH qemu-server v3 3/4] api: create: implement extracting disks when needed for import-from

2024-04-29 Thread Dominik Csapak
when 'import-from' contains a disk image that needs extraction
(currently only from an 'ova' archive), do that in 'create_disks'
and overwrite the '$source' volid.

Collect the names into a 'delete_sources' list, that we use later
to clean it up again (either when we're finished with importing or in an
error case).

Signed-off-by: Dominik Csapak 
---
 PVE/API2/Qemu.pm  | 44 ++-
 PVE/QemuServer.pm |  5 -
 PVE/QemuServer/Helpers.pm | 10 +
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2a349c8c..d32967dc 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -24,6 +24,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 use PVE::ReplicationConfig;
 use PVE::GuestHelpers qw(assert_tag_permissions);
+use PVE::GuestImport;
 use PVE::QemuConfig;
 use PVE::QemuServer;
 use PVE::QemuServer::Cloudinit;
@@ -159,10 +160,19 @@ my $check_storage_access = sub {
 
if (my $src_image = $drive->{'import-from'}) {
my $src_vmid;
-   if (PVE::Storage::parse_volume_id($src_image, 1)) { # PVE-managed 
volume
-   (my $vtype, undef, $src_vmid) = 
PVE::Storage::parse_volname($storecfg, $src_image);
-   raise_param_exc({ $ds => "$src_image has wrong type '$vtype' - 
not an image" })
-   if $vtype ne 'images';
+   if (my ($storeid, $volname) = 
PVE::Storage::parse_volume_id($src_image, 1)) { # PVE-managed volume
+   my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+   my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+   (my $vtype, undef, $src_vmid) = 
$plugin->parse_volname($volname);
+
+   raise_param_exc({ $ds => "$src_image has wrong type '$vtype' - 
needs to be 'images' or 'import'" })
+   if $vtype ne 'images' && $vtype ne 'import';
+
+   if (PVE::GuestImport::copy_needs_extraction($src_image)) {
+   raise_param_exc({ $ds => "$src_image is not on an storage 
with 'images' content type."})
+   if !$scfg->{content}->{images};
+   $rpcenv->check($authuser, "/storage/$storeid", 
['Datastore.AllocateSpace']);
+   }
}
 
if ($src_vmid) { # might be actively used by VM and will be copied 
via clone_disk()
@@ -335,6 +345,7 @@ my sub create_disks : prototype($$) {
 my $res = {};
 
 my $live_import_mapping = {};
+my $delete_sources = [];
 
 my $code = sub {
my ($ds, $disk) = @_;
@@ -392,6 +403,12 @@ my sub create_disks : prototype($$) {
$needs_creation = $live_import;
 
if (PVE::Storage::parse_volume_id($source, 1)) { # PVE-managed 
volume
+   if (PVE::GuestImport::copy_needs_extraction($source)) { # 
needs extraction beforehand
+   print "extracting $source\n";
+   $source = 
PVE::GuestImport::extract_disk_from_import_file($source, $vmid);
+   print "finished extracting to $source\n";
+   push @$delete_sources, $source;
+   }
if ($live_import && $ds ne 'efidisk0') {
my $path = PVE::Storage::path($storecfg, $source)
or die "failed to get a path for '$source'\n";
@@ -514,13 +531,14 @@ my sub create_disks : prototype($$) {
eval { PVE::Storage::vdisk_free($storecfg, $volid); };
warn $@ if $@;
}
+   PVE::QemuServer::Helpers::cleanup_extracted_images($delete_sources);
die $err;
 }
 
 # don't return empty import mappings
 $live_import_mapping = undef if !%$live_import_mapping;
 
-return ($vollist, $res, $live_import_mapping);
+return ($vollist, $res, $live_import_mapping, $delete_sources);
 };
 
 my $check_cpu_model_access = sub {
@@ -1079,6 +1097,7 @@ __PACKAGE__->register_method({
 
my $createfn = sub {
my $live_import_mapping = {};
+   my $delete_sources = [];
 
# ensure no old replication state are exists
PVE::ReplicationState::delete_guest_states($vmid);
@@ -1096,7 +1115,7 @@ __PACKAGE__->register_method({
 
my $vollist = [];
eval {
-   ($vollist, my $created_opts, $live_import_mapping) = 
create_disks(
+   ($vollist, my $created_opts, $live_import_mapping, 
$delete_sources) = create_disks(
$rpcenv,
$authuser,
$conf,
@@ -1149,6 +1168,7 @@ __PACKAGE__->register_method({
eval { PVE::Storage::vdisk_free($storecfg, $volid); };
warn $@ if $@;
}
+   
PVE::QemuServer::Helpers::cleanup_extracted_images($delete_sources);
die "$emsg $err";
 

[pve-devel] [PATCH qemu-server v3 4/4] api: create: add 'import-extraction-storage' parameter

2024-04-29 Thread Dominik Csapak
this is to override the target extraction storage for the option disk
extraction for 'import-from'. This way if the storage does not
supports the content type 'images', one can give an alternative  one.

Signed-off-by: Dominik Csapak 
---
 PVE/API2/Qemu.pm | 56 +---
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index d32967dc..74d0e240 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -128,7 +128,9 @@ my $check_drive_param = sub {
 };
 
 my $check_storage_access = sub {
-   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
+   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage, 
$extraction_storage) = @_;
+
+   my $needs_extraction = 0;
 
$foreach_volume_with_alloc->($settings, sub {
my ($ds, $drive) = @_;
@@ -169,9 +171,13 @@ my $check_storage_access = sub {
if $vtype ne 'images' && $vtype ne 'import';
 
if (PVE::GuestImport::copy_needs_extraction($src_image)) {
-   raise_param_exc({ $ds => "$src_image is not on an storage 
with 'images' content type."})
-   if !$scfg->{content}->{images};
-   $rpcenv->check($authuser, "/storage/$storeid", 
['Datastore.AllocateSpace']);
+   $needs_extraction = 1;
+   if (!defined($extraction_storage)) {
+   raise_param_exc({ $ds => "$src_image is not on an 
storage with 'images'"
+   ." content type and no 'import-extraction-storage' 
was given."})
+   if !$scfg->{content}->{images};
+   $rpcenv->check($authuser, "/storage/$storeid", 
['Datastore.AllocateSpace']);
+   }
}
}
 
@@ -183,6 +189,14 @@ my $check_storage_access = sub {
}
 });
 
+if ($needs_extraction && defined($extraction_storage)) {
+   my $scfg = PVE::Storage::storage_config($storecfg, $extraction_storage);
+   raise_param_exc({ 'import-extraction-storage' => "$extraction_storage 
does not support"
+   ." 'images' content type or is not file based."})
+   if !$scfg->{content}->{images} || !$scfg->{path};
+   $rpcenv->check($authuser, "/storage/$extraction_storage", 
['Datastore.AllocateSpace']);
+}
+
$rpcenv->check($authuser, "/storage/$settings->{vmstatestorage}", 
['Datastore.AllocateSpace'])
if defined($settings->{vmstatestorage});
 };
@@ -326,7 +340,7 @@ my $import_from_volid = sub {
 
 # Note: $pool is only needed when creating a VM, because pool permissions
 # are automatically inherited if VM already exists inside a pool.
-my sub create_disks : prototype($$) {
+my sub create_disks : prototype($$$) {
 my (
$rpcenv,
$authuser,
@@ -338,6 +352,7 @@ my sub create_disks : prototype($$) {
$settings,
$default_storage,
$is_live_import,
+   $extraction_storage,
 ) = @_;
 
 my $vollist = [];
@@ -405,7 +420,8 @@ my sub create_disks : prototype($$) {
if (PVE::Storage::parse_volume_id($source, 1)) { # PVE-managed 
volume
if (PVE::GuestImport::copy_needs_extraction($source)) { # 
needs extraction beforehand
print "extracting $source\n";
-   $source = 
PVE::GuestImport::extract_disk_from_import_file($source, $vmid);
+   $source = 
PVE::GuestImport::extract_disk_from_import_file(
+   $source, $vmid, $extraction_storage);
print "finished extracting to $source\n";
push @$delete_sources, $source;
}
@@ -925,6 +941,12 @@ __PACKAGE__->register_method({
default => 0,
description => "Start VM after it was created 
successfully.",
},
+   'import-extraction-storage' => 
get_standard_option('pve-storage-id', {
+   description => "Storage to put extracted images when using 
'import-from' that"
+   ." needs extraction",
+   optional => 1,
+   completion => \&PVE::QemuServer::complete_storage,
+   }),
},
1, # with_disk_alloc
),
@@ -951,6 +973,7 @@ __PACKAGE__->register_method({
my $storage = extract_param($param, 'storage');
my $unique = extract_param($param, 'unique');
my $live_restore = extract_param($param, 'live-restore');
+   my $extraction_storage = extract_param($param, 
'import-extraction-storage');
 
if (defined(my $ssh_keys = $param->{sshkeys})) {
$ssh_keys = URI::Escape::uri_unescape($ssh_keys);
@@ -1010,7 +1033,8 @@ __PACKAGE__->register_method({
if (scalar(keys $param->%*) > 0) {
&$resolve_cdrom_alias($param);
 
-   &$che

[pve-devel] [PATCH storage v3 01/10] copy OVF.pm from qemu-server

2024-04-29 Thread Dominik Csapak
copies the OVF.pm and relevant ovf tests from qemu-server.
We need it here, and it uses PVE::Storage already, and since there is no
intermediary package/repository we could put it, it seems fitting in
here.

Put it in a new GuestImport module

Signed-off-by: Dominik Csapak 
---
 src/PVE/GuestImport/Makefile  |   3 +
 src/PVE/GuestImport/OVF.pm| 242 ++
 src/PVE/Makefile  |   1 +
 src/PVE/Storage/Makefile  |   1 +
 src/test/Makefile |   5 +-
 src/test/ovf_manifests/Win10-Liz-disk1.vmdk   | Bin 0 -> 65536 bytes
 src/test/ovf_manifests/Win10-Liz.ovf  | 142 ++
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ++
 .../ovf_manifests/Win_2008_R2_two-disks.ovf   | 145 +++
 src/test/ovf_manifests/disk1.vmdk | Bin 0 -> 65536 bytes
 src/test/ovf_manifests/disk2.vmdk | Bin 0 -> 65536 bytes
 src/test/run_ovf_tests.pl |  71 +
 12 files changed, 751 insertions(+), 1 deletion(-)
 create mode 100644 src/PVE/GuestImport/Makefile
 create mode 100644 src/PVE/GuestImport/OVF.pm
 create mode 100644 src/test/ovf_manifests/Win10-Liz-disk1.vmdk
 create mode 100755 src/test/ovf_manifests/Win10-Liz.ovf
 create mode 100755 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
 create mode 100755 src/test/ovf_manifests/Win_2008_R2_two-disks.ovf
 create mode 100644 src/test/ovf_manifests/disk1.vmdk
 create mode 100644 src/test/ovf_manifests/disk2.vmdk
 create mode 100755 src/test/run_ovf_tests.pl

diff --git a/src/PVE/GuestImport/Makefile b/src/PVE/GuestImport/Makefile
new file mode 100644
index 000..5948384
--- /dev/null
+++ b/src/PVE/GuestImport/Makefile
@@ -0,0 +1,3 @@
+.PHONY: install
+install:
+   install -D -m 0644 OVF.pm ${DESTDIR}${PERLDIR}/PVE/GuestImport/OVF.pm
diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
new file mode 100644
index 000..055ebf5
--- /dev/null
+++ b/src/PVE/GuestImport/OVF.pm
@@ -0,0 +1,242 @@
+# Open Virtualization Format import routines
+# https://www.dmtf.org/standards/ovf
+package PVE::GuestImport::OVF;
+
+use strict;
+use warnings;
+
+use XML::LibXML;
+use File::Spec;
+use File::Basename;
+use Data::Dumper;
+use Cwd 'realpath';
+
+use PVE::Tools;
+use PVE::Storage;
+
+# map OVF resources types to descriptive strings
+# this will allow us to explore the xml tree without using magic numbers
+# 
http://schemas.dmtf.org/wbem/cim-html/2/CIM_ResourceAllocationSettingData.html
+my @resources = (
+{ id => 1, dtmf_name => 'Other' },
+{ id => 2, dtmf_name => 'Computer System' },
+{ id => 3, dtmf_name => 'Processor' },
+{ id => 4, dtmf_name => 'Memory' },
+{ id => 5, dtmf_name => 'IDE Controller', pve_type => 'ide' },
+{ id => 6, dtmf_name => 'Parallel SCSI HBA', pve_type => 'scsi' },
+{ id => 7, dtmf_name => 'FC HBA' },
+{ id => 8, dtmf_name => 'iSCSI HBA' },
+{ id => 9, dtmf_name => 'IB HCA' },
+{ id => 10, dtmf_name => 'Ethernet Adapter' },
+{ id => 11, dtmf_name => 'Other Network Adapter' },
+{ id => 12, dtmf_name => 'I/O Slot' },
+{ id => 13, dtmf_name => 'I/O Device' },
+{ id => 14, dtmf_name => 'Floppy Drive' },
+{ id => 15, dtmf_name => 'CD Drive' },
+{ id => 16, dtmf_name => 'DVD drive' },
+{ id => 17, dtmf_name => 'Disk Drive' },
+{ id => 18, dtmf_name => 'Tape Drive' },
+{ id => 19, dtmf_name => 'Storage Extent' },
+{ id => 20, dtmf_name => 'Other storage device', pve_type => 'sata'},
+{ id => 21, dtmf_name => 'Serial port' },
+{ id => 22, dtmf_name => 'Parallel port' },
+{ id => 23, dtmf_name => 'USB Controller' },
+{ id => 24, dtmf_name => 'Graphics controller' },
+{ id => 25, dtmf_name => 'IEEE 1394 Controller' },
+{ id => 26, dtmf_name => 'Partitionable Unit' },
+{ id => 27, dtmf_name => 'Base Partitionable Unit' },
+{ id => 28, dtmf_name => 'Power' },
+{ id => 29, dtmf_name => 'Cooling Capacity' },
+{ id => 30, dtmf_name => 'Ethernet Switch Port' },
+{ id => 31, dtmf_name => 'Logical Disk' },
+{ id => 32, dtmf_name => 'Storage Volume' },
+{ id => 33, dtmf_name => 'Ethernet Connection' },
+{ id => 34, dtmf_name => 'DMTF reserved' },
+{ id => 35, dtmf_name => 'Vendor Reserved'}
+);
+
+sub find_by {
+my ($key, $param) = @_;
+foreach my $resource (@resources) {
+   if ($resource->{$key} eq $param) {
+   return ($resource);
+   }
+}
+return;
+}
+
+sub dtmf_name_to_id {
+my ($dtmf_name) = @_;
+my $found = find_by('dtmf_name', $dtmf_name);
+if ($found) {
+   return $found->{id};
+} else {
+   return;
+}
+}
+
+sub id_to_pve {
+my ($id) = @_;
+my $resource = find_by('id', $id);
+if ($resource) {
+   return $resource->{pve_type};
+} else {
+   return;
+}
+}
+
+# returns two references, $qm which holds qm.conf style key/values, and \@disks
+sub parse

[pve-devel] [PATCH manager v3 6/9] ui: import: improve rendering of volume names

2024-04-29 Thread Dominik Csapak
in directory storages, we don't need the 'import/' part of the volumes,
as that is implied in dir based storages

Signed-off-by: Dominik Csapak 
---
 www/manager6/Utils.js | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index ff2fae25..ea6e30e8 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1024,7 +1024,13 @@ Ext.define('PVE.Utils', {
Ext.String.leftPad(data.channel, 2, '0') +
" ID " + data.id + " LUN " + data.lun;
} else if (data.content === 'import') {
-   result = data.volid.replace(/^.*?:/, '');
+   if (data.volid.match(/^.*?:import\//)) {
+   // dir-based storages
+   result = data.volid.replace(/^.*?:import\//, '');
+   } else {
+   // esxi storage
+   result = data.volid.replace(/^.*?:/, '');
+   }
} else {
result = data.volid.replace(/^.*?:(.*?\/)?/, '');
}
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager v3 9/9] ui: import: show size for dir-based storages

2024-04-29 Thread Dominik Csapak
since there we already have the size information

Signed-off-by: Dominik Csapak 
---
 www/manager6/storage/Browser.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 763abc70..c0b66acc 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -148,7 +148,7 @@ Ext.define('PVE.storage.Browser', {
itemId: 'contentImport',
content: 'import',
useCustomRemoveButton: isEsxi, // hide default remove 
button for esxi
-   showColumns: ['name', 'format'],
+   showColumns: isEsxi ? ['name', 'format'] : ['name', 'size', 
'format'],
enableUploadButton: enableUpload && !isEsxi,
enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
useUploadButton: !isEsxi,
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH qemu] PVE backup: improve error when copy-before-write fails for fleecing

2024-04-29 Thread Fiona Ebner
With fleecing, failure for copy-before-write does not fail the guest
write, but only sets the snapshot error that is associated to the
copy-before-write filter, making further requests to the snapshot
access fail with EACCES, which then also fails the job. But that error
code is not the root cause of why the backup failed, so bubble up the
original snapshot error instead.

Reported-by: Friedrich Weber 
Signed-off-by: Fiona Ebner 
Tested-by: Friedrich Weber 
---

Should have a "fixes #5409:" prefix when applied in the parent git
module. Please consider applying this after the update to QEMU 9.0.0.
Otherwise, I'll have to adapt and re-send that.

 block/copy-before-write.c | 18 --
 block/copy-before-write.h |  1 +
 pve-backup.c  |  9 +
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 9ca5ec5e5c..7d70f221ca 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -27,6 +27,7 @@
 #include "qapi/qmp/qjson.h"
 
 #include "sysemu/block-backend.h"
+#include "qemu/atomic.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "block/block_int.h"
@@ -74,7 +75,8 @@ typedef struct BDRVCopyBeforeWriteState {
  * @snapshot_error is normally zero. But on first copy-before-write failure
  * when @on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT, @snapshot_error takes
  * value of this error (<0). After that all in-flight and further
- * snapshot-API requests will fail with that error.
+ * snapshot-API requests will fail with that error. To be accessed with
+ * atomics.
  */
 int snapshot_error;
 } BDRVCopyBeforeWriteState;
@@ -114,7 +116,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
 return 0;
 }
 
-if (s->snapshot_error) {
+if (qatomic_read(&s->snapshot_error)) {
 return 0;
 }
 
@@ -138,9 +140,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
 WITH_QEMU_LOCK_GUARD(&s->lock) {
 if (ret < 0) {
 assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
-if (!s->snapshot_error) {
-s->snapshot_error = ret;
-}
+qatomic_cmpxchg(&s->snapshot_error, 0, ret);
 } else {
 bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
 }
@@ -214,7 +214,7 @@ cbw_snapshot_read_lock(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
 
 QEMU_LOCK_GUARD(&s->lock);
 
-if (s->snapshot_error) {
+if (qatomic_read(&s->snapshot_error)) {
 g_free(req);
 return NULL;
 }
@@ -594,6 +594,12 @@ void bdrv_cbw_drop(BlockDriverState *bs)
 bdrv_unref(bs);
 }
 
+int bdrv_cbw_snapshot_error(BlockDriverState *bs)
+{
+BDRVCopyBeforeWriteState *s = bs->opaque;
+return qatomic_read(&s->snapshot_error);
+}
+
 static void cbw_init(void)
 {
 bdrv_register(&bdrv_cbw_filter);
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index dc6cafe7fa..a27d2d7d9f 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -44,5 +44,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
   BlockCopyState **bcs,
   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);
+int bdrv_cbw_snapshot_error(BlockDriverState *bs);
 
 #endif /* COPY_BEFORE_WRITE_H */
diff --git a/pve-backup.c b/pve-backup.c
index 00aaff6509..f8fa8e068b 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -385,6 +385,15 @@ static void pvebackup_complete_cb(void *opaque, int ret)
 di->fleecing.snapshot_access = NULL;
 }
 if (di->fleecing.cbw) {
+/*
+ * With fleecing, failure for cbw does not fail the guest write, but 
only sets the snapshot
+ * error, making further requests to the snapshot fail with EACCES, 
which then also fail the
+ * job. But that code is not the root cause and just confusing, so 
update it.
+ */
+int snapshot_error = bdrv_cbw_snapshot_error(di->fleecing.cbw);
+if (di->completed_ret == -EACCES && snapshot_error) {
+di->completed_ret = snapshot_error;
+}
 bdrv_cbw_drop(di->fleecing.cbw);
 di->fleecing.cbw = NULL;
 }
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH qemu] PVE backup: improve error when copy-before-write fails for fleecing

2024-04-29 Thread Fiona Ebner
Am 29.04.24 um 14:53 schrieb Fiona Ebner:
> With fleecing, failure for copy-before-write does not fail the guest
> write, but only sets the snapshot error that is associated to the
> copy-before-write filter, making further requests to the snapshot
> access fail with EACCES, which then also fails the job. But that error
> code is not the root cause of why the backup failed, so bubble up the
> original snapshot error instead.
> 
> Reported-by: Friedrich Weber 
> Signed-off-by: Fiona Ebner 
> Tested-by: Friedrich Weber 
> ---
> 
> Should have a "fixes #5409:" prefix when applied in the parent git

It seems the user does not run into the timeout, but some other issue
https://bugzilla.proxmox.com/show_bug.cgi?id=5409#c15
So while we will better see what the issue is, it's not a fix after all.

> module. Please consider applying this after the update to QEMU 9.0.0.
> Otherwise, I'll have to adapt and re-send that.
> 



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH container/manager 0/2] add deny read/write options for device passthrough

2024-04-29 Thread Filip Schauer
Add the deny_read and deny_write options for device passthrough, to
restrict container access to devices.

This allows for passing through a device in read-only mode without
giving the container full access it.

Up until now a container with a device passed through to it was granted
full access to that device without an option to restrict that access as
pointed out by @Fiona.

pve-container:

Filip Schauer (1):
  add deny read/write options for device passthrough

 src/PVE/LXC.pm| 13 -
 src/PVE/LXC/Config.pm | 10 ++
 2 files changed, 22 insertions(+), 1 deletion(-)


pve-manager:

Filip Schauer (1):
  ui: lxc: add deny read/write options for device passthrough

 www/manager6/lxc/DeviceEdit.js | 16 
 1 file changed, 16 insertions(+)


Summary over all repositories:
  3 files changed, 38 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.6.0


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH container 1/2] add deny read/write options for device passthrough

2024-04-29 Thread Filip Schauer
Add the deny_read and deny_write options for device passthrough, to
restrict container access to devices.

Signed-off-by: Filip Schauer 
---
 src/PVE/LXC.pm| 13 -
 src/PVE/LXC/Config.pm | 10 ++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 65d0fa8..6e2b048 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -651,7 +651,18 @@ sub update_lxc_config {
my $major = PVE::Tools::dev_t_major($rdev);
my $minor = PVE::Tools::dev_t_minor($rdev);
my $device_type_char = S_ISBLK($mode) ? 'b' : 'c';
-   $raw .= "lxc.cgroup2.devices.allow = $device_type_char $major:$minor 
rw\n";
+   my $allow_perms = $device->{deny_read} ? "" : "r";
+   my $deny_perms = $device->{deny_read} ? "r" : "";
+   $allow_perms .= $device->{deny_write} ? "" : "w";
+   $deny_perms .= $device->{deny_write} ? "w" : "";
+
+   if ($allow_perms) {
+   $raw .= "lxc.cgroup2.devices.allow = $device_type_char 
$major:$minor $allow_perms\n";
+   }
+
+   if ($deny_perms) {
+   $raw .= "lxc.cgroup2.devices.deny = $device_type_char $major:$minor 
$deny_perms\n";
+   }
 });
 
 # WARNING: DO NOT REMOVE this without making sure that loop device nodes
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 1664a35..5db9181 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -962,6 +962,16 @@ my $dev_desc = {
minimum => 0,
description => 'Group ID to be assigned to the device node',
 },
+deny_read => {
+   optional => 1,
+   type => 'boolean',
+   description => 'Deny the container to read from the device',
+},
+deny_write => {
+   optional => 1,
+   type => 'boolean',
+   description => 'Deny the container to write to the device',
+},
 };
 
 for (my $i = 0; $i < $MAX_DEVICES; $i++) {
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH manager 2/2] ui: lxc: add deny read/write options for device passthrough

2024-04-29 Thread Filip Schauer
Add checkboxes to the device passthrough dialogue for restricting
container read/write access to a passed through device.

Signed-off-by: Filip Schauer 
---
 www/manager6/lxc/DeviceEdit.js | 16 
 1 file changed, 16 insertions(+)

diff --git a/www/manager6/lxc/DeviceEdit.js b/www/manager6/lxc/DeviceEdit.js
index cdbd9acc..d4b3e34a 100644
--- a/www/manager6/lxc/DeviceEdit.js
+++ b/www/manager6/lxc/DeviceEdit.js
@@ -95,6 +95,20 @@ Ext.define('PVE.lxc.DeviceInputPanel', {
return gettext("Access mode has to be an octal number");
},
},
+   {
+   xtype: 'checkbox',
+   name: 'deny_read',
+   fieldLabel: gettext('Deny Read in CT'),
+   labelWidth: 120,
+   checked: false,
+   },
+   {
+   xtype: 'checkbox',
+   name: 'deny_write',
+   fieldLabel: gettext('Deny Write in CT'),
+   labelWidth: 120,
+   checked: false,
+   },
 ],
 });
 
@@ -145,6 +159,8 @@ Ext.define('PVE.lxc.DeviceEdit', {
mode: data.mode,
uid: data.uid,
gid: data.gid,
+   deny_read: data.deny_read,
+   deny_write: data.deny_write,
};
 
ipanel.setValues(values);
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: [PATCH-SERIES qemu] update to QEMU 9.0.0

2024-04-29 Thread Thomas Lamprecht
Am 25/04/2024 um 17:21 schrieb Fiona Ebner:
> QEMU 8.2.2 required many changes, in particular to the alloc-track
> block driver. It should be the same as [0] just with backup fleecing
> patches added in. See the patch for details.
> 
> The only bigger change in QEMU 9.0.0 is that the AioContext locking
> was removed, and it just required dropping the calls to acquire and
> release the lock. See the patch for details.
> 
> Did not see any outstanding important fixes on the qemu-stable mailing
> list currently, so none picked up.
> 
> [0]: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062422.html
> 
> 
> Fiona Ebner (4):
>   makefile: adapt firmware blob removal to changes for QEMU 8.2
>   makefile: also filter 64-bit hppa ROM for QEMU 8.2
>   update submodule and patches to QEMU 8.2.2
>   update submodule and patches to QEMU 9.0.0
> 

applied series, thanks!

I did a separate upload of 8.2.2 and imported that to pvetest so that we
have it in the repos for quick (regression) testing/comparison, even if
we skip directly to 9.0.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: [PATCH docs v2] fix #5429: network: override device names: include Type=ether

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 09:49 schrieb Friedrich Weber:
> Mention that the systemd link file should contain `Type=ether` in most
> setup, to make sure it only applies to Ethernet devices and does not
> ever apply to e.g. bridges or bonds which inherit the MAC address of
> the Ethernet device. Mention that some setups may require other
> options.
> 
> Reported in the forum [0] and in #5429 [1].
> 
> [0] https://forum.proxmox.com/threads/144557/post-656188
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5429
> 
> Fixes: 96c0261 ("fix #4847: network: extend section on interface naming 
> scheme")
> Signed-off-by: Friedrich Weber 
> ---
> 
> Notes:
> Changes v1 -> v2:
> 
> - link #5429 which was opened in the meantime
> - expand on why Type=ether is recommended
> - mention that some setups may require other choices (thx Thomas)
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-April/063659.html
> 
>  pve-network.adoc | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
>

applied, thanks!


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v2 qemu 2/2] backup: improve error when copy-before-write fails for fleecing

2024-04-29 Thread Fiona Ebner
With fleecing, failure for copy-before-write does not fail the guest
write, but only sets the snapshot error that is associated to the
copy-before-write filter, making further requests to the snapshot
access fail with EACCES, which then also fails the job. But that error
code is not the root cause of why the backup failed, so bubble up the
original snapshot error instead.

Reported-by: Friedrich Weber 
Signed-off-by: Fiona Ebner 
---

No changes in v2, but sent as a patch for the parent git module.

 ...ve-error-when-copy-before-write-fail.patch | 117 ++
 debian/patches/series |   1 +
 2 files changed, 118 insertions(+)
 create mode 100644 
debian/patches/pve/0053-PVE-backup-improve-error-when-copy-before-write-fail.patch

diff --git 
a/debian/patches/pve/0053-PVE-backup-improve-error-when-copy-before-write-fail.patch
 
b/debian/patches/pve/0053-PVE-backup-improve-error-when-copy-before-write-fail.patch
new file mode 100644
index 000..d422157
--- /dev/null
+++ 
b/debian/patches/pve/0053-PVE-backup-improve-error-when-copy-before-write-fail.patch
@@ -0,0 +1,117 @@
+From  Mon Sep 17 00:00:00 2001
+From: Fiona Ebner 
+Date: Mon, 29 Apr 2024 14:43:58 +0200
+Subject: [PATCH] PVE backup: improve error when copy-before-write fails for
+ fleecing
+
+With fleecing, failure for copy-before-write does not fail the guest
+write, but only sets the snapshot error that is associated to the
+copy-before-write filter, making further requests to the snapshot
+access fail with EACCES, which then also fails the job. But that error
+code is not the root cause of why the backup failed, so bubble up the
+original snapshot error instead.
+
+Reported-by: Friedrich Weber 
+Signed-off-by: Fiona Ebner 
+Tested-by: Friedrich Weber 
+---
+ block/copy-before-write.c | 18 --
+ block/copy-before-write.h |  1 +
+ pve-backup.c  |  9 +
+ 3 files changed, 22 insertions(+), 6 deletions(-)
+
+diff --git a/block/copy-before-write.c b/block/copy-before-write.c
+index 7cc5e4f9b9..116b5ed5c9 100644
+--- a/block/copy-before-write.c
 b/block/copy-before-write.c
+@@ -27,6 +27,7 @@
+ #include "qapi/qmp/qjson.h"
+ 
+ #include "sysemu/block-backend.h"
++#include "qemu/atomic.h"
+ #include "qemu/cutils.h"
+ #include "qapi/error.h"
+ #include "block/block_int.h"
+@@ -74,7 +75,8 @@ typedef struct BDRVCopyBeforeWriteState {
+  * @snapshot_error is normally zero. But on first copy-before-write 
failure
+  * when @on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT, @snapshot_error 
takes
+  * value of this error (<0). After that all in-flight and further
+- * snapshot-API requests will fail with that error.
++ * snapshot-API requests will fail with that error. To be accessed with
++ * atomics.
+  */
+ int snapshot_error;
+ } BDRVCopyBeforeWriteState;
+@@ -114,7 +116,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
+ return 0;
+ }
+ 
+-if (s->snapshot_error) {
++if (qatomic_read(&s->snapshot_error)) {
+ return 0;
+ }
+ 
+@@ -138,9 +140,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
+ WITH_QEMU_LOCK_GUARD(&s->lock) {
+ if (ret < 0) {
+ assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
+-if (!s->snapshot_error) {
+-s->snapshot_error = ret;
+-}
++qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+ } else {
+ bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
+ }
+@@ -214,7 +214,7 @@ cbw_snapshot_read_lock(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
+ 
+ QEMU_LOCK_GUARD(&s->lock);
+ 
+-if (s->snapshot_error) {
++if (qatomic_read(&s->snapshot_error)) {
+ g_free(req);
+ return NULL;
+ }
+@@ -594,6 +594,12 @@ void bdrv_cbw_drop(BlockDriverState *bs)
+ bdrv_unref(bs);
+ }
+ 
++int bdrv_cbw_snapshot_error(BlockDriverState *bs)
++{
++BDRVCopyBeforeWriteState *s = bs->opaque;
++return qatomic_read(&s->snapshot_error);
++}
++
+ static void cbw_init(void)
+ {
+ bdrv_register(&bdrv_cbw_filter);
+diff --git a/block/copy-before-write.h b/block/copy-before-write.h
+index dc6cafe7fa..a27d2d7d9f 100644
+--- a/block/copy-before-write.h
 b/block/copy-before-write.h
+@@ -44,5 +44,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
+   BlockCopyState **bcs,
+   Error **errp);
+ void bdrv_cbw_drop(BlockDriverState *bs);
++int bdrv_cbw_snapshot_error(BlockDriverState *bs);
+ 
+ #endif /* COPY_BEFORE_WRITE_H */
+diff --git a/pve-backup.c b/pve-backup.c
+index 00aaff6509..f8fa8e068b 100644
+--- a/pve-backup.c
 b/pve-backup.c
+@@ -385,6 +385,15 @@ static void pvebackup_complete_cb(void *opaque, int ret)
+ di->fleecing.snapshot_access = NULL;
+ }
+ if (di->fleecing.cbw) {
++/*
++  

[pve-devel] [PATCH v2 qemu 1/2] fix #5409: backup: fix copy-before-write timeout

2024-04-29 Thread Fiona Ebner
The type for the copy-before-write timeout in nanoseconds was wrong.
By being just uint32_t, a maximum of slightly over 4 seconds was
possible. Larger values would overflow and thus the 45 seconds set by
Proxmox's backup with fleecing, resulted in effectively 2 seconds
timeout for copy-before-write operations.

Reported-by: Friedrich Weber 
Signed-off-by: Fiona Ebner 
---

New in v2.

Upstream submission of this patch:
https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.eb...@proxmox.com/T/#u

 ...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++
 ...ock-copy-before-write-fix-permission.patch |  2 +-
 ...e-write-support-unligned-snapshot-di.patch |  2 +-
 ...e-write-create-block_copy-bitmap-in-.patch |  2 +-
 ...-backup-add-discard-source-parameter.patch |  4 +--
 ...e-allow-specifying-minimum-cluster-s.patch |  2 +-
 ...um-cluster-size-to-performance-optio.patch |  2 +-
 debian/patches/series |  1 +
 8 files changed, 43 insertions(+), 7 deletions(-)
 create mode 100644 
debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch

diff --git 
a/debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
 
b/debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
new file mode 100644
index 000..b350067
--- /dev/null
+++ 
b/debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
@@ -0,0 +1,35 @@
+From  Mon Sep 17 00:00:00 2001
+From: Fiona Ebner 
+Date: Mon, 29 Apr 2024 15:41:11 +0200
+Subject: [PATCH] block/copy-before-write: use uint64_t for timeout in
+ nanoseconds
+
+rather than the uint32_t for which the maximum is slightly more than 4
+seconds and larger values would overflow. The QAPI interface allows
+specifying the number of seconds, so only values 0 to 4 are safe right
+now, other values lead to a much lower timeout than a user expects.
+
+The block_copy() call where this is used already takes a uint64_t for
+the timeout, so no change required there.
+
+Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option")
+Reported-by: Friedrich Weber 
+Signed-off-by: Fiona Ebner 
+Tested-by: Friedrich Weber 
+---
+ block/copy-before-write.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/block/copy-before-write.c b/block/copy-before-write.c
+index b866e42271..3ee95c0e7a 100644
+--- a/block/copy-before-write.c
 b/block/copy-before-write.c
+@@ -43,7 +43,7 @@ typedef struct BDRVCopyBeforeWriteState {
+ BlockCopyState *bcs;
+ BdrvChild *target;
+ OnCbwError on_cbw_error;
+-uint32_t cbw_timeout_ns;
++uint64_t cbw_timeout_ns;
+ 
+ /*
+  * @lock: protects access to @access_bitmap, @done_bitmap and
diff --git 
a/debian/patches/pve/0046-block-copy-before-write-fix-permission.patch 
b/debian/patches/pve/0046-block-copy-before-write-fix-permission.patch
index 1c2e5bd..5703311 100644
--- a/debian/patches/pve/0046-block-copy-before-write-fix-permission.patch
+++ b/debian/patches/pve/0046-block-copy-before-write-fix-permission.patch
@@ -33,7 +33,7 @@ Signed-off-by: Thomas Lamprecht 
  1 file changed, 7 insertions(+), 3 deletions(-)
 
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index b866e42271..a2dddf6f57 100644
+index 3ee95c0e7a..641236dbf8 100644
 --- a/block/copy-before-write.c
 +++ b/block/copy-before-write.c
 @@ -364,9 +364,13 @@ static void cbw_child_perm(BlockDriverState *bs, 
BdrvChild *c,
diff --git 
a/debian/patches/pve/0047-block-copy-before-write-support-unligned-snapshot-di.patch
 
b/debian/patches/pve/0047-block-copy-before-write-support-unligned-snapshot-di.patch
index 4656750..b02025a 100644
--- 
a/debian/patches/pve/0047-block-copy-before-write-support-unligned-snapshot-di.patch
+++ 
b/debian/patches/pve/0047-block-copy-before-write-support-unligned-snapshot-di.patch
@@ -15,7 +15,7 @@ Signed-off-by: Thomas Lamprecht 
  1 file changed, 13 insertions(+), 3 deletions(-)
 
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index a2dddf6f57..0a219c2b75 100644
+index 641236dbf8..71cf50343f 100644
 --- a/block/copy-before-write.c
 +++ b/block/copy-before-write.c
 @@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
diff --git 
a/debian/patches/pve/0048-block-copy-before-write-create-block_copy-bitmap-in-.patch
 
b/debian/patches/pve/0048-block-copy-before-write-create-block_copy-bitmap-in-.patch
index ab7c0bf..5148257 100644
--- 
a/debian/patches/pve/0048-block-copy-before-write-create-block_copy-bitmap-in-.patch
+++ 
b/debian/patches/pve/0048-block-copy-before-write-create-block_copy-bitmap-in-.patch
@@ -49,7 +49,7 @@ index e13d7bc6b6..b61685f1a2 100644
  if (!copy_bitmap) {
  return NULL;
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 0a219c2b75..d3b95bd600 100644
+index 71cf50343f..ba827b0aba 100644
 --- a/block/copy-before-write.c
 +++ b/block/copy-be

[pve-devel] [PATCH docs] backup: mention where fleecing can be configured

2024-04-29 Thread Fiona Ebner
Reported in the community forum:
https://forum.proxmox.com/threads/145955/post-658380

Signed-off-by: Fiona Ebner 
---
 vzdump.adoc | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 79d4bbc..9b1cb61 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -148,8 +148,12 @@ sectors will be limited by the speed of the backup target.
 With backup fleecing, such old data is cached in a fleecing image rather than
 sent directly to the backup target. This can help guest IO performance and even
 prevent hangs in certain scenarios, at the cost of requiring more storage 
space.
+
 Use e.g. `vzdump 123 --fleecing enabled=1,storage=local-lvm` to enable backup
-fleecing, with fleecing images created on the storage `local-lvm`.
+fleecing, with fleecing images created on the storage `local-lvm`. As always,
+you can set the option for specific backup jobs, or as a node-wide fallback via
+the xref:vzdump_configuration[configuration options]. In the UI, fleecing can 
be
+configured in the 'Advanced' tab when editing a backup job.
 
 The fleecing storage should be a fast local storage, with thin provisioning and
 discard support. Examples are LVM-thin, RBD, ZFS with `sparse 1` in the storage
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: [PATCH v2 qemu 1/2] fix #5409: backup: fix copy-before-write timeout

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 16:27 schrieb Fiona Ebner:
> The type for the copy-before-write timeout in nanoseconds was wrong.
> By being just uint32_t, a maximum of slightly over 4 seconds was
> possible. Larger values would overflow and thus the 45 seconds set by
> Proxmox's backup with fleecing, resulted in effectively 2 seconds
> timeout for copy-before-write operations.
> 
> Reported-by: Friedrich Weber 
> Signed-off-by: Fiona Ebner 
> ---
> 
> New in v2.
> 
> Upstream submission of this patch:
> https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.eb...@proxmox.com/T/#u
> 
>  ...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++
>  ...ock-copy-before-write-fix-permission.patch |  2 +-
>  ...e-write-support-unligned-snapshot-di.patch |  2 +-
>  ...e-write-create-block_copy-bitmap-in-.patch |  2 +-
>  ...-backup-add-discard-source-parameter.patch |  4 +--
>  ...e-allow-specifying-minimum-cluster-s.patch |  2 +-
>  ...um-cluster-size-to-performance-optio.patch |  2 +-
>  debian/patches/series |  1 +
>  8 files changed, 43 insertions(+), 7 deletions(-)
>  create mode 100644 
> debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
> 
>

applied this one to a new stable-8 branch that was split of the last 8.1.5-5
release (8 in the branch name is meaning the PVE major release here though,
can then be merged with master before that switches over to target PVE 9).

Does not apply anymore on master, which is QEMU 9.0 now, so please send a
rebase for that.



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v3 qemu 2/2] backup: improve error when copy-before-write fails for fleecing

2024-04-29 Thread Fiona Ebner
With fleecing, failure for copy-before-write does not fail the guest
write, but only sets the snapshot error that is associated to the
copy-before-write filter, making further requests to the snapshot
access fail with EACCES, which then also fails the job. But that error
code is not the root cause of why the backup failed, so bubble up the
original snapshot error instead.

Reported-by: Friedrich Weber 
Signed-off-by: Fiona Ebner 
---

Changes in v3:
* rebase on master

 ...ve-error-when-copy-before-write-fail.patch | 117 ++
 debian/patches/series |   1 +
 2 files changed, 118 insertions(+)
 create mode 100644 
debian/patches/pve/0051-PVE-backup-improve-error-when-copy-before-write-fail.patch

diff --git 
a/debian/patches/pve/0051-PVE-backup-improve-error-when-copy-before-write-fail.patch
 
b/debian/patches/pve/0051-PVE-backup-improve-error-when-copy-before-write-fail.patch
new file mode 100644
index 000..c7f2ccb
--- /dev/null
+++ 
b/debian/patches/pve/0051-PVE-backup-improve-error-when-copy-before-write-fail.patch
@@ -0,0 +1,117 @@
+From  Mon Sep 17 00:00:00 2001
+From: Fiona Ebner 
+Date: Mon, 29 Apr 2024 14:43:58 +0200
+Subject: [PATCH] PVE backup: improve error when copy-before-write fails for
+ fleecing
+
+With fleecing, failure for copy-before-write does not fail the guest
+write, but only sets the snapshot error that is associated to the
+copy-before-write filter, making further requests to the snapshot
+access fail with EACCES, which then also fails the job. But that error
+code is not the root cause of why the backup failed, so bubble up the
+original snapshot error instead.
+
+Reported-by: Friedrich Weber 
+Signed-off-by: Fiona Ebner 
+Tested-by: Friedrich Weber 
+---
+ block/copy-before-write.c | 18 --
+ block/copy-before-write.h |  1 +
+ pve-backup.c  |  9 +
+ 3 files changed, 22 insertions(+), 6 deletions(-)
+
+diff --git a/block/copy-before-write.c b/block/copy-before-write.c
+index bba58326d7..50cc4c7aae 100644
+--- a/block/copy-before-write.c
 b/block/copy-before-write.c
+@@ -27,6 +27,7 @@
+ #include "qapi/qmp/qjson.h"
+ 
+ #include "sysemu/block-backend.h"
++#include "qemu/atomic.h"
+ #include "qemu/cutils.h"
+ #include "qapi/error.h"
+ #include "block/block_int.h"
+@@ -74,7 +75,8 @@ typedef struct BDRVCopyBeforeWriteState {
+  * @snapshot_error is normally zero. But on first copy-before-write 
failure
+  * when @on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT, @snapshot_error 
takes
+  * value of this error (<0). After that all in-flight and further
+- * snapshot-API requests will fail with that error.
++ * snapshot-API requests will fail with that error. To be accessed with
++ * atomics.
+  */
+ int snapshot_error;
+ } BDRVCopyBeforeWriteState;
+@@ -114,7 +116,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
+ return 0;
+ }
+ 
+-if (s->snapshot_error) {
++if (qatomic_read(&s->snapshot_error)) {
+ return 0;
+ }
+ 
+@@ -138,9 +140,7 @@ static coroutine_fn int 
cbw_do_copy_before_write(BlockDriverState *bs,
+ WITH_QEMU_LOCK_GUARD(&s->lock) {
+ if (ret < 0) {
+ assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
+-if (!s->snapshot_error) {
+-s->snapshot_error = ret;
+-}
++qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+ } else {
+ bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
+ }
+@@ -214,7 +214,7 @@ cbw_snapshot_read_lock(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
+ 
+ QEMU_LOCK_GUARD(&s->lock);
+ 
+-if (s->snapshot_error) {
++if (qatomic_read(&s->snapshot_error)) {
+ g_free(req);
+ return NULL;
+ }
+@@ -585,6 +585,12 @@ void bdrv_cbw_drop(BlockDriverState *bs)
+ bdrv_unref(bs);
+ }
+ 
++int bdrv_cbw_snapshot_error(BlockDriverState *bs)
++{
++BDRVCopyBeforeWriteState *s = bs->opaque;
++return qatomic_read(&s->snapshot_error);
++}
++
+ static void cbw_init(void)
+ {
+ bdrv_register(&bdrv_cbw_filter);
+diff --git a/block/copy-before-write.h b/block/copy-before-write.h
+index dc6cafe7fa..a27d2d7d9f 100644
+--- a/block/copy-before-write.h
 b/block/copy-before-write.h
+@@ -44,5 +44,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
+   BlockCopyState **bcs,
+   Error **errp);
+ void bdrv_cbw_drop(BlockDriverState *bs);
++int bdrv_cbw_snapshot_error(BlockDriverState *bs);
+ 
+ #endif /* COPY_BEFORE_WRITE_H */
+diff --git a/pve-backup.c b/pve-backup.c
+index 7cc1dd3724..07709aa350 100644
+--- a/pve-backup.c
 b/pve-backup.c
+@@ -379,6 +379,15 @@ static void pvebackup_complete_cb(void *opaque, int ret)
+ di->fleecing.snapshot_access = NULL;
+ }
+ if (di->fleecing.cbw) {
++/*
++ * With fleecing, failure

[pve-devel] [PATCH v3 qemu 1/2] fix #5409: backup: fix copy-before-write timeout

2024-04-29 Thread Fiona Ebner
The type for the copy-before-write timeout in nanoseconds was wrong.
By being just uint32_t, a maximum of slightly over 4 seconds was
possible. Larger values would overflow and thus the 45 seconds set by
Proxmox's backup with fleecing, resulted in effectively 2 seconds
timeout for copy-before-write operations.

Reported-by: Friedrich Weber 
Signed-off-by: Fiona Ebner 
---

Changes in v3:
* rebase on master

Upstream submission of this patch:
https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.eb...@proxmox.com/T/#u

 ...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++
 ...ock-copy-before-write-fix-permission.patch |  2 +-
 ...e-write-support-unligned-snapshot-di.patch |  2 +-
 ...e-write-create-block_copy-bitmap-in-.patch |  2 +-
 ...-backup-add-discard-source-parameter.patch |  4 +--
 ...e-allow-specifying-minimum-cluster-s.patch |  2 +-
 ...um-cluster-size-to-performance-optio.patch |  2 +-
 debian/patches/series |  1 +
 8 files changed, 43 insertions(+), 7 deletions(-)
 create mode 100644 
debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch

diff --git 
a/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
 
b/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
new file mode 100644
index 000..a8bdd85
--- /dev/null
+++ 
b/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
@@ -0,0 +1,35 @@
+From  Mon Sep 17 00:00:00 2001
+From: Fiona Ebner 
+Date: Mon, 29 Apr 2024 15:41:11 +0200
+Subject: [PATCH] block/copy-before-write: use uint64_t for timeout in
+ nanoseconds
+
+rather than the uint32_t for which the maximum is slightly more than 4
+seconds and larger values would overflow. The QAPI interface allows
+specifying the number of seconds, so only values 0 to 4 are safe right
+now, other values lead to a much lower timeout than a user expects.
+
+The block_copy() call where this is used already takes a uint64_t for
+the timeout, so no change required there.
+
+Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option")
+Reported-by: Friedrich Weber 
+Signed-off-by: Fiona Ebner 
+Tested-by: Friedrich Weber 
+---
+ block/copy-before-write.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/block/copy-before-write.c b/block/copy-before-write.c
+index 8aba27a71d..026fa9840f 100644
+--- a/block/copy-before-write.c
 b/block/copy-before-write.c
+@@ -43,7 +43,7 @@ typedef struct BDRVCopyBeforeWriteState {
+ BlockCopyState *bcs;
+ BdrvChild *target;
+ OnCbwError on_cbw_error;
+-uint32_t cbw_timeout_ns;
++uint64_t cbw_timeout_ns;
+ 
+ /*
+  * @lock: protects access to @access_bitmap, @done_bitmap and
diff --git 
a/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch 
b/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
index 0d02af9..6a759a4 100644
--- a/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
+++ b/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
@@ -33,7 +33,7 @@ Signed-off-by: Thomas Lamprecht 
  1 file changed, 7 insertions(+), 3 deletions(-)
 
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 8aba27a71d..3e3af30c08 100644
+index 026fa9840f..5a9456d426 100644
 --- a/block/copy-before-write.c
 +++ b/block/copy-before-write.c
 @@ -364,9 +364,13 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, 
BdrvChildRole role,
diff --git 
a/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
 
b/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
index c689750..f651c58 100644
--- 
a/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
+++ 
b/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
@@ -15,7 +15,7 @@ Signed-off-by: Thomas Lamprecht 
  1 file changed, 13 insertions(+), 3 deletions(-)
 
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 3e3af30c08..6d89af0b29 100644
+index 5a9456d426..c0e70669a2 100644
 --- a/block/copy-before-write.c
 +++ b/block/copy-before-write.c
 @@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
diff --git 
a/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
 
b/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
index 0fcda2e..7cd24d0 100644
--- 
a/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
+++ 
b/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
@@ -49,7 +49,7 @@ index 9ee3dd7ef5..8fca2c3698 100644
  if (!copy_bitmap) {
  return NULL;
 diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 6d89af0b29..ed2c228da7 100644
+index c0e70669a2..94db31512d 100644
 --- a/block/copy-