]> git.proxmox.com Git - pve-cluster.git/commitdiff
pmxcfs: add IPC call to get multiple guest config properties at once
authorDominik Csapak <d.csapak@proxmox.com>
Wed, 16 Nov 2022 15:47:55 +0000 (16:47 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 17 Nov 2022 09:01:55 +0000 (10:01 +0100)
Previously we used the existing IPC call just for getting the `lock`
property of virtual guests in the cluster resource API call, but for
the tag display we'll to get another one and calling this twice seems
rather non-ideal.

Thus introduce a successor: CFS_IPC_GET_GUEST_CONFIG_PROPERTIES

It allows one to get multiple properties from a single, or all
virtual guest in-memory configs in one go. Keep the existing IPC call
as is for backward compatibility and add this as separate, new one.

The new IPC command basically behaves the same as the previous
CFS_IPC_GET_GUEST_CONFIG_PROPERTY, but takes a list of properties
instead and returns multiple properties per guest.

The existing perl wrapper around the IPC call to get a single
property was switched over to use the new IPC call too, so we'll
be able to drop the old IPC command with the next major release if
nothing comes up.

= Benchmark =

== Setup ==

Proxmox VE in a VM with CPU type host (Intel 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, ~ 1 KiB)

== Test ==

Average of 100 runs each with time in milliseconds

== Previous Results ==

num props  total time  time per iteration
1          1054.2      10.2

== Results with this Patch ==

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 separate calls
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 <d.csapak@proxmox.com>
 [ T: reword & re-structure commit message ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
data/src/cfs-ipc-ops.h
data/src/server.c
data/src/status.c
data/src/status.h

index 003e233be318dc0a0fade8dcdcc35b9d4efc1ce3..249308d955650045d3c1cb89c070f3b45c97841a 100644 (file)
@@ -43,4 +43,6 @@
 
 #define CFS_IPC_VERIFY_TOKEN 12
 
+#define CFS_IPC_GET_GUEST_CONFIG_PROPERTIES 13
+
 #endif
index 549788a9ee2258f309eab9817073a29183a10dce..80f838b3af7b36aca987c09170cd660dc7861ee0 100644 (file)
@@ -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,62 @@ 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 || current[proplen] != '\0') {
+                               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;
+                           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 validity 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;
index 9bceaeb900d39ea02118191c9cffbbc0b0b83a18..2a54eaf1102372541c1319e781c9c88065947796 100644 (file)
@@ -804,25 +804,63 @@ 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
+// note: line[line_end] needs to be guaranteed a null byte
+char *
+_get_property_value_from_line(char *line, size_t line_len, const char *prop, size_t 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];
+               char *v_end = &line[line_len - 1];
+
+               // drop initial value whitespaces here already
+               while (v_start < v_end && *v_start && isspace(*v_start)) v_start++;
+
+               if (!*v_start) return NULL;
+
+               while (v_end > v_start && isspace(*v_end)) v_end--;
+               if (v_end < &line[line_len - 1]) {
+                   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
+//
+// min..max is the char range of the first character of the given props,
+// so that we can return early when checking the line
+// note: conf must end with a newline
+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;
+       uint8_t count = 0;
+
+       if (conf_size == 0) {
+           return;
+       }
 
        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 +868,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;
-
-               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;
+               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;
+
+               size_t line_len = next_newline - line;
+               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 +912,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 +1001,15 @@ 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, '}');
 
+               // conf needs to be newline terminated
+               if (((char *)tmp)[size - 1] != '\n') {
+                       gpointer new = realloc(tmp, size + 1);
+                       if (new == NULL) goto err;
+                       tmp = new;
+                       ((char *)tmp)[size++] = '\n';
+               }
+               _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 +1025,24 @@ 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;
+
+                       // conf needs to be newline terminated
+                       if (((char *)tmp)[size - 1] != '\n') {
+                           gpointer new = realloc(tmp, size + 1);
+                           if (new == NULL) continue;
+                           tmp = new;
+                           ((char *)tmp)[size++] = '\n';
+                       }
 
-                       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 +1058,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)
 {
index bbf0948123ad71ee68dc10c45dfa883234dfcd7b..041cb343b35433c99e998cba75b7c4dd99c49980 100644 (file)
@@ -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_ */