[pve-devel] [PATCH docs v2] fix #5429: network: override device names: include Type=ether
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
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
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
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
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
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
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
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
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
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
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
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'
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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-