From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 190FA1FF15C for ; Fri, 5 Sep 2025 15:55:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CA39D16684; Fri, 5 Sep 2025 15:55:58 +0200 (CEST) From: Aaron Lauterer To: pve-devel@lists.proxmox.com Date: Fri, 5 Sep 2025 15:55:13 +0200 Message-ID: <20250905135517.4005478-4-a.lauterer@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250905135517.4005478-1-a.lauterer@proxmox.com> References: <20250905135517.4005478-1-a.lauterer@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1757080505611 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.012 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH cluster 3/3] pmxcfs: status.c: always use 9.0 rrd files X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Assuming that we can keep the old RRD files around and fetch data directly there if needed, we can skip the migration step and just write to new 9.0 RRD files. This simplifies the whole approach introduced in 9b00ac9 status: introduce new pve-{type}- rrd and metric format We will always create a new 9.0 file if not present and then need to handle the following cases: * old pve2-{type} format -> pad missing columns * pve-{type}-9.0 matches -> no changes needed * pve-{type}- received, but version doesn't match -> new format, cut at expected columns Since we don't need the old RRD definitions anymore, they are removed. Otherwise the compile will throw an unused variable warning. The `rrd_def_{type}_columns` variables get a `_pve2` suffix to make it clear what they are for. The `checked_mkdir` pattern for VMs is aligned with how we do it for nodes and storages, to call it once more, to be safe. Signed-off-by: Aaron Lauterer --- Notes: if I misunderstood the reason for the doulbe checked_mkdir, then it might only be needed for the storage due to the additional subdirs? src/pmxcfs/status.c | 238 ++++++-------------------------------------- 1 file changed, 30 insertions(+), 208 deletions(-) diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c index 130f08f..cb03e4e 100644 --- a/src/pmxcfs/status.c +++ b/src/pmxcfs/status.c @@ -1097,36 +1097,9 @@ kventry_hash_set(GHashTable *kvhash, const char *key, gconstpointer data, size_t // We create the RRD files with a 60 second stepsize, therefore, RRA timesteps // are alwys per 60 seconds. These 60 seconds are usually showing up in other // code paths where we interact with RRD data! -static const char *rrd_def_node[] = { - "DS:loadavg:GAUGE:120:0:U", - "DS:maxcpu:GAUGE:120:0:U", - "DS:cpu:GAUGE:120:0:U", - "DS:iowait:GAUGE:120:0:U", - "DS:memtotal:GAUGE:120:0:U", - "DS:memused:GAUGE:120:0:U", - "DS:swaptotal:GAUGE:120:0:U", - "DS:swapused:GAUGE:120:0:U", - "DS:roottotal:GAUGE:120:0:U", - "DS:rootused:GAUGE:120:0:U", - "DS:netin:DERIVE:120:0:U", - "DS:netout:DERIVE:120:0:U", - - "RRA:AVERAGE:0.5:1:70", // 1 min avg - one hour - "RRA:AVERAGE:0.5:30:70", // 30 min avg - one day - "RRA:AVERAGE:0.5:180:70", // 3 hour avg - one week - "RRA:AVERAGE:0.5:720:70", // 12 hour avg - one month - "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year - - "RRA:MAX:0.5:1:70", // 1 min max - one hour - "RRA:MAX:0.5:30:70", // 30 min max - one day - "RRA:MAX:0.5:180:70", // 3 hour max - one week - "RRA:MAX:0.5:720:70", // 12 hour max - one month - "RRA:MAX:0.5:10080:70", // 7 day max - ony year - NULL, -}; // This *must* be the number of columns as defined above. -static const int rrd_def_node_columns = 12; +static const int rrd_def_node_columns_pve2 = 12; static const char *rrd_def_node_pve9_0[] = { "DS:loadavg:GAUGE:120:0:U", @@ -1165,34 +1138,8 @@ static const char *rrd_def_node_pve9_0[] = { // This *must* be the number of columns as defined above. static const int rrd_def_node_pve9_0_columns = 19; -static const char *rrd_def_vm[] = { - "DS:maxcpu:GAUGE:120:0:U", - "DS:cpu:GAUGE:120:0:U", - "DS:maxmem:GAUGE:120:0:U", - "DS:mem:GAUGE:120:0:U", - "DS:maxdisk:GAUGE:120:0:U", - "DS:disk:GAUGE:120:0:U", - "DS:netin:DERIVE:120:0:U", - "DS:netout:DERIVE:120:0:U", - "DS:diskread:DERIVE:120:0:U", - "DS:diskwrite:DERIVE:120:0:U", - - "RRA:AVERAGE:0.5:1:70", // 1 min avg - one hour - "RRA:AVERAGE:0.5:30:70", // 30 min avg - one day - "RRA:AVERAGE:0.5:180:70", // 3 hour avg - one week - "RRA:AVERAGE:0.5:720:70", // 12 hour avg - one month - "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year - - "RRA:MAX:0.5:1:70", // 1 min max - one hour - "RRA:MAX:0.5:30:70", // 30 min max - one day - "RRA:MAX:0.5:180:70", // 3 hour max - one week - "RRA:MAX:0.5:720:70", // 12 hour max - one month - "RRA:MAX:0.5:10080:70", // 7 day max - ony year - NULL, -}; - // This *must* be the number of columns as defined above. -static const int rrd_def_vm_columns = 10; +static const int rrd_def_vm_columns_pve2 = 10; static const char *rrd_def_vm_pve9_0[] = { "DS:maxcpu:GAUGE:120:0:U", @@ -1229,24 +1176,6 @@ static const char *rrd_def_vm_pve9_0[] = { // This *must* be the number of columns as defined above. static const int rrd_def_vm_pve9_0_columns = 17; -static const char *rrd_def_storage[] = { - "DS:total:GAUGE:120:0:U", - "DS:used:GAUGE:120:0:U", - - "RRA:AVERAGE:0.5:1:70", // 1 min avg - one hour - "RRA:AVERAGE:0.5:30:70", // 30 min avg - one day - "RRA:AVERAGE:0.5:180:70", // 3 hour avg - one week - "RRA:AVERAGE:0.5:720:70", // 12 hour avg - one month - "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year - - "RRA:MAX:0.5:1:70", // 1 min max - one hour - "RRA:MAX:0.5:30:70", // 30 min max - one day - "RRA:MAX:0.5:180:70", // 3 hour max - one week - "RRA:MAX:0.5:720:70", // 12 hour max - one month - "RRA:MAX:0.5:10080:70", // 7 day max - ony year - NULL, -}; - static const char *rrd_def_storage_pve9_0[] = { "DS:total:GAUGE:120:0:U", "DS:used:GAUGE:120:0:U", @@ -1356,68 +1285,27 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) { } filename = g_strdup_printf(RRDDIR "/pve-node-9.0/%s", node); - char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-node/%s", node); - - int use_pve2_file = 0; - // check existing rrd files and directories - if (g_file_test(filename, G_FILE_TEST_EXISTS)) { - // pve-node-9.0 file exists, we use that - // TODO: get conditions so, that we do not have this empty branch - } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) { - // old file exists, use it - use_pve2_file = 1; - - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; - } else { - // neither file exists, check for directories to decide and create file + if (!g_file_test(filename, G_FILE_TEST_EXISTS)) { + checked_mkdir(RRDDIR "/pve-node-9.0", 0755); - if (g_file_test(RRDDIR "/pve-node-9.0", G_FILE_TEST_IS_DIR)) { + char *dir = g_path_get_dirname(filename); + checked_mkdir(dir, 0755); + g_free(dir); - int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_node_pve9_0); - } else if (g_file_test(RRDDIR "/pve2-node", G_FILE_TEST_IS_DIR)) { - use_pve2_file = 1; - - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; - - char *dir = g_path_get_dirname(filename); - checked_mkdir(dir, 0755); - g_free(dir); - - int argcount = sizeof(rrd_def_node) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_node); - } else { - // no dir exists yet, use new pve-node-9.0 - checked_mkdir(RRDDIR "/pve-node-9.0", 0755); - - char *dir = g_path_get_dirname(filename); - checked_mkdir(dir, 0755); - g_free(dir); - - int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_node_pve9_0); - } + int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1; + create_rrd_file(filename, argcount, rrd_def_node_pve9_0); } skip = 2; // first two columns are live data that isn't archived - if (strncmp(key, "pve2-node/", 10) == 0 && !use_pve2_file) { - padding = rrd_def_node_pve9_0_columns - rrd_def_node_columns; - } else if (strncmp(key, "pve-node-", 9) == 0 && use_pve2_file) { - keep_columns = rrd_def_node_columns; + if (strncmp(key, "pve2-node/", 10) == 0) { + padding = rrd_def_node_pve9_0_columns - rrd_def_node_columns_pve2; } else if (strncmp(key, "pve-node-9.0/", 13) != 0) { // we received an unknown format, expectation is it is newer and has more columns // than we can currently handle keep_columns = rrd_def_node_pve9_0_columns; } - - g_free(filename_pve2); - } else if (strncmp(key, "pve2.3-vm/", 10) == 0 || strncmp(key, "pve-vm-", 7) == 0) { const char *vmid = rrd_skip_data(key, 1, '/'); @@ -1431,59 +1319,28 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) { } filename = g_strdup_printf(RRDDIR "/pve-vm-9.0/%s", vmid); - char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-vm/%s", vmid); - - int use_pve2_file = 0; - - // check existing rrd files and directories - if (g_file_test(filename, G_FILE_TEST_EXISTS)) { - // pve-vm-9.0 file exists, we use that - // TODO: get conditions so, that we do not have this empty branch - } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) { - // old file exists, use it - use_pve2_file = 1; - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; - } else { - // neither file exists, check for directories to decide and create file - - if (g_file_test(RRDDIR "/pve-vm-9.0", G_FILE_TEST_IS_DIR)) { - int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_vm_pve9_0); - } else if (g_file_test(RRDDIR "/pve2-vm", G_FILE_TEST_IS_DIR)) { - use_pve2_file = 1; + if (!g_file_test(filename, G_FILE_TEST_EXISTS)) { + // no dir exists yet, use new pve-vm-9.0 + checked_mkdir(RRDDIR "/pve-vm-9.0", 0755); - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; + char *dir = g_path_get_dirname(filename); + checked_mkdir(dir, 0755); + g_free(dir); - int argcount = sizeof(rrd_def_vm) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_vm); - } else { - // no dir exists yet, use new pve-vm-9.0 - checked_mkdir(RRDDIR "/pve-vm-9.0", 0755); - - int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_vm_pve9_0); - } + int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1; + create_rrd_file(filename, argcount, rrd_def_vm_pve9_0); } skip = 4; // first 4 columns are live data that isn't archived - if (strncmp(key, "pve2.3-vm/", 10) == 0 && !use_pve2_file) { - padding = rrd_def_vm_pve9_0_columns - rrd_def_vm_columns; - } else if (strncmp(key, "pve-vm-", 7) == 0 && use_pve2_file) { - keep_columns = rrd_def_vm_columns; + if (strncmp(key, "pve2.3-vm/", 10) == 0) { + padding = rrd_def_vm_pve9_0_columns - rrd_def_vm_columns_pve2; } else if (strncmp(key, "pve-vm-9.0/", 11) != 0) { // we received an unknown format, expectation is it is newer and has more columns // than we can currently handle keep_columns = rrd_def_vm_pve9_0_columns; } - - g_free(filename_pve2); - } else if (strncmp(key, "pve2-storage/", 13) == 0 || strncmp(key, "pve-storage-", 12) == 0) { const char *node = rrd_skip_data(key, 1, '/'); // will contain {node}/{storage} @@ -1502,49 +1359,17 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) { } filename = g_strdup_printf(RRDDIR "/pve-storage-9.0/%s", node); - char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-storage/%s", node); - - // check existing rrd files and directories - if (g_file_test(filename, G_FILE_TEST_EXISTS)) { - // pve-storage-9.0 file exists, we use that - // TODO: get conditions so, that we do not have this empty branch - } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) { - // old file exists, use it - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; - } else { - // neither file exists, check for directories to decide and create file - - if (g_file_test(RRDDIR "/pve-storage-9.0", G_FILE_TEST_IS_DIR)) { - char *dir = g_path_get_dirname(filename); - checked_mkdir(dir, 0755); - g_free(dir); - - int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_storage_pve9_0); - } else if (g_file_test(RRDDIR "/pve2-storage", G_FILE_TEST_IS_DIR)) { - g_free(filename); - filename = filename_pve2; - filename_pve2 = NULL; - - char *dir = g_path_get_dirname(filename); - checked_mkdir(dir, 0755); - g_free(dir); - - int argcount = sizeof(rrd_def_storage) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_storage); - } else { - // no dir exists yet, use new pve-storage-9.0 - checked_mkdir(RRDDIR "/pve-storage-9.0", 0755); - char *dir = g_path_get_dirname(filename); - checked_mkdir(dir, 0755); - g_free(dir); + if (!g_file_test(filename, G_FILE_TEST_EXISTS)) { + // no dir exists yet, use new pve-storage-9.0 + checked_mkdir(RRDDIR "/pve-storage-9.0", 0755); - int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1; - create_rrd_file(filename, argcount, rrd_def_storage_pve9_0); - } + char *dir = g_path_get_dirname(filename); + checked_mkdir(dir, 0755); + g_free(dir); + + int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1; + create_rrd_file(filename, argcount, rrd_def_storage_pve9_0); } // actual data columns didn't change between pve2-storage and pve-storage-9.0 @@ -1553,9 +1378,6 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) { // than we can currently handle keep_columns = rrd_def_storage_pve9_0_columns; } - - g_free(filename_pve2); - } else { goto keyerror; } -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel