Re: [pve-devel] [PATCH v2 pve-storage 2/2] add lvmqcow2 plugin: (lvm with external qcow2 snapshot)

2024-10-25 Thread Fabian Grünbichler 

> DERUMIER, Alexandre  hat am 23.10.2024 
> 15:45 CEST geschrieben:
> 
>  
> >>I am not yet convinced this is somehow a good idea, but maybe you can
> >>convince me otherwise ;)

I maybe judged this too quickly - I thought this was combining LVM + a 
dir-based storage, but this is putting the qcow2 overlays on LVs, which I 
missed on the first pass!

> >>variant A: this is just useful for very short-lived snapshots
> >>variant B: these snapshots are supposed to be long-lived
> 
> Can you defined "short "/ "long"  ? and the different usecase ?
>
> because for me, a snapshot is a snapshot. Sometime I take a snapshot
> before doing some critical changes, but I can't known if I need to
> rollback in next 2h, or next month.

yes, this would be an example of a short-lived snapshot
 
> I think that "long-lived" usecase is backup (but we don't need it),
> or replication (this could apply here, if we want to add replication
> for disaster recovery)

backup would also be short-lived usually (the snapshot is just to take the 
backup, not to keep a backup). long-lived usually is something like "take daily 
snapshot and keep for a few weeks for file recovery", in addition to regular 
backups. or "snapshot because we just had an incidence and might need this for 
forensics in a few months" (can also be solved with backups, of course ;)).

the main difference between the two is - for short-lived snapshots performance 
implications related to snapshots existing are not that important. I can live 
with a few hours of degraded performance, if the snapshot is part of some 
important process/work flow. with long-lived snapshots there is a requirement 
for them to not hurt performance just by existing, because otherwise you can't 
use them. there is a second reason why long-lived snapshots can be impossible - 
if you need to decide up-front how "big" the delta of that snapshot can grow at 
most, then in PVE context, you always need to allocate the full volume size 
(regular thick LVM had both issues - bad performance, and new writes going into 
a thick snapshot volume).

if you can support long-lived snapshots, then you automatically also support 
short-lived snapshots. the other direction doesn't hold. since PVE only has one 
kind of snapshots, they need to be useful for long-lived snapshots.

> >>A is not something we want. we intentionally don't have non-thin LVM
> >>snapshots for example.
> 
> AFAIK, we never had implemented it because LVM snasphot is slow as
> hell.(as a lvm extent are around 4MB, if you want 4k on a snapshot, you
> need to reread and rewrite the 4MB,  so around 1000x over-
> amplification and slow iops)

see above - there's two issues, one is performance, the other is that you need 
to either
- make the snapshot smaller than the original volume (and risk running out of 
space)
- make the snapshot as big as the original volume (and blow up space 
requirements)

(thick) LVM snapshots basically barely work for the "take a consistent backup 
during quiet periods" use case, and not much else.

> This is really the main blocker for my customers migrating from vmware
> (and to be honest I have some of them going to oracle ovlm (with
> ovirt), because ovirt support it this way). 

> >>B once I create a single snapshot, the "original" storage only
> >>contains the data written up to that point, anything else is stored
> >>on the "snapshot" storage. this means my snapshot storage must be at
> >>least as fast/good/shared/.. as my original storage. in that case, I
> >>can just use the snapshot storage directly and ditch the original
> >>storage?
> 
> Sorry, but I don't understand why you are talking about
> original/snapshot storage ? I never have thinked to use another storage
> for external snapshot.
> 
> The patch is really to add external snapshot on same lvm storage,
> through lvm additional volume, but with qcow2 format to have good
> performance (vs slow lvm snapshot)

see above - I misread and answered too quickly.

I took a closer look and replied inline below with some comments - much of it 
mimics the comments for the dir plugin..

> > Signed-off-by: Alexandre Derumier  > cyllene.com>
> > ---
> >  src/PVE/Storage.pm    |   2 +
> >  src/PVE/Storage/LvmQcow2Plugin.pm | 460
> > ++
> >  src/PVE/Storage/Makefile  |   3 +-
> >  3 files changed, 464 insertions(+), 1 deletion(-)
> >  create mode 100644 src/PVE/Storage/LvmQcow2Plugin.pm
> > 
> > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> > index 57b2038..119998f 100755
> > --- a/src/PVE/Storage.pm
> > +++ b/src/PVE/Storage.pm
> > @@ -28,6 +28,7 @@ use PVE::Storage::Plugin;
> >  use PVE::Storage::DirPlugin;
> >  use PVE::Storage::LVMPlugin;
> >  use PVE::Storage::LvmThinPlugin;
> > +use PVE::Storage::LvmQcow2Plugin;
> >  use PVE::Storage::NFSPlugin;
> >  use PVE::Storage::CIFSPlugin;
> >  use PVE::Storage::ISCSIPlugin;
> > @@ -54,6 +55,7 @@ our $KNOWN_EXPORT_FORMATS = ['raw+size',
> > 'tar+size', 'qcow2+s

Re: [pve-devel] applied: [PATCH container 1/1] status: add some missing descriptions for status return properties

2024-10-25 Thread Thomas Lamprecht 
Am 24/10/2024 um 17:05 schrieb Dominik Csapak:
> On 10/24/24 14:20, Thomas Lamprecht wrote:
>> Am 21/10/2024 um 11:15 schrieb Dominik Csapak:
>>> Signed-off-by: Dominik Csapak 
>>> ---
>>>   src/PVE/LXC.pm | 43 ++-
>>>   1 file changed, 42 insertions(+), 1 deletion(-)
>>>
>>
>>
>> applied, thanks! FYI: I had to fix a duplicate key (see below) and made a few
>> small adjustments to the wording (not just your additions) as a separate 
>> follow-up.
>>
>>> +netin => {
>>> +   description => "The amount of traffic that was sent to the guest since 
>>> the process start,"
>>> +   ." in bytes.",
>>> +   type => 'integer',
>>> +   optional => 1,
>>> +   renderer => 'bytes',
>>> +},
>>> +netin => {
>>
>> changed this to netout and folded that into your commit.
> 
> uff, thanks for that. seems i was too fast with the copy&paste...
> 

yeah, no worries, I just thought fixing that up on applying was overall
quicker compared to requesting a v2 (that I then might overlook/forget for
a few days).

> i wonder if our check target could catch something like that, but
> probably not i guess

Would be nice if perl could warn about those things, FWIW perlcritic does
not catches this either and perl just takes the last value.


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

[pve-devel] applied: [PATCH manager 2/2] api: cluster/resources: add missing return properties

2024-10-25 Thread Thomas Lamprecht 
Am 21/10/2024 um 11:15 schrieb Dominik Csapak:
> used the same description as for the guests.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/Cluster.pm | 34 ++
>  1 file changed, 34 insertions(+)
> 
>

applied, with duplication of netin fixed and similar changes that the container
and qemu-server patches, thanks!


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

Re: [pve-devel] [PATCH v2 qemu-server 1/1] implement external snapshot

2024-10-25 Thread Fabian Grünbichler 


> DERUMIER, Alexandre  hat am 23.10.2024 
> 16:31 CEST geschrieben:
> 
>  
> >>if we want the current volume to keep its name, and the snapshot
> >>volume to actually contain *that* snapshot's data, we need some sort
> >>of rename dance here as well.. i.e., rename the current volume to
> >>have the snapshot volume name, then snapshot it back into the
> >>"current" name. not sure what the proper qmp runes would be to
> >>achieve that?
> 
> I really to check that, but I would like to keep it as most atomic than
> possible (avoid stream, double snapshot, and all fancy stuff just to
> take a snapshot. because shit happen  ^_^ , and generally it'll happen
> when you'll take a snapshot with multiple disk, you'll to manage
> recovery and current state of differents volumes)
> 
> Another stupid way is to use generic name for snapfile (maybe with
> ctime inside the name), and create symlinks when snapshot is taken.(
> qemu follow symlink and use realpaths for backing chain). 
> (and for lvm, it can be done with metadatas).
> 
> I'll really try to find a way with renaming volume, I'll keep you in
> touch.

yes, that might work as well. it's a bit less "clean", since looking at the 
storage itself then requires this extra knowledge to know what's what, but it 
shouldn't be too bad hopefully? curious to see what you come up with :)


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

Re: [pve-devel] [PATCH container] fix #5761: add the "discard" mount option

2024-10-25 Thread Fiona Ebner 
Am 09.10.24 um 16:22 schrieb Filip Schauer:
> Introduce the "discard" mount option for rootfs and mount points. This
> ensures that unused container volume blocks are discarded from the
> underlying storage backend when deleting files within the container.
> 
> Signed-off-by: Filip Schauer 

Reviewed-by: Fiona Ebner 
Tested-by: Fiona Ebner 

However, this misses the UI part or you can get non-editable volumes there.

> ---
>  src/PVE/LXC/Config.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index ce64c4c..e980f8a 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -311,7 +311,7 @@ sub __snapshot_rollback_get_unused {
>  cfs_register_file('/lxc/', \&parse_pct_config, \&write_pct_config);
>  
>  
> -my $valid_mount_option_re = qr/(noatime|lazytime|nodev|nosuid|noexec)/;
> +my $valid_mount_option_re = 
> qr/(discard|lazytime|noatime|nodev|noexec|nosuid)/;
>  
>  sub is_valid_mount_option {
>  my ($option) = @_;


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

[pve-devel] applied: [PATCH container] fix typos on user-visible strings

2024-10-25 Thread Thomas Lamprecht 
Am 11/10/2024 um 11:40 schrieb Maximiliano Sandoval:
> Signed-off-by: Maximiliano Sandoval 
> ---
>  src/PVE/API2/LXC.pm   | 4 ++--
>  src/PVE/CLI/pct.pm| 2 +-
>  src/PVE/LXC/Config.pm | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
>

applied, thanks! But fixed up the following whitespace error:

.git/rebase-apply/patch:25: space before tab in indent.
description => "You need 'VM.Allocate' permission on /vms/{vmid} or on 
the VM pool /pool/{pool}. " .
warning: 1 line adds whitespace errors.


___
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] installer-common: throw setup error if no network interfaces were found

2024-10-25 Thread Christoph Heiss 
We do that check already in the GUI, so add it for TUI (and by
extension, the auto-installer) too.

Signed-off-by: Christoph Heiss 
---
 proxmox-installer-common/src/setup.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/proxmox-installer-common/src/setup.rs 
b/proxmox-installer-common/src/setup.rs
index e4e609b..e9a5b96 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -193,6 +193,8 @@ pub fn installer_setup(in_test_mode: bool) -> 
Result<(SetupInfo, LocaleInfo, Run
 runtime_info.disks.sort();
 if runtime_info.disks.is_empty() {
 Err("The installer could not find any supported hard 
disks.".to_owned())
+} else if runtime_info.network.interfaces.is_empty() {
+Err("The installer could not find any supported network interface 
cards.".to_owned())
 } else {
 Ok((installer_info, locale_info, runtime_info))
 }
-- 
2.46.0



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

[pve-devel] applied: [PATCH manager v3 2/2] ui: lxc: add readonly option for device passthrough

2024-10-25 Thread Thomas Lamprecht 
Am 09/09/2024 um 14:50 schrieb Filip Schauer:
> Add a checkbox to the device passthrough dialogue for restricting
> write access to a device passed through to a container.
> 
> Signed-off-by: Filip Schauer 
> ---
>  www/manager6/lxc/DeviceEdit.js | 8 
>  1 file changed, 8 insertions(+)
> 
>

applied this one now too, thanks!


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

[pve-devel] [PATCH manager 2/2] api: tasks: fix return type of 'starttime'

2024-10-25 Thread Dominik Csapak 
starttime is parsed from a upid with perls `hex` which always returns an
integer

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

diff --git a/PVE/API2/Tasks.pm b/PVE/API2/Tasks.pm
index 4ea9832b..ae60d846 100644
--- a/PVE/API2/Tasks.pm
+++ b/PVE/API2/Tasks.pm
@@ -480,7 +480,7 @@ __PACKAGE__->register_method({
type => 'string',
},
starttime => {
-   type => 'number',
+   type => 'integer',
},
pstart => {
type => 'integer',
-- 
2.39.5



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

[pve-devel] [PATCH manager 1/2] api: cluster resources: add lock and tags to return schema

2024-10-25 Thread Dominik Csapak 
Signed-off-by: Dominik Csapak 
---
 PVE/API2/Cluster.pm | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index ba3c9528..2b53213f 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -315,6 +315,11 @@ __PACKAGE__->register_method({
type => 'string',
optional => 1,
},
+   lock => {
+   description => "The current config lock of the guets (type 
in qemu,lxc)",
+   type => 'string',
+   optional => 1,
+   },
uptime => {
description => "Uptime of node or virtual guest in seconds 
(when type in node,qemu,lxc).",
type => 'integer',
@@ -376,6 +381,11 @@ __PACKAGE__->register_method({
type => 'integer',
optional => 1,
},
+   tags => {
+   description => "The set tags of the guest (type in 
qemu,lxc)",
+   type  => "string",
+   optional => 1,
+   },
template => {
description => "Determines if the guest is a template. 
(type in qemu,lxc)",
type => 'boolean',
-- 
2.39.5



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

[pve-devel] [PATCH manager 12/12] ui: qemu: hardware: add eject button for cdroms

2024-10-25 Thread Daniel Herzig 
Eject by setting file to none.

Signed-off-by: Daniel Herzig 
---
 www/manager6/qemu/HardwareView.js | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/www/manager6/qemu/HardwareView.js 
b/www/manager6/qemu/HardwareView.js
index 59e670db..5d1c18a5 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -538,6 +538,45 @@ Ext.define('PVE.qemu.HardwareView', {
apiurl: '/api2/extjs/' + baseurl,
});
 
+   let eject_btn = new Proxmox.button.Button({
+   text: gettext('Eject'),
+   disabled: true,
+   selModel: sm,
+   RESTMethod: 'POST',
+   confirmMsg: function(rec) {
+   let warn = gettext("Are you sure you want to eject '{0}' ?");
+   let isofile = rec.data.value.split(",")[0];
+   let msg = Ext.String.format(warn, isofile);
+   return msg;
+   },
+   handler: function(btn, e, rec) {
+   let params = {};
+   params[rec.data.key] = 'none,media=cdrom';
+   if (btn.RESTMethod === 'POST') {
+   params.background_delay = 5;
+   }
+   Proxmox.Utils.API2Request({
+   url: '/api2/extjs/' + baseurl,
+   waitMsgTarget: me,
+   method: btn.RESTMethod,
+   params: params,
+   callback: () => me.reload(),
+   failure: response => Ext.Msg.alert('Error', 
response.htmlStatus),
+   success: function(response, options) {
+   if (btn.RESTMethod === 'POST' && response.result.data 
!== null) {
+   Ext.create('Proxmox.window.TaskProgress', {
+   autoShow: true,
+   upid: response.result.data,
+   listeners: {
+   destroy: () => me.reload(),
+   },
+   });
+   }
+   },
+   });
+   },
+   });
+
let efidisk_menuitem = Ext.create('Ext.menu.Item', {
text: gettext('EFI Disk'),
iconCls: 'fa fa-fw fa-hdd-o black',
@@ -608,6 +647,7 @@ Ext.define('PVE.qemu.HardwareView', {
edit_btn.disable();
diskaction_btn.disable();
revert_btn.disable();
+   eject_btn.disable();
return;
}
const { key, value } = rec.data;
@@ -619,6 +659,7 @@ Ext.define('PVE.qemu.HardwareView', {
 
const isCloudInit = isCloudInitKey(value);
const isCDRom = value && !!value.toString().match(/media=cdrom/);
+   const isISO = value && !!value.toString().match(/.iso/);
 
const isUnusedDisk = key.match(/^unused\d+/);
const isUsedDisk = !isUnusedDisk && row.isOnStorageBus && !isCDRom;
@@ -648,6 +689,7 @@ Ext.define('PVE.qemu.HardwareView', {
resize_menuitem.setDisabled(pending || !isUsedDisk);
 
revert_btn.setDisabled(!pending);
+   eject_btn.setDisabled((isCDRom && !cdromCap) || !isISO);
};
 
let editorFactory = (classPath, extraOptions) => {
@@ -751,6 +793,7 @@ Ext.define('PVE.qemu.HardwareView', {
remove_btn,
edit_btn,
diskaction_btn,
+   eject_btn,
revert_btn,
],
rows: rows,
-- 
2.39.5



___
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/1] status: add some missing description for status return properties

2024-10-25 Thread Thomas Lamprecht 
Am 21/10/2024 um 11:15 schrieb Dominik Csapak:
> i omitted the 'disk' property, since it's non functional currently,
> since we don't query the disk usage here (complicated to calculate,
> depending on the storage, or requires guest agent support, which is also
> non-trivial)
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/QemuServer.pm | 34 ++
>  1 file changed, 34 insertions(+)
> 
>

applied, thanks! Also renamed the respective duplicate netin property to
netout and made a similar follow-up as for pve-container


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

[pve-devel] [PATCH storage 1/2] fix #5779: rbd: allow to pass custom krbd map options

2024-10-25 Thread Friedrich Weber 
When KRBD is enabled for an RBD storage, the storage plugin calls out
to `rbd map` to map an RBD image as a block device on the host.
Sometimes it might be necessary to pass custom options to `rbd map`.
For instance, in some setups with Windows VMs, KRBD logs `bad
crc/signature` and VMs performance is degraded unless the `rxbounce`
option is enabled, as reported in the forum [1].

To allow users to specify custom options for KRBD, introduce a
corresponding `krbd-map-options` property to the RBD plugin. The
property is designed to only accept a supported set of map options.
For now, this is only the `rxbounce` map option, but the supported set
can be extended in the future.

The reasoning for constraining the supported set of map options
instead of allowing to pass a free-form option string is as follows:
If `rxbounce` turns out to be a sensible default, accepting a
free-form option string now will make it hard to switch over the
default to `rxbounce` while still allowing users to disable `rxbounce`
if needed. This would require scanning the free-form string for a
`norxbounce` or similar, which is cumbersome.

If users need to set a map option that `krbd-map-options` does not
support (yet), they can alternatively set the RBD config option
`rbd_default_map_options` [2].

[1] https://forum.proxmox.com/threads/155741/
[2] 
https://github.com/ceph/ceph/blob/b2a4bd840/src/common/options/rbd.yaml.in#L507

Signed-off-by: Friedrich Weber 
---
 src/PVE/Storage/Plugin.pm| 10 ++
 src/PVE/Storage/RBDPlugin.pm | 14 +-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 8cc693c..02be257 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -394,6 +394,16 @@ sub verify_dir_override {
 die "invalid override '$value'\n";
 }
 
+PVE::JSONSchema::register_format('pve-storage-krbd-map-option', 
\&verify_krbd_map_option);
+sub verify_krbd_map_option {
+my ($option, $noerr) = @_;
+
+return $option if $option eq 'rxbounce';
+
+return undef if $noerr;
+die "invalid krbd map option '$option'\n";
+}
+
 sub private {
 return $defaultData;
 }
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index f45ad3f..8110cff 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -394,6 +394,12 @@ sub properties {
description => "Client keyring contents (for external clusters).",
type => 'string',
},
+   'krbd-map-options' => {
+   description => "Additional map options (only for krbd). Supported:"
+   ." rxbounce.",
+   type => 'string',
+   format => 'pve-storage-krbd-map-option-list',
+   },
 };
 }
 
@@ -410,6 +416,7 @@ sub options {
krbd => { optional => 1 },
keyring => { optional => 1 },
bwlimit => { optional => 1 },
+   'krbd-map-options' => { optional => 1 },
 };
 }
 
@@ -726,7 +733,12 @@ sub map_volume {
 # features can only be enabled/disabled for image, not for snapshot!
 $krbd_feature_update->($scfg, $storeid, $img_name);
 
-my $cmd = $rbd_cmd->($scfg, $storeid, 'map', $name);
+my @additional_options = ();
+if ($scfg->{'krbd-map-options'}) {
+   @additional_options = ('--options', $scfg->{'krbd-map-options'});
+}
+
+my $cmd = $rbd_cmd->($scfg, $storeid, 'map', $name, @additional_options);
 run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
 
 return $kerneldev;
-- 
2.39.5



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

[pve-devel] [PATCH storage/docs 0/2] fix #5779: storage: rbd: allow setting custom KRBD map option(s)

2024-10-25 Thread Friedrich Weber 
Add an optional `krbd-map-options` property to the RBD storage that can be
used to pass custom map options to KRBD. Currently, only the `rxbounce` option
is supported. Setting this option may be necessary in setups with Windows VMs,
as reported in the forum [1]. For now, the option is not exposed in the GUI.

See patch #1 for more details. Patch #2 adds a section to the docs.

storage:

Friedrich Weber (1):
  fix #5779: rbd: allow to pass custom krbd map options

 src/PVE/Storage/Plugin.pm| 10 ++
 src/PVE/Storage/RBDPlugin.pm | 14 +-
 2 files changed, 23 insertions(+), 1 deletion(-)


docs:

Friedrich Weber (1):
  storage: rbd: document KRBD map options property

 pve-storage-rbd.adoc | 10 ++
 1 file changed, 10 insertions(+)


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

-- 
Generated by git-murpp 0.7.3


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

[pve-devel] [PATCH docs 2/2] storage: rbd: document KRBD map options property

2024-10-25 Thread Friedrich Weber 
Describe the new `krbd-map-options` property, and mention under which
circumstances the `rxbounce` option may be necessary.

Signed-off-by: Friedrich Weber 
---
 pve-storage-rbd.adoc | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/pve-storage-rbd.adoc b/pve-storage-rbd.adoc
index 5fe558a..71fbc02 100644
--- a/pve-storage-rbd.adoc
+++ b/pve-storage-rbd.adoc
@@ -56,6 +56,16 @@ Enforce access to rados block devices through the krbd 
kernel module. Optional.
 
 NOTE: Containers will use `krbd` independent of the option value.
 
+krbd-map-options::
+
+Comma-separated list of additional map options for KRBD, see the Ceph docs.
+footnote:[Kernel RBD (KRBD) options
+{cephdocs-url}/man/8/rbd/#kernel-rbd-krbd-options] Optional.
++
+Currently, only the `rxbounce` option is supported. In case the pool hosts
+Windows VM disks, setting `rxbounce` can be necessary to avoid performance
+degradation and `bad crc/signature` warnings in the host journal.
+
 .Configuration Example for a external Ceph cluster (`/etc/pve/storage.cfg`)
 
 rbd: ceph-external
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server, manager 0/12] bugzilla #4225 -- improve handling of unavailable ISOs

2024-10-25 Thread Daniel Herzig 
Currently VMs refuse to to start if a configured isofile becomes unavailable,
be it a deleted file or an unavailable network storage.

This patch series introduces a new parameter in Drive.pm, called 'required'.
Depending on whether this parameter is set or not, the situation will be handled
differently.

If the parameter is set to 0, the configuration will temporarily changed to use
'none' as file for the cd drive, which allows qemu to start up the machine.
The configuration is not changed in this process to avoid unexpected behaviour.
Instead a log_warn will be issued.

For transition reasons an unset parameter acts like 'required=1'. In this case
the startup process will die earlier than currently, if the file is missing or
the underlying storage not available.

If however a new VM is created from the WebGUI, the corresponding added checkbox
is not checked by default, and the resulting 'required=0' will be written to
the configuration.

To allow for proper testing and building some additions and minor changes
where made to to the testing framework as well.

Not exactly part of #4225, but related to it, this patch series adds an 'Eject'
button to the hardwareview in the WebGUI, which can be used as a convenience
shortcut to manually editing the missing ISO file to 'Do not use any media'.

*** BLURB HERE ***

qemu-server: Daniel Herzig (9):
  fix #4225: qemuserver: drive: add optional required parameter
  qemuserver: add helper function for mocking files
  fix #4225: qemuserver: add function to eject isofiles
  test: chomp all trailing newlines from errors and warnings
  test: mock cifs-store
  test: add nfs-offline storage
  test: mock existing files
  test: mock log_warn warnings
  test: cfg2cmd: add tests for testing the iso required parameter

 PVE/QemuServer.pm | 44 +++
 PVE/QemuServer/Drive.pm   |  9 +++-
 test/cfg2cmd/ide-required-iso-missing.conf| 12 +
 .../cfg2cmd/ide-required-iso-missing.conf.cmd |  0
 .../cfg2cmd/ide-required-iso-offline-nfs.conf | 12 +
 .../ide-required-iso-offline-nfs.conf.cmd |  0
 test/cfg2cmd/ide-required.conf| 14 ++
 test/cfg2cmd/ide-required.conf.cmd| 39 
 test/cfg2cmd/ide-unrequired-iso-missing.conf  | 12 +
 .../ide-unrequired-iso-missing.conf.cmd   | 33 ++
 .../ide-unrequired-iso-offline-nfs.conf   | 12 +
 .../ide-unrequired-iso-offline-nfs.conf.cmd   | 33 ++
 test/run_config2command_tests.pl  | 38 +++-
 13 files changed, 255 insertions(+), 3 deletions(-)
 create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf
 create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf.cmd
 create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf
 create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf.cmd
 create mode 100644 test/cfg2cmd/ide-required.conf
 create mode 100644 test/cfg2cmd/ide-required.conf.cmd
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf.cmd
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf.cmd

manager: Daniel Herzig (3):
  fix #4225: ui: form: isoselector: add optional required checkbox
  fix #4225: ui: qemu: cdedit: enable required checkbox for isos
  ui: qemu: hardware: add eject button for cdroms

 www/manager6/form/IsoSelector.js  | 22 
 www/manager6/qemu/CDEdit.js   |  6 +
 www/manager6/qemu/HardwareView.js | 43 +++
 3 files changed, 71 insertions(+)

-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 3/12] fix #4225: qemuserver: add function to eject isofiles

2024-10-25 Thread Daniel Herzig 
Current behaviour prevents a VM from starting, if an ISO file defined
in the configuration becomes unavailable.

The function eject_nonrequired_isos checks on whether a cdrom drive is
marked as 'required' or not. If the parameter 'required' is not
defined, it will assume 'required' to be true and keep the current
behaviour.

If 'required' is set to 0, the function 'ejects' the ISO file by
setting the drive's file value to 'none', if the underlying storage is
unavailable or if the defined file is unavailable for another reason.

The function is called while config_to_command iterates over all
volumes to allow for early storage activation and early exit in the
case of missing required files.

Signed-off-by: Daniel Herzig 
---
 PVE/QemuServer.pm | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a45bdab9..8fddff92 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3994,6 +3994,8 @@ sub config_to_command {
 PVE::QemuConfig->foreach_volume($conf, sub {
my ($ds, $drive) = @_;
 
+   eject_nonrequired_isos($ds, $drive, $vmid, $storecfg, $conf);
+
if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
check_volume_storage_type($storecfg, $drive->{file});
push @$vollist, $drive->{file};
@@ -8942,6 +8944,43 @@ sub delete_ifaces_ipams_ips {
 }
 }
 
+sub eject_nonrequired_isos {
+my ($ds, $drive, $vmid, $storecfg, $conf) = @_;
+# set 1 to exclude cloudinit. cloudinit isos are always required.
+if (drive_is_cdrom($drive, 1)
+   && $drive->{file} ne 'none'
+   && $drive->{file} ne 'cdrom') {
+   $drive->{required} = 1 if !defined($drive->{required});
+   my $iso_volid = $drive->{file};
+   my $iso_path = get_iso_path($storecfg, $vmid, $drive->{file});
+   my $store_err;
+   if ($iso_volid !~ m|^/|) {
+   my $iso_storage  = PVE::Storage::parse_volume_id($iso_volid, 1);
+   eval { PVE::Storage::activate_storage($storecfg, $iso_storage); };
+   $store_err = $@;
+   }
+   if ($store_err) {
+   if ($drive->{required}) {
+   die "cannot access required file: '${ds}: ${iso_volid}': 
${store_err}\n";
+   } else {
+   log_warn("eject '${ds}: ${iso_volid}': ${store_err}");
+   $drive->{file} = 'none';
+   $conf->{$ds} = print_drive($drive);
+   }
+   } else {
+   if (!file_exists($iso_path)) {
+   if ($drive->{required}) {
+   die "required file does not exist: '${ds}: ${iso_volid}'\n";
+   } else {
+   log_warn("eject '${ds}: ${iso_volid}': file does not 
exist");
+   $drive->{file} = 'none';
+   $conf->{$ds} = print_drive($drive);
+   }
+   }
+   }
+}
+}
+
 sub file_exists {
 my $file_path = shift;
 return -e $file_path
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 2/12] qemuserver: add helper function for mocking files

2024-10-25 Thread Daniel Herzig 
This stub function can be used for mocking a file's existance in testruns.

Signed-off-by: Daniel Herzig 
---
This is just a method to allow for mocking a files (not) existance for testing,
as I did not find a way to mock '-e' itself -- ideas very welcome!
 PVE/QemuServer.pm | 5 +
 1 file changed, 5 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0df3bda0..a45bdab9 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8942,4 +8942,9 @@ sub delete_ifaces_ipams_ips {
 }
 }
 
+sub file_exists {
+my $file_path = shift;
+return -e $file_path
+}
+
 1;
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 4/12] test: chomp all trailing newlines from errors and warnings

2024-10-25 Thread Daniel Herzig 
Ease EXPECT_ERROR and EXPECT_WARN string matching with errors/warnings
that have more than one trailing newline.

Signed-off-by: Daniel Herzig 
---
 test/run_config2command_tests.pl | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 8c525f09..2911483e 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -426,7 +426,9 @@ sub diff($$) {
 
 $SIG{__WARN__} = sub {
 my $warning = shift;
-chomp $warning;
+while ((chomp($warning))) {
+   chomp($warning);
+}
 if (my $warn_expect = $current_test->{expect_warning}) {
if ($warn_expect ne $warning) {
fail($current_test->{testname});
@@ -461,7 +463,9 @@ sub do_test($) {
note("did NOT get any error, but expected error: $err_expect");
return;
}
-   chomp $err;
+   while ((chomp($err))) {
+   chomp($err);
+   }
if ($err ne $err_expect) {
fail("$testname");
note("error does not match expected error: '$err' !~ 
'$err_expect'");
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 1/12] fix #4225: qemuserver: drive: add optional required parameter

2024-10-25 Thread Daniel Herzig 
Add parameter to allow for marking a drive as required.

Signed-off-by: Daniel Herzig 
---
 PVE/QemuServer/Drive.pm | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 6e98c095..c80c0f07 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -154,7 +154,14 @@ my %drivedesc_base = (
verbose_description => "Mark this locally-managed volume as available 
on all nodes.\n\nWARNING: This option does not share the volume automatically, 
it assumes it is shared already!",
optional => 1,
default => 0,
-}
+},
+required => {
+   type => 'boolean',
+   description => 'Mark this iso volume as required for booting the VM.',
+   verbose_description => 'If unset or set to 1, and the iso file is 
unavailable, the VM will not start.\nThis parameter is considered for cdrom iso 
drives only.',
+   optional => 1,
+   default => 1,
+},
 );
 
 my %iothread_fmt = ( iothread => {
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 9/12] test: cfg2cmd: add tests for testing the iso required parameter

2024-10-25 Thread Daniel Herzig 
This adds tests for the errors and warnings issued by
PVE::QemuServer::eject_unrequired_isos.

Empty cmd files were added for the test configurations that are not
expected to have any output (as they die before a command is prepared):
* ide-required-iso-missing.conf
* ide-required-iso-offline-nfs.conf

Signed-off-by: Daniel Herzig 
---
 test/cfg2cmd/ide-required-iso-missing.conf| 12 ++
 .../cfg2cmd/ide-required-iso-missing.conf.cmd |  0
 .../cfg2cmd/ide-required-iso-offline-nfs.conf | 12 ++
 .../ide-required-iso-offline-nfs.conf.cmd |  0
 test/cfg2cmd/ide-required.conf| 14 +++
 test/cfg2cmd/ide-required.conf.cmd| 39 +++
 test/cfg2cmd/ide-unrequired-iso-missing.conf  | 12 ++
 .../ide-unrequired-iso-missing.conf.cmd   | 33 
 .../ide-unrequired-iso-offline-nfs.conf   | 12 ++
 .../ide-unrequired-iso-offline-nfs.conf.cmd   | 33 
 10 files changed, 167 insertions(+)
 create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf
 create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf.cmd
 create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf
 create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf.cmd
 create mode 100644 test/cfg2cmd/ide-required.conf
 create mode 100644 test/cfg2cmd/ide-required.conf.cmd
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf.cmd
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf
 create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf.cmd

diff --git a/test/cfg2cmd/ide-required-iso-missing.conf 
b/test/cfg2cmd/ide-required-iso-missing.conf
new file mode 100644
index ..16ce79ab
--- /dev/null
+++ b/test/cfg2cmd/ide-required-iso-missing.conf
@@ -0,0 +1,12 @@
+# TEST: Config with default machine type, Linux & one IDE CD-ROM with online 
storage and missing required ISO file.
+# EXPECT_ERROR: required file does not exist: 'ide0: 
cifs-store:iso/I_DO_NOT_EXIST.iso'
+bootdisk: scsi0
+cores: 2
+ide0: cifs-store:iso/I_DO_NOT_EXIST.iso,media=cdrom,size=112M
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsi0: local:100/vm-100-disk-2.qcow2,size=10G
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/ide-required-iso-missing.conf.cmd 
b/test/cfg2cmd/ide-required-iso-missing.conf.cmd
new file mode 100644
index ..e69de29b
diff --git a/test/cfg2cmd/ide-required-iso-offline-nfs.conf 
b/test/cfg2cmd/ide-required-iso-offline-nfs.conf
new file mode 100644
index ..47db264d
--- /dev/null
+++ b/test/cfg2cmd/ide-required-iso-offline-nfs.conf
@@ -0,0 +1,12 @@
+# TEST: Config with default machine type, Linux & one IDE CD-ROM with required 
ISO file on offline storage.
+# EXPECT_ERROR: cannot access required file: 'ide0: nfs-offline:iso/any.iso': 
storage 'nfs-offline' is not online
+bootdisk: scsi0
+cores: 2
+ide0: nfs-offline:iso/any.iso,media=cdrom,size=112M
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsi0: local:100/vm-100-disk-2.qcow2,size=10G
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/ide-required-iso-offline-nfs.conf.cmd 
b/test/cfg2cmd/ide-required-iso-offline-nfs.conf.cmd
new file mode 100644
index ..e69de29b
diff --git a/test/cfg2cmd/ide-required.conf b/test/cfg2cmd/ide-required.conf
new file mode 100644
index ..cf56b724
--- /dev/null
+++ b/test/cfg2cmd/ide-required.conf
@@ -0,0 +1,14 @@
+# TEST: Config with default machine type, Linux & four IDE CD-ROMs marked as 
required
+bootdisk: scsi0
+cores: 2
+ide0: cifs-store:iso/zero.iso,media=cdrom,size=112M,required=1
+ide1: cifs-store:iso/one.iso,media=cdrom,size=112M,required=1
+ide2: cifs-store:iso/two.iso,media=cdrom,size=112M,required=1
+ide3: cifs-store:iso/three.iso,media=cdrom,size=112M,required=1
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsi0: local:100/vm-100-disk-2.qcow2,size=10G
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/ide-required.conf.cmd 
b/test/cfg2cmd/ide-required.conf.cmd
new file mode 100644
index ..7fd4888c
--- /dev/null
+++ b/test/cfg2cmd/ide-required.conf.cmd
@@ -0,0 +1,39 @@
+/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' \
+  -smp '2,sockets=1,cores=2,m

[pve-devel] [PATCH qemu-server 5/12] test: mock cifs-store

2024-10-25 Thread Daniel Herzig 
Let cifs-store appear as online to a call from
PVE::Storage::activate_storage.

Signed-off-by: Daniel Herzig 
---
 test/run_config2command_tests.pl | 13 +
 1 file changed, 13 insertions(+)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 2911483e..1a96c278 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -18,6 +18,7 @@ use PVE::QemuServer;
 use PVE::QemuServer::Monitor;
 use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer::CPUConfig;
+use PVE::Storage::CIFSPlugin;
 
 my $base_env = {
 storage_config => {
@@ -392,6 +393,18 @@ $pci_module->mock(
 }
 );
 
+my $pve_storage_cifsplugin_module = 
Test::MockModule->new("PVE::Storage::CIFSPlugin");
+$pve_storage_cifsplugin_module->mock(
+check_connection => sub {
+   return 1;
+},
+cifs_is_mounted => sub {
+   my ($scfg, $mountdata) = @_;
+   my ($mountpoint, $server, $share) = $scfg->@{'path', 'server', 'share'};
+   return $mountpoint;
+},
+);
+
 sub diff($$) {
 my ($a, $b) = @_;
 return if $a eq $b;
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 8/12] test: mock log_warn warnings

2024-10-25 Thread Daniel Herzig 
Strip log_warn wrapper for catching warnings on testruns.

Signed-off-by: Daniel Herzig 
---
 test/run_config2command_tests.pl | 4 
 1 file changed, 4 insertions(+)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 3414eea7..dd6717e2 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -214,6 +214,10 @@ $qemu_server_module->mock(
my $file_path = shift;
return 1 if !($file_path =~ m|I_DO_NOT_EXIST|);
 },
+log_warn => sub {
+   my $logwarn = shift;
+   return warn("${logwarn}\n");
+},
 );
 
 my $qemu_server_config;
-- 
2.39.5



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

[pve-devel] [PATCH manager 10/12] fix #4225: ui: form: isoselector: add optional required checkbox

2024-10-25 Thread Daniel Herzig 
Add a checkbox for marking an iso file as required.

This option is used in the backend to determine if the VM should start
up in case the configured ISO file is not available.

By default this box is not visible and disabled.

Signed-off-by: Daniel Herzig 
---
 www/manager6/form/IsoSelector.js | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/www/manager6/form/IsoSelector.js b/www/manager6/form/IsoSelector.js
index 66229e88..1803cd9d 100644
--- a/www/manager6/form/IsoSelector.js
+++ b/www/manager6/form/IsoSelector.js
@@ -15,12 +15,14 @@ Ext.define('PVE.form.IsoSelector', {
 insideWizard: false,
 labelWidth: undefined,
 labelAlign: 'right',
+showRequired: false,
 
 cbindData: function() {
let me = this;
return {
nodename: me.nodename,
insideWizard: me.insideWizard,
+   showRequired: me.showRequired,
};
 },
 
@@ -113,5 +115,25 @@ Ext.define('PVE.form.IsoSelector', {
},
},
},
+   {
+   xtype: 'proxmoxcheckbox',
+   name: 'required',
+   reference: 'requiredForBoot',
+   inputValue: true,
+   fieldLabel: gettext('Required'),
+   cbind: {
+   nodename: '{nodename}',
+   disabled: '{!showRequired}',
+   hidden: '{!showRequired}',
+   labelWidth: '{labelWidth}',
+   labelAlign: '{labelAlign}',
+   },
+   allowBlank: false,
+   listeners: {
+   change: function() {
+   this.up('pveIsoSelector').checkChange();
+   },
+   },
+   },
 ],
 });
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 7/12] test: mock existing files

2024-10-25 Thread Daniel Herzig 
Let all files checked by file_exists appear as existing if the path
does not contain the string 'I_DO_NOT_EXIST'.

Signed-off-by: Daniel Herzig 
---
 test/run_config2command_tests.pl | 4 
 1 file changed, 4 insertions(+)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index eda89dcb..3414eea7 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -210,6 +210,10 @@ $qemu_server_module->mock(
 cleanup_pci_devices => {
# do nothing
 },
+file_exists => sub {
+   my $file_path = shift;
+   return 1 if !($file_path =~ m|I_DO_NOT_EXIST|);
+},
 );
 
 my $qemu_server_config;
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 6/12] test: add nfs-offline storage

2024-10-25 Thread Daniel Herzig 
Add an nfs-offline storage to allow for comparatative testing
against potentially mocked online storages.

Signed-off-by: Daniel Herzig 
---
 test/run_config2command_tests.pl | 9 +
 1 file changed, 9 insertions(+)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 1a96c278..eda89dcb 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -50,6 +50,15 @@ my $base_env = {
iso => 1,
},
},
+   'nfs-offline' => {
+   type => 'nfs',
+   export => '/srv/nfs/isos',
+   path => '/mnt/pve/nfs-offline',
+   server => '127.0.0.42',
+   content => {
+   iso => 1,
+   },
+   },
'rbd-store' => {
monhost => '127.0.0.42,127.0.0.21,::1',
fsid => 'fc4181a6-56eb-4f68-b452-8ba1f381ca2a',
-- 
2.39.5



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

[pve-devel] [PATCH manager 11/12] fix #4225: ui: qemu: cdedit: enable required checkbox for isos

2024-10-25 Thread Daniel Herzig 
Enables the 'required' checkbox for the IsoSelector.
If the parameter is not set, the backend will use the default (set to
1).

Behaviour:
* Only send parameter if not default (required=0)
* Checked if parameter is missing (default)
* Unchecked when adding a new CD-ROM

Signed-off-by: Daniel Herzig 
---
 www/manager6/qemu/CDEdit.js | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js
index 3cc16205..9f518f68 100644
--- a/www/manager6/qemu/CDEdit.js
+++ b/www/manager6/qemu/CDEdit.js
@@ -12,6 +12,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
me.drive.media = 'cdrom';
if (values.mediaType === 'iso') {
me.drive.file = values.cdimage;
+   me.drive.required = values.required ? undefined : '0';
} else if (values.mediaType === 'cdrom') {
me.drive.file = 'cdrom';
} else {
@@ -44,6 +45,9 @@ Ext.define('PVE.qemu.CDInputPanel', {
} else {
values.mediaType = 'iso';
values.cdimage = drive.file;
+   if (drive.required === '1' || drive.required === undefined) {
+   values.required = '1';
+   }
}
 
me.drive = drive;
@@ -88,6 +92,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
cdImageField.validate();
} else {
cdImageField.reset();
+   delete me.drive.required;
}
},
},
@@ -98,6 +103,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
nodename: me.nodename,
insideWizard: me.insideWizard,
name: 'cdimage',
+   showRequired: true,
});
 
items.push(me.isosel);
-- 
2.39.5



___
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-storage 1/2] add external snasphot support

2024-10-25 Thread DERUMIER, Alexandre via pve-devel 
--- Begin Message ---
 Message initial 
De: Fabian Grünbichler 
À: Proxmox VE development discussion ,
"DERUMIER, Alexandre" 
Cc: Giotta Simon RUAGH 
Objet: Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot
support
Date: 24/10/2024 11:48:03


> Giotta Simon RUAGH via pve-devel  hat am
> 24.10.2024 09:59 CEST geschrieben:
> > I mean, if we don't allow .raw files to be snapshotted then this
> > problem doesn't exist ;)
> 
> Quick comment from the bleacher; Adding a mechanism to shapshot raw
> disks might solve the TPM (tpmstate) snapshotting issue, as well as
> allowing containers to be snapshot.
> 
> For context: 
> When using a storage that does not natively support snapshotting (NFS
> on NetApp or similar enterprise storage, in particular), raw disks
> cannot be snapshot. 
> Since tpmstate disks can only be stored as raw (as I understand they
> are just a binary blob?), this makes it impossible to snapshot or
> (link-)clone any VMs that have a TPM. This especially is an issue for
> current Windows clients.
> Same issue for LXC containers, as their storage format is raw only as
> well.
>  
> https://antiphishing.vadesecure.com/v4?f=OVFyc3FkSEdWUWx0QkZXZpBaFZH9
> xbUoQi0GpC0KVIU1UWG2AZ7f_MrrmMArnShL&i=Sm1YaTk1OUR6bzFoY3JtMLa1y1UZBH
> RmExEJw6jsROc&k=Hbsl&r=dmh0RHJVSG1CUXhDTmJ3UlzJQNCs3CJCbvk0g2AF56AIGO
> 1hR25I2pdFPY1trx1rDP3XHfwmNmQ-
> fWda_VoksA&s=d330b0a625b7cfcbde904428642b953a712c1a40b54a60918ac39b62
> f8ca6535&u=https%3A%2F%2Fbugzilla.proxmox.com%2Fshow_bug.cgi%3Fid%3D4
> 693

>>no it does not - with the mechanisms proposed in this patch series,
>>only the initial volume can be raw, if it is snapshotted, the
>>overlays are qcow2. so anything reading from the volume needs qcow2
>>support, including swtpm. 
>>
>>that's why containers are not on the table for now either..

Hi, I really don't known how swtpm is working, but for containers maybe
it could be possible


not yet merged to kernel, but a dm-qcow2 driver is on the roadmap :)
https://www.youtube.com/watch?v=Z7jPpWydEC8

another possibility is qemu-storage-daemon  + nbd or vdpa export:
https://blog.deckhouse.io/lvm-qcow-csi-driver-shared-san-kubernetes-81455201590e


About vtpm, is it really a problem to not be able to snapshot ? (I
mean, does the content change regulary ? can't we just skip the disk ?
I really don't known how it's working, I don't use tpm :p)

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