[pve-devel] [PATCH proxmox] apt: repositories: document status property for standard repository

2023-09-29 Thread Fiona Ebner
Suggested-by: Dietmar Maurer 
Signed-off-by: Fiona Ebner 
---
 proxmox-apt/src/repositories/standard.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/proxmox-apt/src/repositories/standard.rs 
b/proxmox-apt/src/repositories/standard.rs
index 4bb57d4..29cc788 100644
--- a/proxmox-apt/src/repositories/standard.rs
+++ b/proxmox-apt/src/repositories/standard.rs
@@ -24,7 +24,8 @@ pub struct APTStandardRepository {
 /// Handle referencing a standard repository.
 pub handle: APTRepositoryHandle,
 
-/// Configuration status of the associated repository.
+/// Configuration status of the associated repository, where `None` means
+/// not configured, and `Some(bool)` indicates enabled or disabled.
 #[serde(skip_serializing_if = "Option::is_none")]
 pub status: Option,
 
-- 
2.39.2



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



[pve-devel] [PATCH qemu-server 2/2] migration: add missing eval on nbdstop with tunnel v2.

2023-09-29 Thread Alexandre Derumier
It was already done in tunnel v1.

Avoid to avoid migration (and keep both source/targetvm locked) if nbdstop 
error occur

2023-09-28 16:20:39 ERROR: error - tunnel command '{"cmd":"nbdstop"}' failed - 
failed to handle 'nbdstop' command - VM 140 qmp command 'nbd-server-stop' 
failed - got timeout
2023-09-28 16:20:39 ERROR: migration finished with problems (duration 00:01:42)
---
 PVE/QemuMigrate.pm | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index f41c61f..81880e5 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1475,7 +1475,13 @@ sub phase3_cleanup {
$self->log('info', "stopping NBD storage migration server on 
target.");
# stop nbd server on remote vm - requirement for resume since 2.9
if ($tunnel && $tunnel->{version} && $tunnel->{version} >= 2) {
-   PVE::Tunnel::write_tunnel($tunnel, 30, 'nbdstop');
+   eval {
+   PVE::Tunnel::write_tunnel($tunnel, 30, 'nbdstop');
+   };
+   if (my $err = $@) {
+   $self->log('err', $err);
+   $self->{errors} = 1;
+   }
} else {
my $cmd = [@{$self->{rem_ssh}}, 'qm', 'nbdstop', $vmid];
 
-- 
2.39.2


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



[pve-devel] [PATCH qemu-server 1/2] nbd_stop: increase timeout to 25s

2023-09-29 Thread Alexandre Derumier
---
 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 1b1ccf4..0259c0f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8267,7 +8267,7 @@ sub generate_smbios1_uuid {
 sub nbd_stop {
 my ($vmid) = @_;
 
-mon_cmd($vmid, 'nbd-server-stop');
+mon_cmd($vmid, 'nbd-server-stop', timeout => 25);
 }
 
 sub create_reboot_request {
-- 
2.39.2


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



[pve-devel] [PATCH qemu-server 0/2] migration: fix sporadic nbd-server-stop timeout

2023-09-29 Thread Alexandre Derumier
Hi,

We had some sporadic nbd-stop error when trying to migrate vm with rbd storage 
+ writeback between 2 differents cluster:
(This is without my other targetcpu patch)


2023-09-28 16:20:39 ERROR: error - tunnel command '{"cmd":"nbdstop"}' failed - 
failed to handle 'nbdstop' command - VM 140 qmp command 'nbd-server-stop' 
failed - got timeout
2023-09-28 16:20:39 ERROR: migration finished with problems (duration 00:01:42)


I'm not sure, maybe it's related to writeback, because it never happend with a 
fresh started vm, but vms running since some time can trigger this.
(I'm not sure, maybe nbd need to flush pending datas in cache ?)


Currently, the tunnel command have a 30s timeout, but the qmp command is only 
at 5s.
Also the tunnel v2 command don't have any eval, so the migration abort keeping 
both source && target vm locked.
unlocking target vm and resume it manually is working, so it really seem to be 
a too low timeout.


Alexandre Derumier (2):
  nbd_stop: increase timeout to 25s
  migration: add missing eval on nbdstop with tunnel v2.

 PVE/QemuMigrate.pm | 8 +++-
 PVE/QemuServer.pm  | 2 +-
 2 files changed, 8 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] [PATCH qemu 8/8] cherry-pick stable fixes to avoid crash in IO error scenarios

2023-09-29 Thread Fiona Ebner
Upstream report of the issue [0]. I ran into it too by chance by
filling up my NFS storage with the VM's qcow2 disk.

[0]: https://bugzilla.redhat.com/show_bug.cgi?id=2234374

Signed-off-by: Fiona Ebner 
---
 ...ile-posix-Clear-bs-bl.zoned-on-error.patch | 87 +++
 ...osix-Check-bs-bl.zoned-for-zone-info.patch | 59 +
 ...ix-Fix-zone-update-in-I-O-error-path.patch | 33 +++
 ...-Simplify-raw_co_prw-s-out-zone-code.patch | 58 +
 ...k-file-change-locking-default-to-off.patch |  2 +-
 ...le-posix-make-locking-optiono-on-cre.patch | 14 +--
 debian/patches/series |  4 +
 7 files changed, 249 insertions(+), 8 deletions(-)
 create mode 100644 
debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
 create mode 100644 
debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
 create mode 100644 
debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch
 create mode 100644 
debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch

diff --git 
a/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch 
b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
new file mode 100644
index 000..0e3dc3e
--- /dev/null
+++ b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
@@ -0,0 +1,87 @@
+From  Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek 
+Date: Thu, 24 Aug 2023 17:53:40 +0200
+Subject: [PATCH] file-posix: Clear bs->bl.zoned on error
+
+bs->bl.zoned is what indicates whether the zone information is present
+and valid; it is the only thing that raw_refresh_zoned_limits() sets if
+CONFIG_BLKZONED is not defined, and it is also the only thing that it
+sets if CONFIG_BLKZONED is defined, but there are no zones.
+
+Make sure that it is always set to BLK_Z_NONE if there is an error
+anywhere in raw_refresh_zoned_limits() so that we do not accidentally
+announce zones while our information is incomplete or invalid.
+
+This also fixes a memory leak in the last error path in
+raw_refresh_zoned_limits().
+
+Signed-off-by: Hanna Czenczek 
+Message-Id: <20230824155345.109765-2-hre...@redhat.com>
+Reviewed-by: Sam Li 
+(cherry-picked from commit 56d1a022a77ea2125564913665eeadf3e303a671)
+Signed-off-by: Fiona Ebner 
+---
+ block/file-posix.c | 21 -
+ 1 file changed, 12 insertions(+), 9 deletions(-)
+
+diff --git a/block/file-posix.c b/block/file-posix.c
+index b16e9c21a1..2b88b9eefa 100644
+--- a/block/file-posix.c
 b/block/file-posix.c
+@@ -1412,11 +1412,9 @@ static void raw_refresh_zoned_limits(BlockDriverState 
*bs, struct stat *st,
+ BlockZoneModel zoned;
+ int ret;
+ 
+-bs->bl.zoned = BLK_Z_NONE;
+-
+ ret = get_sysfs_zoned_model(st, &zoned);
+ if (ret < 0 || zoned == BLK_Z_NONE) {
+-return;
++goto no_zoned;
+ }
+ bs->bl.zoned = zoned;
+ 
+@@ -1437,10 +1435,10 @@ static void raw_refresh_zoned_limits(BlockDriverState 
*bs, struct stat *st,
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
+  "sysfs attribute");
+-return;
++goto no_zoned;
+ } else if (!ret) {
+ error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
+-return;
++goto no_zoned;
+ }
+ bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
+ 
+@@ -1448,10 +1446,10 @@ static void raw_refresh_zoned_limits(BlockDriverState 
*bs, struct stat *st,
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Unable to read nr_zones "
+  "sysfs attribute");
+-return;
++goto no_zoned;
+ } else if (!ret) {
+ error_setg(errp, "Read 0 from nr_zones sysfs attribute");
+-return;
++goto no_zoned;
+ }
+ bs->bl.nr_zones = ret;
+ 
+@@ -1472,10 +1470,15 @@ static void raw_refresh_zoned_limits(BlockDriverState 
*bs, struct stat *st,
+ ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "report wps failed");
+-bs->wps = NULL;
+-return;
++goto no_zoned;
+ }
+ qemu_co_mutex_init(&bs->wps->colock);
++return;
++
++no_zoned:
++bs->bl.zoned = BLK_Z_NONE;
++g_free(bs->wps);
++bs->wps = NULL;
+ }
+ #else /* !defined(CONFIG_BLKZONED) */
+ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
diff --git 
a/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch 
b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
new file mode 100644
index 000..a505816
--- /dev/null
+++ b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
@@ -0,0 +1,59 @@
+From  Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek 
+Date: Thu, 24 Aug 2023 17:53:41 +0200
+Subject: [PATCH] f

Re: [pve-devel] [PATCH qemu-server 0/2] migration: fix sporadic nbd-server-stop timeout

2023-09-29 Thread Fiona Ebner
Am 29.09.23 um 10:28 schrieb Alexandre Derumier:
> I'm not sure, maybe it's related to writeback, because it never happend with 
> a fresh started vm, but vms running since some time can trigger this.
> (I'm not sure, maybe nbd need to flush pending datas in cache ?)
> 

It does drain the export's BlockBackend, i.e. wait for all pending IO
before detaching/closing the export. But we did cancel the mirror job,
which should actually wait for any in-flight IO already, so it's a bit
surprising. Maybe there's some cache interaction happening at an
inconvenient time, no idea ¯\_(ツ)_/¯

The other thing it does is closing the connection to the client, so
there is at least that IO interaction and a higher timeout makes sense.

> 
> Alexandre Derumier (2):
>   nbd_stop: increase timeout to 25s
>   migration: add missing eval on nbdstop with tunnel v2.
> 

Patches look good to me, so consider them

Reviewed-by: Fiona Ebner 

but they are missing your Signed-off-by trailer. Can you please add that?


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


Re: [pve-devel] [PATCH proxmox-backup] ui: fix #4260: add dynamic notes in backup group comment

2023-09-29 Thread Philipp Hufnagl



On 9/28/23 15:33, Philipp Hufnagl wrote:
> When there is no comment for a backup group, the comment of the last
> snapshot in this group will be shown slightly grayed out as long as
> the group is collapsed.
> 
> Signed-off-by: Philipp Hufnagl 
> ---
>  www/css/ext6-pbs.css |  3 +++
>  www/datastore/Content.js | 17 ++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
> index 5fd65d25..324f73b8 100644
> --- a/www/css/ext6-pbs.css
> +++ b/www/css/ext6-pbs.css
> @@ -226,6 +226,9 @@ span.snapshot-comment-column {
>  display: inline-block;
>  width: calc(100% - 18px);
>  }
> +span.snapshot-comment-derived {
> +opacity: 0.7;
> +}
>  
>  .x-action-col-icon.good:before {
>  color: #21BF4B;
> diff --git a/www/datastore/Content.js b/www/datastore/Content.js
> index 9fc07d49..926aab89 100644
> --- a/www/datastore/Content.js
> +++ b/www/datastore/Content.js
> @@ -146,6 +146,7 @@ Ext.define('PBS.DataStoreContent', {
>   leaf: false,
>   iconCls: "fa " + cls,
>   expanded: false,
> + comment: item.data.comment,
>   backup_type: item.data["backup-type"],
>   backup_id: item.data["backup-id"],
>   children: [],
> @@ -287,6 +288,7 @@ Ext.define('PBS.DataStoreContent', {
>   if (item["backup-time"] > last_backup && item.size !== 
> null) {
>   last_backup = item["backup-time"];
>   group["backup-time"] = last_backup;
> + group["last-comment"] = item.comment;
>   group.files = item.files;
>   group.size = item.size;
>   group.owner = item.owner;
> @@ -900,16 +902,25 @@ Ext.define('PBS.DataStoreContent', {
>   flex: 1,
>   renderer: (v, meta, record) => {
>   let data = record.data;
> + let additional_classes = "";
>   if (!data || data.leaf || data.root) {
>   return '';
>   }
> - if (v === undefined || v === null) {
> - v = '';
> +
> + // when there is no group comment and the section is collapsed,
> + // display the most recent snapshot comment
> + if (v === undefined || v === null|| v === '') {
> + if (data.expanded === false) {
> + v = data['last-comment'];
> + additional_classes = "snapshot-comment-derived";
> + } else {
> + v = '';
> + }
>   }
>   v = Ext.String.htmlEncode(v);
>   let icon = 'x-action-col-icon fa fa-fw fa-pencil pointer';
>  
> - return `${v}
> + return `${v}
>   `;
>   },
>   listeners: {


Sorry, sent this to the wrong list. I sent it again to the pbs-devel list


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



[pve-devel] [PATCH manager v4 1/3] api: ceph: add endpoint to fetch config keys

2023-09-29 Thread Aaron Lauterer
This new endpoint allows to get the values of config keys that are
either set in the config db or the ceph.conf file.

Values that are set in the ceph.conf file have priority over values set
in the conifg db via 'ceph config set'.

Expects the --config-keys parameter as a semicolon separated list of
":" where the section is a section in the ceph.conf
or config db. For example: global:osd_pool_default_size

Signed-off-by: Aaron Lauterer 
---
changes since
v3:
* rebased

v2:
* fixed small typo

v1:
* use kebab-case parameter names
* use kebab-case for the ceph config parameters, which also are returned
  that way
* improve how we parse and merge the config db and ceph.conf file. This
  way though, we dont warn if we cannot find a config key.
* renamed regex to make the distinctions clearer
* dropped 'format => string-list' as it didn't work when leaving out
  [;, ] from the regex. But we don't need both.

 PVE/API2/Ceph/Cfg.pm | 82 
 1 file changed, 82 insertions(+)

diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
index 2225a1ac..f06c42f4 100644
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use PVE::Ceph::Tools;
+use PVE::Cluster qw(cfs_read_file);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::RADOS;
 use PVE::Tools qw(file_get_contents);
@@ -36,6 +37,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'raw' },
{ name => 'db' },
+   { name => 'value' },
];
 
return $result;
@@ -110,3 +112,83 @@ __PACKAGE__->register_method ({
 
return $res;
 }});
+
+
+my $SINGLE_CONFIGKEY_RE = qr/[0-9a-z\-_\.]+:[0-9a-zA-Z\-_]+/i;
+my $CONFIGKEYS_RE = qr/^(:?${SINGLE_CONFIGKEY_RE})(:?[;, 
]${SINGLE_CONFIGKEY_RE})*$/;
+
+__PACKAGE__->register_method ({
+name => 'value',
+path => 'value',
+method => 'GET',
+proxyto => 'node',
+protected => 1,
+permissions => {
+   check => ['perm', '/', [ 'Sys.Audit' ]],
+},
+description => "Get configured values from either the config file or 
config DB.",
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   node => get_standard_option('pve-node'),
+   'config-keys' => {
+   type => "string",
+   typetext => ":[;:]",
+   pattern => $CONFIGKEYS_RE,
+   description => "List of : items.",
+   }
+   },
+},
+returns => {
+   type => 'object',
+   description => "Contains {section}->{key} children with the values",
+},
+code => sub {
+   my ($param) = @_;
+
+   PVE::Ceph::Tools::check_ceph_inited();
+
+   # Ceph treats '-' and '_' the same in parameter names, stick with '-'
+   my $normalize = sub {
+   my $t = shift;
+   $t =~ s/_/-/g;
+   return $t;
+   };
+
+   my $requested_keys = {};
+   for my $pair (PVE::Tools::split_list($param->{'config-keys'})) {
+   my ($section, $key) = split(":", $pair);
+   $section = $normalize->($section);
+   $key = $normalize->($key);
+
+   $requested_keys->{$section}->{$key} = 1;
+   }
+
+   my $config = {};
+
+   my $rados = PVE::RADOS->new();
+   my $configdb = $rados->mon_command( { prefix => 'config dump', format 
=> 'json' });
+   for my $s (@{$configdb}) {
+   my ($section, $name, $value) = $s->@{'section', 'name', 'value'};
+   my $n_section = $normalize->($section);
+   my $n_name = $normalize->($name);
+
+   $config->{$n_section}->{$n_name} = $value
+   if defined $requested_keys->{$n_section} && $n_name eq $n_name;
+   }
+
+   # read ceph.conf after config db as it has priority if settings are 
present in both
+   my $config_file = cfs_read_file('ceph.conf'); # cfs_read_file to get it 
parsed
+   for my $section (keys %{$config_file}) {
+   my $n_section = $normalize->($section);
+   next if !defined $requested_keys->{$n_section};
+
+   for my $key (keys %{$config_file->{$section}}) {
+   my $n_key = $normalize->($key);
+   $config->{$n_section}->{$n_key} = 
$config_file->{$section}->{$key}
+   if $requested_keys->{$n_section}->{$n_key};
+   }
+   }
+
+   return $config;
+}});
-- 
2.39.2



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



[pve-devel] [PATCH manager v4 3/3] ui: ceph pool edit: rework with controller and formulas

2023-09-29 Thread Aaron Lauterer
instead of relying purely on listeners that then manually change other
components, we can use binds, formulas and a basic controller.

This makes it quite a bit easier to let multiple components react to
changes.

A cbind is used for the size component to set the initial start value.
Other options, like using setValue in the controller init, will trigger
the change listener and therefore can affect the min size without any
user interaction.

Signed-off-by: Aaron Lauterer 
---
changes since
v3:
* rebased

v2:
* don't set default values in controller init() as we expect them to
  always be set via cbinds.

v1:
* moved view between controller and layout
* small code style cleanups

 www/manager6/ceph/Pool.js | 82 ---
 1 file changed, 59 insertions(+), 23 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 0ad59baf..c61d4f71 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -11,6 +11,48 @@ Ext.define('PVE.CephPoolInputPanel', {
 defaultSize: undefined,
 defaultMinSize: undefined,
 
+controller: {
+   xclass: 'Ext.app.ViewController',
+
+   init: function(view) {
+   let vm = this.getViewModel();
+   vm.set('size', Number(view.defaultSize));
+   vm.set('minSize', Number(view.defaultMinSize));
+   },
+   sizeChange: function(field, val) {
+   let vm = this.getViewModel();
+   let minSize = Math.round(val / 2);
+   if (minSize > 1) {
+   vm.set('minSize', minSize);
+   }
+   vm.set('size', val); // bind does not work in a 
pmxDisplayEditField, update manually
+   },
+},
+
+viewModel: {
+   data: {
+   minSize: null,
+   size: null,
+   },
+   formulas: {
+   minSizeLabel: (get) => {
+   if (get('showMinSizeOneWarning') || 
get('showMinSizeHalfWarning')) {
+   return `${gettext('Min. Size')} `;
+   }
+   return gettext('Min. Size');
+   },
+   showMinSizeOneWarning: (get) => get('minSize') === 1,
+   showMinSizeHalfWarning: (get) => {
+   let minSize = get('minSize');
+   let size = get('size');
+   if (minSize === 1) {
+   return false;
+   }
+   return minSize < (size / 2) && minSize !== size;
+   },
+   },
+},
+
 column1: [
{
xtype: 'pmxDisplayEditField',
@@ -38,12 +80,7 @@ Ext.define('PVE.CephPoolInputPanel', {
maxValue: 7,
allowBlank: false,
listeners: {
-   change: function(field, val) {
-   let size = Math.round(val / 2);
-   if (size > 1) {
-   
field.up('inputpanel').down('field[name=min_size]').setValue(size);
-   }
-   },
+   change: 'sizeChange',
},
},
},
@@ -81,7 +118,10 @@ Ext.define('PVE.CephPoolInputPanel', {
 advancedColumn1: [
{
xtype: 'proxmoxintegerfield',
-   fieldLabel: gettext('Min. Size'),
+   bind: {
+   fieldLabel: '{minSizeLabel}',
+   value: '{minSize}',
+   },
name: 'min_size',
cbind: {
value: (get) => get('defaultMinSize'),
@@ -95,28 +135,24 @@ Ext.define('PVE.CephPoolInputPanel', {
},
maxValue: 7,
allowBlank: false,
-   listeners: {
-   change: function(field, minSize) {
-   let panel = field.up('inputpanel');
-   let size = panel.down('field[name=size]').getValue();
-
-   let showWarning = minSize < (size / 2) && minSize !== size;
-
-   let fieldLabel = gettext('Min. Size');
-   if (showWarning) {
-   fieldLabel = gettext('Min. Size') + ' ';
-   }
-   
panel.down('field[name=min_size-warning]').setHidden(!showWarning);
-   field.setFieldLabel(fieldLabel);
-   },
-   },
},
{
xtype: 'displayfield',
-   name: 'min_size-warning',
+   bind: {
+   hidden: '{!showMinSizeHalfWarning}',
+   },
+   hidden: true,
userCls: 'pmx-hint',
value: gettext('min_size < size/2 can lead to data loss, incomplete 
PGs or unfound objects.'),
+   },
+   {
+   xtype: 'displayfield',
+   bind: {
+   hidden: '{!showMinSizeOneWarning}',
+   },
hidden: true,
+   userCls: 'pmx-hint',
+   value: gettext('a min_size of 1 is not recommended and can lead to 
data loss'),
},
{
xtype: 'pmxDisplayEditField',
-- 
2.39.2



___

[pve-devel] [PATCH manager v4 2/3] fix #2515: ui: ceph pool create: use configured defaults for size and min_size

2023-09-29 Thread Aaron Lauterer
Instead of hard coded defaults for the size and min_size parameter,
check if we have defaults configured in the ceph.conf or config db and
use those.

There are clusters where different defaults are needed. For example if
the cluster spans two rooms and needs to survive the loss of one. A
size/min_size of 4/2 are common defaults in such a situation.

Signed-off-by: Aaron Lauterer 
---
changes since
v3:
* rebased

v2:
* default 3/2 values for size/min_size are now set right when we get the
  API response. The component expects to get a value set via cbind.
  This avoids setting default values in multiple places within the
  component.

v1:
* set defaultSize and defaultMinSize to undefined in CephPoolInputPanel
* set them in cbindData in PoolEdit (needed when editing existing pool)
* waitMsgTarget when opening pool create window
* guard returned values for config keys in case they are empty

 www/manager6/ceph/Pool.js | 62 +++
 1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 8de23ecf..0ad59baf 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -7,6 +7,10 @@ Ext.define('PVE.CephPoolInputPanel', {
 onlineHelp: 'pve_ceph_pools',
 
 subject: 'Ceph Pool',
+
+defaultSize: undefined,
+defaultMinSize: undefined,
+
 column1: [
{
xtype: 'pmxDisplayEditField',
@@ -27,7 +31,9 @@ Ext.define('PVE.CephPoolInputPanel', {
name: 'size',
editConfig: {
xtype: 'proxmoxintegerfield',
-   value: 3,
+   cbind: {
+   value: (get) => get('defaultSize'),
+   },
minValue: 2,
maxValue: 7,
allowBlank: false,
@@ -40,7 +46,6 @@ Ext.define('PVE.CephPoolInputPanel', {
},
},
},
-
},
 ],
 column2: [
@@ -78,9 +83,15 @@ Ext.define('PVE.CephPoolInputPanel', {
xtype: 'proxmoxintegerfield',
fieldLabel: gettext('Min. Size'),
name: 'min_size',
-   value: 2,
cbind: {
-   minValue: (get) => get('isCreate') ? 2 : 1,
+   value: (get) => get('defaultMinSize'),
+   minValue: (get) => {
+   if (Number(get('defaultMinSize')) === 1) {
+   return 1;
+   } else {
+   return get('isCreate') ? 2 : 1;
+   }
+   },
},
maxValue: 7,
allowBlank: false,
@@ -195,6 +206,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
 cbindData: {
pool_name: '',
isCreate: (cfg) => !cfg.pool_name,
+   defaultSize: undefined,
+   defaultMinSize: undefined,
 },
 
 cbind: {
@@ -217,6 +230,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
pool_name: '{pool_name}',
isErasure: '{isErasure}',
isCreate: '{isCreate}',
+   defaultSize: '{defaultSize}',
+   defaultMinSize: '{defaultMinSize}',
},
 }],
 });
@@ -397,14 +412,37 @@ Ext.define('PVE.node.Ceph.PoolList', {
{
text: gettext('Create'),
handler: function() {
-   Ext.create('PVE.Ceph.PoolEdit', {
-   title: gettext('Create') + ': Ceph Pool',
-   isCreate: true,
-   isErasure: false,
-   nodename: nodename,
-   autoShow: true,
-   listeners: {
-   destroy: () => rstore.load(),
+   let keys = [
+   'global:osd-pool-default-min-size',
+   'global:osd-pool-default-size',
+   ];
+   let params = {
+   'config-keys': keys.join(';'),
+   };
+
+   Proxmox.Utils.API2Request({
+   url: '/nodes/localhost/ceph/cfg/value',
+   method: 'GET',
+   params,
+   waitMsgTarget: me.getView(),
+   failure: response => 
Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+   success: function({ result: { data } }) {
+   let global = data.global;
+   let defaultSize = 
global?.['osd-pool-default-size'] ?? 3;
+   let defaultMinSize = 
global?.['osd-pool-default-min-size'] ?? 2;
+
+   Ext.create('PVE.Ceph.PoolEdit', {
+   title: gettext('Create') + ': Ceph Pool',
+   isCreate: true,
+   isErasure: false,
+

[pve-devel] [PATCH manager v4 0/3] fix 2515 use size defaults

2023-09-29 Thread Aaron Lauterer
The main goal of this series is to improve the handling of configured
default size & min_size values when creating a new Ceph Pool in the GUI.

A new Ceph API endpoint, 'cfg/value', is added. It allows us to fetch
values for config keys that are set either in the config DB of Ceph or
in the ceph.conf file.

changes since
v3: rebased

v2:
* API rework has been already applied
* cleaned up JS code to set default values right where we get them from
  the API instead of at multiple places in the CephPoolInputPanel
  itself.

Aaron Lauterer (3):
  api: ceph: add endpoint to fetch config keys
  fix #2515: ui: ceph pool create: use configured defaults for size and
min_size
  ui: ceph pool edit: rework with controller and formulas

 PVE/API2/Ceph/Cfg.pm  |  82 ++
 www/manager6/ceph/Pool.js | 144 +-
 2 files changed, 191 insertions(+), 35 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] [PATCH stable-7 qemu 2/2] fix #2874: SATA: avoid unsolicited write to sector 0 during reset

2023-09-29 Thread Fiona Ebner
If there is a pending DMA operation during ide_bus_reset(), the fact
that the IDEstate is already reset before the operation is canceled
can be problematic. In particular, ide_dma_cb() might be called and
then use the reset IDEstate which contains the signature after the
reset. When used to construct the IO operation this leads to
ide_get_sector() returning 0 and nsector being 1. This is particularly
bad, because a write command will thus destroy the first sector which
often contains a partition table or similar.

Upstream discussion:
v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg04239.html
v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg01011.html

Signed-off-by: Fiona Ebner 
---
 ...cel-async-DMA-operation-before-reset.patch | 104 ++
 debian/patches/series |   1 +
 2 files changed, 105 insertions(+)
 create mode 100644 
debian/patches/extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch

diff --git 
a/debian/patches/extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
 
b/debian/patches/extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
new file mode 100644
index 000..45d804b
--- /dev/null
+++ 
b/debian/patches/extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
@@ -0,0 +1,104 @@
+From  Mon Sep 17 00:00:00 2001
+From: Fiona Ebner 
+Date: Wed, 6 Sep 2023 15:09:21 +0200
+Subject: [PATCH] hw/ide: reset: cancel async DMA operation before resetting
+ state
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+If there is a pending DMA operation during ide_bus_reset(), the fact
+that the IDEState is already reset before the operation is canceled
+can be problematic. In particular, ide_dma_cb() might be called and
+then use the reset IDEState which contains the signature after the
+reset. When used to construct the IO operation this leads to
+ide_get_sector() returning 0 and nsector being 1. This is particularly
+bad, because a write command will thus destroy the first sector which
+often contains a partition table or similar.
+
+Traces showing the unsolicited write happening with IDEState
+0x5595af6949d0 being used after reset:
+
+> ahci_port_write ahci(0x5595af6923f0)[0]: port write [reg:PxSCTL] @ 0x2c: 
0x0300
+> ahci_reset_port ahci(0x5595af6923f0)[0]: reset port
+> ide_reset IDEstate 0x5595af6949d0
+> ide_reset IDEstate 0x5595af694da8
+> ide_bus_reset_aio aio_cancel
+> dma_aio_cancel dbs=0x7f64600089a0
+> dma_blk_cb dbs=0x7f64600089a0 ret=0
+> dma_complete dbs=0x7f64600089a0 ret=0 cb=0x5595acd40b30
+> ahci_populate_sglist ahci(0x5595af6923f0)[0]
+> ahci_dma_prepare_buf ahci(0x5595af6923f0)[0]: prepare buf limit=512 
prepared=512
+> ide_dma_cb IDEState 0x5595af6949d0; sector_num=0 n=1 cmd=DMA WRITE
+> dma_blk_io dbs=0x7f6420802010 bs=0x5595ae2c6c30 offset=0 to_dev=1
+> dma_blk_cb dbs=0x7f6420802010 ret=0
+
+> (gdb) p *qiov
+> $11 = {iov = 0x7f647c76d840, niov = 1, {{nalloc = 1, local_iov = {iov_base = 
0x0,
+>   iov_len = 512}}, {__pad = 
"\001\000\000\000\000\000\000\000\000\000\000",
+>   size = 512}}}
+> (gdb) bt
+> #0  blk_aio_pwritev (blk=0x5595ae2c6c30, offset=0, qiov=0x7f6420802070, 
flags=0,
+> cb=0x5595ace6f0b0 , opaque=0x7f6420802010)
+> at ../block/block-backend.c:1682
+> #1  0x5595ace6f185 in dma_blk_cb (opaque=0x7f6420802010, ret=)
+> at ../softmmu/dma-helpers.c:179
+> #2  0x5595ace6f778 in dma_blk_io (ctx=0x5595ae0609f0,
+> sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512,
+> io_func=io_func@entry=0x5595ace6ee30 ,
+> io_func_opaque=io_func_opaque@entry=0x5595ae2c6c30,
+> cb=0x5595acd40b30 , opaque=0x5595af6949d0,
+> dir=DMA_DIRECTION_TO_DEVICE) at ../softmmu/dma-helpers.c:244
+> #3  0x5595ace6f90a in dma_blk_write (blk=0x5595ae2c6c30,
+> sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512,
+> cb=cb@entry=0x5595acd40b30 , 
opaque=opaque@entry=0x5595af6949d0)
+> at ../softmmu/dma-helpers.c:280
+> #4  0x5595acd40e18 in ide_dma_cb (opaque=0x5595af6949d0, ret=)
+> at ../hw/ide/core.c:953
+> #5  0x5595ace6f319 in dma_complete (ret=0, dbs=0x7f64600089a0)
+> at ../softmmu/dma-helpers.c:107
+> #6  dma_blk_cb (opaque=0x7f64600089a0, ret=0) at ../softmmu/dma-helpers.c:127
+> #7  0x5595ad12227d in blk_aio_complete (acb=0x7f6460005b10)
+> at ../block/block-backend.c:1527
+> #8  blk_aio_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524
+> #9  blk_aio_write_entry (opaque=0x7f6460005b10) at 
../block/block-backend.c:1594
+> #10 0x5595ad258cfb in coroutine_trampoline (i0=,
+> i1=) at ../util/coroutine-ucontext.c:177
+
+Signed-off-by: Fiona Ebner 
+Reviewed-by: Philippe Mathieu-Daudé 
+---
+ hw/ide/core.c | 14 +++---
+ 1 file changed, 7 insertions(+), 7 deletions(-)
+
+diff --git a/hw/ide/core.c b/hw/ide/core.c
+index 2f8

Re: [pve-devel] [PATCH v2 common 1/4] fix #3893: network: make bridge vids configurable

2023-09-29 Thread Aaron Lauterer

ping? patches still apply cleanly

On 6/14/23 11:30, Aaron Lauterer wrote:

For that we need to add a new format option that checks against valid
VLAN tags and ranges, for example: 2 4 100-200

The check, if the default value should be used, needs to fail not just
when not defined, but also in case it is an empty string.

Signed-off-by: Aaron Lauterer 
---
no changes since v1.

I think replacing the 'defined' check with 'length' should be fine. We
need to also handle the situation that the parameter is defined, but an
empty string. There should be no autovivification happening. If I missed
a side effect, let me know.

For the new format option I went with singular for the name as it only
checks a single VLAN ID/range from the list, 'pve-bridge-vid', but I am
not sure if it wouldn't be better to call it the actual parameter name
'pve-bridge-vids'.

  src/PVE/INotify.pm|  2 +-
  src/PVE/JSONSchema.pm | 32 
  2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index bc33a8f..14f40ac 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -1270,7 +1270,7 @@ sub __interface_to_string {
  
  	if (defined($d->{bridge_vlan_aware})) {

$raw .= "\tbridge-vlan-aware yes\n";
-   my $vlans = defined($d->{bridge_vids}) ? $d->{bridge_vids} : 
"2-4094";
+   my $vlans = length($d->{bridge_vids}) ? $d->{bridge_vids} : 
"2-4094";
$raw .= "\tbridge-vids $vlans\n";
}
$done->{bridge_vlan_aware} = 1;
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 85d47f2..1051a45 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -78,6 +78,12 @@ register_standard_option('pve-iface', {
  minLength => 2, maxLength => 20,
  });
  
+register_standard_option('pve-bridge-vid', {

+description => "Bridge VLAN ID.",
+type => 'string', format => 'pve-bridge-vid',
+minLength => 1, maxLength => 9,
+});
+
  register_standard_option('pve-storage-id', {
  description => "The storage identifier.",
  type => 'string', format => 'pve-storage-id',
@@ -588,6 +594,32 @@ sub pve_verify_iface {
  return $id;
  }
  
+# bridge vlan id (vids)

+register_format('pve-bridge-vid', \&pve_verify_bridge_vid);
+sub pve_verify_bridge_vid {
+my ($vlan, $noerr) = @_;
+
+my $check_vid = sub {
+   my $id = shift;
+   if ( $id < 2 || $id > 4094) {
+   return undef if $noerr;
+   die "invalid VLAN tag '$id'\n";
+   }
+};
+
+if ($vlan !~ m/^(\d+)([-](\d+))?$/i) {
+   return undef if $noerr;
+   die "invalid VLAN configuration '$vlan'\n";
+}
+$check_vid->($1);
+if ($3) {
+   $check_vid->($3);
+   die "VLAN range must go from lower to higher tag '$vlan'" if $1 > $3 && 
!$noerr;
+}
+
+return $vlan;
+}
+
  # general addresses by name or IP
  register_format('address', \&pve_verify_address);
  sub pve_verify_address {



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