From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH cluster 2/3] Cluster: add get_guest_config_properties
Date: Wed, 16 Mar 2022 11:08:59 +0100 [thread overview]
Message-ID: <1647424380.fqb9guy34f.astroid@nora.none> (raw)
In-Reply-To: <20220314090307.1130083-2-d.csapak@proxmox.com>
this and the other one existing one might also warrant a comment
indicating that you possibly still want to parse/validate/filter the
result before passing it along further up the stack (or that it should
only be used for very simple keys?)
- if no vmid is passed, the result needs to be filtered by access to not
leak information
- if property is anything but a very simple type, validation might be
important (for lock it's not that bad since that is just a simple
enum, so a bogus/invalid value will likely just look weird, for the
`tags` property it's also not that bad since those are just
simple lists of strings, but who knows what this will get used for
down the line ;))
also depending on whether the msg format gets changed this will need
adapation obviously.
On March 14, 2022 10:03 am, Dominik Csapak wrote:
> akin to get_guest_config_property, but with a list of properties.
> uses the new CFS_IPC_GET_GUEST_CONFIG_PROPERTIES
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> data/PVE/Cluster.pm | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index 765fb43..c65ba17 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -340,6 +340,27 @@ sub get_node_kv {
> return $res;
> }
>
> +# properties: an array-ref of config properties you want to get, e.g., this
> +# is perfect to get multiple properties of a guest _fast_
> +# (>100 faster than manual parsing here)
> +# vmid: optional, if a valid is passed we only check that one, else return all
> +# NOTE: does *not* searches snapshot and PENDING entries sections!
> +sub get_guest_config_properties {
> + my ($properties, $vmid) = @_;
> +
> + die "properties required" if !defined($properties);
> +
> + my $bindata = pack "VC", $vmid // 0, scalar(@$properties);
length of $properties actually has a limit which should maybe be checked
here as well? I mean it is rather unlikely to be misused in practice,
and will print a warning that the `C` wraps the value here, but..
> + for my $property (@$properties) {
> + die "property name cannot be longer than 254 chars\n"
> + if length($property) > 254;
> + $bindata .= pack "C/Z*", $property;
like indicated in the other patch's comments, this is a rather strange
encoding (flashbacks to ASN.1 ;)) and shouldn't be needed.
> + }
> + my $res = $ipcc_send_rec_json->(CFS_IPC_GET_GUEST_CONFIG_PROPERTIES, $bindata);
> +
> + return $res;
> +}
> +
> # property: a config property you want to get, e.g., this is perfect to get
> # the 'lock' entry of a guest _fast_ (>100 faster than manual parsing here)
> # vmid: optipnal, if a valid is passed we only check that one, else return all
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
next prev parent reply other threads:[~2022-03-16 10:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-14 9:03 [pve-devel] [PATCH cluster 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method Dominik Csapak
2022-03-14 9:03 ` [pve-devel] [PATCH cluster 2/3] Cluster: add get_guest_config_properties Dominik Csapak
2022-03-16 10:08 ` Fabian Grünbichler [this message]
2022-03-14 9:03 ` [pve-devel] [PATCH cluster 3/3] Cluster: fix typo Dominik Csapak
2022-03-16 9:52 ` [pve-devel] applied: " Fabian Grünbichler
2022-03-16 9:52 ` [pve-devel] [PATCH cluster 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1647424380.fqb9guy34f.astroid@nora.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox