[pve-devel] [PATCH cluster 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method

* [pve-devel] [PATCH cluster 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method
@ 2022-03-14  9:03 Dominik Csapak
  2022-03-14  9:03 ` [pve-devel] [PATCH cluster 2/3] Cluster: add get_guest_config_properties Dominik Csapak
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dominik Csapak @ 2022-03-14  9:03 UTC (permalink / raw)
  To: pve-devel

for getting multiple properties from the in memory config of the
guests. I added a new CSF_IPC_ call to maintain backwards compatibility.

It basically behaves the same as
CFS_IPC_GET_GUEST_CONFIG_PROPERTY, but takes a list of properties
instead.

The old way of getting a single property is now also done by
the new function.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
in my benchmarks with ~10000 "real" vm configs, i could not really
see a difference for the old code vs the new with 2 properties
(~30ms per call)

 data/src/cfs-ipc-ops.h |   2 +
 data/src/server.c      |  47 ++++++++++++
 data/src/status.c      | 166 ++++++++++++++++++++++++++++-------------
 data/src/status.h      |   3 +
 4 files changed, 165 insertions(+), 53 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..d4f6374 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[];
+} cfs_guest_config_properties_get_request_header_t;
+
 typedef struct {
 	struct qb_ipc_request_header req_header;
 	char token[];
@@ -348,6 +355,46 @@ 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;
+
+		int propslen = 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 (propslen <= 0) {
+			cfs_debug("propslen <= 0, %d", propslen);
+			result = -EINVAL;
+		} else {
+			const char **properties = malloc(sizeof(char*) * rh->num_props);
+			char *current = (rh->props);
+			int remaining = propslen;
+			for (uint8_t i = 0; i < rh->num_props; i++) {
+			    uint8_t proplen = (uint8_t)(*current);
+			    if (proplen > --remaining) {
+				cfs_debug("property len > remaining: %d > %d", proplen, remaining);
+				result = -EINVAL;
+				break;
+			    }
+			    properties[i] = ++current;
+			    current[proplen - 1] = '\0'; // ensure property is 0 terminated
+			    remaining -= proplen;
+			    current += proplen;
+			}
+
+			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..c73ad11 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
+// 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, whitout initial whitespace(s), we only deal with the format
 // restricion 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,33 @@ _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) {
+			// found all
+			return;
 		}
 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;
 		}
 		*next_newline = '\0';
 	}
 
-	return NULL; // not found
+	return;
 }
 
 static void
@@ -883,24 +902,36 @@ _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)
+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 +950,25 @@ 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, '}');
+		_get_property_values(values, tmp, size, props, num_props, min, max);
+
+		uint8_t found = 0;
+		for (uint8_t i = 0; i < num_props; i++) {
+			if (values[i] != NULL) {
+				if (found) {
+					g_string_append_c(str, ',');
+				} else {
+					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, '}');
+		}
 	} else {
 		GHashTableIter iter;
 		g_hash_table_iter_init (&iter, ht);
@@ -943,21 +984,34 @@ 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;
-
-			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;
+			if (tmp == NULL) continue;
+
+			memset(values, 0, sizeof(char*)*num_props); // reset array
+			_get_property_values(values, tmp, size, props, num_props, min, max);
+
+			uint8_t found = 0;
+			for (uint8_t i = 0; i < num_props; i++) {
+				if (values[i] != NULL) {
+					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\":{", vminfo->vmid);
+						found = 1;
+					}
+					_g_str_append_kv_jsonescaped(str, props[i], values[i]);
+				}
+			}
 
-			g_string_append_printf(str, "\"%u\":{", vminfo->vmid);
-			_g_str_append_kv_jsonescaped(str, prop, val);
-			g_string_append_c(str, '}');
+			if (found) {
+				g_string_append_c(str, '}');
+			}
 		}
 	}
 ret:
 	g_free(tmp);
+	free(values);
 	if (path != NULL) {
 		g_string_free(path, TRUE);
 	}
@@ -973,6 +1027,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





^ permalink raw reply	[flat|nested] 6+ messages in thread