Re: [pve-devel] [RFC installer 3/6] add answer file fetch script
I think this can be part of the auto-installer itself, instead of introducing another shell script somewhere. While yes, "do one thing and do it well", its a rather small part and not unreasonable to move it directly into the auto-installer. Or have a separate Rust-written executable for this, both have their own merits of course - just not shell :^) If and when we add other methods of retrieving the answer file (as illustrated in the cover letter), it makes sense to write this in a sensible language and have a modular architecture, rather than arcane shell scripting. (Apart from the fact that the latter might even be nearly impossible for some things.) On Tue, Sep 05, 2023 at 03:28:29PM +0200, Aaron Lauterer wrote: > > With the auto installer present, the crucial question is how we get the > answer file. This script implements the way of a local disk/partition > present, labelled 'proxmoxinst', lower or upper case, with the > 'answer.toml' file in the root directory. > > We either want to use it directly and call it from 'unconfigured.sh' or > see it as a first approach to showcase how it could be done. > > Signed-off-by: Aaron Lauterer > --- > start_autoinstall.sh | 50 > 1 file changed, 50 insertions(+) > create mode 100755 start_autoinstall.sh > > [..] ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 docs] sysadmin: add section 'Firmware Updates' and references
personally i would be cautious to include such deep links to vendor sites in our documentation, because they can be outdated fast and it becomes a catch-up game No strong feelings though if the others are fine with it. Besides that, consider this Reviewed-by: Dominik Csapak ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
LGTM and works as advertised. Needs pve-storage >= 8.0.3 (for compression parameter) consider both patches: Reviewed-by: Dominik Csapak Tested-by: Dominik Csapak ___ 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/3] fix #2816: restore: remove timeout when allocating disks
comment inline:
On 9/12/23 11:16, Fiona Ebner wrote:
10 minutes is not long enough when disks are large and/or network
storages are used when preallocation is not disabled. The default is
metadata preallocation for qcow2, so there are still reports of the
issue [0][1]. If allocation really does not finish like the comment
describing the timeout feared, just let the user cancel it.
Also note that when restoring a PBS backup, there is no timeout for
disk allocation, and there don't seem to be any user complaints yet.
The 5 second timeout for receiving the config from vma is kept,
because certain corruptions in the VMA header can lead to the
operation hanging there.
There is no need for the $tmp variable before setting back the old
timeout, because that is at least one second, so we'll always be able
to set the $oldtimeout variable to undef in time in practice.
Currently, there shouldn't even be an outer timeout in the first
place, because the only call path leading to here is via the create
API (also used by qmrestore), both of which don't set a timeout.
[0]: https://forum.proxmox.com/threads/126825/
[1]: https://forum.proxmox.com/threads/128093/
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index bf1de179..05283562 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7483,14 +7483,11 @@ sub restore_vma_archive {
$devinfo->{$devname} = { size => $size, dev_id => $dev_id };
} elsif ($line =~ m/^CTIME: /) {
# we correctly received the vma config, so we can disable
- # the timeout now for disk allocation (set to 10 minutes, so
- # that we always timeout if something goes wrong)
- alarm(600);
+ # the timeout now for disk allocation
+ alarm($oldtimeout || 0);
+ $oldtimeout = undef;
this part looks wrong to me, because AFAIU you want to disable the timeout
(by canceling the alarm), but what you do here is to set it to $oldtimeout
if that was set before?
i guess what we want to do here is:
alarm(0);
<... do stuff ...>
alarm($oldtimeout || 0);
$oldtimeout = undef;
?
&$print_devmap();
print $fifofh "done\n";
- my $tmp = $oldtimeout || 0;
- $oldtimeout = undef;
- alarm($tmp);
close($fifofh);
$fifofh = undef;
}
___
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/3] fix #2816: restore: remove timeout when allocating disks
sry just saw that this was already applied (some version of it) and the discussion had included about oldtimeout always being 0 here, but my point still stands for the applied code in case $oldtimeout is not 0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] tui: honor test mode flag when starting low-level install session
Even if the installer is run in release mode, the test-mode flag should
be honored on whether to start a test-installation or not.
The test mode is always forced on in debug builds, so the cfg()
conditionals can be dropped.
Signed-off-by: Christoph Heiss
---
proxmox-tui-installer/src/main.rs | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs
b/proxmox-tui-installer/src/main.rs
index 23a4ead..3f01713 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -727,6 +727,7 @@ fn install_progress_dialog(siv: &mut Cursive) ->
InstallerView {
let cb_sink = siv.cb_sink().clone();
let state = siv.user_data::().unwrap();
+let in_test_mode = state.in_test_mode;
let progress_text = TextContent::new("starting the installation ..");
let progress_task = {
@@ -736,16 +737,15 @@ fn install_progress_dialog(siv: &mut Cursive) ->
InstallerView {
let child = {
use std::process::{Command, Stdio};
-#[cfg(not(debug_assertions))]
-let (path, args, envs): (&str, [&str; 1], [(&str, &str); 0]) =
-("proxmox-low-level-installer", ["start-session"], []);
-
-#[cfg(debug_assertions)]
-let (path, args, envs) = (
-PathBuf::from("./proxmox-low-level-installer"),
-["-t", "start-session-test"],
-[("PERL5LIB", ".")],
-);
+let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) =
if in_test_mode {
+(
+"./proxmox-low-level-installer",
+&["-t", "start-session-test"],
+vec![("PERL5LIB", ".")],
+)
+} else {
+("proxmox-low-level-installer", &["start-session"], vec![])
+};
Command::new(path)
.args(args)
--
2.41.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
On September 20, 2023 1:07 pm, Dominik Csapak wrote: > LGTM and works as advertised. it breaks downloading container templates that are compressed with one of the "known" compression algorithms (such as gz). probably the detect-compression parameter and handling needs to go back in (that was the reason it was there in the first place!), or some other solution needs to be found.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
On 9/20/23 13:46, Fabian Grünbichler wrote: On September 20, 2023 1:07 pm, Dominik Csapak wrote: LGTM and works as advertised. it breaks downloading container templates that are compressed with one of the "known" compression algorithms (such as gz). probably the detect-compression parameter and handling needs to go back in (that was the reason it was there in the first place!), or some other solution needs to be found.. ah yes ofc, sorry for the oversight couldn't we simply check in the backend for the download for the content type? as we only really need to unpack isos? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
On 9/20/23 13:50, Dominik Csapak wrote: On 9/20/23 13:46, Fabian Grünbichler wrote: On September 20, 2023 1:07 pm, Dominik Csapak wrote: LGTM and works as advertised. it breaks downloading container templates that are compressed with one of the "known" compression algorithms (such as gz). probably the detect-compression parameter and handling needs to go back in (that was the reason it was there in the first place!), or some other solution needs to be found.. ah yes ofc, sorry for the oversight couldn't we simply check in the backend for the download for the content type? as we only really need to unpack isos? ah no that also does not work. so you're right, having a parameter that controls this is probably best ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
On September 20, 2023 1:50 pm, Dominik Csapak wrote: > On 9/20/23 13:46, Fabian Grünbichler wrote: >> On September 20, 2023 1:07 pm, Dominik Csapak wrote: >>> LGTM and works as advertised. >> >> it breaks downloading container templates that are compressed with one >> of the "known" compression algorithms (such as gz). >> >> probably the detect-compression parameter and handling needs to go back >> in (that was the reason it was there in the first place!), or some other >> solution needs to be found.. >> >> > > ah yes ofc, sorry for the oversight > > couldn't we simply check in the backend for the download for the content type? > as we only really need to unpack isos? the "query url" part doesn't know about (storage) content types. and it returns the file name, so we can't let it detect compression but throw that part away, else we get the uncompressed filename instead of the compressed one (exactly what happens with v7 now). that's why we originally made the client/GUI make the choice: iso download dialogue: - query url with compression support - allow overriding (de)compression - pass (de)compression to download if set other download dialogues (currently only templates): - query url without compression support - don't offer (de)compression choice - (de)compression is never set, thus never passed to download in addition, the download backend (which knows about content types) also only allows decompression for isos (at least for the time being, if we ever revisit and allow plain container template archives then all of this is moot anyway ;)) ___ 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/2] ceph: remove global pg bits setting
sadly removing these parameters would be a breaking change
i'd rather mark it deprecatedand non-functional in the description
and remove the functionality
then on the next major release, we can announce the breaking change and
remove the parameter
On 9/7/23 11:49, Maximiliano Sandoval wrote:
This setting was removed in [1] as part of the v13.0.2 tag.
[1] https://github.com/ceph/ceph/commit/e6acf2d1d528a2395947d446a57bec04a3a002dc
Signed-off-by: Maximiliano Sandoval
---
I did a grep search across multiple projects and I was not able to find
more uses of this option. A second pair of eyes might help here.
PVE/API2/Ceph.pm | 15 ---
1 file changed, 15 deletions(-)
diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 7e0763cf..a4101dee 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -158,16 +158,6 @@ __PACKAGE__->register_method ({
minimum => 1,
maximum => 7,
},
- pg_bits => {
- description => "Placement group bits, used to specify the " .
- "default number of placement groups.\n\nNOTE: 'osd pool " .
- "default pg num' does not work for default pools.",
- type => 'integer',
- default => 6,
- optional => 1,
- minimum => 6,
- maximum => 14,
- },
disable_cephx => {
description => "Disable cephx authentication.\n\n" .
"WARNING: cephx is a security feature protecting against " .
@@ -224,11 +214,6 @@ __PACKAGE__->register_method ({
$cfg->{client}->{keyring} =
'/etc/pve/priv/$cluster.$name.keyring';
}
- if ($param->{pg_bits}) {
- $cfg->{global}->{'osd pg bits'} = $param->{pg_bits};
- $cfg->{global}->{'osd pgp bits'} = $param->{pg_bits};
- }
-
if ($param->{network}) {
$cfg->{global}->{'public network'} = $param->{network};
$cfg->{global}->{'cluster network'} = $param->{network};
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
On 9/20/23 14:09, Fabian Grünbichler wrote: On September 20, 2023 1:50 pm, Dominik Csapak wrote: On 9/20/23 13:46, Fabian Grünbichler wrote: On September 20, 2023 1:07 pm, Dominik Csapak wrote: LGTM and works as advertised. it breaks downloading container templates that are compressed with one of the "known" compression algorithms (such as gz). probably the detect-compression parameter and handling needs to go back in (that was the reason it was there in the first place!), or some other solution needs to be found.. ah yes ofc, sorry for the oversight couldn't we simply check in the backend for the download for the content type? as we only really need to unpack isos? the "query url" part doesn't know about (storage) content types. and it returns the file name, so we can't let it detect compression but throw that part away, else we get the uncompressed filename instead of the compressed one (exactly what happens with v7 now). that's why we originally made the client/GUI make the choice: iso download dialogue: - query url with compression support - allow overriding (de)compression - pass (de)compression to download if set other download dialogues (currently only templates): - query url without compression support - don't offer (de)compression choice - (de)compression is never set, thus never passed to download in addition, the download backend (which knows about content types) also only allows decompression for isos (at least for the time being, if we ever revisit and allow plain container template archives then all of this is moot anyway ;)) Thank you for reviewing this! I will make a v8 very soon featuring detect_compression again! ___ 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/2] ceph: remove global pg bits setting
Am 20/09/2023 um 14:10 schrieb Dominik Csapak: > sadly removing these parameters would be a breaking change > @maximiliano: how does ceph react if one sets this currently? I.e., does it silently accepts unknown config keys, or does it break something? Because if it's the latter we can assume that nobody uses this anyway and simply remove it in any release. Otherwise, yes, dropping the usage but keeping the API parameter with reducing the description to something like: "depreacted, will be removed with Proxmox VE 9" would be slightly better, not that I think it will make a difference in practice (this is very niche stuff).. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 docs] sysadmin: add section 'Firmware Updates' and references
Am 20/09/2023 um 11:59 schrieb Dominik Csapak: > personally i would be cautious to include such deep links to vendor sites in > our > documentation, because they can be outdated fast and it becomes a catch-up > game > > No strong feelings though if the others are fine with it. I normally make web.archive scrape it, and sometimes even use that link then (otherwise it's just to have the option to switch to that link if even still relevant when the original breaks) https://web.archive.org/ Can you please do so and use those links instead and send a v2 with Dominik's R-b tag already included? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v12 5/6] add clipboard checkbox to VM Options
Rest of the series looks fine to me (and tested ok), but here i have some
comments/nits inline:
On 9/8/23 13:06, Markus Frank wrote:
Signed-off-by: Markus Frank
---
www/manager6/qemu/DisplayEdit.js | 8 +
www/manager6/qemu/Options.js | 52
2 files changed, 60 insertions(+)
diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js
index 9bb1763e..d7cd51a9 100644
--- a/www/manager6/qemu/DisplayEdit.js
+++ b/www/manager6/qemu/DisplayEdit.js
@@ -4,6 +4,9 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
onlineHelp: 'qm_display',
onGetValues: function(values) {
+ if (typeof this.originalConfig.clipboard !== 'undefined') {
+ values.clipboard = this.originalConfig.clipboard;
+ }
let ret = PVE.Parser.printPropertyString(values, 'type');
if (ret === '') {
return { 'delete': 'vga' };
@@ -11,6 +14,11 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
return { vga: ret };
},
+onSetValues: function(values) {
+ this.originalConfig = values;
+ return values;
+},
+
items: [{
name: 'type',
xtype: 'proxmoxKVComboBox',
diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index 7b112400..7b8283c6 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -154,6 +154,58 @@ Ext.define('PVE.qemu.Options', {
},
} : undefined,
},
+ vga: {
+ header: gettext('Clipboard'),
+ defaultValue: false,
+ renderer: function(value) {
+ let vga = PVE.Parser.parsePropertyString(value, 'type');
+ return vga.clipboard ? vga.clipboard.toUpperCase() : "auto
(SPICE)";
+ },
+ editor: caps.vms['VM.Config.HWType'] ? {
+ xtype: 'proxmoxWindowEdit',
+ subject: gettext('Clipboard'),
+ onlineHelp: 'qm_display',
+ items: {
+ xtype: 'pveDisplayInputPanel',
+ items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ name: 'clipboard',
+ itemId: 'clipboardBox',
+ fieldLabel: gettext('Clipboard'),
+ deleteDefaultValue: true,
+ value: '__default__',
+ comboItems: [
+ ['__default__', 'auto (SPICE)'],
nit: not sure if we really want to use 'auto (SPICE)' (@thomas) ?
wouldn't `${defaulttext} (SPICE)` fit our scheme better ?
+ ['vnc', 'VNC'],
+ ],
+ },
+ {
+ itemId: 'vdagentHint',
+ name: 'vdagentHint',
+ xtype: 'displayfield',
+ userCls: 'pmx-hint',
+ value: 'The SPICE Clipboard stops working when'
+
+ ' you are using the VNC Clipboard, as both'
+
+ ' rely on the same SPICE vdagent.',
+ },
two nits here:
1. we may want to show the hint only when VNC is enabled
2. I'd remove the vdagent reference, since that is only an implementation detail
so i'd write something like this:
Only one of either the SPICE or VNC clipboard can work at a time.
?
(also i'd put it in a gettext)
+ ],
+ onGetValues: function(values) {
+ values = Ext.apply(this.originalConfig, values);
+ if (values.delete === "clipboard") {
+ delete values.clipboard;
+ delete values.delete;
+ }
+ let ret = PVE.Parser.printPropertyString(values,
'type');
+ return { vga: ret };
+ },
not a nit:
this is missing the check if ret === '' (which then must send the delete
parameter)
otherwise you can get the empty string into the config
+ onSetValues: function(values) {
+ this.originalConfig =
PVE.Parser.parsePropertyString(values.vga, 'type');
+ return this.originalConfig;
+ },
+ },
+ } : undefined,
+ },
hotplug: {
header: gettext('Hotplug'),
defaultValue: 'disk,network,usb',
___
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/2] ceph: remove global pg bits setting
Thomas Lamprecht writes: > @maximiliano: how does ceph react if one sets this currently? > I.e., does it silently accepts unknown config keys, or does it break > something? Running ceph config set global osd_pg_bits 42 results in Error EINVAL: unrecognized config option 'osd_pg_bits' Having them set on /etc/pve/ceph.conf on the other hand does not throw any kind of warning as far as I can tell, but I suspect it does not do anything either. Please tell if I am missing something or if I should do a new version of the patch. -- Maximiliano ___ 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/2] ceph: remove global pg bits setting
Am 20/09/2023 um 15:42 schrieb Maximiliano Sandoval: > Thomas Lamprecht writes: > >> @maximiliano: how does ceph react if oe sets this currently? >> I.e., does it silently accepts unknown config keys, or does it break >> something? > > Running > > ceph config set global osd_pg_bits 42 > > results in > > Error EINVAL: unrecognized config option 'osd_pg_bits' > > Having them set on /etc/pve/ceph.conf on the other hand does not throw > any kind of warning as far as I can tell, but I suspect it does not do > anything either. Meh, as we do the latter it means that cephs will eat that value and just ignore it, so this is a question about how strict we want to keep the API "unbroken". Dominik is def. right that just removing the parameter from our API would be a breaking change in the technically correct sense. But, AFAICT, this was never exposed via the pveceph CLI command nor the web UI, so it's not that far-fetched to assume that it has close to zero use in the wild, but we surely cannot be certain about that (some user might have added it to their automatic provision script and us remove it the param now would suddenly cause that to fail after the upgrade). > Please tell if I am missing something or if I should do a new version of > the patch. While I'm not a favor of keeping things around that (Very Highly Probably™) nobody will miss anyway, here just keeping the parameter is simply way too cheap to not do that. So, let's not go for my normal route of ripping it out and see if somebody complains once it hits no-subscription, to then react accordingly. In summary, yes, please send a new version of this patch that keeps a (reduced) "pg_bits" API parameter, drops setting the value (as that is a NO-OP anyway) and maybe even adds a warning like: "the 'pg_bits' is deprecated and will be removed in the future" ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer] tui: honor test mode flag when starting low-level install session
Am 20/09/2023 um 13:39 schrieb Christoph Heiss:
> Even if the installer is run in release mode, the test-mode flag should
> be honored on whether to start a test-installation or not.
>
> The test mode is always forced on in debug builds, so the cfg()
> conditionals can be dropped.
>
> Signed-off-by: Christoph Heiss
> ---
> proxmox-tui-installer/src/main.rs | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
>
applied, thanks!
albeit not sure if returning that tuple is that nice, maybe something like
let exe = match in_test_mode {
true => "./proxmox-low-level-installer",
false => "proxmox-low-level-installer",
};
let mut cmd = Command::new(exe);
if choice {
cmd.args(&["-t", "start-session"])
.envs(vec![("PERL5LIB", ".")]);
} else {
cmd.args(&["start-session"]);
};
cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn();
would be nicer, even if slightly longer (possible it can be even written
shorter).
But no hard feelings, it's not like this is complicated code or the like.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC cluster/manager/network 0/6] Add support for DHCP servers to SDN
Le mercredi 13 septembre 2023 à 13:21 +, DERUMIER, Alexandre a écrit : > yes, use should be able to define his own ip too. (maybe directly in > a > ipam gui on the sdn subnet , or maybe on the vm nic gui (but > registering ip in ipam), I'm really not sure ...) Hi, I have done some tests with differents external ipam, to compare storing or not storing ip on proxmox side. Finally, It's not so easy without writing ip on proxmox side (in vm config or somewhere else), because to retrieve a reserved ip from external ipam when vm start, we need to lookup maybe from mac address, maybe from hostname of the vm, or maybe some custom attributes, but not all ipams accept same attributes. (at least phpipam && netbox don't support all features, or not easyly. Netbox for example, for macaddress need to register the full vm object && interfaces + mac + mapping to ip, Phpipam is a single ip object with mac as attribute). So I think the best way is still to write the ip into the vm config, this allow to inject already reserved ip in dhcp at vm start/migrate without need to call the ipam (also avoid start problem is ipam server is down). and this allow to use it for firewall ipfilter, I see a usecase for sdn vxlan too or special /32 route injection) I just need some protections for snapshot, but nothing too difficult, but we really need to avoid to try to manage in ipam multiple version/snapshot of ip entry for a vm. I had tried 2years ago, it was really painful to handle this in differents ipam. So maybe the best way is to forbid to change ip address when a snapshot already exist. I think we could implement ipam call like: create vm or add a new nic --> - qm create ... -net0 bridge=vnet,,ip=(auto|192.168.0.1|dynamic),ip6=(..) auto : search a free ip in ipam. write the ip address in net0: ...,ip= ip field 192.168.0.1: check if ip is free in ipam && register ip in ipam. write the ip in ip field. dynamic: write "ephemeral" in net0: ,ip=ephemeral (This is a dynamic ip registered at vm start, and release at vm stop) vm start - - if ip=ephemeral, find && register a free ip in ipam, write it in vm net0: ...,ip=192.168.0.10[E] . (maybe with a special flag [E] to indicate it's ephemeral) - read ip from vm config && inject in dhcp vm_stop --- if ip is ephemeral (netX: ip=192.168.0.10[E]), delete ip from ipam, set ip=ephemeral in vm config vm_destroy or nic remove/unplug - if netX: ...,ip=192.168.0.10 , remove ip from ipam nic update when vm is running: -- if ip is defined : netX: ip=192.168.0.10, we don't allow bridge change or ip change, as vm is not notified about theses changes, and still use old ip. We can allow nic hot-unplug && hotplug. (guest os will remove the ip on nic removal, and will call dhcp again on nic hotplug) nic hotplug with ip=auto: - --> add nic in pending state > find ip in ipam && write it in pending ---> do the hotplug in qemu. We need to handle the config revert to remove ip from ipam if the nic hotplug is blocked in pending state(I never see this case until os don't have pci_hotplug module loaded, but it's better to be carefull ) The ipam modules (internal pve, phpipam,netbox) are already for this, I think it shouldn't be too difficult. dnsmasq seem to have a reservation file option, where we can dynamically add ip-mac without need to reload it. I'll try it, re-using your current dnsmasq patches. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
