From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 9EB0889F61 for ; Tue, 18 Oct 2022 16:03:29 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C4BA429737 for ; Tue, 18 Oct 2022 16:02:39 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 18 Oct 2022 16:02:29 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C881644A9C for ; Tue, 18 Oct 2022 16:02:28 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Tue, 18 Oct 2022 16:02:07 +0200 Message-Id: <20221018140226.598710-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221018140226.598710-1-d.csapak@proxmox.com> References: <20221018140226.598710-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.067 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 v8 1/4] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method 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: , X-List-Received-Date: Tue, 18 Oct 2022 14:03:29 -0000 for getting multiple properties from the in memory config of the guests in one go. Keep the existing IPC call as is for backward compatibility and add this as separate, new one. It basically behaves the same as CFS_IPC_GET_GUEST_CONFIG_PROPERTY, but takes a list of properties instead and returns multiple properties per guest. The old way of getting a single property is now also done by the new function, since it basically performs identically. Here a short benchmark: Setup: PVE in a VM with cpu type host (12700k) and 4 cores 10000 typical configs with both 'lock' and 'tags' set at the end and fairly long tags ('test-tag1,test-tag2,test-tag3') (normal vm with a snapshot, ~1KiB) i let it run 100 times each, times in ms old code: total time time per iteration 1054.2 10.2 new code: num props total time time per iter function 2 1332.2 13.2 get_properties 1 1051.2 10.2 get_properties 2 2082.2 20.2 get_property (2 calls to get_property) 1 1034.2 10.2 get_property so a call with the new code for one property is the same as with the old code, and adding a second property only adds a bit of additional time (in this case ~30%) Signed-off-by: Dominik Csapak --- data/src/cfs-ipc-ops.h | 2 + data/src/server.c | 64 +++++++++++++++ data/src/status.c | 177 ++++++++++++++++++++++++++++------------- data/src/status.h | 3 + 4 files changed, 190 insertions(+), 56 deletions(-) diff --git a/data/src/cfs-ipc-ops.h b/data/src/cfs-ipc-ops.h index 003e233..249308d 100644 --- a/data/src/cfs-ipc-ops.h +++ b/data/src/cfs-ipc-ops.h @@ -43,4 +43,6 @@ #define CFS_IPC_VERIFY_TOKEN 12 +#define CFS_IPC_GET_GUEST_CONFIG_PROPERTIES 13 + #endif diff --git a/data/src/server.c b/data/src/server.c index 549788a..ced9cfc 100644 --- a/data/src/server.c +++ b/data/src/server.c @@ -89,6 +89,13 @@ typedef struct { char property[]; } cfs_guest_config_propery_get_request_header_t; +typedef struct { + struct qb_ipc_request_header req_header; + uint32_t vmid; + uint8_t num_props; + char props[]; /* list of \0 terminated properties */ +} cfs_guest_config_properties_get_request_header_t; + typedef struct { struct qb_ipc_request_header req_header; char token[]; @@ -348,6 +355,63 @@ static int32_t s1_msg_process_fn( result = cfs_create_guest_conf_property_msg(outbuf, memdb, rh->property, rh->vmid); } + } else if (request_id == CFS_IPC_GET_GUEST_CONFIG_PROPERTIES) { + + cfs_guest_config_properties_get_request_header_t *rh = + (cfs_guest_config_properties_get_request_header_t *) data; + + size_t remaining = request_size - G_STRUCT_OFFSET(cfs_guest_config_properties_get_request_header_t, props); + + result = 0; + if (rh->vmid < 100 && rh->vmid != 0) { + cfs_debug("vmid out of range %u", rh->vmid); + result = -EINVAL; + } else if (rh->vmid >= 100 && !vmlist_vm_exists(rh->vmid)) { + result = -ENOENT; + } else if (rh->num_props == 0) { + cfs_debug("num_props == 0"); + result = -EINVAL; + } else if (remaining <= 1) { + cfs_debug("property length <= 1, %ld", remaining); + result = -EINVAL; + } else { + const char **properties = malloc(sizeof(char*) * rh->num_props); + char *current = (rh->props); + for (uint8_t i = 0; i < rh->num_props; i++) { + size_t proplen = strnlen(current, remaining); + if (proplen == 0) { + cfs_debug("property length 0"); + result = -EINVAL; + break; + } + if (proplen == remaining) { + cfs_debug("property not \\0 terminated"); + result = -EINVAL; + break; + } + if (current[0] < 'a' || current[0] > 'z') { + cfs_debug("property does not start with [a-z]"); + result = -EINVAL; + break; + } + properties[i] = current; + current[proplen] = '\0'; // ensure property is 0 terminated + remaining -= (proplen + 1); + current += proplen + 1; + } + + if (remaining != 0) { + cfs_debug("leftover data after parsing %u properties", rh->num_props); + result = -EINVAL; + } + + if (result == 0) { + cfs_debug("cfs_get_guest_config_properties: basic valid checked, do request"); + result = cfs_create_guest_conf_properties_msg(outbuf, memdb, properties, rh->num_props, rh->vmid); + } + + free(properties); + } } else if (request_id == CFS_IPC_VERIFY_TOKEN) { cfs_verify_token_request_header_t *rh = (cfs_verify_token_request_header_t *) data; diff --git a/data/src/status.c b/data/src/status.c index 9bceaeb..e2bedaa 100644 --- a/data/src/status.c +++ b/data/src/status.c @@ -804,25 +804,52 @@ cfs_create_vmlist_msg(GString *str) return 0; } -// checks the conf for a line starting with '$prop:' and returns the value -// afterwards, whitout initial whitespace(s), we only deal with the format -// restricion imposed by our perl VM config parser, main reference is +// checks if a config line starts with the given prop. if yes, writes a '\0' +// at the end of the value, and returns the pointer where the value starts +char * +_get_property_value_from_line(char *line, int line_len, const char *prop, int prop_len) +{ + if (line_len <= prop_len + 1) return NULL; + + if (line[prop_len] == ':' && memcmp(line, prop, prop_len) == 0) { // found + char *v_start = &line[prop_len + 1]; + + // drop initial value whitespaces here already + while (*v_start && isspace(*v_start)) v_start++; + + if (!*v_start) return NULL; + + char *v_end = &line[line_len - 1]; + while (v_end > v_start && isspace(*v_end)) v_end--; + v_end[1] = '\0'; + + return v_start; + } + + return NULL; +} + +// checks the conf for lines starting with the given props and +// writes the pointers into the correct positions into the 'found' array +// afterwards, without initial whitespace(s), we only deal with the format +// restriction imposed by our perl VM config parser, main reference is // PVE::QemuServer::parse_vm_config this allows to be very fast and still // relatively simple -// main restrictions used for our advantage is the properties match reges: +// main restrictions used for our advantage is the properties match regex: // ($line =~ m/^([a-z][a-z_]*\d*):\s*(.+?)\s*$/) from parse_vm_config // currently we only look at the current configuration in place, i.e., *no* -// snapshort and *no* pending changes -static char * -_get_property_value(char *conf, int conf_size, const char *prop, int prop_len) +// snapshot and *no* pending changes +void +_get_property_values(char **found, char *conf, int conf_size, const char **props, uint8_t num_props, char min, char max) { const char *const conf_end = conf + conf_size; char *line = conf; - size_t remaining_size; + size_t remaining_size = conf_size; + int count = 0; char *next_newline = memchr(conf, '\n', conf_size); if (next_newline == NULL) { - return NULL; // valid property lines end with \n, but none in the config + return; // valid property lines end with \n, but none in the config } *next_newline = '\0'; @@ -830,41 +857,32 @@ _get_property_value(char *conf, int conf_size, const char *prop, int prop_len) if (!line[0]) goto next; // snapshot or pending section start, but nothing found yet -> not found - if (line[0] == '[') return NULL; - // properties start with /^[a-z]/, so continue early if not - if (line[0] < 'a' || line[0] > 'z') goto next; + if (line[0] == '[') return; + // continue early if line does not begin with the min/max char of the properties + if (line[0] < min || line[0] > max) goto next; int line_len = strlen(line); - if (line_len <= prop_len + 1) goto next; - - if (line[prop_len] == ':' && memcmp(line, prop, prop_len) == 0) { // found - char *v_start = &line[prop_len + 1]; - - // drop initial value whitespaces here already - while (*v_start && isspace(*v_start)) v_start++; - - if (!*v_start) return NULL; - - char *v_end = &line[line_len - 1]; - while (v_end > v_start && isspace(*v_end)) v_end--; - v_end[1] = '\0'; - - return v_start; + for (uint8_t i = 0; i < num_props; i++) { + char * value = _get_property_value_from_line(line, line_len, props[i], strlen(props[i])); + if (value != NULL) { + count += (found[i] != NULL) & 0x1; // count newly found lines + found[i] = value; + } + } + if (count == num_props) { + return; // found all } next: line = next_newline + 1; remaining_size = conf_end - line; - if (remaining_size <= prop_len) { - return NULL; - } next_newline = memchr(line, '\n', remaining_size); if (next_newline == NULL) { - return NULL; // valid property lines end with \n, but none in the config + return; // valid property lines end with \n, but none in the config } *next_newline = '\0'; } - return NULL; // not found + return; } static void @@ -883,24 +901,77 @@ _g_str_append_kv_jsonescaped(GString *str, const char *k, const char *v) } int -cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid) +_print_found_properties( + GString *str, + gpointer conf, + int size, + const char **props, + uint8_t num_props, + uint32_t vmid, + char **values, + char min, + char max, + int first) +{ + _get_property_values(values, conf, size, props, num_props, min, max); + + uint8_t found = 0; + for (uint8_t i = 0; i < num_props; i++) { + if (values[i] == NULL) { + continue; + } + if (found) { + g_string_append_c(str, ','); + } else { + if (!first) { + g_string_append_printf(str, ",\n"); + } else { + first = 0; + } + g_string_append_printf(str, "\"%u\":{", vmid); + found = 1; + } + _g_str_append_kv_jsonescaped(str, props[i], values[i]); + } + + if (found) { + g_string_append_c(str, '}'); + } + + return first; +} + +int +cfs_create_guest_conf_properties_msg(GString *str, memdb_t *memdb, const char **props, uint8_t num_props, uint32_t vmid) { g_return_val_if_fail(cfs_status.vmlist != NULL, -EINVAL); g_return_val_if_fail(str != NULL, -EINVAL); - int prop_len = strlen(prop); - int res = 0; - GString *path = NULL; - // Prelock &memdb->mutex in order to enable the usage of memdb_read_nolock // to prevent Deadlocks as in #2553 g_mutex_lock (&memdb->mutex); g_mutex_lock (&mutex); - g_string_printf(str,"{\n"); + g_string_printf(str, "{\n"); GHashTable *ht = cfs_status.vmlist; + + int res = 0; + GString *path = NULL; gpointer tmp = NULL; + char **values = calloc(num_props, sizeof(char*)); + char min = 'z', max = 'a'; + + for (uint8_t i = 0; i < num_props; i++) { + if (props[i][0] > max) { + max = props[i][0]; + } + + if (props[i][0] < min) { + min = props[i][0]; + } + } + if (!g_hash_table_size(ht)) { goto ret; } @@ -919,15 +990,8 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *pro // use memdb_read_nolock because lock is handled here int size = memdb_read_nolock(memdb, path->str, &tmp); if (tmp == NULL) goto err; - if (size <= prop_len) goto ret; - - char *val = _get_property_value(tmp, size, prop, prop_len); - if (val == NULL) goto ret; - - g_string_append_printf(str, "\"%u\":{", vmid); - _g_str_append_kv_jsonescaped(str, prop, val); - g_string_append_c(str, '}'); + _print_found_properties(str, tmp, size, props, num_props, vmid, values, min, max, 1); } else { GHashTableIter iter; g_hash_table_iter_init (&iter, ht); @@ -943,21 +1007,16 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *pro tmp = NULL; // use memdb_read_nolock because lock is handled here int size = memdb_read_nolock(memdb, path->str, &tmp); - if (tmp == NULL || size <= prop_len) continue; + if (tmp == NULL) continue; - char *val = _get_property_value(tmp, size, prop, prop_len); - if (val == NULL) continue; - - if (!first) g_string_append_printf(str, ",\n"); - else first = 0; - - g_string_append_printf(str, "\"%u\":{", vminfo->vmid); - _g_str_append_kv_jsonescaped(str, prop, val); - g_string_append_c(str, '}'); + memset(values, 0, sizeof(char*)*num_props); // reset array + first = _print_found_properties(str, tmp, size, props, num_props, + vminfo->vmid, values, min, max, first); } } ret: g_free(tmp); + free(values); if (path != NULL) { g_string_free(path, TRUE); } @@ -973,6 +1032,12 @@ enoent: goto ret; } +int +cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid) +{ + return cfs_create_guest_conf_properties_msg(str, memdb, &prop, 1, vmid); +} + void record_memdb_change(const char *path) { diff --git a/data/src/status.h b/data/src/status.h index bbf0948..041cb34 100644 --- a/data/src/status.h +++ b/data/src/status.h @@ -163,4 +163,7 @@ cfs_create_memberlist_msg( int cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid); +int +cfs_create_guest_conf_properties_msg(GString *str, memdb_t *memdb, const char **props, uint8_t num_props, uint32_t vmid); + #endif /* _PVE_STATUS_H_ */ -- 2.30.2