[pve-devel] [PATCH proxmox-perl-rs] update to proxmox-log 0.2
Signed-off-by: Fabian Grünbichler
---
common/src/logger.rs | 4 +---
pmg-rs/Cargo.toml | 2 +-
pmg-rs/debian/control | 2 +-
pve-rs/Cargo.toml | 2 +-
pve-rs/debian/control | 2 +-
5 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/common/src/logger.rs b/common/src/logger.rs
index b54e658..1c8940b 100644
--- a/common/src/logger.rs
+++ b/common/src/logger.rs
@@ -5,9 +5,7 @@ pub fn init(env_var_name: , default_log_level: ) {
if let Err(e) = default_log_level
.parse()
.map_err(Error::from)
-.and_then(|default_log_level| {
-proxmox_log::init_logger(env_var_name, default_log_level, "")
-})
+.and_then(|default_log_level| proxmox_log::init_logger(env_var_name,
default_log_level))
{
eprintln!("could not set up env_logger: {e:?}");
}
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 6296b64..2af92ee 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -35,7 +35,7 @@ proxmox-apt-api-types = "1.0"
proxmox-config-digest = "0.1"
proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] }
proxmox-http-error = "0.1.0"
-proxmox-log = "0.1"
+proxmox-log = "0.2"
proxmox-notify = "0.4"
proxmox-subscription = "0.4"
proxmox-sys = "0.6"
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
index 24ecdc4..2904b55 100644
--- a/pmg-rs/debian/control
+++ b/pmg-rs/debian/control
@@ -25,7 +25,7 @@ Build-Depends: cargo:native ,
librust-proxmox-http-0.9+client-trait-dev,
librust-proxmox-http-0.9+default-dev,
librust-proxmox-http-error-0.1+default-dev,
- librust-proxmox-log-0.1+default-dev,
+ librust-proxmox-log-0.2+default-dev,
librust-proxmox-notify-0.4+default-dev,
librust-proxmox-subscription-0.4+default-dev,
librust-proxmox-sys-0.6+default-dev,
diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index ddae988..b6abf6c 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -36,7 +36,7 @@ proxmox-apt-api-types = "1.0"
proxmox-config-digest = "0.1"
proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] }
proxmox-http-error = "0.1.0"
-proxmox-log = "0.1"
+proxmox-log = "0.2"
proxmox-notify = { version = "0.4", features = ["pve-context"] }
proxmox-openid = "0.10"
proxmox-resource-scheduling = "0.3.0"
diff --git a/pve-rs/debian/control b/pve-rs/debian/control
index 0fabbe9..6191fdb 100644
--- a/pve-rs/debian/control
+++ b/pve-rs/debian/control
@@ -23,7 +23,7 @@ Build-Depends: cargo:native ,
librust-proxmox-http-0.9+client-trait-dev,
librust-proxmox-http-0.9+default-dev,
librust-proxmox-http-error-0.1+default-dev,
- librust-proxmox-log-0.1+default-dev,
+ librust-proxmox-log-0.2+default-dev,
librust-proxmox-notify-0.4+default-dev,
librust-proxmox-notify-0.4+pve-context-dev,
librust-proxmox-openid-0.10+default-dev,
--
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] applied: [PATCH v1 pve-common 0/3] Section Config: Documentation & Code Cleanup
On July 24, 2024 1:34 pm, Fiona Ebner wrote: > Am 04.06.24 um 11:28 schrieb Max Carrara: >> >> Max Carrara (3): >> section config: document package and its methods with POD >> section config: update code style >> section config: clean up parser logic >> >> src/PVE/SectionConfig.pm | 982 +++ >> 1 file changed, 798 insertions(+), 184 deletions(-) >> > > For the record, it seems like this already got applied: > https://git.proxmox.com/?p=pve-common.git;a=commit;h=6cba8d7660b62002ffdc81655b8e8668952614a9 > > Could you re-send the changes for v2 as follow-ups? Or v1 could also be > reverted and v2 applied? I reverted them for now - it seems they accidentally slipped in when I applied an unrelated patch cause of a dirty repo state I missed before pushing.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] S3 Support
> Tiziano Bacocco via pve-devel hat am 16.07.2024 > 00:20 CEST geschrieben: > Hi, to everyone, i'm interested in native S3 support on proxmox, is anyone > working on it ( to avoid double work )? None of us is, but there is an external developer that is trying to implement something like this. The whole topic might be a better fit for pbs-devel though ;) > Second question, my idea is to reuse existing pbs code but upload chunks > and index to s3, sounds good? The thing puzzling me is that a lot of code > will end up duplicated , when actually only the part uploading to S3 > instead of pbs would be different, chunking, fleeching, restoring, backup, > qemu interface would be almost identical https://bugzilla.proxmox.com/show_bug.cgi?id=2943 backing up directly to S3 will be hard for a multitude of reasons: - you don't want to write individual chunks as objects (too expensive) - you don't want to slow down the guest because of the speed of the object storage - .. instead, a similar approach to the tape backup feature would be needed: - define retention and bucket allocation policies - combine metadata (snapshots/..) and chunks of a local datastore in a meaningful way into buckets - have some sort of index/catalog that tells you which buckets you need to restore which snapshots into a datastore - .. hope this makes sense! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control] api: ACL update: fix handling of Permissions.Modify
with 8.x, the scope of non-"Permissions.Modify"-based ACL update privileges
were reduced (so that users with for example, VM.Allocate on a VM could only
delegate their own privileges, but not arbitrary other ones). that additional
logic had a wrong guard and was accidentally triggered for calls where the user
had the "Permissions.Modify" privilege on the modified ACL path, but without
propagation set.
a user with "Permissions.Modify" on a path should be able to set arbitrary
ACLs for that path, even without propagation.
reported on the forum:
https://forum.proxmox.com/threads/privilege-permissions-modify-on-pool-will-not-propagade-to-contained-vms-anymore.151032/
Fixes: 46bfd59dfca655b263d1f905be37d985416717ac ("acls: restrict
less-privileged ACL modifications")
Signed-off-by: Fabian Grünbichler
---
src/PVE/API2/ACL.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
index 93adb78..2a4d4ff 100644
--- a/src/PVE/API2/ACL.pm
+++ b/src/PVE/API2/ACL.pm
@@ -166,7 +166,8 @@ __PACKAGE__->register_method ({
die "role '$role' does not exist\n"
if !$cfg->{roles}->{$role};
- if (!$auth_user_privs->{'Permissions.Modify'}) {
+ # permissions() returns set privs as key, and propagate bit
as value!
+ if (!defined($auth_user_privs->{'Permissions.Modify'})) {
# 'perm-modify' allows /vms/* with VM.Allocate and
similar restricted use cases
# filter those to only allow handing out a subset of
currently active privs
my $role_privs = $cfg->{roles}->{$role};
--
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 pve-kernel] fix #5558: cherry-pick NFSv4 fix
picked from v6.9.8, the bug can cause lost NFS connections according to
upstream, and possibly corrupt backups according to our user report.
Signed-off-by: Fabian Grünbichler
---
numbered after Fiona's two cherry-picks already on the list, assuming those
will all be applied in one go ;)
...0-SUNRPC-Fix-backchannel-reply-again.patch | 58 +++
1 file changed, 58 insertions(+)
create mode 100644 patches/kernel/0020-SUNRPC-Fix-backchannel-reply-again.patch
diff --git a/patches/kernel/0020-SUNRPC-Fix-backchannel-reply-again.patch
b/patches/kernel/0020-SUNRPC-Fix-backchannel-reply-again.patch
new file mode 100644
index 000..7fe2703
--- /dev/null
+++ b/patches/kernel/0020-SUNRPC-Fix-backchannel-reply-again.patch
@@ -0,0 +1,58 @@
+From Mon Sep 17 00:00:00 2001
+From: Chuck Lever
+Date: Wed, 19 Jun 2024 09:51:08 -0400
+Subject: [PATCH] SUNRPC: Fix backchannel reply, again
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+[ Upstream commit 6ddc9deacc1312762c2edd9de00ce76b00f69f7c ]
+
+I still see "RPC: Could not send backchannel reply error: -110"
+quite often, along with slow-running tests. Debugging shows that the
+backchannel is still stumbling when it has to queue a callback reply
+on a busy transport.
+
+Note that every one of these timeouts causes a connection loss by
+virtue of the xprt_conditional_disconnect() call in that arm of
+call_cb_transmit_status().
+
+I found that setting to_maxval is necessary to get the RPC timeout
+logic to behave whenever to_exponential is not set.
+
+Fixes: 57331a59ac0d ("NFSv4.1: Use the nfs_client's rpc timeouts for
backchannel")
+Signed-off-by: Chuck Lever
+Reviewed-by: Benjamin Coddington
+Signed-off-by: Trond Myklebust
+Signed-off-by: Sasha Levin
+(cherry picked from commit bd1e42e0f2567c911d3df761cf7a33b021fdceeb)
+Signed-off-by: Fabian Grünbichler
+---
+ net/sunrpc/svc.c | 5 -
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
+index b969e505c7b7..5fc974dda811 100644
+--- a/net/sunrpc/svc.c
b/net/sunrpc/svc.c
+@@ -1548,9 +1548,11 @@ void svc_process(struct svc_rqst *rqstp)
+ */
+ void svc_process_bc(struct rpc_rqst *req, struct svc_rqst *rqstp)
+ {
++ struct rpc_timeout timeout = {
++ .to_increment = 0,
++ };
+ struct rpc_task *task;
+ int proc_error;
+- struct rpc_timeout timeout;
+
+ /* Build the svc_rqst used by the common processing routine */
+ rqstp->rq_xid = req->rq_xid;
+@@ -1603,6 +1605,7 @@ void svc_process_bc(struct rpc_rqst *req, struct
svc_rqst *rqstp)
+ timeout.to_initval = req->rq_xprt->timeout->to_initval;
+ timeout.to_retries = req->rq_xprt->timeout->to_retries;
+ }
++ timeout.to_maxval = timeout.to_initval;
+ memcpy(>rq_snd_buf, >rq_res, sizeof(req->rq_snd_buf));
+ task = rpc_run_bc_task(req, );
+
--
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 pve-docs] vzdump: add section describing PBS change detection mode
On June 26, 2024 10:13 am, Christian Ebner wrote: > Add a concise section about what PBS change detection mode is and > what it affects, including a table with a description of the modes. > > Signed-off-by: Christian Ebner > --- > vzdump.adoc | 24 > 1 file changed, 24 insertions(+) > > diff --git a/vzdump.adoc b/vzdump.adoc > index 79d4bbc..958ee12 100644 > --- a/vzdump.adoc > +++ b/vzdump.adoc > @@ -171,6 +171,30 @@ fleecing image up-front. On a thinly provisioned > storage, the fleecing image can > grow to the same size as the original image only if the guest re-writes a > whole > disk while the backup is busy with another disk. > > +CT Change Detection Mode > + > + > +Setting the change detection mode defines the encoding format for the pxar > +archives and how changed and unchanged files are handled for container > backups > +with Proxmox Backup Server as target. > + > +Backups of VMs or to other storage backends are not affected by this setting. > + > +There are 3 change detection modes available: > + > +[width="100%",cols=" +|=== > +| Mode | Description > +| Default | The legacy mode, read and encode all files in a self contained > +pxar archive with format version 1 being uploaded to the Proxmox Backup > Server. I'd shorten this: Read and encode all files into a single archive, using the pxar format version 1. > +| Data | (EXPERIMENTAL): Read and encode all files, but split data and > +metadata into separate streams, using the pxar format version 2. > +| Metadata | (EXPERIMENTAL): Use the metadata archive of the previous backup > +run to detect unchanged files and reuse the files data already backed up in > the > +previous backup for the current backup run when possible. Data and metadata > are > +encoded into separate streams, using the pxar format version 2. Split streams and archive format version 2 like `Data`, but use metadata archive of previous snapshot (if one exists) to detect unchanged files, and reuse their data chunks without reading file contents from disk, whenever possible. This is easier to parse (I hope) and both explains the difference to `Data`, and the reason why you'd want to use it ;) > +|=== > + > Backup File Names > - > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 1/4] ui: homogenize uses of Zstd and SCSI
On April 23, 2024 10:27 am, Maximiliano Sandoval wrote:
> See [1, 2] for their spelling.
>
> [1] https://en.wikipedia.org/wiki/Zstd
neither the upstream repo, nor the corresponding RFC indicate that this
is the desired spelling (it is "Zstandard", but the short variant is
called "zstd" in both places, except for a single "Zstd" in the Readme
of the repo, which might as well be a typo ;)). you have to ignore all
the occurences at the start of a sentence of course..
> [2] https://en.wikipedia.org/wiki/SCSI
>
> Signed-off-by: Maximiliano Sandoval
> ---
> www/manager6/form/BackupCompressionSelector.js | 2 +-
> www/manager6/panel/BackupAdvancedOptions.js| 2 +-
> www/manager6/window/DownloadUrlToStorage.js| 2 +-
> www/manager6/window/GuestImport.js | 1 +
> 4 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/www/manager6/form/BackupCompressionSelector.js
> b/www/manager6/form/BackupCompressionSelector.js
> index 014b8f7e..2efc2919 100644
> --- a/www/manager6/form/BackupCompressionSelector.js
> +++ b/www/manager6/form/BackupCompressionSelector.js
> @@ -5,6 +5,6 @@ Ext.define('PVE.form.BackupCompressionSelector', {
> ['0', Proxmox.Utils.noneText],
> ['lzo', 'LZO (' + gettext('fast') + ')'],
> ['gzip', 'GZIP (' + gettext('good') + ')'],
funnily enough, GZIP would be the more likely candidate for such a
change, after all it is normally just spelled `gzip`..
> -['zstd', 'ZSTD (' + gettext('fast and good') + ')'],
> +['zstd', 'Zstd (' + gettext('fast and good') + ')'],
> ],
> });
> diff --git a/www/manager6/panel/BackupAdvancedOptions.js
> b/www/manager6/panel/BackupAdvancedOptions.js
> index c79c31cb..a9c8a05a 100644
> --- a/www/manager6/panel/BackupAdvancedOptions.js
> +++ b/www/manager6/panel/BackupAdvancedOptions.js
> @@ -136,7 +136,7 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
> endFlex: 2,
> endColumn: {
> xtype: 'displayfield',
> - value: `${gettext('Threads used for zstd compression
> (non-PBS).')} ${Ext.String.format(gettext("Schema default: {0}"), 1)}`,
> + value: `${gettext('Threads used for Zstd compression
> (non-PBS).')} ${Ext.String.format(gettext("Schema default: {0}"), 1)}`,
> },
> },
> {
> diff --git a/www/manager6/window/DownloadUrlToStorage.js
> b/www/manager6/window/DownloadUrlToStorage.js
> index 5523a152..f165e3a9 100644
> --- a/www/manager6/window/DownloadUrlToStorage.js
> +++ b/www/manager6/window/DownloadUrlToStorage.js
> @@ -227,7 +227,7 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
> ['__default__', Proxmox.Utils.NoneText],
> ['lzo', 'LZO'],
> ['gz', 'GZIP'],
> - ['zst', 'ZSTD'],
> + ['zst', 'Zstd'],
> ],
> cbind: {
> hidden: get => get('content') !== 'iso',
> diff --git a/www/manager6/window/GuestImport.js
> b/www/manager6/window/GuestImport.js
> index 944d275b..69e7c9bb 100644
> --- a/www/manager6/window/GuestImport.js
> +++ b/www/manager6/window/GuestImport.js
> @@ -924,6 +924,7 @@ Ext.define('PVE.window.GuestImport', {
> 'cdrom-image-ignored': gettext("CD-ROM images cannot get
> imported, if required you can reconfigure the '{0}' drive in the 'Advanced'
> tab."),
> 'nvme-unsupported': gettext("NVMe disks are currently not
> supported, '{0}' will get attaced as SCSI"),
> 'ovmf-with-lsi-unsupported': gettext("OVMF is built without LSI
> drivers, scsi hardware was set to '{1}'"),
> + 'ovmf-with-lsi-unsupported': gettext("OVMF is built without LSI
> drivers, SCSI hardware was set to '{1}'"),
if the two corrections were separate patches, I could have already
applied this one, alas..
> 'serial-port-socket-only': gettext("Serial socket '{0}' will be
> mapped to a socket"),
> 'guest-is-running': gettext('Virtual guest seems to be running
> on source host. Import might fail or have inconsistent state!'),
> 'efi-state-lost': Ext.String.format(
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-firewall 1/1] rules: allow vital ICMP(v6) types
On June 10, 2024 2:52 pm, Stefan Hanreich wrote:
> There are certain ICMP messages that should always pass through a
> firewall irregardless of any other rules. This is particularly
> important for ICMPv6. While we already handled NDP, there are certain
> control messages that should always be able to pass through any
> firewall, according to RFC 4890.
>
> For ICMP we additionally allow 'Source Quench' as well.
>
> Signed-off-by: Stefan Hanreich
> ---
> While Source Quench is deprecated, there might be niche use cases
> using it and allowing it shouldn't really hurt so I've thrown it into
> the mix as well.
>
> .../resources/proxmox-firewall.nft| 22 +--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/proxmox-firewall/resources/proxmox-firewall.nft
> b/proxmox-firewall/resources/proxmox-firewall.nft
> index 537ba88..ea2cd7d 100644
> --- a/proxmox-firewall/resources/proxmox-firewall.nft
> +++ b/proxmox-firewall/resources/proxmox-firewall.nft
> @@ -16,6 +16,7 @@ add chain inet proxmox-firewall allow-ndp-out
> add chain inet proxmox-firewall block-ndp-out
> add chain inet proxmox-firewall block-conntrack-invalid
> add chain inet proxmox-firewall block-smurfs
> +add chain inet proxmox-firewall allow-icmp
> add chain inet proxmox-firewall log-drop-smurfs
> add chain inet proxmox-firewall default-in
> add chain inet proxmox-firewall default-out
> @@ -32,6 +33,7 @@ add chain bridge proxmox-firewall-guests allow-ndp-out
> add chain bridge proxmox-firewall-guests block-ndp-out
> add chain bridge proxmox-firewall-guests allow-ra-out
> add chain bridge proxmox-firewall-guests block-ra-out
> +add chain bridge proxmox-firewall-guests allow-icmp
> add chain bridge proxmox-firewall-guests do-reject
> add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting
> priority 0; policy accept;}
> add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting
> priority 0; policy accept;}
> @@ -47,6 +49,7 @@ flush chain inet proxmox-firewall allow-ndp-out
> flush chain inet proxmox-firewall block-ndp-out
> flush chain inet proxmox-firewall block-conntrack-invalid
> flush chain inet proxmox-firewall block-smurfs
> +flush chain inet proxmox-firewall allow-icmp
> flush chain inet proxmox-firewall log-drop-smurfs
> flush chain inet proxmox-firewall default-in
> flush chain inet proxmox-firewall default-out
> @@ -63,6 +66,7 @@ flush chain bridge proxmox-firewall-guests allow-ndp-out
> flush chain bridge proxmox-firewall-guests block-ndp-out
> flush chain bridge proxmox-firewall-guests allow-ra-out
> flush chain bridge proxmox-firewall-guests block-ra-out
> +flush chain bridge proxmox-firewall-guests allow-icmp
> flush chain bridge proxmox-firewall-guests do-reject
> flush chain bridge proxmox-firewall-guests vm-out
> flush chain bridge proxmox-firewall-guests vm-in
> @@ -175,9 +179,16 @@ table inet proxmox-firewall {
> drop
> }
>
> +chain allow-icmp {
> +icmp type { destination-unreachable, source-quench, time-exceeded }
> accept
> +# based on RFC 4890 - NDP is handled separately
> +icmpv6 type { destination-unreachable, packet-too-big,
> time-exceeded, parameter-problem } accept
> +}
> +
> chain default-in {
> iifname "lo" accept
>
> +jump allow-icmp
> ct state related,established accept
>
> meta l4proto igmp accept
> @@ -185,8 +196,6 @@ table inet proxmox-firewall {
> tcp dport { 8006, 5900-5999, 3128, 22 } jump accept-management
> udp dport 5405-5412 accept
>
> -meta l4proto icmp icmp type { destination-unreachable, time-exceeded
> } accept
> -
> # Drop Microsoft SMB noise
> udp dport { 135, 137-139, 445 } goto do-reject
> udp sport 137 udp dport 1024-65535 goto do-reject
> @@ -203,6 +212,7 @@ table inet proxmox-firewall {
> chain default-out {
> oifname "lo" accept
>
> +jump allow-icmp
> ct state vmap { invalid : drop, established : accept, related :
> accept }
> }
>
> @@ -284,6 +294,12 @@ table bridge proxmox-firewall-guests {
> icmpv6 type { nd-router-advert, nd-redirect } drop
> }
>
> +chain allow-icmp {
> +icmp type { destination-unreachable, source-quench, time-exceeded }
> accept
> +# based on RFC 4890 - NDP is handled separately
> +icmpv6 type { destination-unreachable, packet-too-big,
> time-exceeded, parameter-problem } accept
> +}
> +
> chain do-reject {
> meta pkttype broadcast drop
> ip saddr 224.0.0.0/4 drop
> @@ -297,12 +313,14 @@ table bridge proxmox-firewall-guests {
>
> chain vm-out {
> type filter hook prerouting priority 0; policy accept;
> +jump allow-icmp
> ether type != arp ct state vmap { established : accept, related :
> accept, invalid : drop }
> iifname vmap @vm-map-out
> }
>
>
Re: [pve-devel] [PATCH proxmox-offline-mirror 2/2] build: execute dh-cargo-built-using
On July 10, 2024 1:03 pm, Fabian Grünbichler wrote: > On July 10, 2024 12:42 pm, Fabian Grünbichler wrote: >> while it is pending a bug fix at the moment to properly pick up all >> dependencies, this ensures the X-Cargo-Built-Using (and soon, >> Static-Built-Using) substvars are actually filled with contents, and allow to >> find out which rustc version and dependency versions were used to build a >> particular binary package. > > correction: it's not dh-cargo that's broken, somehow when running plain > `make deb` our top-level cargo config is still preferred over the one > generate by the cargo wrapper, and thus the crates are picked up > directly from /usr/share/cargo/registry instead of via the symlinks in > debian/cargo_registry like dh-cargo-built-using expects.. > > a build in a clean env via source package/sbuild works as expected. I'll > try to debug this further! found the culprit, sent v2 ;) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror v2 2/2] build: execute dh-cargo-built-using
this ensures the X-Cargo-Built-Using (and soon, Static-Built-Using) substvars are actually filled with contents, and allow to find out which rustc version and dependency versions were used to build a particular binary package. Signed-off-by: Fabian Grünbichler --- debian/rules | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debian/rules b/debian/rules index 5a34545..0d3c755 100644 --- a/debian/rules +++ b/debian/rules @@ -35,6 +35,9 @@ override_dh_auto_test: # skip for now to avoid additional debug builds - no tests anyway # dh_auto_test -- test --all +execute_after_dh_auto_install: + /usr/share/cargo/bin/dh-cargo-built-using $(DEB_CARGO_PACKAGE) + override_dh_missing: dh_missing --fail-missing -- 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 proxmox-offline-mirror v2 1/2] build: use cargo wrapper
for package builds to ensure all common flags are actually set.
Signed-off-by: Fabian Grünbichler
---
Notes:
v2: symlink wrapper config in place
Makefile | 9 +++--
debian/rules | 11 ---
docs/Makefile | 4 ++--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index c652bda..ab23b73 100644
--- a/Makefile
+++ b/Makefile
@@ -19,10 +19,15 @@ DEBS = $(DEB) $(HELPER_DEB) $(DBG_DEB) $(HELPER_DBG_DEB)
$(DOC_DEB)
ifeq ($(BUILD_MODE), release)
CARGO_BUILD_ARGS += --release
COMPILEDIR := target/release
+else ifeq ($(BUILD_MODE), release-deb)
+CARGO_BUILD_ARGS += --release
+COMPILEDIR := target/$(DEB_HOST_RUST_TYPE)/release
else
COMPILEDIR := target/debug
endif
+CARGO ?= cargo
+
USR_BIN := \
proxmox-offline-mirror \
proxmox-offline-mirror-helper
@@ -34,7 +39,7 @@ all: cargo-build $(SUBDIRS)
.PHONY: cargo-build
cargo-build:
- cargo build $(CARGO_BUILD_ARGS)
+ $(CARGO) build $(CARGO_BUILD_ARGS)
.PHONY: $(SUBDIRS)
$(SUBDIRS): cargo-build
@@ -98,7 +103,7 @@ distclean: clean
.PHONY: clean
clean:
- cargo clean
+ $(CARGO) clean
rm -f *.deb *.build *.buildinfo *.changes *.dsc rust-$(PACKAGE)*.tar*
rm -rf $(PACKAGE)-[0-9]*/
find . -name '*~' -exec rm {} ';'
diff --git a/debian/rules b/debian/rules
index 58b733f..5a34545 100644
--- a/debian/rules
+++ b/debian/rules
@@ -3,13 +3,13 @@
include /usr/share/dpkg/pkg-info.mk
include /usr/share/rustc/architecture.mk
-export BUILD_MODE=release
+export BUILD_MODE=release-deb
-CARGO=/usr/share/cargo/bin/cargo
+export CARGO=/usr/share/cargo/bin/cargo
export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS
export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE
-export CARGO_HOME = $(CURDIR)/debian/cargo_home
+export CARGO_HOME=$(CURDIR)/debian/cargo_home
export DEB_CARGO_CRATE=proxmox-offline-mirror_$(DEB_VERSION_UPSTREAM)
export DEB_CARGO_PACKAGE=proxmox-offline-mirror
@@ -24,6 +24,11 @@ override_dh_auto_configure:
@perl -ne 'if (/^version\s*=\s*"(\d+(?:\.\d+)+)"/) { my $$v_cargo =
$$1; my $$v_deb = "$(DEB_VERSION_UPSTREAM)"; \
die "ERROR: d/changelog <-> Cargo.toml version mismatch: $$v_cargo
!= $$v_deb\n" if $$v_cargo ne $$v_deb; exit(0); }' Cargo.toml
$(CARGO) prepare-debian $(CURDIR)/debian/cargo_registry
--link-from-system
+ # `cargo build` and `cargo install` have different config precedence,
symlink
+ # the wrapper config into a place where `build` picks it up as well..
+ #
https://doc.rust-lang.org/cargo/commands/cargo-install.html#configuration-discovery
+ mkdir -p $(CURDIR)/.cargo
+ ln -s $(CARGO_HOME)/config.toml $(CURDIR)/.cargo/config.toml
dh_auto_configure
override_dh_auto_test:
diff --git a/docs/Makefile b/docs/Makefile
index 973355a..fa52867 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -21,10 +21,10 @@ SPHINXBUILD = sphinx-build
BUILDDIR = output
ifeq ($(BUILD_MODE), release)
-COMPILEDIR := ../target/release
+COMPILEDIR := ../target/$release
SPHINXOPTS+= -t release
else ifeq ($(BUILD_MODE), release-deb)
-COMPILEDIR :=
../target/$(DEB_TARGET_GNU_CPU)-unknown-$(DEB_TARGET_GNU_SYSTEM)/release
+COMPILEDIR := ../target/$(DEB_HOST_RUST_TYPE)/release
SPHINXOPTS+= -t release
else
COMPILEDIR := ../target/debug
--
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 proxmox-offline-mirror 2/2] build: execute dh-cargo-built-using
On July 10, 2024 12:42 pm, Fabian Grünbichler wrote: > while it is pending a bug fix at the moment to properly pick up all > dependencies, this ensures the X-Cargo-Built-Using (and soon, > Static-Built-Using) substvars are actually filled with contents, and allow to > find out which rustc version and dependency versions were used to build a > particular binary package. correction: it's not dh-cargo that's broken, somehow when running plain `make deb` our top-level cargo config is still preferred over the one generate by the cargo wrapper, and thus the crates are picked up directly from /usr/share/cargo/registry instead of via the symlinks in debian/cargo_registry like dh-cargo-built-using expects.. a build in a clean env via source package/sbuild works as expected. I'll try to debug this further! > > Signed-off-by: Fabian Grünbichler > --- > debian/rules | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/debian/rules b/debian/rules > index 9729be1..badace9 100644 > --- a/debian/rules > +++ b/debian/rules > @@ -30,6 +30,9 @@ override_dh_auto_test: > # skip for now to avoid additional debug builds - no tests anyway > # dh_auto_test -- test --all > > +execute_after_dh_auto_install: > + /usr/share/cargo/bin/dh-cargo-built-using $(DEB_CARGO_PACKAGE) > + > override_dh_missing: > dh_missing --fail-missing > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror 2/2] build: execute dh-cargo-built-using
while it is pending a bug fix at the moment to properly pick up all dependencies, this ensures the X-Cargo-Built-Using (and soon, Static-Built-Using) substvars are actually filled with contents, and allow to find out which rustc version and dependency versions were used to build a particular binary package. Signed-off-by: Fabian Grünbichler --- debian/rules | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debian/rules b/debian/rules index 9729be1..badace9 100644 --- a/debian/rules +++ b/debian/rules @@ -30,6 +30,9 @@ override_dh_auto_test: # skip for now to avoid additional debug builds - no tests anyway # dh_auto_test -- test --all +execute_after_dh_auto_install: + /usr/share/cargo/bin/dh-cargo-built-using $(DEB_CARGO_PACKAGE) + override_dh_missing: dh_missing --fail-missing -- 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 proxmox-offline-mirror 1/2] build: use cargo wrapper
for package builds to ensure all common flags are actually set.
Signed-off-by: Fabian Grünbichler
---
Makefile | 9 +++--
debian/rules | 4 ++--
docs/Makefile | 4 ++--
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index c652bda..ab23b73 100644
--- a/Makefile
+++ b/Makefile
@@ -19,10 +19,15 @@ DEBS = $(DEB) $(HELPER_DEB) $(DBG_DEB) $(HELPER_DBG_DEB)
$(DOC_DEB)
ifeq ($(BUILD_MODE), release)
CARGO_BUILD_ARGS += --release
COMPILEDIR := target/release
+else ifeq ($(BUILD_MODE), release-deb)
+CARGO_BUILD_ARGS += --release
+COMPILEDIR := target/$(DEB_HOST_RUST_TYPE)/release
else
COMPILEDIR := target/debug
endif
+CARGO ?= cargo
+
USR_BIN := \
proxmox-offline-mirror \
proxmox-offline-mirror-helper
@@ -34,7 +39,7 @@ all: cargo-build $(SUBDIRS)
.PHONY: cargo-build
cargo-build:
- cargo build $(CARGO_BUILD_ARGS)
+ $(CARGO) build $(CARGO_BUILD_ARGS)
.PHONY: $(SUBDIRS)
$(SUBDIRS): cargo-build
@@ -98,7 +103,7 @@ distclean: clean
.PHONY: clean
clean:
- cargo clean
+ $(CARGO) clean
rm -f *.deb *.build *.buildinfo *.changes *.dsc rust-$(PACKAGE)*.tar*
rm -rf $(PACKAGE)-[0-9]*/
find . -name '*~' -exec rm {} ';'
diff --git a/debian/rules b/debian/rules
index 58b733f..9729be1 100644
--- a/debian/rules
+++ b/debian/rules
@@ -3,9 +3,9 @@
include /usr/share/dpkg/pkg-info.mk
include /usr/share/rustc/architecture.mk
-export BUILD_MODE=release
+export BUILD_MODE=release-deb
-CARGO=/usr/share/cargo/bin/cargo
+export CARGO=/usr/share/cargo/bin/cargo
export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS
export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE
diff --git a/docs/Makefile b/docs/Makefile
index 973355a..fa52867 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -21,10 +21,10 @@ SPHINXBUILD = sphinx-build
BUILDDIR = output
ifeq ($(BUILD_MODE), release)
-COMPILEDIR := ../target/release
+COMPILEDIR := ../target/$release
SPHINXOPTS+= -t release
else ifeq ($(BUILD_MODE), release-deb)
-COMPILEDIR :=
../target/$(DEB_TARGET_GNU_CPU)-unknown-$(DEB_TARGET_GNU_SYSTEM)/release
+COMPILEDIR := ../target/$(DEB_HOST_RUST_TYPE)/release
SPHINXOPTS+= -t release
else
COMPILEDIR := ../target/debug
--
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-series: [PATCH proxmox-offline-mirror v2 0/5] remove snapshot directories vanished on source also on medium
thanks! On July 9, 2024 12:47 pm, Stoiko Ivanov wrote: > supersedes: > https://lists.proxmox.com/pipermail/pve-devel/2024-June/064278.html > > v1->v2: > * had quite a few chats with Fabian off-list, as noted by him - Thanks! > * noticed that the core-issue was probably an error in using path.is_empty > (which checks for an empty path '', not for an empty directory) fixed with > patch 3/5 > * noticed that the current gc-code removes orphaned files (a.k.a. files > not hardlinked to a checksum file) in any case - so dropped the idea of > an explicit command-line switch (as sync_pool calls gc on the target > pool as well) - can gladly rework both to add this as explicit > safe-guard, but currently do not think it has much merit > * patch 1/5 addresses the recent changes to proxmox-apt - I hope it's ok > as is. > * patch 2/5 is unrelated, but it confused me enough while going through > the code. > > tested with a local setup. > > original cover-letter for v1: > > This patchset fixes a small glitch that we noticed in a pom-setup, creating > regular snapshots, without cleaning them up regularly. > > Eventually medium sync becomes quite slow. > After removing many snapshots and running garbage collection both on the > mirror as well as on the medium the run-time for the sync still took quite > long. strace showed that the process still walked through the directories > for each snapshot on the medium - they were not cleaned up after all the > files inside were removed. > > tested the patch locally (which is the reason for patch 1/2). > > > Stoiko Ivanov (5): > bump proxmox-apt to 0.11 and adapt to changes. > pool: drop superfluous check for impossible path combination > pool: unlink_file: fix check for empty directory > pool: gc: remove empty directories under link_dir > pool: remove unused imports > > Cargo.toml | 3 ++- > debian/control | 3 ++- > src/lib.rs | 5 +++-- > src/mirror.rs | 11 +-- > src/pool.rs| 24 ++-- > 5 files changed, 26 insertions(+), 20 deletions(-) > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Bug/enhancement 4300 implementation
> Francois Prowse via pve-devel hat am 10.07.2024 > 02:11 CEST geschrieben: > Is it possible to understand how the enhancement (4300) I submitted and > implemented by the dev team will be rolled into the master branch? the patch series got reviewed and is waiting for a v2 as far as I can tell: https://lists.proxmox.com/pipermail/pve-devel/2024-June/064222.html (and replies) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu] zeroinit: fix regression with filename parsing
with the missing link added to the commit message.
On July 8, 2024 12:09 pm, Fiona Ebner wrote:
> As reported in the community forum [0], cloning or importing images
> to RBD storages (without the krbd setting) was broken. This is a
> result of no filename parsing happening anymore in bdrv_open_child()
> after commit b242e7f ("backport fix for CVE-2024-4467"), which the
> zeroinit relied on for passing along the RBD filename+key-value pairs.
>
> There is a dedicated function for opening the file child which still
> does filename parsing. Use that for opening the file child. Role and
> flags should still be the same as with the manual bdrv_open_child(),
> because the zeroinit driver is a filter, and the assignment bs->file
> is also done by bdrv_open_file_child().
>
> Fixes: b242e7f ("backport fix for CVE-2024-4467")
> Signed-off-by: Fiona Ebner
> ---
> ...add-the-zeroinit-block-driver-filter.patch | 24 +++
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git
> a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
>
> b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> index 34a7efe..7464ca5 100644
> ---
> a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> +++
> b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> @@ -5,12 +5,13 @@ Subject: [PATCH] PVE: block: add the zeroinit block driver
> filter
>
> Signed-off-by: Thomas Lamprecht
> [FE: adapt to changed function signatures
> - adhere to block graph lock requirements]
> + adhere to block graph lock requirements
> + use dedicated function to open file child]
> Signed-off-by: Fiona Ebner
> ---
> block/meson.build | 1 +
> - block/zeroinit.c | 214 ++
> - 2 files changed, 215 insertions(+)
> + block/zeroinit.c | 207 ++
> + 2 files changed, 208 insertions(+)
> create mode 100644 block/zeroinit.c
>
> diff --git a/block/meson.build b/block/meson.build
> @@ -27,10 +28,10 @@ index e1f03fd773..b530e117b5 100644
> system_ss.add(when: 'CONFIG_TCG', if_true: files('blkreplay.c'))
> diff --git a/block/zeroinit.c b/block/zeroinit.c
> new file mode 100644
> -index 00..696558d8d6
> +index 00..7998c9332d
> --- /dev/null
> +++ b/block/zeroinit.c
> -@@ -0,0 +1,214 @@
> +@@ -0,0 +1,207 @@
> +/*
> + * Filter to fake a zero-initialized block device.
> + *
> @@ -96,7 +97,6 @@ index 00..696558d8d6
> + Error **errp)
> +{
> +BDRVZeroinitState *s = bs->opaque;
> -+BdrvChild *file = NULL;
> +QemuOpts *opts;
> +Error *local_err = NULL;
> +int ret;
> @@ -112,15 +112,9 @@ index 00..696558d8d6
> +}
> +
> +/* Open the raw file */
> -+file = bdrv_open_child(qemu_opt_get(opts, "x-next"), options, "next",
> bs,
> -+ _of_bds,
> -+ BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, false,
> -+ _err);
> -+bdrv_graph_wrlock();
> -+bs->file = file;
> -+bdrv_graph_wrunlock();
> -+if (local_err) {
> -+ret = -EINVAL;
> ++ret = bdrv_open_file_child(qemu_opt_get(opts, "x-next"), options,
> "next",
> ++ bs, _err);
> ++if (ret < 0) {
> +error_propagate(errp, local_err);
> +goto fail;
> +}
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC qemu] fix #3231+#3631: PVE backup: fail backup rather than guest write when backup target cannot be reached or is too slow
Quoting Fiona Ebner (2024-06-10 14:59:35) > A long-standing issue with VM backups in Proxmox VE is that a slow or > unreachable target would lead to a copy-before-write (cbw) operation > to break the guest write rather than abort the backup. This is > unexpected to users and the will end up without a successful backup > and without a working guest in such cases. This series prevents the > latter by changing the behavior to fail the backup instead of the > guest write. > > This is done by re-using the already existing 'on-cbw-error' and > 'cbw-timeout' options that are already used for fleecing and having > regular backup also check for the cbw's snapshot_error (unfortunately > this becomes a bit of a misnomer). If a given copy-before-write > operation cannot complete within 45 seconds, it's extremely likely > that aborting the backup is the better choice than keeping the guest > IO blocked. > > Just checking for the error already makes it work (i.e. without the > last two patches), but backup can only check the error at the end. To > abort backup immediately, an error callback for the copy-before-write > node is introduced. A potential alternative would be give the > block-copy operation a pointer to the snapshot_error and have it check > it during its operation, but my initial attempt failed. Likely I > missed adapting certain logic that checks for whether the block-copy > operation failed and it's questionable if this approach would be > cleaner. An error callback is nice and explicit. > > Note for testers: if e.g. the PBS is compeletly unreachable, the > backup job still will need to wait until the in-flight request is > aborted after 15 minutes. But the guest writes should be fast again. > > Should it really be required to make the option more flexible, i.e. > allow users to specify a custom timeout or go back to the old behavior > then the 'backup' QMP call can be extended with those parameters. > > Unfortunately, this is a non-trivial amount of code to make it work, > but there is quite a bit of boilerplate and some comments, so > hopefully the logic is straight-forward enough. this sentence here made me except a lot worse ;) code seems very straight-forward and clean, two small comments inline. not sure whether we want to entangle this with 9.0, but I think this could be applied soonish after some more in-depth testing, since it should solve a pretty big pain point user consistenly run into.. I am sure we will have users clamoring for a configurable timeout soon after though ;) > > The first patch can be applied regardless of whether we want to go > with this or not. > > > Fiona Ebner (7): > PVE backup: fleecing: properly set minimum cluster size > block/copy-before-write: allow passing additional options for > bdrv_cbw_append() > block/backup: allow passing additional options for copy-before-write > upon job creation > block/backup: make cbw error also fail backup that does not use > fleecing > fix #3231+#3631: PVE backup: add timeout for copy-before-write > operations and fail backup instead of guest writes > block/copy-before-write: allow specifying error callback > block/backup: set callback for cbw errors > > block/backup.c | 57 +- > block/copy-before-write.c | 41 +++--- > block/copy-before-write.h | 9 +++- > block/replication.c| 2 +- > blockdev.c | 2 +- > include/block/block_int-global-state.h | 2 + > pve-backup.c | 13 +- > 7 files changed, 115 insertions(+), 11 deletions(-) > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC qemu 7/7] block/backup: set callback for cbw errors
Quoting Fiona Ebner (2024-06-10 14:59:42)
> The callback is invoked when cbw is configured to not break the guest
> write and will abort a backup job immediately. Currently the backup
> has to wait for the rest of the block copy operation to finish before
> checking the cbw error state.
>
> Signed-off-by: Fiona Ebner
> ---
>
> Note for testers: if e.g. the PBS is compeletly unreachable, the
> backup job still will need to wait until the in-flight request is
> aborted after 15 minutes. But the guest writes should be fast again.
could we improve that by checking the status in the pbs-qemu lib periodically,
and aborting there as well?
how is the bitmap handled in case of a cbw-timeout/error?
>
> block/backup.c | 41 +
> 1 file changed, 41 insertions(+)
>
> diff --git a/block/backup.c b/block/backup.c
> index ba153110d3..43d34ce4c2 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -32,6 +32,45 @@
>
> static const BlockJobDriver backup_job_driver;
>
> +typedef struct {
> +Job *job;
> +int ret;
> +} BackupOnCbwError;
> +
> +static void backup_on_cbw_error_cb_bh(void *opaque)
> +{
> +BackupOnCbwError *data = opaque;
> +if (data->job) {
> +WITH_JOB_LOCK_GUARD() {
> +if (!job_is_completed_locked(data->job)) {
> +error_report("backup was cancelled because of
> copy-before-write error: %s",
> + strerror(-data->ret));
> +job_cancel_locked(data->job, true);
> +}
> +}
> +} else {
> +error_report("backup_on_cbw_error_cb_bh: no job! Error: %s",
> strerror(-data->ret));
> +}
> +
> +g_free(data);
> +}
> +
> +static void backup_on_cbw_error_cb(void *opaque, int ret)
> +{
> +BackupOnCbwError *data = g_new0(BackupOnCbwError, 1);
> +data->job = opaque;
> +data->ret = ret;
> +
> +/*
> + * backup_cancel() cannot run in coroutine context.
> + */
> +if (qemu_in_coroutine()) {
> +aio_bh_schedule_oneshot(qemu_get_aio_context(),
> backup_on_cbw_error_cb_bh, data);
> +} else {
> +backup_on_cbw_error_cb_bh(data);
> +}
> +}
> +
> static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
> {
> BdrvDirtyBitmap *bm;
> @@ -477,6 +516,8 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
> goto error;
> }
>
> +bdrv_cbw_set_error_cb(cbw, backup_on_cbw_error_cb, job);
> +
> job->cbw = cbw;
> job->source_bs = bs;
> job->target_bs = target;
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC qemu 3/7] block/backup: allow passing additional options for copy-before-write upon job creation
Quoting Fiona Ebner (2024-06-10 14:59:38)
> In particular, useful for setting the 'on-cbw-error' and 'cbw-timeout'
> options (see BlockdevOptionsCbw in QAPI).
>
> Signed-off-by: Fiona Ebner
> ---
> block/backup.c | 10 +++---
> block/replication.c| 2 +-
> blockdev.c | 2 +-
> include/block/block_int-global-state.h | 2 ++
> pve-backup.c | 2 +-
> 5 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index e0acfe6758..82fedf1680 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -336,6 +336,7 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
>bool compress, bool discard_source,
>const char *filter_node_name,
>BackupPerf *perf,
> + QDict *cbw_opts,
>BlockdevOnError on_source_error,
>BlockdevOnError on_target_error,
>int creation_flags,
> @@ -347,7 +348,6 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
> int64_t cluster_size;
> BlockDriverState *cbw = NULL;
> BlockCopyState *bcs = NULL;
> -QDict *cbw_opts = NULL;
>
> assert(bs);
> assert(target);
> @@ -436,8 +436,12 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
> }
>
> if (perf->has_min_cluster_size) {
> -cbw_opts = qdict_new();
> -qdict_put_int(cbw_opts, "min-cluster-size", perf->min_cluster_size);
> +if (!cbw_opts) {
> +cbw_opts = qdict_new();
> +}
> +if (!qdict_haskey(cbw_opts, "min-cluster-size")) {
> +qdict_put_int(cbw_opts, "min-cluster-size",
> perf->min_cluster_size);
so here cbw_opts "wins" without any indication that this happens
> +}
> }
>
> cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
> diff --git a/block/replication.c b/block/replication.c
> index 0415a5e8b7..c5a27f593e 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -583,7 +583,7 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> s->backup_job = backup_job_create(
> NULL, s->secondary_disk->bs,
> s->hidden_disk->bs,
> 0, MIRROR_SYNC_MODE_NONE, NULL, 0, false,
> false,
> -NULL, ,
> +NULL, , NULL,
> BLOCKDEV_ON_ERROR_REPORT,
> BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
> backup_job_completed, bs, NULL, _err);
> diff --git a/blockdev.c b/blockdev.c
> index cbe224387b..55ca967430 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2732,7 +2732,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
> backup->sync, bmap, backup->bitmap_mode,
> backup->compress, backup->discard_source,
> backup->filter_node_name,
> -,
> +, NULL,
> backup->on_source_error,
> backup->on_target_error,
> job_flags, NULL, NULL, txn, errp);
> diff --git a/include/block/block_int-global-state.h
> b/include/block/block_int-global-state.h
> index f0c642b194..7332680087 100644
> --- a/include/block/block_int-global-state.h
> +++ b/include/block/block_int-global-state.h
> @@ -179,6 +179,7 @@ void mirror_start(const char *job_id, BlockDriverState
> *bs,
> * @bitmap_mode: The bitmap synchronization policy to use.
> * @perf: Performance options. All actual fields assumed to be present,
> *all ".has_*" fields are ignored.
> + * @cbw_opts: Additional options to configure cbw filter with.
from this, I'd have guessed the other way round ;) should the precedence be
made explicit somewhere? or, for this particular option, should the higher
value win? but such logic might quickly get complicated once more parameters
need to be handled..
> * @on_source_error: The action to take upon error reading from the source.
> * @on_target_error: The action to take upon error writing to the target.
> * @creation_flags: Flags that control the behavior of the Job lifetime.
> @@ -198,6 +199,7 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
> bool compress, bool discard_source,
> const char *filter_node_name,
> BackupPerf *perf,
> +QDict *cbw_opts,
> BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> int creation_flags,
> diff --git
Re: [pve-devel] [PATCH pve-container 2/2] fix #5339: api: lxc: ip: add 'all' option so that all addresses can be returned.
> Wolfgang Bumiller hat am 05.07.2024 09:45 CEST
> geschrieben:
>
>
> On Tue, Jul 02, 2024 at 04:29:25PM GMT, Fabian Grünbichler wrote:
> > apologies again for the long delay!
> >
> > > Johannes Cornelis Draaijer via pve-devel
> > > hat am 18.04.2024 22:49 CEST geschrieben:
> >
> > > Signed-off-by: Johannes Cornelis Draaijer
> > > ---
> > > src/PVE/API2/LXC.pm | 16 +---
> > > src/PVE/LXC.pm | 9 +++--
> > > 2 files changed, 20 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> > > index 89ba64c..3561317 100644
> > > --- a/src/PVE/API2/LXC.pm
> > > +++ b/src/PVE/API2/LXC.pm
> > > @@ -2533,6 +2533,12 @@ __PACKAGE__->register_method({
> > > properties => {
> > > node => get_standard_option('pve-node'),
> > > vmid => get_standard_option('pve-vmid', { completion =>
> > > \::LXC::complete_ctid }),
> > > + all => {
> > > + type => 'boolean',
> > > + default => 0,
> > > + optional => 1,
> > > + description => 'Return all adresses of each interface instead
> > > of only one',
> >
> > typo: s/adresses/addresses
> >
> > > + }
> > > },
> > > },
> > > returns => {
> > > @@ -2552,12 +2558,14 @@ __PACKAGE__->register_method({
> > > },
> > > inet => {
> > > type => 'string',
> > > - description => 'The IPv4 address of the interface',
> > > + format => 'CIDRv4-list',
> >
> > this format here and the code below don't agree. a string type with the
> > -list suffix needs actually be a string with the list elements delimited by
> > either space, ',' or ';'. in this case, comma or semicolon is probably okay.
> >
>
> Since the caller needs to specify a new parameter, shouldn't we just
> return an actual array instead?
the retun schema wouldn't match then for either variant of the call..
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs v7 19/19] notifications: backport some rephrased parts from PBS docs
Quoting Lukas Wagner (2024-06-10 10:40:38)
> Most of the changes were done when adapting the PVE docs to
> the new PBS notification system, so now we 'backport' those
> improvements.
>
> Signed-off-by: Lukas Wagner
> ---
> notifications.adoc | 99 +++---
> 1 file changed, 67 insertions(+), 32 deletions(-)
>
> diff --git a/notifications.adoc b/notifications.adoc
> index 9c5228c..bdfebd0 100644
> --- a/notifications.adoc
> +++ b/notifications.adoc
> @@ -9,37 +9,38 @@ Overview
>
> [thumbnail="screenshot/gui-datacenter-notification-overview.png"]
>
> -{pve} will send notifications if case of noteworthy events in the system.
> -
> -There are a number of different xref:notification_events[notification
> events],
> -each with their own set of metadata fields that can be used in
> -notification matchers.
> -
> -A xref:notification_matchers[notification matcher] determines
> -_which_ notifications shall be sent _where_.
> -A matcher has _match rules_, that can be used to
> -match on certain notification properties (e.g. timestamp, severity,
> -metadata fields).
> -If a matcher matches a notification, the notification will be routed
> -to a configurable set of notification targets.
> -
> -A xref:notification_targets[notification target] is an abstraction for a
> -destination where a notification should be sent to - for instance,
> -a Gotify server instance, or a set of email addresses.
> -There are multiple types of notification targets, including
> -`sendmail`, which uses the system's sendmail command to send emails,
> -or `gotify`, which sends a notification to a Gotify instance.
> +* {pve} emits xref:notification_events[Notification Events] in case of
> + storage replication failures, node fencing, finished/failed backups
> + and other events.
> + These events are handled by the notification system. A notification
> + event has metadata, for example a timestamp, a severity level,
> + a type, and other optional metadata fields.
> +* xref:notification_matchers[Notification Matchers] route a notification
> + event to one or more notification targets. A matcher can have match
> + rules to selectively route based on the metadata of a notification event.
> +* xref:notification_targets[Notification Targets] are a destination to
> + which a notification event is routed to by a matcher.
> + There are multiple types of target, mail-based (Sendmail and SMTP)
> + and Gotify.
> +
> +Backup jobs have a configurable xref:notification_mode[Notification Mode].
> +It allows you to choose between the notification system and a legacy mode
> +for sending notification emails. The legacy mode is equivalent to the
> +way notifications were handled before {pve} 8.1.
>
> The notification system can be configured in the GUI under
> -Datacenter -> Notifications. The configuration is stored in
> +Datacenter → Notifications. The configuration is stored in
> `/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` -
> the latter contains sensitive configuration options such as
> -passwords or authentication tokens for notification targets.
> +passwords or authentication tokens for notification targets and can
> +only be read by `root`.
>
> [[notification_targets]]
> Notification Targets
>
>
> +{pve} offers multiple types of notification targets.
> +
> [[notification_targets_sendmail]]
> Sendmail
>
> @@ -50,14 +51,19 @@ that handles the sending of email messages.
> It is a command-line utility that allows users and applications to send
> emails
> directly from the command line or from within scripts.
>
> -The sendmail notification target uses the `sendmail` binary to send emails.
> -
> +The sendmail notification target uses the `sendmail` binary to send emails
> to a
> +list of configured users or email addresses. If a user is selected as a
> recipient,
> +the email address configured in user's settings will be used.
> +For the `root@pam` user, this is the email address entered during
> installation.
> +A user's email address can be configured in
> +`Datacenter → Permissions → Users`.
> +If a user has no associated email address, no email will be sent.
>
> NOTE: In standard {pve} installations, the `sendmail` binary is provided by
> -Postfix. For this type of target to work correctly, it might be necessary to
> -change Postfix's configuration so that it can correctly deliver emails.
> -For cluster setups it is necessary to have a working Postfix configuration on
> -every single cluster node.
> +Postfix. It may be necessary to configure Postfix so that it can deliver
> +mails correctly - for example by setting an external mail relay (smart host).
> +In case of failed delivery, check the system logs for messages logged by
> +the Postfix daemon.
>
> The configuration for Sendmail target plugins has the following options:
>
> @@ -90,6 +96,12 @@ SMTP
>
[pve-devel] applied: [PATCH docs v7 18/19] notifications: fix typo in 'notification'
with some context adaptations Quoting Lukas Wagner (2024-06-10 10:40:37) > Signed-off-by: Lukas Wagner > --- > notifications.adoc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/notifications.adoc b/notifications.adoc > index 07f0b3e..9c5228c 100644 > --- a/notifications.adoc > +++ b/notifications.adoc > @@ -295,7 +295,7 @@ Notification Events > [width="100%",options="header"] > |=== > | Field name| Description > -| `type`| Type of the notifcation > +| `type`| Type of the notification > | `hostname`| Hostname, without domain (e.g. `pve1`) > | `job-id` | Job ID > |=== > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain
Quoting Lukas Wagner (2024-06-10 10:40:34) > This was a bit inconsistent between the different notification types: > - APT/VZDump included the domain part > - fence notifications did not > > A decision has been made to unify this by removing the domain part > from APT/VZDump notifications. > > Signed-off-by: Lukas Wagner > --- > notifications.adoc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/notifications.adoc b/notifications.adoc > index 46aff6a..57053c8 100644 > --- a/notifications.adoc > +++ b/notifications.adoc > @@ -301,7 +301,7 @@ Notification Events > |=== > | Field name | Description > | `type` | Type of the notifcation > -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`) > +| `hostname` | Hostname, without domain (e.g. `pve1`) > |=== > > System Mail Forwarding > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager v7 07/19] api: replication: include 'hostname' field for notifications
and this one as well..
Quoting Lukas Wagner (2024-06-10 10:40:26)
> The field contains the hostname of the host (without any domain part)
> which sends the notification. This field can be used in match-field
> match rules.
>
> Signed-off-by: Lukas Wagner
> ---
> PVE/API2/Replication.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> index 6208a1a2..e4a7180f 100644
> --- a/PVE/API2/Replication.pm
> +++ b/PVE/API2/Replication.pm
> @@ -125,6 +125,8 @@ my sub _handle_job_err {
> my $metadata_fields = {
> type => "replication",
> "job-id" => $job->{id},
> + # Hostname (without domain part)
> + hostname => PVE::INotify::nodename(),
> };
>
> eval {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager v7 06/19] vzdump: apt: notification: do not include domain in 'hostname' field
independent as well..
Quoting Lukas Wagner (2024-06-10 10:40:25)
> - The man page warns about the usage of `hostname -f`, since a host
>may have multiple domains (or none at all)
> - The fallback PVE::INotify::nodename() already only returned the
>hostname without the domain part
> - Fencing notifications didn't include the domain part anyway
>
> This may result in soft-breakage for any users who have already relied
> on the domain being present. If there is need for it, it could include
> a fqdn metadata field.
>
> The hostname property used for rendering the notification template
> is unaffected for now.
>
> Signed-off-by: Lukas Wagner
> ---
> PVE/API2/APT.pm | 3 ++-
> PVE/VZDump.pm | 8
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
> index ec7c21b2..47c50961 100644
> --- a/PVE/API2/APT.pm
> +++ b/PVE/API2/APT.pm
> @@ -348,7 +348,8 @@ __PACKAGE__->register_method({
> # matchers.
> my $metadata_fields = {
> type => 'package-updates',
> - hostname => $hostname,
> + # Hostname (without domain part)
> + hostname => PVE::INotify::nodename(),
> };
>
> PVE::Notify::info(
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 2167b289..db3a02a9 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -517,10 +517,9 @@ sub send_notification {
> "See Task History for details!\n";
> };
>
> -my $hostname = get_hostname();
> -
> my $notification_props = {
> - "hostname" => $hostname,
> + # Hostname, might include domain part
> + "hostname" => get_hostname(),
> "error-message" => $err,
> "guest-table" => build_guest_table($tasklist),
> "logs" => $text_log_part,
> @@ -531,7 +530,8 @@ sub send_notification {
>
> my $fields = {
> type => "vzdump",
> - hostname => $hostname,
> + # Hostname (without domain part)
> + hostname => PVE::INotify::nodename(),
> };
> # Add backup-job metadata field in case this is a backup job.
> $fields->{'job-id'} = $job_id if $job_id;
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager v7 05/19] api: replication: add 'job-id' to notification metadata
since this one is independent of the things with open questions.
Quoting Lukas Wagner (2024-06-10 10:40:24)
> This allows users to create notification match rules for specific
> replication jobs, if they so desire.
>
> Signed-off-by: Lukas Wagner
> ---
> PVE/API2/Replication.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> index d84ac1ab..6208a1a2 100644
> --- a/PVE/API2/Replication.pm
> +++ b/PVE/API2/Replication.pm
> @@ -123,8 +123,8 @@ my sub _handle_job_err {
> };
>
> my $metadata_fields = {
> - # TODO: Add job-id?
> type => "replication",
> + "job-id" => $job->{id},
> };
>
> eval {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements
Quoting Lukas Wagner (2024-06-10 10:40:19) > This patch series attempts to improve the user experience when creating > notification matchers. > > Some of the noteworthy changes: > - Fixup inconsistent 'hostname' field. Some notification events sent > the hostname including a domain, while other did not. > This series unifies the behavior, now the field only includes the hostname > without a domain. Also updated the docs to reflect this change. > - Allow setting a custom backup job ID, similar how we handle it for > sync/prune jobs in PBS (to allow recognizable names used in matchers) > - Adding columns for backup job ID/replication job ID in the UI > - New metadata fields: > - job-id: Job ID for backup-jobs or replication-jobs > - Add an API that enumerates known notification metadata fields/values > - Suggest known fields/values in match rule window > - Some code clean up for match rule edit window > - Extended the 'exact' match-field mode - it now allows setting multiple > allowed values, separated via ',': > e.g. `match-field exact:type=replication,fencing > Originally, I created a separate 'list' match type for this, but > since the semantics for a list with one value and 'exact' mode > are identical, I decided to just extend 'exact'. > This is a safe change since there are are no values where a ',' > makes any sense (config IDs, hostnames) > > NOTE: Might need a versionened break, since the widget-toolkit-patches > depend on new APIs provided by pve-manager. If the API is not present, > creating matchers with 'match-field' does not work (cannot load lists > of known values/fields) > > Inter-Dependencies: > - the widget-toolkit dep in pve-manager needs to be bumped > to at least 4.1.4 > (we need "utils: add mechanism to add and override translatable > notification event > descriptions in the product specific UIs", otherwise the UI breaks > with the pve-manager patches applied) --> already included a patch for > this > - widget-toolkit relies on a new API endpoint provided by pve-manager: > --> we require a versioned break in widget-toolkit on pve-manager pve-guest-common is also needed by pve-manager AFAICT? and manual invocations of backup jobs are broken in a cluster if the target node is not yet upgraded, since that would set the still unknown job-id parameter.. combined with the "job-id value can't be trusted" aspect, it might be better to skip setting it for manual invocations? > > Changelog: > - v7: incorporated some more feedback from @Fiona, thx! > - Fixed error when switching from 'exact' to 'regex' if the text field > was empty > - rebased to latest master > - 'backport' doc improvements from PBS > - bumped widget-toolkit dep > - v6: incorporate feedback from @Fiona, thx! > - rename 'id' -> 'job-id' in VZDump API handler > - consolidate 'replication-job'/'backup-job' to 'job-id' > - Move 'job-id' setting to advanced tab in backup job edit. > - Don't use 'internal' flag to mark translatable fields, since > the only field where that's necessary is 'type' for now - so > just add a hardcoded check > - v5: > - Rebased onto latest master, resolving some small conflict > - v4: > - widget-toolkit: break out changes for the utils module so that they > can be applied ahead of time to ease dep bumping > - don't show Job IDs in the backup/replication job columns > - v3: > - Drop already applied patches for `proxmox` > - Rebase onto latest master - minor conflict resolution was needed > - v2: > - include 'type' metadata field for forwarded mails > --> otherwise it's not possible to match them > - include Maximilliano's T-b trailer in UI patches > > pve-guest-common: > > Lukas Wagner (1): > vzdump: common: allow 'job-id' as a parameter without being in schema > > src/PVE/VZDump/Common.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > pve-manager: > > Lukas Wagner (9): > api: jobs: vzdump: pass job 'job-id' parameter > ui: dc: backup: send 'job-id' property when starting a backup job > manually > ui: dc: backup: allow to set custom job id in advanced settings > api: replication: add 'job-id' to notification metadata > vzdump: apt: notification: do not include domain in 'hostname' field > api: replication: include 'hostname' field for notifications > api: notification: add API for getting known metadata fields/values > ui: utils: add overrides for translatable notification fields/values > d/control: bump proxmox-widget-toolkit dependency to 4.1.4 > > PVE/API2/APT.pm | 3 +- > PVE/API2/Cluster/Notifications.pm | 139 > PVE/API2/Replication.pm | 4 +- > PVE/API2/VZDump.pm | 8 ++ > PVE/Jobs/VZDump.pm | 4 +- > PVE/VZDump.pm
Re: [pve-devel] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter
Quoting Lukas Wagner (2024-06-10 10:40:21)
> This allows us to access us the backup job id in the send_notification
> function, where we can set it as metadata for the notification.
should we have some sort of safeguard against passing in a bogus/fake job-id?
e.g., right now, I could call this API endpoint with arbitrary job-id values
and (potentially) trigger notifications to other users..
some possible avenues would be:
- limit the parameter to root (but that means only scheduled executions can set
it, manual invocations can't)
- limit to existing job-ids (doesn't provide much benefit)
- ..
>
> Signed-off-by: Lukas Wagner
> ---
> PVE/API2/VZDump.pm | 8
> PVE/Jobs/VZDump.pm | 4 +++-
> PVE/VZDump.pm | 6 +++---
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
> index 7f92e7ec..84dbc100 100644
> --- a/PVE/API2/VZDump.pm
> +++ b/PVE/API2/VZDump.pm
> @@ -53,6 +53,14 @@ __PACKAGE__->register_method ({
> parameters => {
> additionalProperties => 0,
> properties => PVE::VZDump::Common::json_config_properties({
> + 'job-id' => {
> + description => "The ID of the backup job. If set, the
> 'backup-job' metadata field"
> + . " of the backup notification will be set to this
> value.",
> + type => 'string',
> + format => 'pve-configid',
> + maxLength => 256,
> + optional => 1,
> + },
> stdout => {
> type => 'boolean',
> description => "Write tar to stdout, not to a file.",
> diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
> index b8e57945..2dad3f55 100644
> --- a/PVE/Jobs/VZDump.pm
> +++ b/PVE/Jobs/VZDump.pm
> @@ -12,7 +12,7 @@ use PVE::API2::VZDump;
> use base qw(PVE::VZDump::JobBase);
>
> sub run {
> -my ($class, $conf) = @_;
> +my ($class, $conf, $job_id) = @_;
>
> my $props = $class->properties();
> # remove all non vzdump related options
> @@ -20,6 +20,8 @@ sub run {
> delete $conf->{$opt} if !defined($props->{$opt});
> }
>
> +$conf->{'job-id'} = $job_id;
> +
> # Required as string parameters # FIXME why?! we could just check ref()
> for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
> if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 5b7080f3..2167b289 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -483,6 +483,7 @@ sub send_notification {
> my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
>
> my $opts = $self->{opts};
> +my $job_id = $opts->{'job-id'};
> my $mailto = $opts->{mailto};
> my $cmdline = $self->{cmdline};
> my $policy = $opts->{mailnotification} // 'always';
> @@ -529,12 +530,11 @@ sub send_notification {
> };
>
> my $fields = {
> - # TODO: There is no straight-forward way yet to get the
> - # backup job id here... (I think pvescheduler would need
> - # to pass that to the vzdump call?)
> type => "vzdump",
> hostname => $hostname,
> };
> +# Add backup-job metadata field in case this is a backup job.
> +$fields->{'job-id'} = $job_id if $job_id;
>
> my $severity = $failed ? "error" : "info";
> my $email_configured = $mailto && scalar(@$mailto);
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 2/2] cfg2cmd: split out helper for vga properties
Quoting Fiona Ebner (2024-05-31 17:13:30)
> To remove some line bloat from the config_to_command() function.
>
> Signed-off-by: Fiona Ebner
> ---
>
> Original motivation was to prepare for a deprectation warning for
> display type 'cirrus'. However, it might not get deprecated after
> all:
> https://lore.kernel.org/qemu-devel/usd6hvncbao47zklcb5qlpvjcuk7odryu57f45imxienyltlec@2ujm6g2gr2od/
>
> Still, adding the helper doesn't hurt IMHO.
>
> PVE/QemuServer.pm | 35 ++-
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index cf593383..a1f8adea 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3493,6 +3493,27 @@ my sub print_ovmf_drive_commandlines {
> return ("if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code",
> $var_drive_str);
> }
>
> +my sub get_vga_properties {
> +my ($conf, $arch, $machine_version, $winversion) = @_;
> +
> +my $vga = parse_vga($conf->{vga});
> +
> +my $qxlnum = vga_conf_has_spice($conf->{vga});
> +$vga->{type} = 'qxl' if $qxlnum;
> +
> +if (!$vga->{type}) {
> + if ($arch eq 'aarch64') {
> + $vga->{type} = 'virtio';
> + } elsif (min_version($machine_version, 2, 9)) {
> + $vga->{type} = (!$winversion || $winversion >= 6) ? 'std' :
> 'cirrus';
> + } else {
> + $vga->{type} = ($winversion >= 6) ? 'std' : 'cirrus';
> + }
> +}
> +
> +return ($vga, $qxlnum);
> +}
> +
> sub config_to_command {
> my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
> $live_restore_backing) = @_;
> @@ -3644,20 +3665,8 @@ sub config_to_command {
> my @usbcontrollers = PVE::QemuServer::USB::get_usb_controllers(
> $conf, $bridges, $arch, $machine_type, $machine_version);
> push @$devices, @usbcontrollers if @usbcontrollers;
> -my $vga = parse_vga($conf->{vga});
>
> -my $qxlnum = vga_conf_has_spice($conf->{vga});
> -$vga->{type} = 'qxl' if $qxlnum;
> -
> -if (!$vga->{type}) {
> - if ($arch eq 'aarch64') {
> - $vga->{type} = 'virtio';
> - } elsif (min_version($machine_version, 2, 9)) {
> - $vga->{type} = (!$winversion || $winversion >= 6) ? 'std' :
> 'cirrus';
> - } else {
> - $vga->{type} = ($winversion >= 6) ? 'std' : 'cirrus';
> - }
> -}
> +my ($vga, $qxlnum) = get_vga_properties($conf, $arch, $machine_version,
> $winversion);
>
> # enable absolute mouse coordinates (needed by vnc)
> my $tablet = $conf->{tablet};
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 1/2] schema: vga: mention that type 'cirrus' should not be used
Quoting Fiona Ebner (2024-05-31 17:13:29)
> [0]: https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> [1]:
> https://lore.kernel.org/qemu-devel/usd6hvncbao47zklcb5qlpvjcuk7odryu57f45imxienyltlec@2ujm6g2gr2od/
>
> Signed-off-by: Fiona Ebner
> ---
> PVE/QemuServer.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 5df0c96d..cf593383 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -179,7 +179,7 @@ my $agent_fmt = {
>
> my $vga_fmt = {
> type => {
> - description => "Select the VGA type.",
> + description => "Select the VGA type. Using type 'cirrus' is not
> recommended.",
> type => 'string',
> default => 'std',
> optional => 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 mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-firewall 1/1] service: flush firewall rules on force disable
Quoting Stefan Hanreich (2024-05-29 15:25:26)
> When disabling the nftables firewall again, there is a race condition
> where the nftables ruleset never gets flushed and persists after
> disabling. In practice this almost never happens due to pve-firewall
> running every 10 seconds, and proxmox-firewall running every 5
> seconds, so the proxmox-firewall main loop almost always runs at least
> once before the force disable file gets created and flushes the
> ruleset.
so if I understand this correctly, it should handle the following case:
proxmox-firewall runs and sets up NFT rules
user disables NFT
pve-firewall runs and sets up legacy rules and force disable file
proxmox-firewall runs and disables NFT rules
as opposed to the following sequence
proxmox-firewall runs and sets up NFT rules
user disables NFT
proxmox-firewall runs and disables NFT rules
pve-firewall runs and sets up legacy rules and force disable file
which is already handled..
I don't see why the first cast should "almost never happen", just because the
loops have a different period - it all comes down to alignment of the periods
and timing of the user action?
in other words, you have a sequence
0:N
5:N
5+X: L
10: N
15: N
15+X: L
20: N
25: N
25+X: L
where the gap between N and N is 5 seconds, and the gap between N and L and L
and N together is also 5 seconds. on average (assuming random alignment of the
periods), there's an X=2.5s window (out of 10) that the user action must hit to
trigger the issue (in the gap between L and N, since X can be between 0 and
5s)?
FWIW, the change itself looks good to me, but the commit message might need
some adaptation ;)
>
> Reported-by: Hannes Laimer
> Signed-off-by: Stefan Hanreich
> ---
> proxmox-firewall/src/bin/proxmox-firewall.rs | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs
> b/proxmox-firewall/src/bin/proxmox-firewall.rs
> index f7e816e..5133cbf 100644
> --- a/proxmox-firewall/src/bin/proxmox-firewall.rs
> +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
> @@ -91,6 +91,10 @@ fn main() -> Result<(), std::io::Error> {
>
> while !term.load(Ordering::Relaxed) {
> if force_disable_flag.exists() {
> +if let Err(error) = remove_firewall() {
> +log::error!("unable to disable firewall: {error:#}");
> +}
> +
> std::thread::sleep(Duration::from_secs(5));
> continue;
> }
> --
> 2.39.2
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common] abstract config: snapshot create/rollback/remove: print more log statements
Quoting Dominik Csapak (2024-05-29 13:45:19)
> this increases verbosity of the actions, especially when including the
> snapshot name, since that will appear in the task logs wereas before
> there was no mention of any action, just the storage specific output for
> creating/rollback/removal.
>
> Logs are printed at all main actions that can fail or take potentially
> long, so that it's clear what started/finished.
>
> Signed-off-by: Dominik Csapak
> ---
> replaces https://lists.proxmox.com/pipermail/pve-devel/2024-May/064010.html
>
> src/PVE/AbstractConfig.pm | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
> index 5d5f9b4..b685b7f 100644
> --- a/src/PVE/AbstractConfig.pm
> +++ b/src/PVE/AbstractConfig.pm
> @@ -806,6 +806,8 @@ sub __snapshot_activate_storages {
> sub snapshot_create {
> my ($class, $vmid, $snapname, $save_vmstate, $comment) = @_;
>
> +print "Creating snapshot '$snapname'\n";
> +
> my $snap = $class->__snapshot_prepare($vmid, $snapname, $save_vmstate,
> $comment);
>
> $save_vmstate = 0 if !$snap->{vmstate};
> @@ -820,24 +822,30 @@ sub snapshot_create {
> $class->__snapshot_activate_storages($conf, 0);
>
> if ($freezefs) {
> + print "Trying to freeze guest filesystems\n";
> $class->__snapshot_freeze($vmid, 0);
> + print "Succesfully frozen guest filesystems\n";
> }
>
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running,
> "before");
>
> + print "Creating volume snapshots\n";
> $class->foreach_volume($snap, sub {
> my ($vs, $volume) = @_;
>
> $class->__snapshot_create_vol_snapshot($vmid, $vs, $volume,
> $snapname);
> $drivehash->{$vs} = 1;
> });
> + print "Succesfully created volume snapshots\n";
> };
> my $err = $@;
>
> if ($running) {
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running,
> "after");
> if ($freezefs) {
> + print "Trying to thaw guest filesystems\n";
> $class->__snapshot_freeze($vmid, 1);
> + print "Succesfully thawed guest filesystems\n";
> }
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running,
> "after-unfreeze");
> }
> @@ -850,6 +858,8 @@ sub snapshot_create {
> }
>
> $class->__snapshot_commit($vmid, $snapname);
> +
> +print "Succesfully created snapshot '$snapname'\n";
> }
>
> # Check if the snapshot might still be needed by a replication job.
> @@ -895,6 +905,8 @@ my $snapshot_delete_assert_not_needed_by_replication =
> sub {
> sub snapshot_delete {
> my ($class, $vmid, $snapname, $force, $drivehash) = @_;
>
> +print "Removing snapshot '$snapname'\n";
> +
> my $unused = [];
>
> my $conf = $class->load_config($vmid);
> @@ -963,12 +975,15 @@ sub snapshot_delete {
>
> # now remove vmstate file
> if ($snap->{vmstate}) {
> + print "Removing vmstate file\n";
> $class->__snapshot_delete_vmstate_file($snap, $force);
>
> # save changes (remove vmstate from snapshot)
> $class->lock_config($vmid, $remove_drive, 'vmstate') if !$force;
> + print "Succesfully removed vmstate file\n";
> };
>
> +print "Removing volume snapshots\n";
> # now remove all volume snapshots
> $class->foreach_volume($snap, sub {
> my ($vs, $volume) = @_;
> @@ -985,6 +1000,7 @@ sub snapshot_delete {
> # save changes (remove drive from snapshot)
> $class->lock_config($vmid, $remove_drive, $vs) if !$force;
> });
> +print "Succesfully removed volume snapshots\n";
>
> # now cleanup config
> $class->lock_config($vmid, sub {
> @@ -1010,6 +1026,8 @@ sub snapshot_delete {
>
> $class->write_config($vmid, $conf);
> });
> +
> +print "Succesfully removed snapshot '$snapname'\n";
> }
>
> # Remove replication snapshots to make a rollback possible.
> @@ -1082,6 +1100,8 @@ my $rollback_remove_replication_snapshots = sub {
> sub snapshot_rollback {
> my ($class, $vmid, $snapname) = @_;
>
> +print "Rolling back to snapshot '$snapname'\n";
> +
> my $prepare = 1;
>
> my $data = {};
> @@ -1121,6 +1141,7 @@ sub snapshot_rollback {
>
> if ($prepare) {
> $class->check_lock($conf);
> + print "Stopping guest\n";
nit: this can be a bit misleading, since it is also printed if the guest isn't
running.. for containers, the implementation of this is guarded by
check_running, for VMs it basically is as well (since this only calls vm_stop,
which returns immediately if the VM is not running).
should we just add that check to the if condition here? and then drop it from
LXC::Config, since it's redundant then?
> $class->__snapshot_rollback_vm_stop($vmid);
> }
>
> @@ -1149,20 +1170,25 @@ sub
[pve-devel] applied: [PATCH common] fix #5486: tools: encode_text: add '%' to list of encoded characters
Quoting Dominik Csapak (2024-05-28 13:10:02)
> all text that is going through encode_text will at a later point be
> decoded by 'decode_text'. The latter is decoding all percent encoded
> characters, even those not originally encoded by 'encode_text'.
>
> This means, to preserve the original data, we first have to at least
> percent encode the '%' itself, otherwise it's impossible to properly
> store e.g. '%20' there.
>
> It would get saved as '%20' directly, but on the next read, it gets
> decoded to ' ', which is not the original data. instead we have to save
> it as '%2520', which gets then correctly decoded to '%20' again
>
> This is especially important for the vm/ct/node description, as there
> users can store external links, which already include percent encoded
> characters.
>
> Signed-off-by: Dominik Csapak
> ---
> AFAICS, we only use this for comment fields + first/lastname in
> access-control, so we should be ok here
and also for the worker ID for download-from-url workers, which should be fine
as well.
> src/PVE/Tools.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 766c809..59cc5c9 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -1246,8 +1246,8 @@ sub upid_normalize_status_type {
> sub encode_text {
> my ($text) = @_;
>
> -# all control and hi-bit characters, and ':'
> -my $unsafe = "^\x20-\x39\x3b-\x7e";
> +# all control and hi-bit characters, ':' and '%'
> +my $unsafe = "^\x20-\x24\x26-\x39\x3b-\x7e";
> return uri_escape(Encode::encode("utf8", $text), $unsafe);
> }
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 manager] pve7to8: allow arbitrary newer running '-pve' kernels after upgrade
with the last return adapted to explicitly return 0.
Quoting Fiona Ebner (2024-05-28 12:59:23)
> As recently reported in the community forum [0], 6.8 pve kernels would
> not be detected correctly by the script. Allow arbitrary newer
> versions if already upgraded for future-proofing.
>
> [0]: https://forum.proxmox.com/threads/145723/post-664612
>
> Suggested-by: Dominik Csapak
> Signed-off-by: Fiona Ebner
> ---
>
> Changes in v2:
> * Allow arbitrary newer '-pve' kernels like Dominik suggested.
>
> PVE/CLI/pve7to8.pm | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
> index b34c8362..5c308907 100644
> --- a/PVE/CLI/pve7to8.pm
> +++ b/PVE/CLI/pve7to8.pm
> @@ -204,17 +204,30 @@ sub check_pve_packages {
> }
>
> # FIXME: better differentiate between 6.2 from bullseye or bookworm
> - my ($krunning, $kinstalled) =
> (qr/6\.(?:2\.(?:[2-9]\d+|1[6-8]|1\d\d+)|5)[^~]*$/, 'proxmox-kernel-6.2');
> + my $kinstalled = 'proxmox-kernel-6.2';
> if (!$upgraded) {
> # we got a few that avoided 5.15 in cluster with mixed CPUs, so
> allow older too
> - ($krunning, $kinstalled) = (qr/(?:5\.(?:13|15)|6\.2)/,
> 'pve-kernel-5.15');
> + $kinstalled = 'pve-kernel-5.15';
> }
>
> + my $kernel_version_is_expected = sub {
> + my ($version) = @_;
> +
> + return $version =~ m/^(?:5\.(?:13|15)|6\.2)/ if !$upgraded;
> +
> + if ($version =~ m/^6\.(?:2\.(?:[2-9]\d+|1[6-8]|1\d\d+)|5)[^~]*$/)
> {
> + return 1;
> + } elsif ($version =~ m/^(\d+).(\d+)[^~]*-pve$/) {
> + return $1 >= 6 && $2 >= 2;
> + }
> + return;
> + };
> +
> print "\nChecking running kernel version..\n";
> my $kernel_ver = $proxmox_ve->{RunningKernel};
> if (!defined($kernel_ver)) {
> log_fail("unable to determine running kernel version.");
> - } elsif ($kernel_ver =~ /^$krunning/) {
> + } elsif ($kernel_version_is_expected->($kernel_ver)) {
> if ($upgraded) {
> log_pass("running new kernel '$kernel_ver' after upgrade.");
> } else {
> @@ -227,7 +240,7 @@ sub check_pve_packages {
> log_warn("unexpected running and installed kernel
> '$kernel_ver'.");
> }
>
> - if ($upgraded && $kernel_ver =~ /^$krunning/) {
> + if ($upgraded && $kernel_version_is_expected->($kernel_ver)) {
> my $outdated_kernel_meta_pkgs = [];
> for my $kernel_meta_version ('5.4', '5.11', '5.13', '5.15') {
> my $pkg = "pve-kernel-${kernel_meta_version}";
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC v2 qemu-server 4/4] move helper to check running QEMU version out of the 'Machine' module
On May 28, 2024 10:50 am, Fiona Ebner wrote:
> The version of the running QEMU binary is not related to the machine
> version and so it's a bit confusing to have the helper in the
> 'Machine' module. It cannot live in the 'Helpers' module, because that
> would lead to a cyclic inclusion Helpers <-> Monitor. Thus,
> 'QMPHelpers' is chosen as the new home.
>
> Signed-off-by: Fiona Ebner
Acked-by: Fabian Grünbichler
but needs the first patch to be applied, or a re-order to move this
first ;)
> ---
>
> New in v2.
>
> PVE/QemuMigrate.pm| 3 ++-
> PVE/QemuServer/Machine.pm | 12
> PVE/QemuServer/QMPHelpers.pm | 13 +
> test/MigrationTest/QemuMigrateMock.pm | 4
> test/run_config2command_tests.pl | 4 ++--
> 5 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 34fc46ee..e71face4 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -30,6 +30,7 @@ use PVE::QemuServer::Helpers qw(min_version);
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> use PVE::QemuServer::Memory qw(get_current_memory);
> +use PVE::QemuServer::QMPHelpers;
> use PVE::QemuServer;
>
> use PVE::AbstractMigrate;
> @@ -1140,7 +1141,7 @@ sub phase2 {
> PVE::QemuServer::qemu_drive_mirror($vmid, $drive, $nbd_uri, $vmid,
> undef, $self->{storage_migration_jobs}, 'skip', undef, $bwlimit, $bitmap);
> }
>
> - if (PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid, 8, 2)) {
> + if (PVE::QemuServer::QMPHelpers::runs_at_least_qemu_version($vmid, 8,
> 2)) {
> $self->log('info', "switching mirror jobs to actively synced mode");
> PVE::QemuServer::qemu_drive_mirror_switch_to_active_mode(
> $vmid,
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index cc92e7e6..a3917dae 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -161,18 +161,6 @@ sub can_run_pve_machine_version {
> return 0;
> }
>
> -# dies if a) VM not running or not exisiting b) Version query failed
> -# So, any defined return value is valid, any invalid state can be caught by
> eval
> -sub runs_at_least_qemu_version {
> -my ($vmid, $major, $minor, $extra) = @_;
> -
> -my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
> -die "could not query currently running version for VM $vmid\n" if
> !defined($v);
> -$v = $v->{qemu};
> -
> -return PVE::QemuServer::Helpers::version_cmp($v->{major}, $major,
> $v->{minor}, $minor, $v->{micro}, $extra) >= 0;
> -}
> -
> sub qemu_machine_pxe {
> my ($vmid, $conf) = @_;
>
> diff --git a/PVE/QemuServer/QMPHelpers.pm b/PVE/QemuServer/QMPHelpers.pm
> index d3a52327..0269ea46 100644
> --- a/PVE/QemuServer/QMPHelpers.pm
> +++ b/PVE/QemuServer/QMPHelpers.pm
> @@ -3,6 +3,7 @@ package PVE::QemuServer::QMPHelpers;
> use warnings;
> use strict;
>
> +use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Monitor qw(mon_cmd);
>
> use base 'Exporter';
> @@ -45,4 +46,16 @@ sub qemu_objectdel {
> return 1;
> }
>
> +# dies if a) VM not running or not exisiting b) Version query failed
> +# So, any defined return value is valid, any invalid state can be caught by
> eval
> +sub runs_at_least_qemu_version {
> +my ($vmid, $major, $minor, $extra) = @_;
> +
> +my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
> +die "could not query currently running version for VM $vmid\n" if
> !defined($v);
> +$v = $v->{qemu};
> +
> +return PVE::QemuServer::Helpers::version_cmp($v->{major}, $major,
> $v->{minor}, $minor, $v->{micro}, $extra) >= 0;
> +}
> +
> 1;
> diff --git a/test/MigrationTest/QemuMigrateMock.pm
> b/test/MigrationTest/QemuMigrateMock.pm
> index f5b44424..11c58c08 100644
> --- a/test/MigrationTest/QemuMigrateMock.pm
> +++ b/test/MigrationTest/QemuMigrateMock.pm
> @@ -188,6 +188,10 @@ $qemu_server_machine_module->mock(
> if !defined($vm_status->{runningmachine});
> return $vm_status->{runningmachine};
> },
> +);
> +
> +my $qemu_server_qmphelpers_module =
> Test::MockModule->new("PVE::QemuServer::QMPHelpers");
> +$qemu_server_qmphelpers_module->mock(
> runs_at_least_qemu_version => sub {
> return 1;
> },
> diff --git a/test/run_config2command_tests.pl
> b/test/run_config2command_tests.pl
> index 7212acc4..d48ef562 100755
> --- a/test/run_config2command_tests.pl
> +++ b
Re: [pve-devel] [PATCH v2 qemu-server 1/4] migration: avoid crash with heavy IO on local VM disk
On May 28, 2024 10:50 am, Fiona Ebner wrote:
> There is a possibility that the drive-mirror job is not yet done when
> the migration wants to inactivate the source's blockdrives:
>
>> bdrv_co_write_req_prepare: Assertion `!(bs->open_flags & BDRV_O_INACTIVE)'
>> failed.
>
> This can be prevented by using the 'write-blocking' copy mode (also
> called active mode) for the mirror. However, with active mode, the
> guest write speed is limited by the synchronous writes to the mirror
> target. For this reason, a way to start out in the faster 'background'
> mode and later switch to active mode was introduced in QEMU 8.2.
>
> The switch is done once the mirror job for all drives is ready to be
> completed to reduce the time spent where guest IO is limited.
>
> Reported rarely, but steadily over the years:
> https://forum.proxmox.com/threads/78954/post-353651
> https://forum.proxmox.com/threads/78954/post-380015
> https://forum.proxmox.com/threads/100020/post-431660
> https://forum.proxmox.com/threads/111831/post-482425
> https://forum.proxmox.com/threads/111831/post-499807
> https://forum.proxmox.com/threads/137849/
>
> Signed-off-by: Fiona Ebner
> ---
>
> Changes in v2:
> * check for running QEMU version instead of installed version
>
> PVE/QemuMigrate.pm| 8 ++
> PVE/QemuServer.pm | 41 +++
> test/MigrationTest/QemuMigrateMock.pm | 6
> 3 files changed, 55 insertions(+)
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 33d5b2d1..d7ee4a5b 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -1145,6 +1145,14 @@ sub phase2 {
> $self->log('info', "$drive: start migration to $nbd_uri");
> PVE::QemuServer::qemu_drive_mirror($vmid, $drive, $nbd_uri, $vmid,
> undef, $self->{storage_migration_jobs}, 'skip', undef, $bwlimit, $bitmap);
> }
> +
> + if (PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid, 8, 2)) {
> + $self->log('info', "switching mirror jobs to actively synced mode");
> + PVE::QemuServer::qemu_drive_mirror_switch_to_active_mode(
> + $vmid,
> + $self->{storage_migration_jobs},
> + );
> + }
> }
>
> $self->log('info', "starting online/live migration on $migrate_uri");
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 5df0c96d..d472e805 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -8122,6 +8122,47 @@ sub qemu_blockjobs_cancel {
> }
> }
>
> +# Callers should version guard this (only available with a binary >= QEMU
> 8.2)
> +sub qemu_drive_mirror_switch_to_active_mode {
> +my ($vmid, $jobs) = @_;
> +
> +my $switching = {};
> +
> +for my $job (sort keys $jobs->%*) {
> + print "$job: switching to actively synced mode\n";
> +
> + eval {
> + mon_cmd(
> + $vmid,
> + "block-job-change",
> + id => $job,
> + type => 'mirror',
> + 'copy-mode' => 'write-blocking',
> + );
> + $switching->{$job} = 1;
> + };
> + die "could not switch mirror job $job to active mode - $@\n" if $@;
> +}
> +
> +while (1) {
> + my $stats = mon_cmd($vmid, "query-block-jobs");
> +
> + my $running_jobs = {};
> + $running_jobs->{$_->{device}} = $_ for $stats->@*;
> +
> + for my $job (sort keys $switching->%*) {
> + if ($running_jobs->{$job}->{'actively-synced'}) {
> + print "$job: successfully switched to actively synced mode\n";
> + delete $switching->{$job};
> + }
> + }
> +
> + last if scalar(keys $switching->%*) == 0;
> +
> + sleep 1;
> +}
so what could be the cause here for a job not switching? and do we
really want to loop forever if it happens?
> +}
> +
> # Check for bug #4525: drive-mirror will open the target drive with the same
> aio setting as the
> # source, but some storages have problems with io_uring, sometimes even
> leading to crashes.
> my sub clone_disk_check_io_uring {
> diff --git a/test/MigrationTest/QemuMigrateMock.pm
> b/test/MigrationTest/QemuMigrateMock.pm
> index 1efabe24..f5b44424 100644
> --- a/test/MigrationTest/QemuMigrateMock.pm
> +++ b/test/MigrationTest/QemuMigrateMock.pm
> @@ -152,6 +152,9 @@ $MigrationTest::Shared::qemu_server_module->mock(
> }
> return;
> },
> +qemu_drive_mirror_switch_to_active_mode => sub {
> + return;
> +},
> set_migration_caps => sub {
> return;
> },
> @@ -185,6 +188,9 @@ $qemu_server_machine_module->mock(
> if !defined($vm_status->{runningmachine});
> return $vm_status->{runningmachine};
> },
> +runs_at_least_qemu_version => sub {
> + return 1;
> +},
> );
>
> my $ssh_info_module = Test::MockModule->new("PVE::SSHInfo");
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
>
[pve-devel] applied: [PATCH v2 qemu-server 2/4] migration: handle replication: remove outdated and inaccurate check for QEMU version
On May 28, 2024 10:50 am, Fiona Ebner wrote:
> In Proxmox VE 8, the oldest supported QEMU version is 8.0, so a
> check for version 4.2 is not required anymore. The check was also
> wrong, because it checked the installed version and not the currently
> running one.
>
> Signed-off-by: Fiona Ebner
> ---
>
> New in v2.
>
> PVE/QemuMigrate.pm | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index d7ee4a5b..34fc46ee 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -544,12 +544,6 @@ sub handle_replication {
> if $self->{opts}->{remote};
>
> if ($self->{running}) {
> -
> - my $version = PVE::QemuServer::kvm_user_version();
> - if (!min_version($version, 4, 2)) {
> - die "can't live migrate VM with replicated volumes, pve-qemu to old
> (< 4.2)!\n"
> - }
> -
> my @live_replicatable_volumes = $self->filter_local_volumes('online',
> 1);
> foreach my $volid (@live_replicatable_volumes) {
> my $drive = $local_volumes->{$volid}->{drivename};
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
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-server 3/4] backup: prepare: remove outdated QEMU version check
On May 28, 2024 10:50 am, Fiona Ebner wrote:
> In Proxmox VE 8, the oldest supported QEMU version is 8.0, so a check
> for version 4.0.1 is not required anymore.
>
> Signed-off-by: Fiona Ebner
> ---
>
> New in v2.
>
> PVE/VZDump/QemuServer.pm | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index 8c97ee62..5248c6eb 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -90,10 +90,6 @@ sub prepare {
> if (!$volume->{included}) {
> $self->loginfo("exclude disk '$name' '$volid' ($volume->{reason})");
> next;
> - } elsif ($self->{vm_was_running} && $volume_config->{iothread} &&
> - !PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid,
> 4, 0, 1)) {
> - die "disk '$name' '$volid' (iothread=on) can't use backup feature
> with running QEMU " .
> - "version < 4.0.1! Either set backup=no for this drive or
> upgrade QEMU and restart VM\n";
> } else {
> my $log = "include disk '$name' '$volid'";
> if (defined(my $size = $volume_config->{size})) {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-firmware] d/control: add Conflicts and Replaces for ath9k-htc firmware
On May 17, 2024 11:07 am, Fiona Ebner wrote:
> Reported in the community forum [0].
>
> Both firmware files that are in the package [1]
>
>> /lib/firmware/ath9k_htc/htc_7010-1.4.0.fw
>> /lib/firmware/ath9k_htc/htc_9271-1.4.0.fw
>
> seem to have been present since the linux-firmware submodule commit
> 3de1c437 ("Add v1.4.0 firmware for ath9k_htc.") which is present since
> tag 20190717.
>
> [0]: https://forum.proxmox.com/threads/135758/post-664862
> [1]: https://packages.debian.org/bookworm/all/firmware-ath9k-htc/filelist
>
> Signed-off-by: Fiona Ebner
> ---
> debian/control | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/debian/control b/debian/control
> index 3c21b90..1fc601b 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -11,6 +11,7 @@ Architecture: all
> Depends: ${misc:Depends},
> Suggests: linux-image,
> Conflicts: firmware-amd-graphics,
> + firmware-ath9k-htc,
> firmware-atheros,
> firmware-bnx2,
> firmware-bnx2x,
> @@ -33,6 +34,7 @@ Conflicts: firmware-amd-graphics,
> firmware-siano,
> firmware-ti-connectivity,
> Replaces: firmware-amd-graphics,
> + firmware-ath9k-htc,
>firmware-atheros,
>firmware-bnx2,
>firmware-bnx2x,
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs] backup: mention where fleecing can be configured
On April 29, 2024 4:49 pm, Fiona Ebner wrote: > 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 mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu] fix #4726: avoid superfluous check in vma code
On June 14, 2024 12:50 pm, Fiona Ebner wrote:
> The 'status' pointer is dereferenced after the NULL check. Since all
> callers pass in the address of a struct on the stack, the pointer can
> never be NULL. Remove the superfluous check and add an assert instead.
>
> Signed-off-by: Fiona Ebner
Reviewed-by: Fabian Grünbichler
> ---
> ...VE-Backup-add-vma-backup-format-code.patch | 23 +--
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git
> a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
> b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
> index ee40ab8..d6d7767 100644
> --- a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
> +++ b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
> @@ -16,10 +16,10 @@ Signed-off-by: Fiona Ebner
> block/meson.build | 2 +
> meson.build | 5 +
> vma-reader.c | 870
> - vma-writer.c | 818 +
> + vma-writer.c | 817 +
> vma.c | 901 ++
> vma.h | 150
> - 6 files changed, 2746 insertions(+)
> + 6 files changed, 2745 insertions(+)
> create mode 100644 vma-reader.c
> create mode 100644 vma-writer.c
> create mode 100644 vma.c
> @@ -939,10 +939,10 @@ index 00..d0b6721812
> +
> diff --git a/vma-writer.c b/vma-writer.c
> new file mode 100644
> -index 00..126b296647
> +index 00..a466652a5d
> --- /dev/null
> +++ b/vma-writer.c
> -@@ -0,0 +1,818 @@
> +@@ -0,0 +1,817 @@
> +/*
> + * VMA: Virtual Machine Archive
> + *
> @@ -1517,17 +1517,16 @@ index 00..126b296647
> +int i;
> +
> +g_assert(vmaw != NULL);
> ++g_assert(status != NULL);
> +
> -+if (status) {
> -+status->status = vmaw->status;
> -+g_strlcpy(status->errmsg, vmaw->errmsg, sizeof(status->errmsg));
> -+for (i = 0; i <= 255; i++) {
> -+status->stream_info[i] = vmaw->stream_info[i];
> -+}
> -+
> -+uuid_unparse_lower(vmaw->uuid, status->uuid_str);
> ++status->status = vmaw->status;
> ++g_strlcpy(status->errmsg, vmaw->errmsg, sizeof(status->errmsg));
> ++for (i = 0; i <= 255; i++) {
> ++status->stream_info[i] = vmaw->stream_info[i];
> +}
> +
> ++uuid_unparse_lower(vmaw->uuid, status->uuid_str);
> ++
> +status->closed = vmaw->closed;
> +
> +return vmaw->status;
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-container 2/2] fix #5339: api: lxc: ip: add 'all' option so that all addresses can be returned.
apologies again for the long delay!
> Johannes Cornelis Draaijer via pve-devel hat am
> 18.04.2024 22:49 CEST geschrieben:
> Signed-off-by: Johannes Cornelis Draaijer
> ---
> src/PVE/API2/LXC.pm | 16 +---
> src/PVE/LXC.pm | 9 +++--
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 89ba64c..3561317 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -2533,6 +2533,12 @@ __PACKAGE__->register_method({
> properties => {
> node => get_standard_option('pve-node'),
> vmid => get_standard_option('pve-vmid', { completion =>
> \::LXC::complete_ctid }),
> + all => {
> + type => 'boolean',
> + default => 0,
> + optional => 1,
> + description => 'Return all adresses of each interface instead
> of only one',
typo: s/adresses/addresses
> + }
> },
> },
> returns => {
> @@ -2552,12 +2558,14 @@ __PACKAGE__->register_method({
> },
> inet => {
> type => 'string',
> - description => 'The IPv4 address of the interface',
> + format => 'CIDRv4-list',
this format here and the code below don't agree. a string type with the -list
suffix needs actually be a string with the list elements delimited by either
space, ',' or ';'. in this case, comma or semicolon is probably okay.
> + description => 'A list of IPv4 CIDRs. This will only
> contain a single address if \'all\' is not set to true',
> optional => 1,
> },
> inet6 => {
> type => 'string',
> - description => 'The IPv6 address of the interface',
> + format => 'CIDRv6-list',
same here
> + description => 'A list of IPv6 CIDRs. This will only
> contain a single address if \'all\' is not set to true',
> optional => 1,
> },
> }
> @@ -2565,8 +2573,10 @@ __PACKAGE__->register_method({
> },
> code => sub {
> my ($param) = @_;
> + my $vmid = extract_param($param, 'vmid');
> + my $alladdrs = extract_param($param, 'all') // 0;
existing code is not always consistent, but this should be all_addrs
>
> - return PVE::LXC::get_interfaces($param->{vmid});
> + return PVE::LXC::get_interfaces($vmid, $alladdrs);
> }});
>
> __PACKAGE__->register_method({
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 7883cfb..6d00141 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1088,7 +1088,7 @@ sub hotplug_net {
> }
>
> sub get_interfaces {
> -my ($vmid) = @_;
> +my ($vmid, $alladdrs) = @_;
>
> my $pid = eval { find_lxc_pid($vmid); };
> return if $@;
> @@ -1104,7 +1104,12 @@ sub get_interfaces {
> for my $interface ($config->@*) {
> my $obj = { name => $interface->{ifname} };
> for my $ip ($interface->{addr_info}->@*) {
> - $obj->{$ip->{family}} = $ip->{local} . "/" . $ip->{prefixlen};
> + my $cidr = $ip->{local} . "/" . $ip->{prefixlen};
> + if ($alladdrs eq 1) {
eq is for string comparison, this can just use
if ($all_addrs) {
but for regular comparison you'd use `==` otherwise :)
> + push(@{$obj->{$ip->{family}}}, $cidr);
if you do this, then you'd need to `join` this list into a string at the end.
or you could just build the string directly here (special casing the first and
subsequent addresses), both would work in this case.
or you do this here unconditionally, and then after the loop either join (if
$all_addr) or just pick the first element, which would be equivalent to the old
code here as well.
> +} else {
> + $obj->{$ip->{family}} = $cidr;
> + }
> }
> $obj->{hwaddr} = $interface->{address};
> push @$res, $obj
> --
> 2.34.1
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-container 1/2] api: lxc: add 'interfaces' endpoint to the index
sorry it took me so long to get back to this, applied this one!
> Johannes Cornelis Draaijer via pve-devel Signed-off-by: Johannes Cornelis Draaijer
> ---
> src/PVE/API2/LXC.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index fd42ccf..89ba64c 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -590,6 +590,7 @@ __PACKAGE__->register_method({
> { subdir => 'firewall' },
> { subdir => 'snapshot' },
> { subdir => 'resize' },
> + { subdir => 'interfaces' }
> ];
>
> return $res;
> --
> 2.34.1
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 storage] plugin: move definition for 'port' option to base plugin
On April 18, 2024 10:33 am, Fiona Ebner wrote:
> Commit 7020491 ("esxi: add 'port' config parameter") started using
> the 'port' option in a second plugin, but the definition stayed in the
> PBS plugin. Avoid the hidden dependency and move the definition to the
> base plugin instead.
>
> It is necessary to mark it as optional or it would be required always.
>
> Clarify that the option is not used by NFS and CIFS.
>
> Signed-off-by: Fiona Ebner
> ---
>
> Changes in v2:
> * do not use a definite list of plugins that use the option
> * clarify that NFS and CIFS do not use the option
>
> src/PVE/Storage/PBSPlugin.pm | 6 --
> src/PVE/Storage/Plugin.pm| 9 +
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
> index 08ceb88..0808bcc 100644
> --- a/src/PVE/Storage/PBSPlugin.pm
> +++ b/src/PVE/Storage/PBSPlugin.pm
> @@ -49,12 +49,6 @@ sub properties {
> description => "Base64-encoded, PEM-formatted public RSA key. Used
> to encrypt a copy of the encryption-key which will be added to each encrypted
> backup.",
> type => 'string',
> },
> - port => {
> - description => "For non default port.",
> - type => 'integer',
> - minimum => 1,
> - maximum => 65535,
> - },
> };
> }
>
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 22a9729..4c8f672 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -205,6 +205,15 @@ my $defaultData = {
> format => 'pve-storage-options',
> optional => 1,
> },
> + port => {
> + description => "Use this port to connect to the storage instead of
> the default one (for"
> + ." example, with PBS or ESXi). For NFS and CIFS, use the
> 'options' option to"
> + ." configure the port via the mount options.",
> + type => 'integer',
> + minimum => 1,
> + maximum => 65535,
> + optional => 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 mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH-SERIES v2 qemu-server] improve error detection/messages for some block jobs
On April 11, 2024 1:16 pm, Fiona Ebner wrote: > Changes in v2: > * Also do not auto-dismiss for the stream job for the new > live-import feature. > > When auto-dismiss=true (the default), a failed job can disappear very > quickly from the job list and there might not be any chance to see the > error in the result of 'query-block-jobs'. For jobs with $completion > being 'auto', like 'block-stream', it couldn't even be detected that > the job failed. > > Jobs with auto-dismiss=false on the other hand, will wait in > 'concluded' state until manually dismissed. For those, it will be > possible to query the error if the job failed. > > This series makes 'drive-mirror' and 'block-stream' jobs do just that. > > There doesn't seem to be a way to have only failed jobs stay around, > e.g. something like auto-dismiss=on-success. > > > Fiona Ebner (4): > blockjob: anticipate jobs with auto-dismiss=false for better error > messages and detection > mirror: do not auto-dismiss to allow getting error message from job > live restore: do not auto-dismiss stream job to improve error message > and detection > live import: do not auto-dismiss stream job to improve error message > and detection > > PVE/QemuServer.pm | 35 +-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Feature Request: Enhancing Error Messaging
> Rovshan Pashayev hat am 02.07.2024 15:02 CEST > geschrieben: > Hello Fabian, > > Example: > If we attempt to start VM with problematic storages from WebUI, we might see > following error (example): > https://i.ibb.co/Lx5QjCg/image-2.png > > But if we attempt start action via SSH request we get both output in the one > line (example): > WARN: no efidisk configured! Using temporary efivars disk. storage > 'NFS_pve-storage' does not exist > > Which causes confusion, during differentiating errors. please post "pveversion -v" and the contents of the corresponding task file for that start task (/var/log/pve/tasks/...). it seems very likely that this issue is on your end (i.e., with the terminal/shell/.. you are using?)). the screenshot would indicate that the task log is correct. if I manually try to provoke the same error and run "qm start XXX" in a shell, there is a proper new line between the warning line and the final line with the fatal error: $ qm start 740001 generating cloud-init ISO WARN: no efidisk configured! Using temporary efivars disk. storage 'ext3' does not exist note that everything but the last line is printed on STDOUT, while the last line is printed on STDERR.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Feature Request: Enhancing Error Messaging
> Rovshan Pashayev via pve-devel hat am > 02.07.2024 12:50 CEST geschrieben: > We kindly request the implementation of enhanced error messaging. Currently, > all errors and failures are combined into a single string, making it > difficult to identify and address specific issues. which errors? for which actions/API endpoints/..? > Please consider introducing error codes or a similar mechanism to provide a > structured format for error reporting. This would improve the user experience > and expedite troubleshooting during third-party backup processes. we try to keep the errors meaningful and easy to understand. if you have concrete examples that could benefit from more detail or changed contents, feel free to file issues at https://bugzilla.proxmox.com we are not planning to add error codes (like E1234) to our error messages at the moment - in our experience, those don't provide much added benefit at the level of interaction PVE usually provides. structured error types can make sense for low level interfaces (where callers might decide on a course of action based on the error, like retrying). note that we do differentiate between lacking permissions, bad parameters and other errors also on the API level. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Strange Behavior – ZFS (and other) Storage Configuration in PVE (plain text)
On July 2, 2024 10:48 am, Rovshan Pashayev via pve-devel wrote: > We would like to report a strange behavior that we have encountered in > Proxmox, version 8.2.2. The issue is as follows: > We noticed that ZFS storage configuration is missing mountpoint line > randomly. And workaround is manually adding missing line. what exactly do you mean by this? the entry in /etc/pve/storage.cfg? that one should only change if you call the corresponding API endpoint (or CLI command). please double-check the logs that no such invocation is happening. if you mean something else, please clearly describe how your system is configured and what the unexpected thing that is happening is. > We also can see same behavior was reported in your forum: > NFS: [SOLVED] - Proxmox 3.1 Lost Mount Point in Cluster | Proxmox Support > Forum > (https://forum.proxmox.com/threads/proxmox-3-1-lost-mount-point-in-cluster.16664/) this is about a mount not working because of a misconfiguration > ZFS: mountpoint missing after restoring a backup | Proxmox Support Forum > (https://forum.proxmox.com/threads/mountpoint-missing-after-restoring-a-backup.110012/) this is about a container mountpoint that cannot be restored unless the restoring user is root@pam. so two very different things, and both seem to also be something completely different from the issue you describe, but maybe I misunderstood that part ;) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH access-control] api: Prevent TFA from being set up for openid users
doesn't apply anymore!
On March 13, 2024 2:18 pm, Markus Frank wrote:
> Currently it is possible to set up TFA for an OpenID user (as root user),
> but it is never requested during the login process for that user.
> This patch prevents this and displays an error message with the instruction
> to set up TFA using the OpenId server.
>
> Signed-off-by: Markus Frank
> ---
> src/PVE/API2/TFA.pm | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm
> index 13ffc59..5e7e9eb 100644
> --- a/src/PVE/API2/TFA.pm
> +++ b/src/PVE/API2/TFA.pm
> @@ -381,6 +381,13 @@ __PACKAGE__->register_method ({
> my ($userid, $realm) =
> root_permission_check($rpcenv, $authuser, $param->{userid},
> $param->{password});
>
> + my $domain_cfg = cfs_read_file('domains.cfg');
> + my $realm_cfg = $domain_cfg->{ids}->{$realm};
> + if ($realm_cfg->{type} eq "openid") {
> + die "Users of the realm '$realm' with type 'openid' cannot use TFA."
> + ." Using the OpenID server to set up TFA is recommended.\n";
> + }
> +
> my $type = delete $param->{type};
> my $value = delete $param->{value};
> if ($type eq 'yubico') {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] vzdump: fix unit for bandwidth limit in log message
On June 26, 2024 4:32 pm, Fiona Ebner wrote:
> The documentation 'man vzdump' states that the value is in KiB/s. This
> is correct, as seen in the plugin implementations, where the value is
> multiplied by 1024.
>
> Signed-off-by: Fiona Ebner
> ---
> PVE/VZDump.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 5b7080f3..33e08f9e 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -1084,7 +1084,7 @@ sub exec_backup_task {
> $task->{mode} = $mode;
>
> debugmsg ('info', "backup mode: $mode", $logfd);
> - debugmsg ('info', "bandwidth limit: $opts->{bwlimit} KB/s", $logfd) if
> $opts->{bwlimit};
> + debugmsg ('info', "bandwidth limit: $opts->{bwlimit} KiB/s", $logfd)
> if $opts->{bwlimit};
> debugmsg ('info', "ionice priority: $opts->{ionice}", $logfd);
>
> if ($mode eq 'stop') {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 2/2] fix #3352: templates: minimize config when starting templates
we could also think about using qemu-storage-daemon and skip starting of
the VM entirely.. that might be interesting for other use cases as well
(mostly storage-agnostic replication via block-mirror, I guess?)
On June 20, 2024 12:09 pm, Dominik Csapak wrote:
> templates can only be started in context of a pbs backup, and there we
> don't need or want to use most of the config, since they cannot be
> started normally anyway.
>
> We minimize the config by copying some specific relevant options (see
> the comments for why the options were chosen) and all disk
> configurations.
>
> Since we change the qemu commandline for templates, we now have to adapt
> the tests involving templates.
>
> Without this, users can get into a situation where the template cannot
> be backed up when there are some resources not available (such as cpu
> cores, kvm, pci devices, etc.) even if the backup process does not need
> them.
>
> This change has some nice side effects, such as we don't need to
> allocate the full amount of memory anymore for templates that have a
> hostpci device configured, the configured bridges don't have to exist,
> etc.
>
> Signed-off-by: Dominik Csapak
> ---
> this replaces: "fix #5543: pci: don't use pci devices when starting templates"
> https://lists.proxmox.com/pipermail/pve-devel/2024-June/064238.html
>
>
> PVE/QemuServer.pm | 25 +
> test/cfg2cmd/efi-raw-template.conf.cmd| 9 +++--
> .../q35-linux-hostpci-template.conf.cmd | 35 ++-
> test/cfg2cmd/simple1-template.conf.cmd| 18 --
> 4 files changed, 46 insertions(+), 41 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7815b608..0d998798 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3497,6 +3497,28 @@ sub config_to_command {
> my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
> $live_restore_backing) = @_;
>
> +# minimize config for templates, they can only start for backup,
> +# so most options besides the disks are irrelevant
> +if (PVE::QemuConfig->is_template($conf)) {
> + my $newconf = {
> + template => 1, # in case below code checks that
> + kvm => 0, # to prevent an error on hosts without virtualization
> extensions
> + vga => 'none', # to not start a vnc server
> + scsihw => $conf->{scsihw}, # so that the scsi disks are correctly
> added
> + bios => $conf->{bios}, # so efidisk gets included if it exists
> + name => $conf->{name}, # so it's correct in the process list
> + };
> +
> + # copy all disks over
> + for my $device (PVE::QemuServer::Drive::valid_drive_names()) {
> + $newconf->{$device} = $conf->{$device};
> + }
> +
> + # remaining configs stay default
> +
> + $conf = $newconf;
> +}
> +
> my ($globalFlags, $machineFlags, $rtcFlags) = ([], [], []);
> my $devices = [];
> my $bridges = {};
> @@ -6137,6 +6159,9 @@ sub get_vm_volumes {
> sub cleanup_pci_devices {
> my ($vmid, $conf) = @_;
>
> +# templates don't use pci devices
> +return if $conf->{template};
> +
> foreach my $key (keys %$conf) {
> next if $key !~ m/^hostpci(\d+)$/;
> my $hostpciindex = $1;
> diff --git a/test/cfg2cmd/efi-raw-template.conf.cmd
> b/test/cfg2cmd/efi-raw-template.conf.cmd
> index b1d4d1f6..3e90c335 100644
> --- a/test/cfg2cmd/efi-raw-template.conf.cmd
> +++ b/test/cfg2cmd/efi-raw-template.conf.cmd
> @@ -8,21 +8,20 @@
>-mon 'chardev=qmp-event,mode=control' \
>-pidfile /var/run/qemu-server/8006.pid \
>-daemonize \
> - -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
>-drive
> 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
> \
>-drive
> 'if=pflash,unit=1,id=drive-efidisk0,format=raw,file=/var/lib/vz/images/100/base-disk-100-0.raw,size=131072,readonly=on'
> \
>-smp '1,sockets=1,cores=1,maxcpus=1' \
>-nodefaults \
>-boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> - -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
> - -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> + -vga none \
> + -nographic \
> + -cpu qemu64 \
>-m 512 \
>-device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
>-device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
>-device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
>-device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
> - -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
>-device
> 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
>-iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
> - -machine 'type=pc+pve0' \
> + -machine 'accel=tcg,type=pc+pve0' \
>-snapshot
> diff --git a/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd
>
[pve-devel] applied: [PATCH qemu-server 1/2] tests: cfg2cmd: add test for templates with more options
On June 20, 2024 12:09 pm, Dominik Csapak wrote: > during pbs backups, we need to start templates, so add a test for that. > We already have some tests for templates, but none with hostpci,tpm, > etc. > > Signed-off-by: Dominik Csapak > --- > test/cfg2cmd/q35-linux-hostpci-template.conf | 23 ++ > .../q35-linux-hostpci-template.conf.cmd | 45 +++ > 2 files changed, 68 insertions(+) > create mode 100644 test/cfg2cmd/q35-linux-hostpci-template.conf > create mode 100644 test/cfg2cmd/q35-linux-hostpci-template.conf.cmd > > diff --git a/test/cfg2cmd/q35-linux-hostpci-template.conf > b/test/cfg2cmd/q35-linux-hostpci-template.conf > new file mode 100644 > index ..dfbf1322 > --- /dev/null > +++ b/test/cfg2cmd/q35-linux-hostpci-template.conf > @@ -0,0 +1,23 @@ > +# TEST: Config with q35, NUMA, hostpci passthrough, EFI, TPM & Linux as a > template > +bios: ovmf > +bootdisk: scsi0 > +cores: 1 > +efidisk0: local:100/base-100-disk-1.qcow2,size=128K > +hostpci0: 00:ff.1 > +hostpci1: d0:13.0,pcie=1 > +hostpci2: 00:f4.0 > +hostpci3: d0:15.1,pcie=1 > +hostpci4: d0:17.0,pcie=1,rombar=0 > +hostpci7: d0:15.2,pcie=1 > +machine: q35 > +memory: 512 > +net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0 > +numa: 1 > +ostype: l26 > +scsihw: virtio-scsi-pci > +scsi0: local:100/base-100-disk-2.raw,size=10G > +smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687 > +sockets: 2 > +template: 1 > +tpmstate0: local:100/base-100-disk-1.raw,size=4M,version=v2.0 > +vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d > diff --git a/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd > b/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd > new file mode 100644 > index ..f9b9bf69 > --- /dev/null > +++ b/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd > @@ -0,0 +1,45 @@ > +/usr/bin/kvm \ > + -id 8006 \ > + -name 'vm8006,debug-threads=on' \ > + -no-shutdown \ > + -chardev > 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \ > + -mon 'chardev=qmp,mode=control' \ > + -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \ > + -mon 'chardev=qmp-event,mode=control' \ > + -pidfile /var/run/qemu-server/8006.pid \ > + -daemonize \ > + -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \ > + -drive > 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' > \ > + -drive > 'if=pflash,unit=1,id=drive-efidisk0,format=qcow2,file=/var/lib/vz/images/100/base-100-disk-1.qcow2,readonly=on' > \ > + -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \ > + -smp '2,sockets=2,cores=1,maxcpus=2' \ > + -nodefaults \ > + -boot > 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' > \ > + -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \ > + -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \ > + -m 512 \ > + -object 'memory-backend-ram,id=ram-node0,size=256M' \ > + -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \ > + -object 'memory-backend-ram,id=ram-node1,size=256M' \ > + -numa 'node,nodeid=1,cpus=1,memdev=ram-node1' \ > + -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \ > + -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \ > + -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \ > + -device 'vfio-pci,host=:00:ff.1,id=hostpci0,bus=pci.0,addr=0x10' \ > + -device > 'vfio-pci,host=:d0:13.0,id=hostpci1,bus=ich9-pcie-port-2,addr=0x0' \ > + -device 'vfio-pci,host=:00:f4.0,id=hostpci2,bus=pci.0,addr=0x1b' \ > + -device > 'vfio-pci,host=:d0:15.1,id=hostpci3,bus=ich9-pcie-port-4,addr=0x0' \ > + -device > 'pcie-root-port,id=ich9-pcie-port-5,addr=10.0,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=5,chassis=5' > \ > + -device > 'vfio-pci,host=:d0:17.0,id=hostpci4,bus=ich9-pcie-port-5,addr=0x0,rombar=0' > \ > + -device > 'pcie-root-port,id=ich9-pcie-port-8,addr=10.3,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=8,chassis=8' > \ > + -device > 'vfio-pci,host=:d0:15.2,id=hostpci7,bus=ich9-pcie-port-8,addr=0x0' \ > + -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \ > + -device > 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \ > + -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ > + -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \ > + -drive > 'file=/var/lib/vz/images/100/base-100-disk-2.raw,if=none,id=drive-scsi0,format=raw,cache=none,aio=io_uring,detect-zeroes=on,readonly=on' > \ > + -device > 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' > \ > + -netdev > 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' > \ > + -device > 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' > \ > + -machine 'type=q35+pve0' \ > + -snapshot > -- > 2.39.2 > >
[pve-devel] applied: [PATCH storage] style: remove goto statements
applied with those suggestions folded in!
On June 27, 2024 3:28 pm, Fiona Ebner wrote:
> Am 26.06.24 um 10:56 schrieb Fabian Grünbichler:
>> these can just as well be `die` statements right there, there is no
>> complicated
>> cleanup that would warrant a goto statement..
>>
>> Signed-off-by: Fabian Grünbichler
>
> Reviewed-by: Fiona Ebner
>
> but with some suggestions:
>
>> ---
>> src/PVE/Storage/Plugin.pm | 12 ++--
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
>> index b5a54c1..f8dc9a2 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -1586,13 +1586,14 @@ sub read_common_header($) {
>> # Export a volume into a file handle as a stream of desired format.
>> sub volume_export {
>> my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
>> $base_snapshot, $with_snapshots) = @_;
>
> While at it, we could add a blank line here ;)
>
>> +my $unsupported = "volume export format $format not available for
>> $class\n";
>
> Maybe add _msg or _error or similar to improve the variable name?
>
>> if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
>> my $file = $class->path($scfg, $volname, $storeid)
>> -or goto unsupported;
>> +or die $unsupported;
>
> Could be moved to the previous line.
>
>> my ($size, $file_format) = file_size_info($file);
>>
>> if ($format eq 'raw+size') {
>> -goto unsupported if $with_snapshots || $file_format eq 'subvol';
>> +die $unsupported if $with_snapshots || $file_format eq 'subvol';
>> write_common_header($fh, $size);
>> if ($file_format eq 'raw') {
>> run_command(['dd', "if=$file", "bs=4k", "status=progress"],
>> output => '>&'.fileno($fh));
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 0/3] minor tpm state handling improvements
with the fixes: prefix dropped for the last patch On June 27, 2024 1:03 pm, Fiona Ebner wrote: > > Fiona Ebner (3): > fix #5562: tpm: avoid warning about undefined value when version is > not explicitly set > drive: tpm: fix default version in schema > fix #5563: api: update vm: prohibit changing version of TPM state > > PVE/API2/Qemu.pm| 19 +++ > PVE/QemuServer.pm | 6 +++--- > PVE/QemuServer/Drive.pm | 2 +- > 3 files changed, 23 insertions(+), 4 deletions(-) > > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH ifupdown2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
small nit below, otherwise
Reviewed-by: Fabian Grünbichler
On June 27, 2024 5:01 pm, Stefan Hanreich wrote:
> This can lead to issue when upgrading from ifupdown to ifupdown2. The
> particular issue this fixes occurs in the following scenario:
>
> * Suppose there is a legacy Debian host with ifupdown and ifenslave
> installed that has a bond configured in /etc/network/interfaces.
> * ifenslave installs a script /etc/network/if-pre-up.d/ifenslave.
> * Now, an upgrade creates a second script
> /etc/network/if-pre-up.d/ifenslave.dpkg-new. As ifupdown executes
> network scripts via run-parts which ignores scripts with . in their
> name, ifenslave.dpkg-new has no effect.
> * If the host switches over to ifupdown2 by installing it (removing
> ifupdown, keeping ifenslave) and reboots, the network will not come
> up:
> /etc/network/if-pre-up.d/ifenslave still exists, but is ignored
> by ifupdown2's bond addon [1]
> /etc/network/if-pre-up.d/ifenslave.dpkg-new is executed by ifupdown2
> because it executes all scripts in /etc/network/if-pre-up.d, even if
> their name contains a dot
>
> This leads to ifreload failing on upgrades, which in turn causes
> issues with the networking of upgraded hosts.
>
> Also submitted upstream at [2]
>
> [1]
> https://github.com/CumulusNetworks/ifupdown2/blob/ccdc386cfab70703b657fe7c0ffceb95448a9c2b/ifupdown2/addons/bond.py#L45
> [2] https://github.com/CumulusNetworks/ifupdown2/pull/304
>
> Signed-off-by: Stefan Hanreich
> ---
> ...dpkg-files-when-running-hook-scripts.patch | 54 +++
> debian/patches/series | 1 +
> 2 files changed, 55 insertions(+)
> create mode 100644
> debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
>
> diff --git
> a/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
>
> b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
> new file mode 100644
> index 000..eea615f
> --- /dev/null
> +++
> b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
> @@ -0,0 +1,54 @@
> +From dbb759a1383cf736a0fa769c5c5827e1e7f8145c Mon Sep 17 00:00:00 2001
> +From: Stefan Hanreich
> +Date: Tue, 4 Jun 2024 16:17:54 +0200
> +Subject: [PATCH] main: ignore dpkg files when running hook scripts
> +
> +Currently ifupdown2 executes scripts that are backed up by dpkg (e.g.
> +foo.dpkg-old). This can lead to issues with hook scripts getting
> +executed after upgrading ifupdown2 via dpkg, even though they should
> +not be executed.
> +
> +This also brings ifupdown2 closer on par with the behavior of
> +ifupdown, which did not execute hook scripts with dpkg suffixes.
some nits for phrasing here:
- this not only affects ifupdown2 upgrades - any package shipping an
ifupdown hook script can potentially trigger this
- the 'via dpkg' is redundant IMHO (package updates always happen via
apt and dpkg ;))
- 'closer on par' is a bit of an oxymoron, I'd just say 'closer to' and
maybe add that ifupdown used/uses run-parts, and doesn't execute
dpkg-created copies of scripts for that reason
> +
> +Signed-off-by: Stefan Hanreich
> +---
> + ifupdown2/ifupdown/ifupdownmain.py | 4 +++-
> + ifupdown2/ifupdown/utils.py| 6 ++
> + 2 files changed, 9 insertions(+), 1 deletion(-)
> +
> +diff --git a/ifupdown2/ifupdown/ifupdownmain.py
> b/ifupdown2/ifupdown/ifupdownmain.py
> +index 51f5460..e6622f0 100644
> +--- a/ifupdown2/ifupdown/ifupdownmain.py
> b/ifupdown2/ifupdown/ifupdownmain.py
> +@@ -1540,7 +1540,9 @@ class ifupdownMain:
> + try:
> + module_list = os.listdir(msubdir)
> + for module in module_list:
> +-if self.modules.get(module) or module in
> self.overridden_ifupdown_scripts:
> ++if (self.modules.get(module)
> ++or module in self.overridden_ifupdown_scripts
> ++or utils.is_dpkg_file(module)):
> + continue
> + self.script_ops[op].append(msubdir + '/' + module)
> + except Exception:
> +diff --git a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py
> +index 05c7e48..3085e82 100644
> +--- a/ifupdown2/ifupdown/utils.py
> b/ifupdown2/ifupdown/utils.py
> +@@ -212,6 +212,12 @@ class utils():
> + # what we have in the cache (data retrieved via a netlink dump by
> + # nlmanager). nlmanager return all macs in lower-case
> +
> ++_dpkg_suffixes = (".dpkg-old", ".dpkg-dist", ".dpkg-new", ".dpkg-tmp")
> ++
> ++@staticmethod
> ++def is_dpkg_file(name):
&g
Re: [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror
as discussed off-list, we might want to offer a
"--remove-orphaned-files" mode for sync and gc, and clean up orphaned
dirs (need to be detected first) and files (already are) in addition to
this.
this patch already gets rid of the orphaned dirs created during regular
operations with a very low chance of false positives, but an additional
opt-in check in remove_dir so that only (recursively) empty dir trees
are removed would be great!
On June 24, 2024 9:57 pm, Stoiko Ivanov wrote:
> Currently a medium sync takes care to remove all files and the links
> in the individual snapshot directories, but leaves the empty snapshot
> hierarchy in place. This causes a medium sync to take longer than
> necessary, as we walk through the complete mirror on the medium,
> including all empty link directories.
>
> The status output is misleading as well, still showing more snapshots
> than actually present (in the sense of containing any files).
>
> Noticed on a system, which creates snapshots on a daily basis, after a
> cleanup removing many snapshots and running garbage collection on the
> mirror and the medium, still caused the medium sync to be quite slow.
>
> Signed-off-by: Stoiko Ivanov
> ---
> src/medium.rs | 18 +-
> src/types.rs | 2 +-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/medium.rs b/src/medium.rs
> index 53df50d..2c2d24e 100644
> --- a/src/medium.rs
> +++ b/src/medium.rs
> @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
>
> use crate::{
> config::{self, ConfigLockGuard, MediaConfig, MirrorConfig},
> -generate_repo_file_line,
> +generate_repo_file_line, mirror,
> mirror::pool,
> pool::Pool,
> types::{Diff, Snapshot, SNAPSHOT_REGEX},
> @@ -404,6 +404,22 @@ pub fn sync(
> Pool::create(_base, _pool)?
> };
>
> +let source_snapshots: HashSet =
> +mirror::list_snapshots()?.iter().cloned().collect();
> +let target_snapshots: HashSet =
> list_snapshots(medium_base, )?
> +.iter()
> +.cloned()
> +.collect();
> +let target_only_snapshots =
> target_snapshots.difference(_snapshots);
> +for snapshot in target_only_snapshots.into_iter() {
> +let path =
> target_pool.get_path(Path::new(_string()))?;
> +println!(
> +"Removing snapshot no longer present on mirror: {:?}",
> +_string()
> +);
> +target_pool.lock()?.remove_dir()?;
> +}
> +
> let source_pool: Pool = pool()?;
> source_pool.lock()?.sync_pool(_pool, medium.verify)?;
>
> diff --git a/src/types.rs b/src/types.rs
> index 7544d5e..94eee79 100644
> --- a/src/types.rs
> +++ b/src/types.rs
> @@ -71,7 +71,7 @@ const_regex! {
> type: String,
> format: ::Pattern(_REGEX),
> )]
> -#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)]
> +#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Hash)]
> /// Mirror snapshot
> pub struct Snapshot(i64);
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 storage] fix #5191: api, cli: implement moving a volume between storages
On June 25, 2024 4:53 pm, Filip Schauer wrote:
> Add the ability to move a backup, ISO, container template or snippet
> between storages and nodes via an API method. Moving a VMA backup to a
> Proxmox Backup Server requires the proxmox-vma-to-pbs package to be
> installed. Currently only VMA backups can be moved to a Proxmox Backup
> Server and moving backups from a Proxmox Backup Server is not yet
> supported.
>
> The method can be called from the PVE shell with `pvesm move-volume`:
>
> ```
> pvesm move-volume [--target-node ]
> [--delete]
> ```
>
> For example to move a VMA backup to a Proxmox Backup Server:
>
> ```
> pvesm move-volume \
> local:backup/vzdump-qemu-100-2024_06_25-13_08_56.vma.zst pbs
> ```
>
> Or move a container template to another node and delete the source:
>
> ```
> pvesm move-volume \
> local:vztmpl/devuan-4.0-standard_4.0_amd64.tar.gz local \
> --target-node pvenode2 --delete
> ```
>
> Or use curl to call the API method:
>
> ```
> curl
> https://$APINODE:8006/api2/json/nodes/$SOURCENODE/storage/$SOURCESTORAGE/content/$SOURCEVOLUME
> \
> --insecure --cookie "$( --data-raw "target-storage=$TARGETSTORAGE=$TARGETNODE"
> ```
>
> Signed-off-by: Filip Schauer
> ---
> This patch depends on
> [PATCH backup-qemu/vma-to-pbs 0/2] add support for notes and logs
> https://lists.proxmox.com/pipermail/pbs-devel/2024-May/009445.html
nit: it also needs a new dependency on proxmox-vma-to-pbs in d/control!
>
> Changes since v1:
> * Rename pvesm command to move-volume
> * Add a delete option to control whether the source volume should be
> kept
> * Move the API method to the POST endpoint of
> /nodes/{node}/storage/{storage}/content/{volume}, replacing the
> experimental copy method that has not been used since its introduction
> in October 2011 883eeea6.
> * Implement migrating volumes between nodes
>
> src/PVE/API2/Storage/Content.pm | 69 +-
> src/PVE/CLI/pvesm.pm| 60 +
> src/PVE/Storage.pm | 222
> src/PVE/Storage/Plugin.pm | 105 +++
> 4 files changed, 367 insertions(+), 89 deletions(-)
>
> diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
> index fe0ad4a..e4bf0cb 100644
> --- a/src/PVE/API2/Storage/Content.pm
> +++ b/src/PVE/API2/Storage/Content.pm
> @@ -484,10 +484,10 @@ __PACKAGE__->register_method ({
> }});
>
> __PACKAGE__->register_method ({
> -name => 'copy',
> +name => 'move',
> path => '{volume}',
> method => 'POST',
> -description => "Copy a volume. This is experimental code - do not use.",
> +description => "Move a volume.",
> protected => 1,
> proxyto => 'node',
> parameters => {
> @@ -499,14 +499,21 @@ __PACKAGE__->register_method ({
> description => "Source volume identifier",
> type => 'string',
> },
> - target => {
> - description => "Target volume identifier",
> + 'target-storage' => {
> + description => "Target storage",
> type => 'string',
> },
> - target_node => get_standard_option('pve-node', {
> + 'target-node' => get_standard_option('pve-node', {
> description => "Target node. Default is local node.",
> optional => 1,
> }),
> + delete => {
> + type => 'boolean',
> + description => "Delete the original volume after a successful
> copy. " .
> + "By default the original is kept.",
> + optional => 1,
> + default => 0,
> + },
> },
> },
this is missing `permissions => ..`, so it's root-only. is that
intentional?
> returns => {
> @@ -515,43 +522,33 @@ __PACKAGE__->register_method ({
> code => sub {
> my ($param) = @_;
>
> - my $rpcenv = PVE::RPCEnvironment::get();
> -
> - my $user = $rpcenv->get_user();
> -
> - my $target_node = $param->{target_node} || PVE::INotify::nodename();
> - # pvesh examples
> - # cd /nodes/localhost/storage/local/content
> - # pve:/> create local:103/vm-103-disk-1.raw -target
> local:103/vm-103-disk-2.raw
> - # pve:/> create 103/vm-103-disk-1.raw -target 103/vm-103-disk-3.raw
> -
> my $src_volid = &$real_volume_id($param->{storage}, $param->{volume});
> - my $dst_volid = &$real_volume_id($param->{storage}, $param->{target});
> -
> - print "DEBUG: COPY $src_volid TO $dst_volid\n";
> + my $dst_storeid = $param->{'target-storage'};
> + my ($src_storeid, $src_volname) =
> PVE::Storage::parse_volume_id($src_volid);
> + my $src_node = PVE::INotify::nodename();
> + my $dst_node = $param->{'target-node'} || $src_node;
> + my $delete = $param->{delete};
>
> my $cfg = PVE::Storage::config();
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $user = $rpcenv->get_user();
>
> - # do all parameter
[pve-devel] [PATCH storage] style: remove goto statements
these can just as well be `die` statements right there, there is no complicated
cleanup that would warrant a goto statement..
Signed-off-by: Fabian Grünbichler
---
src/PVE/Storage/Plugin.pm | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index b5a54c1..f8dc9a2 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1586,13 +1586,14 @@ sub read_common_header($) {
# Export a volume into a file handle as a stream of desired format.
sub volume_export {
my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
$base_snapshot, $with_snapshots) = @_;
+my $unsupported = "volume export format $format not available for
$class\n";
if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
my $file = $class->path($scfg, $volname, $storeid)
- or goto unsupported;
+ or die $unsupported;
my ($size, $file_format) = file_size_info($file);
if ($format eq 'raw+size') {
- goto unsupported if $with_snapshots || $file_format eq 'subvol';
+ die $unsupported if $with_snapshots || $file_format eq 'subvol';
write_common_header($fh, $size);
if ($file_format eq 'raw') {
run_command(['dd', "if=$file", "bs=4k", "status=progress"],
output => '>&'.fileno($fh));
@@ -1603,20 +1604,19 @@ sub volume_export {
return;
} elsif ($format =~ /^(qcow2|vmdk)\+size$/) {
my $data_format = $1;
- goto unsupported if !$with_snapshots || $file_format ne
$data_format;
+ die $unsupported if !$with_snapshots || $file_format ne
$data_format;
write_common_header($fh, $size);
run_command(['dd', "if=$file", "bs=4k", "status=progress"], output
=> '>&'.fileno($fh));
return;
} elsif ($format eq 'tar+size') {
- goto unsupported if $file_format ne 'subvol';
+ die $unsupported if $file_format ne 'subvol';
write_common_header($fh, $size);
run_command(['tar', @COMMON_TAR_FLAGS, '-cf', '-', '-C', $file,
'.'],
output => '>&'.fileno($fh));
return;
}
}
- unsupported:
-die "volume export format $format not available for $class";
+die $unsupported;
}
sub volume_export_formats {
--
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 proxmox-offline-mirror 1/2] bump proxmox-time to 2.0
applied this one already :)
On June 24, 2024 9:57 pm, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov
> ---
> Cargo.toml | 2 +-
> debian/control | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index bf7d754..edcdb87 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -35,4 +35,4 @@ proxmox-section-config = "2"
> proxmox-serde = "0.1"
> proxmox-subscription = { version = "0.4.2", features = [ "api-types" ] }
> proxmox-sys = { version = "0.5", features = [ "timer" ] }
> -proxmox-time = { version = "1.1.5" }
> +proxmox-time = "2"
> diff --git a/debian/control b/debian/control
> index b8c8ddc..86b6cfb 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -31,7 +31,7 @@ Build-Depends: bash-completion,
> librust-proxmox-subscription-0.4+default-dev (>= 0.4.2~),
> librust-proxmox-sys-0.5+default-dev,
> librust-proxmox-sys-0.5+timer-dev,
> - librust-proxmox-time-1+default-dev (>= 1.1.5~),
> + librust-proxmox-time-2+default-dev,
> librust-regex-1+default-dev (>= 1.5.5-~~),
> librust-sequoia-openpgp-1+default-dev (>= 1.7-~~),
> librust-serde-1+default-dev,
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-secure-boot-support] ship apt pinning snippet
this should ensure that a shim-signed package from a non-Proxmox repository cannot overtake ours, even if the version is newer. since proxmox-secure-boot-support is optional, this is entirely opt-in. Signed-off-by: Fabian Grünbichler --- not the most elegant solution, but the only one I could come up with. the next bookworm point release will likely ship with a shim-signed version higher than our current one, so we probably want to roll this out rather fast.. debian/99-proxmox-secure-boot-support | 7 +++ debian/proxmox-secure-boot-support.install | 1 + 2 files changed, 8 insertions(+) create mode 100644 debian/99-proxmox-secure-boot-support create mode 100644 debian/proxmox-secure-boot-support.install diff --git a/debian/99-proxmox-secure-boot-support b/debian/99-proxmox-secure-boot-support new file mode 100644 index 000..03c4b89 --- /dev/null +++ b/debian/99-proxmox-secure-boot-support @@ -0,0 +1,7 @@ +# automatically added by proxmox-secure-boot-support, to ensure Proxmox version +# of shim-signed stays installed even if Debian repositories contain an +# upgraded version earlier than Proxmox ones, since they embed different +# certificates and are incompatible. +Package: shim-signed +Pin: release o=Proxmox +Pin-Priority: 900 diff --git a/debian/proxmox-secure-boot-support.install b/debian/proxmox-secure-boot-support.install new file mode 100644 index 000..f10aab3 --- /dev/null +++ b/debian/proxmox-secure-boot-support.install @@ -0,0 +1 @@ +debian/99-proxmox-secure-boot-support /etc/apt/preferences.d/ -- 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 proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml
with a follow-up to adapt the Makefiles - please test builds when
touching the build system ;)
On June 20, 2024 10:59 am, Lukas Wagner wrote:
> Fixes the following new warning that appeared after switching
> to rust 1.77:
>
> warning: `proxmox-perl-rs/pve-rs/.cargo/config` is deprecated in
> favor of `config.toml`
>
> Signed-off-by: Lukas Wagner
> ---
> pmg-rs/.cargo/{config => config.toml} | 0
> pve-rs/.cargo/{config => config.toml} | 0
> 2 files changed, 0 insertions(+), 0 deletions(-)
> rename pmg-rs/.cargo/{config => config.toml} (100%)
> rename pve-rs/.cargo/{config => config.toml} (100%)
>
> diff --git a/pmg-rs/.cargo/config b/pmg-rs/.cargo/config.toml
> similarity index 100%
> rename from pmg-rs/.cargo/config
> rename to pmg-rs/.cargo/config.toml
> diff --git a/pve-rs/.cargo/config b/pve-rs/.cargo/config.toml
> similarity index 100%
> rename from pve-rs/.cargo/config
> rename to pve-rs/.cargo/config.toml
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-kernel] fix #5448: cherry-pick SCSI VPD fix
via -stable cherry-pick, but otherwise identical. On June 17, 2024 9:49 am, Fiona Ebner wrote: > Am 06.06.24 um 09:02 schrieb Fabian Grünbichler: >> ping, it's in linux-next in the meantime :) >> > > And now in some mainline stable kernels and v6.10-rc3. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 manager] vzdump config: add fleecing property string
thanks!
On June 12, 2024 10:16 am, Fiona Ebner wrote:
> This makes it clear(er) that fleecing can be configured as a node-wide
> default too.
>
> Signed-off-by: Fiona Ebner
> ---
>
> Changes in v2:
> * rebase
>
> Since the new pbs-change-detection-mode got added, would be nice to
> add fleecing soon too to avoid pestering admins twice with the
> ==> Package distributor has shipped an updated version.
> dialoge during update.
>
> configs/vzdump.conf | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configs/vzdump.conf b/configs/vzdump.conf
> index 54913073..1835e208 100644
> --- a/configs/vzdump.conf
> +++ b/configs/vzdump.conf
> @@ -17,3 +17,4 @@
> #pigz: N
> #notes-template: {{guestname}}
> #pbs-change-detection-mode: legacy|data|metadata
> +#fleecing: enabled=BOOLEAN,storage=STORAGE_ID
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-backup-qemu] tree-wide: fix typos in comments
thanks!
On January 19, 2024 10:57 am, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner
> ---
> header-preamble.c | 4 ++--
> src/backup.rs | 2 +-
> src/commands.rs | 6 +++---
> src/lib.rs| 24
> 4 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/header-preamble.c b/header-preamble.c
> index b44ab32..e6a6068 100644
> --- a/header-preamble.c
> +++ b/header-preamble.c
> @@ -19,6 +19,6 @@
> * result: *mut c_int,
> * error: *mut *mut c_char,
> *
> - * The callback function is called when the the async function is
> - * ready. Possible errors are returned in 'error'.
> + * The callback function is called when the async function is ready.
> + * Possible errors are returned in 'error'.
> */
> diff --git a/src/backup.rs b/src/backup.rs
> index bbe4f00..d662520 100644
> --- a/src/backup.rs
> +++ b/src/backup.rs
> @@ -34,7 +34,7 @@ pub(crate) struct BackupTask {
> registry: Arc>>,
> known_chunks: Arc>>,
> abort: tokio::sync::broadcast::Sender<()>,
> -aborted: OnceCell, // set on abort, conatins abort reason
> +aborted: OnceCell, // set on abort, contains abort reason
> }
>
> impl BackupTask {
> diff --git a/src/commands.rs b/src/commands.rs
> index 37d653c..ba1832f 100644
> --- a/src/commands.rs
> +++ b/src/commands.rs
> @@ -58,7 +58,7 @@ pub(crate) fn deserialize_state(data: &[u8]) -> Result<(),
> Error> {
> Ok(())
> }
>
> -// Note: We alway register/upload a chunk containing zeros
> +// Note: We always register/upload a chunk containing zeros
> async fn register_zero_chunk(
> client: Arc,
> crypt_config: Option>,
> @@ -439,7 +439,7 @@ pub(crate) async fn write_data(
>
> match upload_queue {
> Some(ref mut upload_queue) => {
> -// Phase 2: send reponse future to other task
> +// Phase 2: send response future to other task
> if upload_queue.send(upload_future).await.is_err() {
> let upload_result = {
> let mut guard = registry.lock().unwrap();
> @@ -463,7 +463,7 @@ pub(crate) async fn write_data(
> }
> }
>
> -//println!("upload chunk sucessful");
> +//println!("upload chunk successful");
>
> Ok(if reused { 0 } else { size as c_int })
> }
> diff --git a/src/lib.rs b/src/lib.rs
> index 02e74f7..3b0c1fa 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -98,7 +98,7 @@ macro_rules! param_not_null {
>
> /// Returns the text presentation (relative path) for a backup snapshot
> ///
> -/// The resturned value is allocated with strdup(), and can be freed
> +/// The returned value is allocated with strdup(), and can be freed
> /// with free().
> #[no_mangle]
> #[allow(clippy::not_unsafe_ptr_arg_deref)]
> @@ -142,7 +142,7 @@ pub(crate) struct BackupSetup {
> pub fingerprint: Option,
> }
>
> -// helper class to implement synchrounous interface
> +// helper class to implement synchronous interface
> struct GotResultCondition {
> lock: Mutex,
> cond: Condvar,
> @@ -327,7 +327,7 @@ fn backup_handle_to_task(handle: *mut
> ProxmoxBackupHandle) -> Arc {
> /// Open connection to the backup server (sync)
> ///
> /// Returns:
> -/// 0 ... Sucecss (no prevbious backup)
> +/// 0 ... Success (no previous backup)
> /// 1 ... Success (found previous backup)
> /// -1 ... Error
> #[no_mangle]
> @@ -358,7 +358,7 @@ pub extern "C" fn proxmox_backup_connect(
> /// Open connection to the backup server
> ///
> /// Returns:
> -/// 0 ... Sucecss (no prevbious backup)
> +/// 0 ... Success (no previous backup)
> /// 1 ... Success (found previous backup)
> /// -1 ... Error
> #[no_mangle]
> @@ -565,7 +565,7 @@ pub extern "C" fn proxmox_backup_add_config_async(
> ///
> /// Returns:
> /// -1: on error
> -/// 0: successful, chunk already exists on server, so it was resued
> +/// 0: successful, chunk already exists on server, so it was reused
> /// size: successful, chunk uploaded
> #[no_mangle]
> #[allow(clippy::not_unsafe_ptr_arg_deref)]
> @@ -608,11 +608,11 @@ pub extern "C" fn proxmox_backup_write_data(
> /// (only allowed if size == chunk_size)
> ///
> /// Note: The data pointer needs to be valid until the async
> -/// opteration is finished.
> +/// operation is finished.
> ///
> /// Returns:
> /// -1: on error
> -/// 0: successful, chunk already exists on server, so it was resued
> +/// 0: successful, chunk already exists on server, so it was reused
> /// size: successful, chunk uploaded
> #[no_mangle]
> #[allow(clippy::not_unsafe_ptr_arg_deref)]
> @@ -772,7 +772,7 @@ fn restore_handle_to_task(handle: *mut
> ProxmoxRestoreHandle) -> Arc
> Arc::clone(restore_task)
> }
>
> -/// DEPRECATED: Connect the the backup server for restore (sync)
> +/// DEPRECATED: Connect to the backup server for restore (sync)
> ///
> /// Deprecated in favor of `proxmox_restore_new_ns` which includes a
> namespace parameter.
> /// Also, it used "lossy" utf8
[pve-devel] [PATCH docs] fix #5525: storage: pbs: improve master-pubkey docs
add the information that the parameter is special like other secret ones, and add the resulting config to the example to make it even more obvious. Signed-off-by: Fabian Grünbichler --- pve-storage-pbs.adoc | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pve-storage-pbs.adoc b/pve-storage-pbs.adoc index 84d598f..3140135 100644 --- a/pve-storage-pbs.adoc +++ b/pve-storage-pbs.adoc @@ -64,8 +64,11 @@ Optional. master-pubkey:: A public RSA key used to encrypt the backup encryption key as part of the -backup task. The encrypted copy will be appended to the backup and stored on -the Proxmox Backup Server instance for recovery purposes. +backup task. Will be saved in a file under +`/etc/pve/priv/storage/.master.pem` with access restricted to the +root user. +The encrypted copy of the backup encryption key will be appended to each backup +and stored on the Proxmox Backup Server instance for recovery purposes. Optional, requires `encryption-key`. .Configuration Example (`/etc/pve/storage.cfg`) @@ -77,6 +80,8 @@ pbs: backup fingerprint 09:54:ef:..snip..:88:af:47:fe:4c:3b:cf:8b:26:88:0b:4e:3c:b2 prune-backups keep-all=1 username archiver@pbs +encryption-key a9:ee:c8:02:13:..snip..:2d:53:2c:98 +master-pubkey = 1 Storage Features -- 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-series: [PATCH-SERIES qemu-server/manager] fix #5440: HMP/disk timeout improvements
thanks!
On May 3, 2024 1:19 pm, Fiona Ebner wrote:
> Currently, all HMP commands (issued as a "human-monitor-command" QMP
> command) use the default timeout for QMP commands of just 5 seconds.
>
> For drive-related commands, this is not always enough, as reported in
> bug #5440. Other drive-related QMP commands use 10 minutes, so it is
> natural to increase the limit for the HMP command for detaching a
> drive to that too.
>
> Other HMP commands that are issued are limited by being in a
> synchronous API call or interactive monitor session, but can still
> benefit from an increased timeout over the 5 second default.
>
> Lastly, the UI is adapted to use the asynchronous API call when
> hot-unplugging a drive, to be more in-line with the backend.
>
>
> qemu-server:
>
> Fiona Ebner (5):
> monitor: allow passing timeout for a HMP command
> api: human monitor: increase timeout to 25 seconds
> vzdump: increase timeout for attaching drives to 60 seconds
> cli: qm: increase timeout for monitor commands to 30 seconds
> fix #5440: hmp helpers: drive{add,del}: increase timeout
>
> PVE/API2/Qemu.pm | 2 +-
> PVE/CLI/qm.pm | 2 +-
> PVE/QemuServer.pm | 4 ++--
> PVE/QemuServer/Monitor.pm | 4 ++--
> PVE/VZDump/QemuServer.pm | 4 ++--
> 5 files changed, 8 insertions(+), 8 deletions(-)
>
>
> pve-manager:
>
> Fiona Ebner (2):
> ui: qemu: hardware: use background delay for asynchronous remove tasks
> ui: qemu: hardware: use asynchronous remove API call for disk
> hot-unplug
>
> www/manager6/qemu/HardwareView.js | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH container] backup: log errors from rsync
thanks!
On May 6, 2024 2:40 pm, Fiona Ebner wrote:
> Commit 5582b0c ("vzdump: rsync: make less verbose") talks about making
> the output less verbose, but it likely did not intended to get rid of
> the error messages from rsync, but only the uninteresting messages to
> stdout.
>
> The currently used log function is only matching the total
> transferred and ignores everything else. Split it into an output and
> error function and log all error messages from rsync.
>
> Excerpt from the output with the patch:
>
>> INFO: starting first sync /proc/55667/root/ to /mnt/tmp/vzdumptmp62235_113/
>> ERROR: rsync: [sender] send_files failed to open
>> "/proc/55667/root/foo/file": Input/output error (5)
>> ERROR: rsync error: some files/attrs were not transferred (see previous
>> errors) (code 23) at main.c(1338) [sender=3.2.7]
>> ERROR: Backup of VM 113 failed - command 'rsync --stats -h -X -A
>> --numeric-ids -aH --delete --no-whole-file --sparse --one-file-system
>> --relative '--exclude=/tmp/?*' '--exclude=/var/tmp/?*'
>> '--exclude=/var/run/?*.pid' /proc/55667/root//./ /proc/55667/root//./foo
>> /mnt/tmp/vzdumptmp62235_113/' failed: exit code 23
>> INFO: Failed at 2024-05-06 14:21:58
>> INFO: Backup job finished with errors
>
> Without the patch, the first two error messages with the root cause of
> the issue would not be shown, confusing users and leaving developers
> in the dark when trying to help.
>
> Examples from the forum:
> https://forum.proxmox.com/threads/146415/post-660946
> https://forum.proxmox.com/threads/101810/
> https://forum.proxmox.com/threads/101560/
> https://forum.proxmox.com/threads/79572/post-352377
>
> Fixes: 5582b0c ("vzdump: rsync: make less verbose")
> Signed-off-by: Fiona Ebner
> ---
> src/PVE/VZDump/LXC.pm | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
> index 8c28a5e..671f5f8 100644
> --- a/src/PVE/VZDump/LXC.pm
> +++ b/src/PVE/VZDump/LXC.pm
> @@ -52,13 +52,15 @@ my $rsync_vm = sub {
> }
>
> my $transferred = '';
> -my $logfunc = sub {
> +my $outfunc = sub {
> return if $_[0] !~ /^Total transferred file size: (.+)$/;
> $transferred = $1;
> };
>
> +my $errfunc = sub { $self->logerr($_[0]); };
> +
> my $starttime = time();
> -PVE::Tools::run_command([@$rsync, $to], logfunc => $logfunc);
> +PVE::Tools::run_command([@$rsync, $to], outfunc => $outfunc, errfunc =>
> $errfunc);
> my $delay = time () - $starttime;
>
> $self->loginfo ("$text sync finished - transferred $transferred in
> ${delay}s");
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v1 pve-common 1/3] section config: document package and its methods with POD
On June 4, 2024 11:28 am, Max Carrara wrote:
> Apart from the obvious benefits that documentation has, this also
> allows LSPs to provide docstrings e.g. via 'textDocument/hover' [0].
>
> Tested with Perl Navigator [1].
>
> [0]:
> https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_hover
> [1]: https://github.com/bscan/PerlNavigator
>
> Signed-off-by: Max Carrara
> ---
> src/PVE/SectionConfig.pm | 737 +++
> 1 file changed, 672 insertions(+), 65 deletions(-)
>
> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
> index a18e9d8..99ee348 100644
> --- a/src/PVE/SectionConfig.pm
> +++ b/src/PVE/SectionConfig.pm
> @@ -10,65 +10,102 @@ use PVE::Exception qw(raise_param_exc);
> use PVE::JSONSchema qw(get_standard_option);
> use PVE::Tools;
>
> -# This package provides a way to have multiple (often similar) types of
> entries
> -# in the same config file, each in its own section, thus "Section Config".
> -#
> -# The intended structure is to have a single 'base' plugin that inherits from
> -# this class and provides meaningful defaults in its '$defaultData', e.g. a
> -# default list of the core properties in its propertyList (most often only
> 'id'
> -# and 'type')
> -#
> -# Each 'real' plugin then has it's own package that should inherit from the
> -# 'base' plugin and returns it's specific properties in the 'properties'
> method,
> -# its type in the 'type' method and all the known options, from both parent
> and
> -# itself, in the 'options' method.
> -# The options method can also be used to define if a property is 'optional'
> or
> -# 'fixed' (only settable on config entity-creation), for example:
> -#
> -#
> -# sub options {
> -# return {
> -# 'some-optional-property' => { optional => 1 },
> -# 'a-fixed-property' => { fixed => 1 },
> -# 'a-required-but-not-fixed-property' => {},
> -# };
> -# }
> -# ```
> -#
> -# 'fixed' options can be set on create, but not changed afterwards.
> -#
> -# To actually use it, you have to first register all the plugins and then
> init
> -# the 'base' plugin, like so:
> -#
> -# ```
> -# use PVE::Dummy::Plugin1;
> -# use PVE::Dummy::Plugin2;
> -# use PVE::Dummy::BasePlugin;
> -#
> -# PVE::Dummy::Plugin1->register();
> -# PVE::Dummy::Plugin2->register();
> -# PVE::Dummy::BasePlugin->init();
> -# ```
> -#
> -# There are two modes for how properties are exposed, the default 'unified'
> -# mode and the 'isolated' mode.
> -# In the default unified mode, there is only a global list of properties
> -# which the plugins can use, so you cannot define the same property name
> twice
> -# in different plugins. The reason for this is to force the use of identical
> -# properties for multiple plugins.
> -#
> -# The second way is to use the 'isolated' mode, which can be achieved by
> -# calling init with `1` as its parameter like this:
> -#
> -# ```
> -# PVE::Dummy::BasePlugin->init(property_isolation => 1);
> -# ```
> -#
> -# With this, each plugin get's their own isolated list of properties which it
> -# can use. Note that in this mode, you only have to specify the property in
> the
> -# options method when it is either 'fixed' or comes from the global list of
> -# properties. All locally defined ones get automatically added to the schema
> -# for that plugin.
> +=pod
> +
> +=head1 NAME
> +
> +SectionConfig
> +
> +=head1 DESCRIPTION
> +
> +This package provides a way to have multiple (often similar) types of entries
> +in the same config file, each in its own section, thus I.
> +
> +Under the hood, this package automatically creates and manages a matching
> +I for one's plugin architecture that is used to represent data
> +that is read from and written to the config file.
this sentence is too long and hard to parse.
For each SectionConfig-based config file, a I is derived
automatically. This schema can be used to implement CRUD operations for
the config data.
or something like that?
> +
> +Where this config file is located, as well as its permissions and other
> related
> +things, is up to the plugin author and is not handled by
> C
> +at all.
it's not really up to the plugin author? it's up to the author of the
code using SectionConfig, which might be someone else entirely. the
plugin author might be some third party..
> +
> +=head1 USAGE
> +
> +The intended structure is to have a single I that inherits from
> +this class and provides meaningful defaults in its C<$defaultData>, such as a
s/inherits from this class/uses the SectionConfig module as base module
in Perl, a class is a perl structure that has been blessed. while the
code here uses '$class', it's not really one.
> +default list of core C I. The I is
> +thus very similar to an I.
> +
> +Each I is then defined in its own package that should inherit
> +from the I and defines which I it itself provides
> and
> +uses, as well as which I it uses from the I.
> +
> +The
Re: [pve-devel] [PATCH v1 pve-common 3/3] section config: clean up parser logic
On June 4, 2024 11:28 am, Max Carrara wrote:
> In order to make the parser somewhat more maintainable in the future,
> this commit cleans up its logic and makes its control flow easier to
> follow.
it also mixes deeper changes with things like variable renaming, which
makes it unnecessarily hard to review..
>
> Signed-off-by: Max Carrara
> ---
> src/PVE/SectionConfig.pm | 189 ---
> 1 file changed, 98 insertions(+), 91 deletions(-)
>
> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
> index a6b0183..30faaa4 100644
> --- a/src/PVE/SectionConfig.pm
> +++ b/src/PVE/SectionConfig.pm
> @@ -1014,25 +1014,26 @@ The error.
> sub parse_config {
> my ($class, $filename, $raw, $allow_unknown) = @_;
>
> -my $pdata = $class->private();
> +if (!defined($raw)) {
> + return {
> + ids => {},
> + order => {},
> + digest => Digest::SHA::sha1_hex(''),
> + };
> +}
> +
> +my $re_begins_with_comment = qr/^\s*#/;
> +my $re_kv_pair = qr/^\s+ (\S+) (\s+ (.*\S) )? \s*$/x;
I am not sure this is really more readable? especially in a RE that is
basically only concerned with whitespace.. and for a RE that is only
used once..
>
> my $ids = {};
> my $order = {};
> -
> -$raw = '' if !defined($raw);
> -
> my $digest = Digest::SHA::sha1_hex($raw);
>
> -my $pri = 1;
> +my $current_order = 1;
this is actually a worse name than before. this would be something like
current_index? current_position? section_no?
> +my $line_no = 0;
>
> -my $lineno = 0;
> my @lines = split(/\n/, $raw);
> -my $nextline = sub {
> - while (defined(my $line = shift @lines)) {
> - $lineno++;
> - return $line if ($line !~ /^\s*#/);
> - }
> -};
> +my @errors;
what do we gain from this?
>
> my $is_array = sub {
> my ($type, $key) = @_;
> @@ -1043,106 +1044,112 @@ sub parse_config {
> return $schema->{type} eq 'array';
> };
>
> -my $errors = [];
> -while (@lines) {
> - my $line = $nextline->();
> +my $get_next_line = sub {
> + while (scalar(@lines)) {
> + my $line = shift(@lines);
> + $line_no++;
> +
> + next if ($line =~ m/$re_begins_with_comment/);
> +
> + return $line;
> + }
> +
> + return undef;
> +};
> +
> +my $skip_to_next_empty_line = sub {
> + while ($get_next_line->() ne '') {}
> +};
> +
> +while (defined(my $line = $get_next_line->())) {
> next if !$line;
>
> - my $errprefix = "file $filename line $lineno";
> + my $errprefix = "file $filename line $line_no";
>
> - my ($type, $sectionId, $errmsg, $config) =
> $class->parse_section_header($line);
> - if ($config) {
> - my $skip = 0;
> - my $unknown = 0;
> + my ($type, $section_id, $errmsg, $config) =
> $class->parse_section_header($line);
>
> - my $plugin;
> + if (!defined($config)) {
> + warn "$errprefix - ignore config line: $line\n";
> + next;
> + }
>
> - if ($errmsg) {
> - $skip = 1;
> - chomp $errmsg;
> - warn "$errprefix (skip section '$sectionId'): $errmsg\n";
> - } elsif (!$type) {
> - $skip = 1;
> - warn "$errprefix (skip section '$sectionId'): missing type -
> internal error\n";
> - } else {
> - if (!($plugin = $pdata->{plugins}->{$type})) {
> - if ($allow_unknown) {
> - $unknown = 1;
> - } else {
> - $skip = 1;
> - warn "$errprefix (skip section '$sectionId'):
> unsupported type '$type'\n";
> - }
> - }
> - }
> + if ($errmsg) {
> + chomp $errmsg;
> + warn "$errprefix (skip section '$section_id'): $errmsg\n";
> + $skip_to_next_empty_line->();
> + next;
> + }
> +
> + if (!$type) {
> + warn "$errprefix (skip section '$section_id'): missing type -
> internal error\n";
> + $skip_to_next_empty_line->();
> + next;
> + }
> +
> + my $plugin = eval { $class->lookup($type) };
> + my $is_unknown_type = defined($@) && $@ ne '';
> +
> + if ($is_unknown_type && !$allow_unknown) {
> + warn "$errprefix (skip section '$section_id'): unsupported type
> '$type'\n";
> + $skip_to_next_empty_line->();
> + next;
> + }
> +
> + # Parse kv-pairs of section - will go on until empty line is encountered
> + while (my $section_line = $get_next_line->()) {
> + if ($section_line =~ m/$re_kv_pair/) {
> + my ($key, $value) = ($1, $3);
>
> - while ($line = $nextline->()) {
> - next if $skip;
> -
> - $errprefix = "file $filename line $lineno";
> -
> - if ($line =~ m/^\s+(\S+)(\s+(.*\S))?\s*$/) {
> - my ($k, $v) = ($1, $3);
> -
>
[pve-devel] applied-series: [PATCH v4 pve-guest-common pve-container pve-manager 0/4] change-detection-mode for PBS
On June 10, 2024 11:57 am, Christian Ebner wrote:
> This patch series defines the vzdump configuration description as well
> as backup job configuration option in the WebUI to set the
> experimental `change-detection-mode` parameter to the
> proxmox-backup-client invocation for container backups having a Proxmox
> Backup Server storage target.
>
> This allows to either set the parameter to the node wide vzdump.conf
> or on a per job level. The following shows an exemplary job
> configuration taken from /etc/pve/jobs.cfg:
>
> ```
> vzdump: backup-ab2edf62-d43c
> schedule yearly
> compress zstd
> enabled 1
> fleecing 0
> mode snapshot
> notes-template {{guestname}}
> pbs-change-detection-mode metadata
> storage pbs-local
> vmid 100
> ```
>
> This patches depend on a proxmox-backup-client compiled with the
> following 2 patches applied:
> https://lists.proxmox.com/pipermail/pbs-devel/2024-June/009779.html
>
> Changes since version 3:
> - Use PBS default instead of redefining on PVE side
>
> pve-guest-common:
>
> Christian Ebner (1):
> vzdump: add PBS change detection mode configuration
>
> src/PVE/VZDump/Common.pm | 7 +++
> 1 file changed, 7 insertions(+)
>
> pve-container:
>
> Christian Ebner (1):
> vzdump: conditionally set PBS change detection mode option
>
> src/PVE/VZDump/LXC.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> pve-manager:
>
> Christian Ebner (2):
> www: advanced backup: add pbs change detection mode selector
> vzdump: add pbs-change-detection-mode to config template
>
> configs/vzdump.conf | 1 +
> www/manager6/panel/BackupAdvancedOptions.js | 23 +
> 2 files changed, 24 insertions(+)
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 pve-guest-common pve-container pve-manager 0/4] change-detection-mode for PBS
Reviewed-by: Fabian Grünbichler
with one slight caveat - I'd like to change the "Default" enum value in
PBS to something else "V1", "Legacy", .., which has some consequences
here, so I held off applying this for now..
On May 29, 2024 2:07 pm, Christian Ebner wrote:
> This patch series defines the vzdump configuration description as well
> as backup job configuration option in the WebUI to set the
> experimental `change-detection-mode` parameter to the
> proxmox-backup-client invocation for container backups having a Proxmox
> Backup Server storage target.
>
> This allows to either set the parameter to the node wide vzdump.conf
> or on a per job level. The following shows an exemplary job
> configuration taken from /etc/pve/jobs.cfg:
>
> ```
> vzdump: backup-ab2edf62-d43c
> schedule yearly
> compress zstd
> enabled 1
> fleecing 0
> mode snapshot
> notes-template {{guestname}}
> pbs-change-detection-mode metadata
> storage pbs-local
> vmid 100
> ```
>
> This patches depend on a proxmox-backup-client compiled with the
> following patch series applied:
> https://lists.proxmox.com/pipermail/pbs-devel/2024-May/009526.html
>
> pve-guest-common:
>
> Christian Ebner (1):
> vzdump: add PBS change detection mode configuration
>
> src/PVE/VZDump/Common.pm | 8
> 1 file changed, 8 insertions(+)
>
> pve-container:
>
> Christian Ebner (1):
> vzdump: conditionally set PBS change detection mode option
>
> src/PVE/VZDump/LXC.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> pve-manager:
>
> Christian Ebner (2):
> www: advanced backup: add pbs change detection mode selector
> vzdump: add pbs-change-detection-mode to config template
>
> configs/vzdump.conf | 1 +
> www/manager6/panel/BackupAdvancedOptions.js | 20
> 2 files changed, 21 insertions(+)
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox] fix #5513: apt: do not assume that sources.list file exists
thanks!
On June 5, 2024 10:29 am, Fiona Ebner wrote:
> Some users might want to switch to using only the newer .sources files
> already, which Debian is going to switch to in the long run.
>
> Signed-off-by: Fiona Ebner
> ---
> proxmox-apt/src/repositories/mod.rs | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-apt/src/repositories/mod.rs
> b/proxmox-apt/src/repositories/mod.rs
> index 7bfe9cae..26e4192c 100644
> --- a/proxmox-apt/src/repositories/mod.rs
> +++ b/proxmox-apt/src/repositories/mod.rs
> @@ -146,12 +146,21 @@ pub fn repositories() -> Result {
>
> let sources_list_d_path = PathBuf::from(APT_SOURCES_LIST_DIRECTORY);
>
> -match APTRepositoryFile::new(sources_list_path) {
> -Ok(Some(mut file)) => match file.parse() {
> -Ok(()) => files.push(file),
> -Err(err) => errors.push(err),
> -},
> -_ => bail!("internal error with '{}'", APT_SOURCES_LIST_FILENAME),
> +if sources_list_path.exists() {
> +if sources_list_path.is_file() {
> +match APTRepositoryFile::new(sources_list_path) {
> +Ok(Some(mut file)) => match file.parse() {
> +Ok(()) => files.push(file),
> +Err(err) => errors.push(err),
> +},
> +_ => bail!("internal error with '{}'",
> APT_SOURCES_LIST_FILENAME),
> +}
> +} else {
> +errors.push(APTRepositoryFileError {
> +path: APT_SOURCES_LIST_FILENAME.to_string(),
> +error: "not a regular file!".to_string(),
> +});
> +}
> }
>
> if !sources_list_d_path.exists() {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-kernel] fix #5448: cherry-pick SCSI VPD fix
ping, it's in linux-next in the meantime :)
On May 23, 2024 11:38 am, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler
> ---
> 6.8/master only, patch straight from the linked LKML thread
>
> affected user confirmed it fixes the issue, patch is very small and
> straight-forward.
>
> ...-devices-which-return-an-unusually-l.patch | 51 +++
> 1 file changed, 51 insertions(+)
> create mode 100644
> patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
>
> diff --git
> a/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
>
> b/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
> new file mode 100644
> index 000..513d79b
> --- /dev/null
> +++
> b/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
> @@ -0,0 +1,51 @@
> +From Mon Sep 17 00:00:00 2001
> +From: "Martin K. Petersen"
> +Date: Mon, 20 May 2024 22:30:40 -0400
> +Subject: [PATCH] scsi: core: Handle devices which return an unusually large
> + VPD page count
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Peter Schneider reported that a system would no longer boot after
> +updating to 6.8.4. Peter bisected the issue and identified commit
> +b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
> +fetching page") as being the culprit.
> +
> +Turns out the enclosure device in Peter's system reports a byteswapped
> +page length for VPD page 0. It reports "02 00" as page length instead
> +of "00 02". This causes us to attempt to access 516 bytes (page length
> ++ header) of information despite only 2 pages being present.
> +
> +Limit the page search scope to the size of our VPD buffer to guard
> +against devices returning a larger page count than requested.
> +
> +Cc: sta...@vger.kernel.org
> +Reported-by: Peter Schneider
> +Tested-by: Peter Schneider
> +Fixes: b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
> fetching page")
> +Link:
> https://lore.kernel.org/all/eec6ebbf-061b-4a7b-96dc-ea748aa4d...@googlemail.com/
> +Signed-off-by: Martin K. Petersen
> +Signed-off-by: Fabian Grünbichler
> +---
> + drivers/scsi/scsi.c | 7 +++
> + 1 file changed, 7 insertions(+)
> +
> +diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> +index 8cad9792a562..b3ff3a633a1e 100644
> +--- a/drivers/scsi/scsi.c
> b/drivers/scsi/scsi.c
> +@@ -350,6 +350,13 @@ static int scsi_get_vpd_size(struct scsi_device *sdev,
> u8 page)
> + if (result < SCSI_VPD_HEADER_SIZE)
> + return 0;
> +
> ++if (result > sizeof(vpd)) {
> ++dev_warn_once(>sdev_gendev,
> ++ "%s: long VPD page 0 length: %d bytes\n",
> ++ __func__, result);
> ++result = sizeof(vpd);
> ++}
> ++
> + result -= SCSI_VPD_HEADER_SIZE;
> + if (!memchr([SCSI_VPD_HEADER_SIZE], page, result))
> + return 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 mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs] intro: add information on copyright files
might not be obvious for users that are not involved with Debian.
Signed-off-by: Fabian Grünbichler
---
obviously, the same sentence with a different package name can be added to pmg
and pbs docs as well, if the wording is settled..
pve-intro.adoc | 4
1 file changed, 4 insertions(+)
diff --git a/pve-intro.adoc b/pve-intro.adoc
index a494efe..12129dd 100644
--- a/pve-intro.adoc
+++ b/pve-intro.adoc
@@ -174,6 +174,10 @@ https://www.gnu.org/licenses/agpl-3.0.html[GNU Affero
General Public
License, version 3]. This means that you are free to inspect the
source code at any time or contribute to the project yourself.
+Detailed license and copyright information for all installed packages can be
+found in `/usr/share/doc/{PACKAGE}/copyright` (replace `{PACKAGE}` with the
+package name, for example, `proxmox-ve`).
+
At Proxmox we are committed to use open source software whenever
possible. Using open source software guarantees full access to all
functionalities - as well as high security and reliability. We think
--
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-server] api: snapshot create/delete/rollback: print log line that mentions snapshot name
On May 29, 2024 1:07 pm, Dominik Csapak wrote: > On 5/29/24 12:09, Fabian Grünbichler wrote: >> On May 29, 2024 11:20 am, Dominik Csapak wrote: >>> so that an admin can see from the task logs which snapshot was rolled >>> back/created/removed without the need to look into the journal (to which >>> the admin might not have access to). >> >> good idea in general, but wouldn't it be better to log it before >> attempting the operation, so that the context is in the task log >> irrespective of how a potential error message looks like? > > i'd probably log both logging the end doesn't give you much (if the name is mentioned earlier already), since the task already has a status, so it's (heavily) implied ;) but it also doesn't really hurt :) >> (side note, we might want to think about making the whole snapshot >> process a bit more verbose? i.e., log the individual parts as well?) > > i noticed that i missed doing that for containers . > also for some storages there is already output (e.g. ceph logs the > snapshot progress) > > For a v2 i'd do the logging in 'snapshot_create' etc. in AbstractConfig, > that should handle both vms and ct, and there we can print between > phases too > > does that sound ok? sure! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server] api: snapshot create/delete/rollback: print log line that mentions snapshot name
On May 29, 2024 11:20 am, Dominik Csapak wrote:
> so that an admin can see from the task logs which snapshot was rolled
> back/created/removed without the need to look into the journal (to which
> the admin might not have access to).
good idea in general, but wouldn't it be better to log it before
attempting the operation, so that the context is in the task log
irrespective of how a potential error message looks like?
(side note, we might want to think about making the whole snapshot
process a bit more verbose? i.e., log the individual parts as well?)
>
> Signed-off-by: Dominik Csapak
> ---
> noticed while trying to find out, to which snapshot a vm was rolled back
> to on a host where i don't have access to the syslog ;)
>
> PVE/API2/Qemu.pm | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 2a1d4d79..903c60a4 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -5196,6 +5196,7 @@ __PACKAGE__->register_method({
> PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid:
> $snapname");
> PVE::QemuConfig->snapshot_create($vmid, $snapname,
> $param->{vmstate},
>$param->{description});
> + print "Created snapshot '$snapname'.\n";
> };
>
> return $rpcenv->fork_worker('qmsnapshot', $vmid, $authuser, $realcmd);
> @@ -5376,6 +5377,7 @@ __PACKAGE__->register_method({
> my $realcmd = sub {
> PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM
> $vmid: $snapname");
> PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
> + print "Rolled back to snapshot '$snapname'.\n";
>
> if ($param->{start} &&
> !PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
> PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node });
> @@ -5435,6 +5437,7 @@ __PACKAGE__->register_method({
> $lock_obtained = 1;
> PVE::Cluster::log_msg('info', $authuser, "delete snapshot VM $vmid:
> $snapname");
> PVE::QemuConfig->snapshot_delete($vmid, $snapname, $param->{force});
> + print "Removed snapshot '$snapname'.\n";
> };
>
> my $realcmd = sub {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import
On May 23, 2024 12:40 pm, Dominik Csapak wrote:
> On 5/22/24 12:08, Fabian Grünbichler wrote:
>> On April 29, 2024 1:21 pm, Dominik Csapak wrote:
>>> 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;
>>
>> another circular module dependency..
>>
true, sorry for the noise! :)
> why do you think so? nothing in storage is using PVE::GuestImport only
> PVE::GuestImport::OVF ?
>
>>> +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\+(.*)$/;
>>> +}
>>
>> this could just as well live in qemu-server, there's only two call sites
>> in one module there.. one of them even already has the parsed volname ;)
>
> true
>
>>
>>> +
>>> +sub
[pve-devel] [PATCH pve-kernel] fix #5448: cherry-pick SCSI VPD fix
Signed-off-by: Fabian Grünbichler
---
6.8/master only, patch straight from the linked LKML thread
affected user confirmed it fixes the issue, patch is very small and
straight-forward.
...-devices-which-return-an-unusually-l.patch | 51 +++
1 file changed, 51 insertions(+)
create mode 100644
patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
diff --git
a/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
b/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
new file mode 100644
index 000..513d79b
--- /dev/null
+++
b/patches/kernel/0016-scsi-core-Handle-devices-which-return-an-unusually-l.patch
@@ -0,0 +1,51 @@
+From Mon Sep 17 00:00:00 2001
+From: "Martin K. Petersen"
+Date: Mon, 20 May 2024 22:30:40 -0400
+Subject: [PATCH] scsi: core: Handle devices which return an unusually large
+ VPD page count
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Peter Schneider reported that a system would no longer boot after
+updating to 6.8.4. Peter bisected the issue and identified commit
+b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
+fetching page") as being the culprit.
+
+Turns out the enclosure device in Peter's system reports a byteswapped
+page length for VPD page 0. It reports "02 00" as page length instead
+of "00 02". This causes us to attempt to access 516 bytes (page length
++ header) of information despite only 2 pages being present.
+
+Limit the page search scope to the size of our VPD buffer to guard
+against devices returning a larger page count than requested.
+
+Cc: sta...@vger.kernel.org
+Reported-by: Peter Schneider
+Tested-by: Peter Schneider
+Fixes: b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
fetching page")
+Link:
https://lore.kernel.org/all/eec6ebbf-061b-4a7b-96dc-ea748aa4d...@googlemail.com/
+Signed-off-by: Martin K. Petersen
+Signed-off-by: Fabian Grünbichler
+---
+ drivers/scsi/scsi.c | 7 +++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
+index 8cad9792a562..b3ff3a633a1e 100644
+--- a/drivers/scsi/scsi.c
b/drivers/scsi/scsi.c
+@@ -350,6 +350,13 @@ static int scsi_get_vpd_size(struct scsi_device *sdev, u8
page)
+ if (result < SCSI_VPD_HEADER_SIZE)
+ return 0;
+
++ if (result > sizeof(vpd)) {
++ dev_warn_once(>sdev_gendev,
++"%s: long VPD page 0 length: %d bytes\n",
++__func__, result);
++ result = sizeof(vpd);
++ }
++
+ result -= SCSI_VPD_HEADER_SIZE;
+ if (!memchr([SCSI_VPD_HEADER_SIZE], page, result))
+ return 0;
--
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-server 1/2] migration: avoid crash with heavy IO on local VM disk
one nit below, otherwise very nice to have this!
On May 3, 2024 10:34 am, Fiona Ebner wrote:
> There is a possibility that the drive-mirror job is not yet done when
> the migration wants to inactivate the source's blockdrives:
>
>> bdrv_co_write_req_prepare: Assertion `!(bs->open_flags & BDRV_O_INACTIVE)'
>> failed.
>
> This can be prevented by using the 'write-blocking' copy mode (also
> called active mode) for the mirror. However, with active mode, the
> guest write speed is limited by the synchronous writes to the mirror
> target. For this reason, a way to start out in the faster 'background'
> mode and later switch to active mode was introduced in QEMU 8.2.
>
> The switch is done once the mirror job for all drives is ready to be
> completed to reduce the time spent where guest IO is limited.
>
> Reported rarely, but steadily over the years:
> https://forum.proxmox.com/threads/78954/post-353651
> https://forum.proxmox.com/threads/78954/post-380015
> https://forum.proxmox.com/threads/100020/post-431660
> https://forum.proxmox.com/threads/111831/post-482425
> https://forum.proxmox.com/threads/111831/post-499807
> https://forum.proxmox.com/threads/137849/
>
> Signed-off-by: Fiona Ebner
> ---
> PVE/QemuMigrate.pm| 9 ++
> PVE/QemuServer.pm | 41 +++
> test/MigrationTest/QemuMigrateMock.pm | 3 ++
> 3 files changed, 53 insertions(+)
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35ae..649cfec4 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -1142,6 +1142,15 @@ sub phase2 {
> $self->log('info', "$drive: start migration to $nbd_uri");
> PVE::QemuServer::qemu_drive_mirror($vmid, $drive, $nbd_uri, $vmid,
> undef, $self->{storage_migration_jobs}, 'skip', undef, $bwlimit, $bitmap);
> }
> +
> + my $kvm_version = PVE::QemuServer::kvm_user_version();
wouldn't this need to check the *running* qemu binary for the migrated
VM, not the *installed* qemu binary on the system?
> + if (min_version($kvm_version, 8, 2)) {
> + $self->log('info', "switching mirror jobs to actively synced mode");
> + PVE::QemuServer::qemu_drive_mirror_switch_to_active_mode(
> + $vmid,
> + $self->{storage_migration_jobs},
> + );
> + }
> }
>
> $self->log('info', "starting online/live migration on $migrate_uri");
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 82e7d6a6..5bc313e2 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -8118,6 +8118,47 @@ sub qemu_blockjobs_cancel {
> }
> }
>
> +# Callers should version guard this (only available with a binary >= QEMU
> 8.2)
> +sub qemu_drive_mirror_switch_to_active_mode {
> +my ($vmid, $jobs) = @_;
> +
> +my $switching = {};
> +
> +for my $job (sort keys $jobs->%*) {
> + print "$job: switching to actively synced mode\n";
> +
> + eval {
> + mon_cmd(
> + $vmid,
> + "block-job-change",
> + id => $job,
> + type => 'mirror',
> + 'copy-mode' => 'write-blocking',
> + );
> + $switching->{$job} = 1;
> + };
> + die "could not switch mirror job $job to active mode - $@\n" if $@;
> +}
> +
> +while (1) {
> + my $stats = mon_cmd($vmid, "query-block-jobs");
> +
> + my $running_jobs = {};
> + $running_jobs->{$_->{device}} = $_ for $stats->@*;
> +
> + for my $job (sort keys $switching->%*) {
> + if ($running_jobs->{$job}->{'actively-synced'}) {
> + print "$job: successfully switched to actively synced mode\n";
> + delete $switching->{$job};
> + }
> + }
> +
> + last if scalar(keys $switching->%*) == 0;
> +
> + sleep 1;
> +}
> +}
> +
> # Check for bug #4525: drive-mirror will open the target drive with the same
> aio setting as the
> # source, but some storages have problems with io_uring, sometimes even
> leading to crashes.
> my sub clone_disk_check_io_uring {
> diff --git a/test/MigrationTest/QemuMigrateMock.pm
> b/test/MigrationTest/QemuMigrateMock.pm
> index 1efabe24..2b9e91b3 100644
> --- a/test/MigrationTest/QemuMigrateMock.pm
> +++ b/test/MigrationTest/QemuMigrateMock.pm
> @@ -152,6 +152,9 @@ $MigrationTest::Shared::qemu_server_module->mock(
> }
> return;
> },
> +qemu_drive_mirror_switch_to_active_mode => sub {
> + return;
> +},
> set_migration_caps => sub {
> return;
> },
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 2/2] migration: add missing use statements
but slight follow-up question below
On May 3, 2024 10:34 am, Fiona Ebner wrote:
> There's functions from all of those being used, but without importing
> first.
>
> Signed-off-by: Fiona Ebner
> ---
> PVE/QemuMigrate.pm | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 649cfec4..312ddaf8 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -7,14 +7,17 @@ use IO::File;
> use IPC::Open2;
> use Time::HiRes qw( usleep );
>
> +use PVE::AccessControl;
> use PVE::Cluster;
> use PVE::Format qw(render_bytes);
> use PVE::GuestHelpers qw(safe_boolean_ne safe_string_ne);
> use PVE::INotify;
> +use PVE::JSONSchema;
> use PVE::RPCEnvironment;
> use PVE::Replication;
> use PVE::ReplicationConfig;
> use PVE::ReplicationState;
> +use PVE::Storage::Plugin;
this one is only used for a single check_connection call in `prepare`:
if ($scfg->{shared}) {
# PVE::Storage::activate_storage checks this for non-shared storages
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
warn "Used shared storage '$sid' is not online on source node!\n"
if !$plugin->check_connection($sid, $scfg);
}
can't we get rid of this? either we are live-migrating, then the storage
is already active and in use. or we are migrating offline, then shared
storages are irrelevant (unless it's a remote migration, but then we
will activate the storage anyhow)?
or, couldn't we replace it with an eval-ed call to activate_storage for
the same effect (to get a warning if the storage is somehow broken, even
though it is not a pre-requisite for migration)?
I'm currently trying to eliminate direct calls to plugin code (i.e., not
via PVE::Storage itself as entry point), and this seems like low-hanging
fruit ;) the same also exists in pve-container for migration..
> use PVE::Storage;
> use PVE::StorageTunnel;
> use PVE::Tools;
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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_path) {
> + die "only regular files are allowed\n";
> + }
> +
> + my $target_diskname
> + = $target_plugin->find_free_diskname($target_storeid, $target_scfg,
> $vmid, $fmt, 1);
thought some more about this part. I don't think we currently consider
find_free_diskname to be public API for consumption outside of plugins
(rightfully so, IMHO).
I wonder how we could avoid that problem here. we could extend the
existing rename feature to allow moving from an arbitrary path to the
target storage (but that is risky, since it might mean we are copying
and not moving/renaming, unless we add extra checks)?
or we could handle the "temp extracted volume" as a volume, allowing a
regular PVE::Storage::rename_volume call to work?
maybe would risk breaking existing external plugins, but we could bump
APIVER and APIAGE and check APIVER to only allow storages as extraction
target that explicitly opted into it?
> + $target_volname = "$vmid/" . $target_diskname;
> + $target_path = $target_plugin->filesystem_path($target_scfg,
> $target_volname);
> +
> + print "renaming $source_path to $target_path\n";
> + my $imagedir = $target_plugin->get_subdir($target_scfg, 'images');
> + mkpath "$imagedir/$vmid";
> +
> + rename($source_path, $target_path) or die "unable to move - $!\n";
> +};
> +if (my $err = $@) {
> + unlink $source_path;
> + unlink $target_path if defined($target_path);
> + rmdir $destdir;
> + die "error during extraction: $err\n";
> +}
> +
> +rmdir $destdir;
> +
> +return "$target_storeid:$target_volname";
> +}
> +
> +sub cleanup_extracted_image {
> +my ($source) = @_;
> +
> +my $cfg = PVE::Storage::config();
> +PVE::Storage::vdisk_free($cfg, $source);
> +}
> +
> +1;
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v3 3/4] api: create: implement extracting disks when needed for import-from
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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);
please use PVE::Storage instead!
> +
> + 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)) {
like noted in the patch introducing that helper, it could just be
inlined here..
> + 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 = [];
we already have a list of created volumes here that are cleaned up on
error ($vollist), so this is just to also clean them up after importing
if that worked? and then, it's basically just for live importing (since
for non-live imports, we can just free the volume right after the import
was successful?)? but live imports already have their own hash anyway
($live_import_mapping), we could just annotate the volume there?
>
> 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";
this is a bit hard to follow, it might be more readable to do
my $extracted_volid = ..;
$source = $extracted_volid;
even if the end result is the same, it makes it much more explicit what
is happening here with $source?
> + push @$delete_sources, $source;
this could just push to $vollist I think..
> + }
> 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);
then this would not be needed, since we cleanup all the freshly
allocated volumes above anyway..
> 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,
Re: [pve-devel] [PATCH qemu-server v3 4/4] api: create: add 'import-extraction-storage' parameter
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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;
this is not needed
>
> $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']);
> +}
> +
because this can just move up to/merged with the code where the
no-explicit-extraction-storage case is handled..
> $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";
should we mention the storage here as well?
> - $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",
something something "temporary" ;)
maybe
"Storage for temporarily extracted `import-from` image files (default:
import source storage)."
or something like that?
> + optional => 1,
> + completion => \::QemuServer::complete_storage,
> + }),
> },
> 1, # with_disk_alloc
> ),
> @@ -951,6 +973,7 @@ __PACKAGE__->register_method({
> my
Re: [pve-devel] [PATCH qemu-server v3 1/4] api: delete unused OVF.pm
On May 22, 2024 12:25 pm, Fabian Grünbichler wrote: > On April 29, 2024 1:21 pm, Dominik Csapak wrote: >> 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 -- > > as noted in the pve-storage patch, this should also drop > libxml-libxml-perl from d/control here.. sorry, this was meant for the next patch in qemu-server ;) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v3 1/4] api: delete unused OVF.pm
On April 29, 2024 1:21 pm, Dominik Csapak wrote: > 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 -- as noted in the pve-storage patch, this should also drop libxml-libxml-perl from d/control here.. > 2 files changed, 1 insertion(+), 54 deletions(-) > delete mode 100644 PVE/API2/Qemu/OVF.pm > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v3 08/10] api: allow ova upload/download
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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$!) {
was a bit stumped here, but the others have it as well - $filename is
normalized first and that removes any slashes anyway. this also means
uploaded OVAs only have a subset of characters compared to what we
accept otherwise. do we still want to be extra-cautious in case we relax
the normalization in the future, and check for the same characters we
allow otherwise? would be rather weird if users can upload files but
possible not even see them afterwards ^^
> + 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
>
>
>
>
Re: [pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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;
another circular module dependency..
> +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\+(.*)$/;
> +}
this could just as well live in qemu-server, there's only two call sites
in one module there.. one of them even already has the parsed volname ;)
> +
> +sub extract_disk_from_import_file {
I don't really like that this is using lots of plugin stuff..
> +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);
could be PVE::Storage::parse_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);
could be PVE::Storage::path
> +
> +my $target_scfg = PVE::Storage::storage_config($cfg, $target_storeid);
> +my
Re: [pve-devel] [PATCH storage v3 01/10] copy OVF.pm from qemu-server
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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;
Re: [pve-devel] [PATCH storage v3 02/10] plugin: dir: implement import content type
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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,
syntax error here (cleaned up in next patch)
> };
> 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};
> +
Re: [pve-devel] [PATCH storage v3 01/10] copy OVF.pm from qemu-server
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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;
this means the libxml-libxml-perl dependency should also move from
qemu-server to libpve-storage-perl
> +use File::Spec;
> +use File::Basename;
> +use Data::Dumper;
not used?
> +use Cwd 'realpath';
> +
> +use PVE::Tools;
> +use PVE::Storage;
this one here makes a circular dependency, since the DirPlugin then uses
this module.. it is within the repository though, which we have quite
often, but it's a bit of a bummer..
> +
> [..]
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH cluster v2] fix #5461: pvecm: use ssh_info_to_command for intra cluster ssh
On May 16, 2024 2:08 pm, Aaron Lauterer wrote:
> because otherwise the SSH calls to other nodes in the cluster will fail
> on newer clusters that only have the ssh host keys located in the
> pmxcfs.
>
> With ssh_info_to_command we get all the needed SSH options that set the
> alias and point to the right known_hosts file.
>
> Signed-off-by: Aaron Lauterer
> ---
> changes since v1:
> * use ssh_info_to_command to get the cmd call & options in one go
> this gives us short enough line lenghts
> * remove $ssh_cmd from qdevice remove
> * cleaned up \&$outsub -> $outsub symbol salad ;)
>
> src/PVE/CLI/pvecm.pm | 40 +---
> 1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
> index 0e8ca8f..cc8b3d3 100755
> --- a/src/PVE/CLI/pvecm.pm
> +++ b/src/PVE/CLI/pvecm.pm
> @@ -18,6 +18,7 @@ use PVE::PTY;
> use PVE::API2::ClusterConfig;
> use PVE::Corosync;
> use PVE::Cluster::Setup;
> +use PVE::SSHInfo;
>
> use base qw(PVE::CLIHandler);
>
> @@ -173,10 +174,11 @@ __PACKAGE__->register_method ({
> run_command([@$scp_cmd, "root\@\[$qnetd_addr\]:$ca_export_file",
> "/etc/pve/$ca_export_base"]);
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_cluster_cmd = PVE::SSHInfo::ssh_info_to_command({ ip =>
> $ip, name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> run_command(
> - [@$ssh_cmd, $ip, $qdevice_certutil, "-i", "-c",
> "/etc/pve/$ca_export_base"],
> - noerr => 1, outfunc => \&$outsub
> + [@$ssh_cluster_cmd, '--', $qdevice_certutil, "-i", "-c",
> "/etc/pve/$ca_export_base"],
> + noerr => 1, outfunc => $outsub
> );
> });
> unlink "/etc/pve/$ca_export_base";
> @@ -206,11 +208,12 @@ __PACKAGE__->register_method ({
> run_command([@$scp_cmd, "$db_dir_node/$p12_file_base", "/etc/pve/"]);
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_cluster_cmd = PVE::SSHInfo::ssh_info_to_command({ ip =>
> $ip, name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> - run_command([
> - @$ssh_cmd, $ip, "$qdevice_certutil", "-m", "-c",
> - "/etc/pve/$p12_file_base"], outfunc => \&$outsub
> - );
> + run_command(
> + [@$ssh_cluster_cmd, '--', "$qdevice_certutil", "-m", "-c",
> "/etc/pve/$p12_file_base"],
> + outfunc => $outsub
> + );
> });
> unlink "/etc/pve/$p12_file_base";
>
> @@ -243,10 +246,17 @@ __PACKAGE__->register_method ({
>
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_cluster_cmd = PVE::SSHInfo::ssh_info_to_command({ ip =>
> $ip, name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> print "\nINFO: start and enable corosync qdevice daemon on node
> '$node'...\n";
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'start',
> 'corosync-qdevice'], outfunc => \&$outsub);
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'enable',
> 'corosync-qdevice'], outfunc => \&$outsub);
> + run_command(
> + [@$ssh_cluster_cmd, '--', 'systemctl', 'start',
> 'corosync-qdevice'],
> + outfunc => $outsub
> + );
> + run_command(
> + [@$ssh_cluster_cmd, '--', 'systemctl', 'enable',
> 'corosync-qdevice'],
> + outfunc => $outsub
> + );
> });
>
> run_command(['corosync-cfgtool', '-R']); # do cluster wide config reload
> @@ -276,8 +286,6 @@ __PACKAGE__->register_method ({
> if !$members->{$node}->{online};
> }
>
> - my $ssh_cmd = ['ssh', '-o', 'BatchMode=yes', '-lroot'];
> -
> my $code = sub {
> my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> my $quorum_section = $conf->{main}->{quorum};
> @@ -291,8 +299,9 @@ __PACKAGE__->register_method ({
> # cleanup qdev state (cert storage)
> my $qdev_state_dir = "/etc/corosync/qdevice";
> $foreach_member->(sub {
> - my (undef, $ip) = @_;
> - run_command([@$ssh_cmd, $ip, '--', 'rm', '-rf',
> $qdev_state_dir]);
> + my ($node, $ip) = @_;
> + my $ssh_cluster_cmd = PVE::SSHInfo::ssh_info_to_command({ ip =>
> $ip, name => $node });
> + run_command([@$ssh_cluster_cmd, '--', 'rm', '-rf',
> $qdev_state_dir]);
> });
> };
>
> @@ -300,9 +309,10 @@ __PACKAGE__->register_method ({
> die $@ if $@;
>
> $foreach_member->(sub {
> - my (undef, $ip) = @_;
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'stop',
> 'corosync-qdevice']);
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'disable',
> 'corosync-qdevice']);
> + my ($node, $ip) = @_;
> + my $ssh_cluster_cmd =
Re: [pve-devel] [PATCH cluster 1/2] fix #5461: pvecm: ssh: adapt intra cluster ssh options
On May 15, 2024 12:32 pm, Aaron Lauterer wrote:
> because otherwise the SSH calls to other nodes in the cluster will fail
> on newer clusters that only have the ssh known host keys located in the
> pmxcfs.
>
> By utilizing SSHInfo::ssh_info_to_ssh_opts we can add the needed options
> to the SSH call to have the node name aliased correctly and pointing SSH
> to the correct known hosts file.
couldn't this completely be switched over to use ssh_info_to_command ?
then we'd also benefit from other existing and future additions there
ssh_info_to_ssh_opts is basically the escape hatch for cases where that
does not work, like scp ;)
> Signed-off-by: Aaron Lauterer
> ---
> src/PVE/CLI/pvecm.pm | 24 +++-
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
> index 0e8ca8f..5c285a9 100755
> --- a/src/PVE/CLI/pvecm.pm
> +++ b/src/PVE/CLI/pvecm.pm
> @@ -18,6 +18,7 @@ use PVE::PTY;
> use PVE::API2::ClusterConfig;
> use PVE::Corosync;
> use PVE::Cluster::Setup;
> +use PVE::SSHInfo;
>
> use base qw(PVE::CLIHandler);
>
> @@ -173,9 +174,10 @@ __PACKAGE__->register_method ({
> run_command([@$scp_cmd, "root\@\[$qnetd_addr\]:$ca_export_file",
> "/etc/pve/$ca_export_base"]);
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> run_command(
> - [@$ssh_cmd, $ip, $qdevice_certutil, "-i", "-c",
> "/etc/pve/$ca_export_base"],
> + [@$ssh_cmd, @$ssh_options, $ip, $qdevice_certutil, "-i", "-c",
> "/etc/pve/$ca_export_base"],
> noerr => 1, outfunc => \&$outsub
> );
> });
> @@ -206,9 +208,10 @@ __PACKAGE__->register_method ({
> run_command([@$scp_cmd, "$db_dir_node/$p12_file_base", "/etc/pve/"]);
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> run_command([
> - @$ssh_cmd, $ip, "$qdevice_certutil", "-m", "-c",
> + @$ssh_cmd, @$ssh_options, $ip, "$qdevice_certutil", "-m",
> "-c",
> "/etc/pve/$p12_file_base"], outfunc => \&$outsub
> );
> });
> @@ -243,10 +246,11 @@ __PACKAGE__->register_method ({
>
> $foreach_member->(sub {
> my ($node, $ip) = @_;
> + my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> print "\nINFO: start and enable corosync qdevice daemon on node
> '$node'...\n";
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'start',
> 'corosync-qdevice'], outfunc => \&$outsub);
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'enable',
> 'corosync-qdevice'], outfunc => \&$outsub);
> + run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'start',
> 'corosync-qdevice'], outfunc => \&$outsub);
> + run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'enable',
> 'corosync-qdevice'], outfunc => \&$outsub);
> });
>
> run_command(['corosync-cfgtool', '-R']); # do cluster wide config reload
> @@ -291,8 +295,9 @@ __PACKAGE__->register_method ({
> # cleanup qdev state (cert storage)
> my $qdev_state_dir = "/etc/corosync/qdevice";
> $foreach_member->(sub {
> - my (undef, $ip) = @_;
> - run_command([@$ssh_cmd, $ip, '--', 'rm', '-rf',
> $qdev_state_dir]);
> + my ($node, $ip) = @_;
> + my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip =>
> $ip, name => $node });
> + run_command([@$ssh_cmd, @$ssh_options, $ip, '--', 'rm', '-rf',
> $qdev_state_dir]);
> });
> };
>
> @@ -300,9 +305,10 @@ __PACKAGE__->register_method ({
> die $@ if $@;
>
> $foreach_member->(sub {
> - my (undef, $ip) = @_;
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'stop',
> 'corosync-qdevice']);
> - run_command([@$ssh_cmd, $ip, 'systemctl', 'disable',
> 'corosync-qdevice']);
> + my ($node, $ip) = @_;
> + my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> + run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'stop',
> 'corosync-qdevice']);
> + run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'disable',
> 'corosync-qdevice']);
> });
>
> run_command(['corosync-cfgtool', '-R']);
> --
> 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 cluster 2/2] pvecm: qdevice: adjust line lengths
On May 15, 2024 12:32 pm, Aaron Lauterer wrote:
> The first instance had the line break mid array. It now is a bit over
> the limit, but follows the style guide closely: putting each argument to
> the function in a newline.
>
> Signed-off-by: Aaron Lauterer
> ---
> src/PVE/CLI/pvecm.pm | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
> index 5c285a9..53e9cac 100755
> --- a/src/PVE/CLI/pvecm.pm
> +++ b/src/PVE/CLI/pvecm.pm
> @@ -210,10 +210,10 @@ __PACKAGE__->register_method ({
> my ($node, $ip) = @_;
> my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> - run_command([
> - @$ssh_cmd, @$ssh_options, $ip, "$qdevice_certutil", "-m",
> "-c",
> - "/etc/pve/$p12_file_base"], outfunc => \&$outsub
> - );
> + run_command(
> + [@$ssh_cmd, @$ssh_options, $ip, "$qdevice_certutil", "-m",
> "-c", "/etc/pve/$p12_file_base"],
> + outfunc => \&$outsub
this would then be even shorter if the options can be dropped
altogether. and while we are at it:
outfunc => $outsub
no need for symbol salad here :)
I don't mind the arg-per-line even if it would be below the limit then,
but as an alternative, you can always construct the full command first
to make the run_command line shorter.
e.g.:
my $cluster_ssh_cmd = PVE::SSHInfo::ssh_info_to_command({ ip => $ip, name =>
$node });
my $cmd = [@$cluster_ssh_cmd, '--', $qdevice_certutil, '-m', '-c',
"/etc/pve/$p12_file_base"];
run_command($cmd, outfunc => $outsub);
but that is mainly a matter of taste I'd say.
> + );
> });
> unlink "/etc/pve/$p12_file_base";
>
> @@ -249,8 +249,14 @@ __PACKAGE__->register_method ({
> my $ssh_options = PVE::SSHInfo::ssh_info_to_ssh_opts ({ ip => $ip,
> name => $node });
> my $outsub = sub { print "\nnode '$node': " . shift };
> print "\nINFO: start and enable corosync qdevice daemon on node
> '$node'...\n";
> - run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'start',
> 'corosync-qdevice'], outfunc => \&$outsub);
> - run_command([@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'enable',
> 'corosync-qdevice'], outfunc => \&$outsub);
> + run_command(
> + [@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'start',
> 'corosync-qdevice'],
> + outfunc => \&$outsub
> + );
> + run_command(
> + [@$ssh_cmd, @$ssh_options, $ip, 'systemctl', 'enable',
> 'corosync-qdevice'],
> + outfunc => \&$outsub
> + );
same as above applies here as well
> });
>
> run_command(['corosync-cfgtool', '-R']); # do cluster wide config reload
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] auto-installer: fix answer-request charset
'utf-' is a typo, and can trip up some servers that do strict
checking/matching.
Signed-off-by: Fabian Grünbichler
---
Notes:
see
https://forum.proxmox.com/threads/invalid-charset-on-automated-install-answer-http-fetch.145856/
proxmox-fetch-answer/src/fetch_plugins/http.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs
b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 1c5e9ea..21bc6a2 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -211,7 +211,7 @@ mod http_post {
answer = agent
.post()
-.set("Content-type", "application/json; charset=utf-")
+.set("Content-type", "application/json; charset=utf-8")
.send_string()?
.into_string()?;
} else {
@@ -231,7 +231,7 @@ mod http_post {
.build();
answer = agent
.post()
-.set("Content-type", "application/json; charset=utf-")
+.set("Content-type", "application/json; charset=utf-8")
.timeout(std::time::Duration::from_secs(60))
.send_string()?
.into_string()?;
--
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] fix #5414: use proper percentages in `pct df`
while some people write percentages as 0.XX , putting a % next to that is just
confusing. also, combined with the format modifier this would be rather lossy,
and also not match regular `df` output..
Signed-off-by: Fabian Grünbichler
---
src/PVE/CLI/pct.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
index c48321d..4504b54 100755
--- a/src/PVE/CLI/pct.pm
+++ b/src/PVE/CLI/pct.pm
@@ -433,7 +433,7 @@ __PACKAGE__->register_method({
my $used = $format->($df->{used});
my $avail = $format->($df->{avail});
- my $pc = sprintf('%.1f', $df->{used}/$df->{total});
+ my $pc = sprintf('%.1f', 100 * $df->{used} / $df->{total});
my $entry = [ $name, $mp->{volume}, $total, $used, $avail,
$pc, $path ];
push @list, $entry;
--
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 installer 2/2] skip proxmox-secure-boot-support if secureboot is not enabled
while it doesn't hurt to be installed, it also doesn't help in any fashion on
such systems.
Signed-off-by: Fabian Grünbichler
---
Notes:
only makes sense if proxmox-secure-boot-support is on the ISO
Proxmox/Install.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 82619ae..e2f8ad9 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1098,6 +1098,7 @@ _EOD
# upon upgrade - and conflict with each other - install the fitting
one only
next if ($deb =~ /grub-pc_/ && $run_env->{boot_type} ne 'bios');
next if ($deb =~ /grub-efi-amd64_/ && $run_env->{boot_type} ne
'efi');
+ next if ($deb =~ /^proxmox-secure-boot-support_/ &&
!$run_env->{secure_boot});
update_progress($count/$pkg_count, 0.5, 0.75, "extracting $deb");
print STDERR "extracting: $deb\n";
--
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 installer 1/2] move secure boot state to RunEnv
as preparation for using it in more than one place.
Signed-off-by: Fabian Grünbichler
---
Proxmox/Install.pm| 18 +-
Proxmox/Install/RunEnv.pm | 12 +++-
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 19f7dc1..82619ae 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -15,7 +15,7 @@ use Proxmox::Install::StorageConfig;
use Proxmox::Sys::Block qw(get_cached_disks wipe_disk partition_bootable_disk);
use Proxmox::Sys::Command qw(run_command syscmd);
-use Proxmox::Sys::File qw(file_read_all file_read_firstline file_write_all);
+use Proxmox::Sys::File qw(file_read_firstline file_write_all);
use Proxmox::UI;
# TODO: move somewhere better?
@@ -576,20 +576,12 @@ my sub chroot_chmod {
}
sub prepare_proxmox_boot_esp {
-my ($espdev, $targetdir) = @_;
+my ($espdev, $targetdir, $secureboot) = @_;
my $mode = '';
-# detect secure boot being enabled and switch to grub-on-ESP if it is
-if (-d "/sys/firmware/efi") {
- my $content = eval {
file_read_all("/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c")
};
- if ($@) {
- warn "Failed to read secure boot state: $@\n";
- } else {
- my @secureboot = unpack("C", $content);
- $mode = 'grub' if $secureboot[4] == 1;
- }
-}
+# if secure boot is enabled switch to grub-on-ESP
+$mode = 'grub' if $secureboot;
syscmd("chroot $targetdir proxmox-boot-tool init $espdev $mode") == 0 ||
die "unable to init ESP and install proxmox-boot loader on '$espdev'\n";
@@ -1237,7 +1229,7 @@ _EOD
foreach my $di (@$bootdevinfo) {
my $dev = $di->{devname};
if ($use_zfs) {
- prepare_proxmox_boot_esp($di->{esp}, $targetdir);
+ prepare_proxmox_boot_esp($di->{esp}, $targetdir,
$run_env->{secure_boot});
} else {
if (!$native_4k_disk_bootable) {
eval {
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 39505d0..7eaf96a 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -8,7 +8,7 @@ use JSON qw(from_json to_json);
use Proxmox::Log;
use Proxmox::Sys::Command qw(run_command CMD_FINISHED);
-use Proxmox::Sys::File qw(file_read_firstline);
+use Proxmox::Sys::File qw(file_read_all file_read_firstline);
use Proxmox::Sys::Block;
use Proxmox::Sys::Net;
@@ -285,6 +285,16 @@ sub query_installation_environment : prototype() {
$output->{hvm_supported} = query_cpu_hvm_support();
$output->{boot_type} = -d '/sys/firmware/efi' ? 'efi' : 'bios';
+if ($output->{boot_type} eq 'efi') {
+ my $content = eval {
file_read_all("/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c")
};
+ if ($@) {
+ log_warn("Failed to read secure boot state: $@\n");
+ } else {
+ my @secureboot = unpack("C", $content);
+ $output->{secure_boot} = $secureboot[4] == 1;
+ }
+}
+
my $err;
my $country;
if ($routes->{gateway4}) {
--
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 many 00/19] notifications: move template strings to template files; PBS preparations
On April 19, 2024 12:09 pm, Fiona Ebner wrote: > Am 09.04.24 um 15:25 schrieb Lukas Wagner: >> Bumps/dependencies: >> - proxmox_notify >> - libproxmox-rs-perl/libpve-rs-perl (needs bumped proxmox_notify) >> - libpve-notify-perl (needs bumped >> libproxmox-rs-perl/libpve-rs-perl) >> - pve-ha-manager (needs bumped libpve-notify-perl) >> - pve-manager (needs bumped libpve-notify-perl) >> - proxmox-mail-forward (optional. should not be affected by any >> changes, but I think >> it should be also be bumped for any larger proxmox_notify changes to >> avoid any >> weird hidden regressions due to mismatches of proxmox_notify >> > > Versioned breaks required: > - new pve-cluster will break old pve-manager and old pve-ha-manager > - new libproxmox-rs-perl/libpve-rs-perl will break old pve-cluster > > Still not too sure about putting the templates in > /usr/share/proxmox-ve/, but maybe @Thomas or @Fabian can chime in on that. without in-depth look at all the changes in this series, I think it would make most sense for the package shipping (most?) templates to "own" the dir where they are shipped. that seems to be pve-manager, so maybe /usr/share/pve-manager would be an okay fit? or, if we want to avoid having "pve-manager" there, we could of course also create /usr/share/pve or /usr/lib/pve or one of those with "proxmox" instead of "pve", and have that owned by pve-manager.. I dislike proxmox-ve - it might not be the case that it is always installed, which means potential pitfalls for such non-standard systems or if we ever make it optional like we did for PMG/PBS (granted, not very likely, but still..) - normally the /usr/share/$package dir should only be used by $package ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server] clone disk: prevent 'uninitialized value' warning for unused check
On April 19, 2024 8:51 am, Dominik Csapak wrote:
> since commit
> 1f743141 (fix #1905: Allow moving unused disks)
>
> we want to check the source drive name for 'unused', but in case of
> importing a volume from the 'import' content type (e.g. from esxi),
> there is no source drive name. So we have to first check if it's
> defined.
>
> Signed-off-by: Dominik Csapak
> ---
> i did not add a 'fixes' trailer, because when the patch was written
> there was no 'import' content-type where that could have failed.
>
> also at the time the 'import' was written, the patch checking for unused
> was not applied yet, so there no fault there too..
>
>
> PVE/QemuServer.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 661613df..28e630d3 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -8154,7 +8154,7 @@ sub clone_disk {
> my ($newvmid, $dst_drivename, $efisize) = $dest->@{qw(vmid drivename
> efisize)};
> my ($storage, $format) = $dest->@{qw(storage format)};
>
> -my $unused = $src_drivename =~ /^unused/;
> +my $unused = defined($src_drivename) && $src_drivename =~ /^unused/;
> my $use_drive_mirror = $full && $running && $src_drivename && !$snapname
> && !$unused;
>
> if ($src_drivename && $dst_drivename && $src_drivename ne
> $dst_drivename) {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
