[pve-devel] applied-series: [PATCH v2 cluster 1/3] pvecm: fix weirdly spaced double-prompt for password on join
applied whole series, thanks!
On November 19, 2019 10:28 am, Stefan Reiter wrote:
> Not only did it display two prompts with identical meaning, the second
> was indented to the end of the first in my terminal for some reason.
>
> Signed-off-by: Stefan Reiter
> ---
>
> Series is already based on Fabian's refactoring (i.e. latest master).
>
> v2:
> * Move variable definitons with assert_joinable 2/3
> * Rename $silent -> $noerr in 3/3
>
> data/PVE/CLI/pvecm.pm | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
> index d3fde3c..c13d0e7 100755
> --- a/data/PVE/CLI/pvecm.pm
> +++ b/data/PVE/CLI/pvecm.pm
> @@ -365,8 +365,7 @@ __PACKAGE__->register_method ({
> my $worker = sub {
>
> if (!$param->{use_ssh}) {
> - print "Please enter superuser (root) password for '$host':\n";
> - my $password = PVE::PTY::read_password("Password for
> root\@$host: ");
> + my $password = PVE::PTY::read_password("Please enter superuser
> (root) password for '$host': ");
>
> delete $param->{use_ssh};
> $param->{password} = $password;
> --
> 2.20.1
>
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] apply pending changes in lxc poststop hook
On 11/21/19 5:48 PM, Oguz Bektas wrote:
> diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
> index 19d0b52..64fe54d 100755
> --- a/src/lxc-pve-poststop-hook
> +++ b/src/lxc-pve-poststop-hook
> @@ -38,6 +38,14 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
> PVE::Network::veth_delete("veth${vmid}i$ind");
> }
>
> +my $config_updated = 0;
> +if ($conf->{pending}) {
> + PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg);
we may want to eval this statement, and just warn the errors it throws,
It may error but that should _not_ abort a reboot, config re-generation
can be done nonetheless, so just above would be enough to wrap:
eval { PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg) };
warn "$@" if $@;
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH container] stop/reboot: handle pending changes errors as non-fatal
Note them in the log, but do not die - the pending changes should be
kept if the did not apply and we do not want to cancel a reboot.
Signed-off-by: Thomas Lamprecht
---
src/lxc-pve-poststop-hook | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index 64fe54d..438836c 100755
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -40,7 +40,8 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
my $config_updated = 0;
if ($conf->{pending}) {
- PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg);
+ eval { PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf,
$storage_cfg) };
+ warn "$@" if $@;
PVE::LXC::update_lxc_config($vmid, $conf);
$config_updated = 1;
}
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH cluster 1/5] corosync: add verify_conf
On November 20, 2019 5:43 pm, Stefan Reiter wrote:
> It does some basic sanity checking, warns the user about encryption
> settings and unresolved hostnames, and finally makes sure that all nodes
> have the same links configured (as well as comparing the configured
> links to specified interfaces, if there are any).
>
> A corosync.conf that has been created and modified strictly through our
> API should *always* be valid.
>
> verify_conf is called in 'addnode', warnings and errors are returned via
> the API to be displayed in the task log of the node asking to join. If a
> verification error occurs, it is handled specially via a "raise" outside
> of any lock code that strips extra information from an Exception
> instance. This ensures that multi-line formatted errors can be returned.
> Warnings are always returned as array, to be printed on the caller.
>
> Includes testing.
>
> Signed-off-by: Stefan Reiter
this would be great to re-use for pve5to6 (and more importantly, a
future pve6to7 ;))
> ---
> data/PVE/API2/ClusterConfig.pm| 27 +++-
> data/PVE/Cluster/Setup.pm | 20 +-
> data/PVE/Corosync.pm | 103 +-
> data/test/corosync_parser_test.pl | 28
> 4 files changed, 175 insertions(+), 3 deletions(-)
>
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index c426a30..3d778b8 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -3,6 +3,7 @@ package PVE::API2::ClusterConfig;
> use strict;
> use warnings;
>
> +use PVE::Exception;
> use PVE::Tools;
> use PVE::SafeSyslog;
> use PVE::RESTHandler;
> @@ -219,7 +220,13 @@ __PACKAGE__->register_method ({
> },
> corosync_conf => {
> type => 'string',
> - }
> + },
> + warnings => {
> + type => 'array',
> + items => {
> + type => 'string',
> + },
> + },
> },
> },
> code => sub {
> @@ -227,11 +234,17 @@ __PACKAGE__->register_method ({
>
> PVE::Cluster::check_cfs_quorum();
>
> + my $vc_errors;
> + my $vc_warnings;
> +
> my $code = sub {
> my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> my $nodelist = PVE::Corosync::nodelist($conf);
> my $totem_cfg = PVE::Corosync::totem_config($conf);
>
> + ($vc_errors, $vc_warnings) = PVE::Corosync::verify_conf($conf);
> + die if scalar(@$vc_errors);
> +
> my $name = $param->{node};
>
> # ensure we do not reuse an address, that can crash the whole
> cluster!
> @@ -317,11 +330,23 @@ __PACKAGE__->register_method ({
> };
>
> $config_change_lock->($code);
> +
> + # If vc_errors is set, we died because of verify_conf.
> + # Raise this error, since it contains more information than just a
> + # single-line string.
> + if (defined($vc_errors) && scalar(@$vc_errors)) {
> + my @combined = ();
> + push @combined, @$vc_errors, @$vc_warnings;
> + my %err_hash = map { $_ => $combined[$_] } 0..$#combined;
> + PVE::Exception::raise("invalid corosync.conf\n", errors =>
> \%err_hash);
it might make sense to somehow differentiate between errors and warnings
here? so that a caller/user knows which parts are actually causing this
to fail..
> + }
> +
> die $@ if $@;
>
> my $res = {
> corosync_authkey => PVE::Tools::file_get_contents($authfile),
> corosync_conf => PVE::Tools::file_get_contents($clusterconf),
> + warnings => $vc_warnings,
> };
>
> return $res;
> diff --git a/data/PVE/Cluster/Setup.pm b/data/PVE/Cluster/Setup.pm
> index 81e3ef8..5569d6c 100644
> --- a/data/PVE/Cluster/Setup.pm
> +++ b/data/PVE/Cluster/Setup.pm
> @@ -667,7 +667,25 @@ sub join {
> $args->{link1} = $param->{link1} if defined($param->{link1});
>
> print "Request addition of this node\n";
> -my $res = $conn->post("/cluster/config/nodes/$nodename", $args);
> +my $res = eval { $conn->post("/cluster/config/nodes/$nodename", $args);
> };
> +if ($@) {
if (my $err = $@) {
the block below is already big, who knows what happens in the future
that might (re)set $@ half-way through ;)
> + if ($@->{msg} && $@->{errors}) {
would it make sense to check whether $@ is a PVE::Exception here?
> + # we received additional info about the error, show the user
> + $@->{msg} =~ s/^(\n|\s)*|(\n|\s)*$//g;
PVE::Tools::trim ? do we actually care about newlines here (or why do we
include one below, just to remove it again here)?
> + warn "An error occured on the cluster node: $@->{msg}\n";
> + foreach my $err (keys %{$@->{errors}}) {
> + warn "* $@->{errors}->{$err}\n"
> + }
> + die "Cluster join aborted!\n";
> + }
> + die $@;
> +}
> +
> +if (defined($res->{warnings})) {
> + for
Re: [pve-devel] [PATCH cluster 2/5] Enable support for up to 8 corosync links
small nit inline
On November 20, 2019 5:43 pm, Stefan Reiter wrote:
> add_corosync_link_properties/extract_corosync_link_args are introduced
> as helpers to avoid hardcoding links in parameters=>properties on
> several occasions, while still providing autocompletion with pvecm by
> being seperate parameters instead of an array.
>
> Maximum number of links is given as constant MAX_LINK_COUNT, should it
> change in the future.
>
> All necessary functions have been updated to
> use the new $links array format instead of seperate $link0/$link1
> parameters, and call sites changed accordingly.
>
> Signed-off-by: Stefan Reiter
> ---
>
> This patch intentionally removes some verification and fallback code to be
> reintroduced in the next one.
>
> data/PVE/API2/ClusterConfig.pm | 51 +++
> data/PVE/CLI/pvecm.pm | 20
> data/PVE/Cluster/Setup.pm | 22 +
> data/PVE/Corosync.pm | 89 +++---
> 4 files changed, 100 insertions(+), 82 deletions(-)
>
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index 3d778b8..241c14e 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -68,10 +68,11 @@ __PACKAGE__->register_method ({
> path => '',
> method => 'POST',
> protected => 1,
> -description => "Generate new cluster configuration.",
> +description => "Generate new cluster configuration. If no links given,"
> + . " default to local IP address as link0.",
> parameters => {
> additionalProperties => 0,
> - properties => {
> + properties => PVE::Corosync::add_corosync_link_properties({
> clustername => {
> description => "The name of the cluster.",
> type => 'string', format => 'pve-node',
> @@ -84,9 +85,7 @@ __PACKAGE__->register_method ({
> minimum => 1,
> optional => 1,
> },
> - link0 => get_standard_option('corosync-link'),
> - link1 => get_standard_option('corosync-link'),
> - },
> + }),
> },
> returns => { type => 'string' },
> code => sub {
> @@ -110,7 +109,7 @@ __PACKAGE__->register_method ({
> my $nodename = PVE::INotify::nodename();
>
> # get the corosync basis config for the new cluster
> - my $config = PVE::Corosync::create_conf($nodename, %$param);
> + my $config = PVE::Corosync::create_conf($nodename, $param);
>
> print "Writing corosync config to /etc/pve/corosync.conf\n";
> PVE::Corosync::atomic_write_conf($config);
> @@ -194,7 +193,7 @@ __PACKAGE__->register_method ({
> description => "Adds a node to the cluster configuration. This call is
> for internal use.",
> parameters => {
> additionalProperties => 0,
> - properties => {
> + properties => PVE::Corosync::add_corosync_link_properties({
> node => get_standard_option('pve-node'),
> nodeid => get_standard_option('corosync-nodeid'),
> votes => {
> @@ -208,9 +207,7 @@ __PACKAGE__->register_method ({
> description => "Do not throw error if node already exists.",
> optional => 1,
> },
> - link0 => get_standard_option('corosync-link'),
> - link1 => get_standard_option('corosync-link'),
> - },
> + }),
> },
> returns => {
> type => "object",
> @@ -256,7 +253,7 @@ __PACKAGE__->register_method ({
> while (my ($k, $v) = each %$nodelist) {
> next if $k eq $name; # allows re-adding a node if force is
> set
>
> - for my $linknumber (0..1) {
> + for my $linknumber (0..$PVE::Corosync::max_link_iter) {
> my $id = "ring${linknumber}_addr";
> next if !defined($v->{$id});
>
> @@ -266,20 +263,10 @@ __PACKAGE__->register_method ({
> }
> };
>
> - my $link0 = PVE::Corosync::parse_corosync_link($param->{link0});
> - my $link1 = PVE::Corosync::parse_corosync_link($param->{link1});
> -
> - $check_duplicate_addr->($link0);
> - $check_duplicate_addr->($link1);
> -
> - # FIXME: handle all links (0-7), they're all independent now
> - $link0->{address} //= $name if exists($totem_cfg->{interface}->{0});
> -
> - die "corosync: using 'link1' parameter needs a interface with
> linknumber '1' configured!\n"
> - if $link1 && !defined($totem_cfg->{interface}->{1});
> -
> - die "corosync: totem interface with linknumber 1 configured but
> 'link1' parameter not defined!\n"
> - if defined($totem_cfg->{interface}->{1}) && !defined($link1);
> + my $links = PVE::Corosync::extract_corosync_link_args($param);
> + foreach my $link (values %$links) {
> + $check_duplicate_addr->($link);
> + }
>
> if (defined(my $res = $node
Re: [pve-devel] RFC V2 for ACME DNS Challenge
On 11/5/19 1:44 PM, Wolfgang Link wrote: > > These patches are just the acme.sh thin wrapper part for the reuse of the > dnsapi. > In this version I use curl instead of wget. > Curl is already part of PMG, and PVE has libcurl packages dependency. > This script has additionl an additional on > - coreutils (an we ignore) > - sed (must be installed) > - curl (just the frontend) OK, if wget cannot be used easily. > - idn (just dns_cyon is using it so that I wouldn't make a dependency for > it. > If a user likes to use cyon, he will get an error to install idn.) installing 'idn' seems to be pretty straightforward, on a normal PVE host I do not get additional packages pulled in, and it's installed size is 314 kB, so fine to add too. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH frr] bump to 7.2
>>applied and pushed the 7.1 tag to the mirror_frr, thanks! Thanks ! >>as Dietmar said, we have a bit limited resources for such a core >>thematic like SDN, which needs a good experience with Networks and >>PVE developing, and those people having that here are currently working >>on other stuff.. yes no problem. I'll send a v2 next week from my last sdn patches (I'm working to finish permissions) I have finished network reload api for all hosts from datacenter level. I have a basic gui working for pve-manager using last apis. - Mail original - De: "Thomas Lamprecht" À: "pve-devel" , "aderumier" Envoyé: Vendredi 15 Novembre 2019 09:01:16 Objet: applied: [pve-devel] [PATCH frr] bump to 7.2 On 11/14/19 11:32 PM, Alexandre Derumier wrote: > This release fix last evpn bugs. > (need to update proxmox frr mirror to frr-7.2 tag) > --- > Makefile | 2 +- > debian/changelog | 6 ++ > debian/control | 18 -- > 3 files changed, 23 insertions(+), 3 deletions(-) > applied and pushed the 7.1 tag to the mirror_frr, thanks! as Dietmar said, we have a bit limited resources for such a core thematic like SDN, which needs a good experience with Networks and PVE developing, and those people having that here are currently working on other stuff.. I would like to see this, as I'd prefer it much more than OVS, but it seems like we currently only can do best-effort on/off review/apply of this SDN stuff, it's not the smallest feature.. just as heads-up :) ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] fix #2462: ACMEAccount: make tos in get_tos optional
On November 13, 2019 10:15 am, Dominik Csapak wrote:
> the code returns undef in case there is no 'tos', and the code
> calling this api call handles a non-existing tos already, but
> fails in that case becasue of the failing return value verification
>
> Signed-off-by: Dominik Csapak
> ---
> PVE/API2/ACMEAccount.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/API2/ACMEAccount.pm b/PVE/API2/ACMEAccount.pm
> index 90620dea..29d5dcf3 100644
> --- a/PVE/API2/ACMEAccount.pm
> +++ b/PVE/API2/ACMEAccount.pm
> @@ -322,6 +322,7 @@ __PACKAGE__->register_method ({
> },
> returns => {
> type => 'string',
> + optional => 1,
> description => 'ACME TermsOfService URL.',
> },
> code => sub {
> --
> 2.20.1
>
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] apply pending changes in lxc poststop hook
hi,
On Fri, Nov 22, 2019 at 09:18:34AM +0100, Thomas Lamprecht wrote:
> On 11/21/19 5:48 PM, Oguz Bektas wrote:
> > diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
> > index 19d0b52..64fe54d 100755
> > --- a/src/lxc-pve-poststop-hook
> > +++ b/src/lxc-pve-poststop-hook
> > @@ -38,6 +38,14 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
> > PVE::Network::veth_delete("veth${vmid}i$ind");
> > }
> >
> > +my $config_updated = 0;
> > +if ($conf->{pending}) {
> > + PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg);
>
> we may want to eval this statement, and just warn the errors it throws,
> It may error but that should _not_ abort a reboot, config re-generation
> can be done nonetheless, so just above would be enough to wrap:
>
> eval { PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg) };
> warn "$@" if $@;
that shouldn't happen i think.
vmconfig_apply_pending's default behaviour is not to die and just log the
errors. so with an un-appliable pending change the task isn't aborted.
is there a case where it actually aborts the reboot?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server 2/2] fix #2367: do not allow snapshot with name PENDING
hi,
will this get applied?
On Thu, Oct 24, 2019 at 01:53:09PM +0200, Oguz Bektas wrote:
> or any other variant of the word 'pending'.
>
> note that we can actually allow this snapshot after PVE 7.0, since
> pending section and snapshots will be properly namespaced.
> ([pve:pending] and [snap:$snapname] or similar).
>
> Signed-off-by: Oguz Bektas
> ---
> PVE/QemuServer.pm | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 8d7994e..3669643 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2773,7 +2773,10 @@ sub write_vm_config {
> &$cleanup_config($conf->{pending}, 1);
>
> foreach my $snapname (keys %{$conf->{snapshots}}) {
> - die "internal error" if $snapname eq 'pending';
> + # TODO: we can allow snapshots with name 'pending' after PVE 7.0
> + # since pending section is namespaced with 'pve:'
> + # but for now, we should forbid it to avoid confusion in parser
> + die "internal error: snapshot name '$snapname' is forbidden" if
> lc($snapname) eq 'pending';
> &$cleanup_config($conf->{snapshots}->{$snapname}, undef, $snapname);
> }
>
> --
> 2.20.1
>
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] apply pending changes in lxc poststop hook
On 11/22/19 12:32 PM, Oguz Bektas wrote:
> hi,
>
> On Fri, Nov 22, 2019 at 09:18:34AM +0100, Thomas Lamprecht wrote:
>> On 11/21/19 5:48 PM, Oguz Bektas wrote:
>>> diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
>>> index 19d0b52..64fe54d 100755
>>> --- a/src/lxc-pve-poststop-hook
>>> +++ b/src/lxc-pve-poststop-hook
>>> @@ -38,6 +38,14 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
>>> PVE::Network::veth_delete("veth${vmid}i$ind");
>>> }
>>>
>>> +my $config_updated = 0;
>>> +if ($conf->{pending}) {
>>> + PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg);
>>
>> we may want to eval this statement, and just warn the errors it throws,
>> It may error but that should _not_ abort a reboot, config re-generation
>> can be done nonetheless, so just above would be enough to wrap:
>>
>> eval { PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg)
>> };
>> warn "$@" if $@;
>
> that shouldn't happen i think.
>
> vmconfig_apply_pending's default behaviour is not to die and just log the
> errors. so with an un-appliable pending change the task isn't aborted.
>
> is there a case where it actually aborts the reboot?
>
no idea, but better safe than sorry.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v4 qemu-server 5/8] refactor: extract QEMU machine related helpers to package
On 11/19/19 12:23 PM, Stefan Reiter wrote:
> ...PVE::QemuServer::Machine.
>
> qemu_machine_feature_enabled is exported since it has a *lot* of users
> in PVE::QemuServer and a long enough name as it is.
>
> Signed-off-by: Stefan Reiter
> ---
>
> I left the code exporting qemu_machine_feature_enabled from v2 and only remove
> it in the next patch, just to make sure this patch works standalone as well.
>
> PVE/QemuConfig.pm | 3 +-
> PVE/QemuMigrate.pm| 3 +-
> PVE/QemuServer.pm | 103 +++---
> PVE/QemuServer/Machine.pm | 101 +
> PVE/QemuServer/Makefile | 1 +
> PVE/VZDump/QemuServer.pm | 3 +-
> 6 files changed, 116 insertions(+), 98 deletions(-)
> create mode 100644 PVE/QemuServer/Machine.pm
>
> 8< SNIP 8<
>
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> new file mode 100644
> index 000..c3e0004
> --- /dev/null
> +++ b/PVE/QemuServer/Machine.pm
> @@ -0,0 +1,101 @@
> +package PVE::QemuServer::Machine;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::QemuServer::Monitor;
> +
> +use base 'Exporter';
> +our @EXPORT_OK = qw(
> +qemu_machine_feature_enabled
> +);
> +
> +sub machine_type_is_q35 {
> +my ($conf) = @_;
> +
> +return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
> +}
> +
> +# this only works if VM is running
> +sub get_current_qemu_machine {
> +my ($vmid) = @_;
> +
> +my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-machines');
> +
> +my ($current, $default);
> +foreach my $e (@$res) {
> + $default = $e->{name} if $e->{'is-default'};
> + $current = $e->{name} if $e->{'is-current'};
> +}
> +
> +# fallback to the default machine if current is not supported by qemu
> +return $current || $default || 'pc';
> +}
> +
> +sub qemu_machine_feature_enabled {
> +my ($machine, $kvmver, $version_major, $version_minor) = @_;
> +
> +my $current_major;
> +my $current_minor;
> +
> +if ($machine && $machine =~
> m/^((?:pc(-i440fx|-q35)?|virt)-(\d+)\.(\d+))/) {
> +
> + $current_major = $3;
> + $current_minor = $4;
> +
> +} elsif ($kvmver =~ m/^(\d+)\.(\d+)/) {
> +
> + $current_major = $1;
> + $current_minor = $2;
> +}
> +
> +return 1 if version_cmp($current_major, $version_major, $current_minor,
> $version_minor) >= 0;
> +}
> +
> +# gets in pairs the versions you want to compares, i.e.:
> +# ($a-major, $b-major, $a-minor, $b-minor, $a-extra, $b-extra, ...)
> +# returns 0 if same, -1 if $a is older than $b, +1 if $a is newer than $b
> +sub version_cmp {
> +my @versions = @_;
> +
> +my $size = scalar(@versions);
> +
> +return 0 if $size == 0;
> +die "cannot compare odd count of versions" if $size & 1;
> +
> +for (my $i = 0; $i < $size; $i += 2) {
> + my ($a, $b) = splice(@versions, 0, 2);
> + $a //= 0;
> + $b //= 0;
> +
> + return 1 if $a > $b;
> + return -1 if $a < $b;
> +}
> +return 0;
> +}
> +
> +# dies if a) VM not running or not exisiting b) Version query failed
> +# So, any defined return value is valid, any invalid state can be caught by
> eval
> +sub runs_at_least_qemu_version {
> +my ($vmid, $major, $minor, $extra) = @_;
> +
> +my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
> +die "could not query currently running version for VM $vmid\n" if
> !defined($v);
> +$v = $v->{qemu};
> +
> +return version_cmp($v->{major}, $major, $v->{minor}, $minor,
> $v->{micro}, $extra) >= 0;
this broke a few things like backup.. version_vmp is a (non-exported)
method of the PVE::QemuServer::Helpers module, so this cannot work.
Can you see if you add some tests for this, either standalone or integrated
into the cfg2cmd tests?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Revision of the pvesr documentation
Some hints from my side inline. Rephrasing some passages trying to make it easier to read and understand. Did some sentence splitting an smaller corrections as well. Please check if the rephrased sections are still correct from a technical POV. On 11/15/19 9:51 AM, Wolfgang Link wrote: Improvement of grammar and punctuation. Clarify the HA limitations. Remove future tense in some sentences. It is not good to use it in technical/scientific papers. Rewrite some sentences to improve understanding. --- pvesr.adoc | 108 ++--- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/pvesr.adoc b/pvesr.adoc index 83ab268..7934a84 100644 --- a/pvesr.adoc +++ b/pvesr.adoc @@ -31,34 +31,34 @@ local storage and reduces migration time. It replicates guest volumes to another node so that all data is available without using shared storage. Replication uses snapshots to minimize traffic sent over the network. Therefore, new data is sent only incrementally after -an initial full sync. In the case of a node failure, your guest data is +the initial full sync. In the case of a node failure, your guest data is still available on the replicated node. -The replication will be done automatically in configurable intervals. -The minimum replication interval is one minute and the maximal interval is +The replication is done automatically in configurable intervals. +The minimum replication interval is one minute, and the maximal interval is once a week. The format used to specify those intervals is a subset of `systemd` calendar events, see xref:pvesr_schedule_time_format[Schedule Format] section: -Every guest can be replicated to multiple target nodes, but a guest cannot -get replicated twice to the same target node. +The storage replication can replicate a guest to multiple target nodes, +but a guest cannot get replicated twice to the same target node. It is possible to replicate a guest to multiple target nodes, but not twice to the same target node. Each replications bandwidth can be limited, to avoid overloading a storage or server. -Virtual guest with active replication cannot currently use online migration. -Offline migration is supported in general. If you migrate to a node where -the guests data is already replicated only the changes since the last -synchronisation (so called `delta`) must be sent, this reduces the required -time significantly. In this case the replication direction will also switch -nodes automatically after the migration finished. +Virtual guests with active replication cannot currently use online migration. +The migration offline is supported in general. If you migrate to a node where +the guest's data is already replicated only the changes since the last +synchronization (so called `delta`) must be sent, this reduces the required +time significantly. In this case, the replication direction has switched +the nodes automatically after the migration finished. Guests with replication enabled can currently only be migrated offline. Only changes since the last replication (so called `deltas`) need to be transferred if the guest is migrated to a node to which it already is replicated. This reduces the time needed significantly. The replication direction is switched automatically if you migrate a guest to the replication target node. For example: VM100 is currently on `nodeA` and gets replicated to `nodeB`. You migrate it to `nodeB`, so now it gets automatically replicated back from `nodeB` to `nodeA`. If you migrate to a node where the guest is not replicated, the whole disk -data must send over. After the migration the replication job continues to +data must send over. After the migration, the replication job continues to replicate this guest to the configured nodes. [IMPORTANT] @@ -66,8 +66,8 @@ replicate this guest to the configured nodes. High-Availability is allowed in combination with storage replication, but it has the following implications: -* redistributing services after a more preferred node comes online will lead - to errors. +* consider the live migration is currently not yet supported, + a migration error occurs when a more preferred node goes online * live migration of replicated guests if not yet supported - If a preferred node is configured the try to live migrate the guest to it, after it is back online, will fail. * recovery works, but there may be some data loss between the last synced time and the time a node failed. @@ -98,24 +98,25 @@ Such a calendar event uses the following format: [day(s)] [[start-time(s)][/repetition-time(s)]] -This allows you to configure a set of days on which the job should run. -You can also set one or more start times, it tells the replication scheduler +This format allows you to configure a set of days on which the job should run. +You can also set one or more start times. It tells th
[pve-devel] applied: [PATCH storage] RBD: fix ceph version detection
Signed-off-by: Thomas Lamprecht
---
PVE/Storage/RBDPlugin.pm | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 214b732..d78a785 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -74,8 +74,8 @@ my $librados_connect = sub {
my $krbd_feature_disable = sub {
my ($scfg, $storeid, $name) = @_;
-my ($major, undef, undef, undef) = ceph_version();
-return 1 if $major < 10;
+my ($versionparts) = ceph_version();
+return 1 if $versionparts->[0] < 10;
my $krbd_feature_blacklist = ['deep-flatten', 'fast-diff', 'object-map',
'exclusive-lock'];
my (undef, undef, undef, undef, $features) = rbd_volume_info($scfg,
$storeid, $name);
@@ -90,33 +90,32 @@ my $krbd_feature_disable = sub {
};
my $ceph_version_parser = sub {
-my $line = shift;
-if ($line =~ m/^ceph version ((\d+)\.(\d+)\.(\d+))(?: \([a-fA-F0-9]+\))/) {
- return ($2, $3, $4, $1);
-} else {
- warn "Could not parse Ceph version: '$line'\n";
+my $ceph_version = shift;
+# FIXME this is the same as pve-manager PVE::Ceph::Tools get_local_version
+if ($ceph_version =~
/^ceph.*\s(\d+(?:\.\d+)+(?:-pve\d+)?)\s+(?:\(([a-zA-Z0-9]+)\))?/) {
+ my ($version, $buildcommit) = ($1, $2);
+ my $subversions = [ split(/\.|-/, $version) ];
+
+ return ($subversions, $version, $buildcommit);
}
+warn "Could not parse Ceph version: '$ceph_version'\n";
};
sub ceph_version {
my ($cache) = @_;
my $version_string = $cache;
-
-my $major;
-my $minor;
-my $bugfix;
-
-if (defined($version_string)) {
- ($major, $minor, $bugfix, $version_string) =
&$ceph_version_parser($version_string);
-} else {
+if (!defined($version_string)) {
run_command('ceph --version', outfunc => sub {
- my $line = shift;
- ($major, $minor, $bugfix, $version_string) =
&$ceph_version_parser($line);
+ $version_string = shift;
});
}
return undef if !defined($version_string);
-return wantarray ? ($major, $minor, $bugfix, $version_string) :
$version_string;
+# subversion is an array ref. with the version parts from major to minor
+# version is the filtered version string
+my ($subversions, $version) = $ceph_version_parser->($version_string);
+
+return wantarray ? ($subversions, $version) : $version;
}
sub run_rbd_command {
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH cluster] fix #2479: use correct sub in create_conf
'pvecm create' fails since the subroutine doesn't exist in PVE::Cluster
but in PVE::Corosync
Signed-off-by: Oguz Bektas
---
data/PVE/Corosync.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
index 1b92ea3..7fb52e6 100644
--- a/data/PVE/Corosync.pm
+++ b/data/PVE/Corosync.pm
@@ -239,7 +239,7 @@ sub create_conf {
my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
-my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
+my $link0 = PVE::Corosync::parse_corosync_link($param{link0});
$link0->{address} //= $local_ip_address;
my $conf = {
@@ -278,7 +278,7 @@ sub create_conf {
$totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
if defined($link0->{priority});
-my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
+my $link1 = PVE::Corosync::parse_corosync_link($param{link1});
if ($link1->{address}) {
$conf->{totem}->{interface}->{1} = {
linknumber => 1,
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH cluster] fix #2479: use correct sub in create_conf
On 11/22/19 4:13 PM, Oguz Bektas wrote:
> 'pvecm create' fails since the subroutine doesn't exist in PVE::Cluster
> but in PVE::Corosync
>
first, great catch! But...
> Signed-off-by: Oguz Bektas
> ---
> data/PVE/Corosync.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> index 1b92ea3..7fb52e6 100644
> --- a/data/PVE/Corosync.pm
> +++ b/data/PVE/Corosync.pm
> @@ -239,7 +239,7 @@ sub create_conf {
>
> my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
>
> -my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
> +my $link0 = PVE::Corosync::parse_corosync_link($param{link0});
we're in the corosync module, so just use
parse_corosync_link($param{link0});
directly? :)
> $link0->{address} //= $local_ip_address;
>
> my $conf = {
> @@ -278,7 +278,7 @@ sub create_conf {
> $totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
> if defined($link0->{priority});
>
> -my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
> +my $link1 = PVE::Corosync::parse_corosync_link($param{link1});
> if ($link1->{address}) {
> $conf->{totem}->{interface}->{1} = {
> linknumber => 1,
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH cluster] fix #2479: use correct sub in create_conf
On Fri, Nov 22, 2019 at 04:15:30PM +0100, Thomas Lamprecht wrote:
> On 11/22/19 4:13 PM, Oguz Bektas wrote:
> > 'pvecm create' fails since the subroutine doesn't exist in PVE::Cluster
> > but in PVE::Corosync
> >
>
> first, great catch! But...
>
> > Signed-off-by: Oguz Bektas
> > ---
> > data/PVE/Corosync.pm | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> > index 1b92ea3..7fb52e6 100644
> > --- a/data/PVE/Corosync.pm
> > +++ b/data/PVE/Corosync.pm
> > @@ -239,7 +239,7 @@ sub create_conf {
> >
> > my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
> >
> > -my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
> > +my $link0 = PVE::Corosync::parse_corosync_link($param{link0});
>
> we're in the corosync module, so just use
> parse_corosync_link($param{link0});
> directly? :)
hahahah, yeah. i'll send v2 :)
>
> > $link0->{address} //= $local_ip_address;
> >
> > my $conf = {
> > @@ -278,7 +278,7 @@ sub create_conf {
> > $totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
> > if defined($link0->{priority});
> >
> > -my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
> > +my $link1 = PVE::Corosync::parse_corosync_link($param{link1});
> > if ($link1->{address}) {
> > $conf->{totem}->{interface}->{1} = {
> > linknumber => 1,
> >
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 cluster] fix #2479: use correct sub in create_conf
'pvecm create' fails since the subroutine doesn't exist in PVE::Cluster
but in PVE::Corosync
Signed-off-by: Oguz Bektas
---
v1->v2:
* PVE::Corosync is redundant since we're already in that module
data/PVE/Corosync.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
index 1b92ea3..3208a6b 100644
--- a/data/PVE/Corosync.pm
+++ b/data/PVE/Corosync.pm
@@ -239,7 +239,7 @@ sub create_conf {
my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
-my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
+my $link0 = parse_corosync_link($param{link0});
$link0->{address} //= $local_ip_address;
my $conf = {
@@ -278,7 +278,7 @@ sub create_conf {
$totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
if defined($link0->{priority});
-my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
+my $link1 = parse_corosync_link($param{link1});
if ($link1->{address}) {
$conf->{totem}->{interface}->{1} = {
linknumber => 1,
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 cluster] fix #2479: use correct sub in create_conf
On 11/22/19 4:21 PM, Oguz Bektas wrote:
> 'pvecm create' fails since the subroutine doesn't exist in PVE::Cluster
> but in PVE::Corosync
>
> Signed-off-by: Oguz Bektas
> ---
>
> v1->v2:
> * PVE::Corosync is redundant since we're already in that module
>
> data/PVE/Corosync.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> index 1b92ea3..3208a6b 100644
> --- a/data/PVE/Corosync.pm
> +++ b/data/PVE/Corosync.pm
> @@ -239,7 +239,7 @@ sub create_conf {
>
> my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
>
> -my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
> +my $link0 = parse_corosync_link($param{link0});
> $link0->{address} //= $local_ip_address;
>
> my $conf = {
> @@ -278,7 +278,7 @@ sub create_conf {
> $totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
> if defined($link0->{priority});
>
> -my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
> +my $link1 = parse_corosync_link($param{link1});
> if ($link1->{address}) {
> $conf->{totem}->{interface}->{1} = {
> linknumber => 1,
>
applied, thanks!
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH cluster] fix buildsys for new library packages
This was forgotten to add/update when the package was refactored ...
Signed-off-by: Thomas Lamprecht
---
Makefile | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 6adbdb4..527edb5 100644
--- a/Makefile
+++ b/Makefile
@@ -6,6 +6,8 @@ PACKAGE=pve-cluster
GITVERSION:=$(shell git rev-parse HEAD)
DEB=${PACKAGE}_${DEB_VERSION_UPSTREAM_REVISION}_${DEB_BUILD_ARCH}.deb
+LIBDEB = libpve-cluster-perl_${DEB_VERSION_UPSTREAM_REVISION}_all.deb
+LIBDEB += libpve-cluster-api-perl_${DEB_VERSION_UPSTREAM_REVISION}_all.deb
DBGDEB=${PACKAGE}-dbgsym_${DEB_VERSION_UPSTREAM_REVISION}_${DEB_BUILD_ARCH}.deb
PERL_APIVER := `perl -MConfig -e 'print
$$Config{debian_abi}//$$Config{version};'`
@@ -16,11 +18,11 @@ cpgtest: cpgtest.c
gcc -Wall cpgtest.c $(shell pkg-config --cflags --libs libcpg libqb) -o
cpgtest
.PHONY: dinstall
-dinstall: ${DEB}
- dpkg -i ${DEB}
+dinstall: ${DEB} ${LIBDEB}
+ dpkg -i $^
.PHONY: deb
-deb ${DBG_DEB}: ${DEB}
+deb ${DBG_DEB} ${LIBDEB}: ${DEB}
${DEB}:
rm -f *.deb
rm -rf build
@@ -32,8 +34,8 @@ ${DEB}:
.PHONY: upload
-upload: ${DEB} ${DBG_DEB}
- tar cf - ${DEB} ${DBGDEB}| ssh -X repo...@repo.proxmox.com -- upload
--product pve --dist buster --arch ${DEB_BUILD_ARCH}
+upload: ${DEB} ${DBG_DEB} ${LIBDEB}
+ tar cf - ${DEB} ${DBGDEB} ${LIBDEB}| ssh -X repo...@repo.proxmox.com --
upload --product pve --dist buster --arch ${DEB_BUILD_ARCH}
.PHONY: clean
clean:
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH storage] RBD: fix ceph version detection
Hi,
>> my $krbd_feature_blacklist = ['deep-flatten', 'fast-diff', 'object-map',
>> 'exclusive-lock'];
since kernel 5.3, all theses features are supported now :) (it was missing
fast-diff && object-map before).
- Mail original -
De: "Thomas Lamprecht"
À: "pve-devel"
Envoyé: Vendredi 22 Novembre 2019 16:09:23
Objet: [pve-devel] applied: [PATCH storage] RBD: fix ceph version detection
Signed-off-by: Thomas Lamprecht
---
PVE/Storage/RBDPlugin.pm | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 214b732..d78a785 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -74,8 +74,8 @@ my $librados_connect = sub {
my $krbd_feature_disable = sub {
my ($scfg, $storeid, $name) = @_;
- my ($major, undef, undef, undef) = ceph_version();
- return 1 if $major < 10;
+ my ($versionparts) = ceph_version();
+ return 1 if $versionparts->[0] < 10;
my $krbd_feature_blacklist = ['deep-flatten', 'fast-diff', 'object-map',
'exclusive-lock'];
my (undef, undef, undef, undef, $features) = rbd_volume_info($scfg, $storeid,
$name);
@@ -90,33 +90,32 @@ my $krbd_feature_disable = sub {
};
my $ceph_version_parser = sub {
- my $line = shift;
- if ($line =~ m/^ceph version ((\d+)\.(\d+)\.(\d+))(?: \([a-fA-F0-9]+\))/) {
- return ($2, $3, $4, $1);
- } else {
- warn "Could not parse Ceph version: '$line'\n";
+ my $ceph_version = shift;
+ # FIXME this is the same as pve-manager PVE::Ceph::Tools get_local_version
+ if ($ceph_version =~
/^ceph.*\s(\d+(?:\.\d+)+(?:-pve\d+)?)\s+(?:\(([a-zA-Z0-9]+)\))?/) {
+ my ($version, $buildcommit) = ($1, $2);
+ my $subversions = [ split(/\.|-/, $version) ];
+
+ return ($subversions, $version, $buildcommit);
}
+ warn "Could not parse Ceph version: '$ceph_version'\n";
};
sub ceph_version {
my ($cache) = @_;
my $version_string = $cache;
-
- my $major;
- my $minor;
- my $bugfix;
-
- if (defined($version_string)) {
- ($major, $minor, $bugfix, $version_string) =
&$ceph_version_parser($version_string);
- } else {
+ if (!defined($version_string)) {
run_command('ceph --version', outfunc => sub {
- my $line = shift;
- ($major, $minor, $bugfix, $version_string) = &$ceph_version_parser($line);
+ $version_string = shift;
});
}
return undef if !defined($version_string);
- return wantarray ? ($major, $minor, $bugfix, $version_string) :
$version_string;
+ # subversion is an array ref. with the version parts from major to minor
+ # version is the filtered version string
+ my ($subversions, $version) = $ceph_version_parser->($version_string);
+
+ return wantarray ? ($subversions, $version) : $version;
}
sub run_rbd_command {
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH storage] RBD: fix ceph version detection
On 11/22/19 5:12 PM, Alexandre DERUMIER wrote: > Hi, > >>> my $krbd_feature_blacklist = ['deep-flatten', 'fast-diff', >>> 'object-map', 'exclusive-lock']; > > > since kernel 5.3, all theses features are supported now :) (it was missing > fast-diff && object-map before). Yeah, that's why I did not run into this issue on my test systems and never saw that this parser was a bit limited.. I'd say that we add the check if the running kernel is 5.3 and skip the disabling then also. Just need to confirm that we do not get live-migration issues for VMs with krbd on.. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC common] fix misleading warning caused by Devel::Cycle
When validating the parameters (or return values) of an API call against the
schema we run the parameter hash through Devel::Cycle::find_cycle.
find_cycle has no handler for GLOB types (e.g. filehandles).
Recently we introduced passing a filehandle as return attribute in
PMG::API2::Quarantine::download
When find_cycle encounters such an object it emits a warning:
`Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
which ends up in the journal of pmgproxy (in that example).
Since the line might mislead someone to think that this is a more serious bug
we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for the
time when find_cycle runs, to ignore messages starting with
'Unhandled type: GLOB at'. The particular location of and in in Devel::Cycle
might change and is left out of the match.
[0] man perlvar
[1] perldoc -f warn
Signed-off-by: Stoiko Ivanov
---
sending as RFC, since the comment above the patch already indicate that we
could do away with the cycle-search on every call
The choice of saving the old handler and restoring it instead of using
local $SIG{'__WARN__'} was done to minimize the effect - although local'izing
is a bit more idiomatic it seems.
Caught this in my PMG's journal - traced it down, by editing
/usr/share/perl5/Devel/Cycle and adding a Carp::cluck at the location the
warning came from)
at /usr/share/perl5/Devel/Cycle.pm line 108.
Devel::Cycle::_find_cycle_dispatch(IO::File=GLOB(0x559b8b002420),
HASH(0x559b8ad928b8), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 97
Devel::Cycle::_find_cycle(IO::File=GLOB(0x559b8b002420),
HASH(0x559b8b0b6c80), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 156
Devel::Cycle::_find_cycle_HASH(HASH(0x559b8b002450),
HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at
/usr/share/perl5/Devel/Cycle.pm line 114
Devel::Cycle::_find_cycle_dispatch(HASH(0x559b8b002450),
HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at
/usr/share/perl5/Devel/Cycle.pm line 97
Devel::Cycle::_find_cycle(HASH(0x559b8b002450), HASH(0x559b87def760),
CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at
/usr/share/perl5/Devel/Cycle.pm line 70
Devel::Cycle::find_cycle(HASH(0x559b8b002450), CODE(0x559b8acfe4f0))
called at /usr/share/perl5/PVE/JSONSchema.pm line 1053
PVE::JSONSchema::validate(HASH(0x559b8b002450), HASH(0x559b8ab6d330),
"Result verification failed\x{a}") called at
/usr/share/perl5/PVE/RESTHandler.pm line 449
PVE::RESTHandler::handle("PMG::API2::Quarantine", HASH(0x559b8ab6d738),
HASH(0x559b8ad23c10)) called at /usr/share/perl5/PMG/HTTPServer.pm line 157
eval {...} called at /usr/share/perl5/PMG/HTTPServer.pm line 111
PMG::HTTPServer::rest_handler(PMG::HTTPServer=HASH(0x559b8ac60ad8),
"192.168.16.68", "GET", "/quarantine/download", HASH(0x559b8acc0300),
HASH(0x559b8706b9c8), "json") called at
/usr/share/perl5/PVE/APIServer/AnyEvent.pm line 725
eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 699
PVE::APIServer::AnyEvent::handle_api2_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
"/api2/json/quarantine/download") called at
/usr/share/perl5/PVE/APIServer/AnyEvent.pm line 950
eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 942
PVE::APIServer::AnyEvent::handle_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
"/api2/json/quarantine/download") called at
/usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1342
eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
1160
PVE::APIServer::AnyEvent::__ANON__(AnyEvent::Handle=HASH(0x559b87057838), "",
"\x{d}\x{a}") called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm
line 1541
AnyEvent::Handle::__ANON__(AnyEvent::Handle=HASH(0x559b87057838))
called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1315
AnyEvent::Handle::_drain_rbuf(AnyEvent::Handle=HASH(0x559b87057838))
called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2100
AnyEvent::Handle::_dotls(AnyEvent::Handle=HASH(0x559b87057838)) called
at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2013
AnyEvent::Handle::__ANON__(EV::IO=SCALAR(0x559b8acbfcd0), 1) called at
/usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
eval {...} called at
/usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
AnyEvent::CondVar::Base::_wait(AnyEvent::CondVar=HASH(0x559b8708f8b0))
called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent.pm line 2026
AnyEvent::CondVar::Base::recv(AnyEvent::CondVar=HASH(0x559b8708f8b0))
called
Re: [pve-devel] [RFC common] fix misleading warning caused by Devel::Cycle
On 11/22/19 6:21 PM, Stoiko Ivanov wrote:
> When validating the parameters (or return values) of an API call against the
> schema we run the parameter hash through Devel::Cycle::find_cycle.
>
> find_cycle has no handler for GLOB types (e.g. filehandles).
> Recently we introduced passing a filehandle as return attribute in
> PMG::API2::Quarantine::download
>
> When find_cycle encounters such an object it emits a warning:
> `Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
> which ends up in the journal of pmgproxy (in that example).
>
> Since the line might mislead someone to think that this is a more serious bug
> we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for the
> time when find_cycle runs, to ignore messages starting with
> 'Unhandled type: GLOB at'. The particular location of and in in Devel::Cycle
> might change and is left out of the match.
>
> [0] man perlvar
> [1] perldoc -f warn
>
> Signed-off-by: Stoiko Ivanov
> ---
>
> sending as RFC, since the comment above the patch already indicate that we
> could do away with the cycle-search on every call
I'd really rather to that, this is a build check, and thus we
do not want to do it on every validation - besides those errors
you ran into, it's also a bit costly..
I'd check for an environment variable, e.g. for "DEB_BUILD_ARCH",
which we always have set when building. As some may want to have
validation also in other cases we could also check a opt-in one,
for example:
return if $ENV{DEB_BUILD_ARCH} && !$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
>
> The choice of saving the old handler and restoring it instead of using
> local $SIG{'__WARN__'} was done to minimize the effect - although local'izing
> is a bit more idiomatic it seems.
>
> Caught this in my PMG's journal - traced it down, by editing
> /usr/share/perl5/Devel/Cycle and adding a Carp::cluck at the location the
> warning came from)
>
> at /usr/share/perl5/Devel/Cycle.pm line 108.
> Devel::Cycle::_find_cycle_dispatch(IO::File=GLOB(0x559b8b002420),
> HASH(0x559b8ad928b8), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
> ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(IO::File=GLOB(0x559b8b002420),
> HASH(0x559b8b0b6c80), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
> ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 156
> Devel::Cycle::_find_cycle_HASH(HASH(0x559b8b002450),
> HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called
> at /usr/share/perl5/Devel/Cycle.pm line 114
> Devel::Cycle::_find_cycle_dispatch(HASH(0x559b8b002450),
> HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called
> at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(HASH(0x559b8b002450), HASH(0x559b87def760),
> CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at
> /usr/share/perl5/Devel/Cycle.pm line 70
> Devel::Cycle::find_cycle(HASH(0x559b8b002450), CODE(0x559b8acfe4f0))
> called at /usr/share/perl5/PVE/JSONSchema.pm line 1053
> PVE::JSONSchema::validate(HASH(0x559b8b002450), HASH(0x559b8ab6d330),
> "Result verification failed\x{a}") called at
> /usr/share/perl5/PVE/RESTHandler.pm line 449
> PVE::RESTHandler::handle("PMG::API2::Quarantine",
> HASH(0x559b8ab6d738), HASH(0x559b8ad23c10)) called at
> /usr/share/perl5/PMG/HTTPServer.pm line 157
> eval {...} called at /usr/share/perl5/PMG/HTTPServer.pm line 111
> PMG::HTTPServer::rest_handler(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> "192.168.16.68", "GET", "/quarantine/download", HASH(0x559b8acc0300),
> HASH(0x559b8706b9c8), "json") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 725
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 699
>
> PVE::APIServer::AnyEvent::handle_api2_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
> "/api2/json/quarantine/download") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 950
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 942
>
> PVE::APIServer::AnyEvent::handle_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
> "/api2/json/quarantine/download") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1342
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 1160
>
> PVE::APIServer::AnyEvent::__ANON__(AnyEvent::Handle=HASH(0x559b87057838), "",
> "\x{d}\x{a}") called at
> /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1541
> AnyEvent::Handle::__ANON__(AnyEvent::Handle=HASH(0x559b87057838))
> called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1315
> AnyEvent::Handle::_drain_rbuf(AnyEvent::Handle=HASH(0x559b87057838))
> called at /usr/lib/x86_64-li
Re: [pve-devel] [RFC common] fix misleading warning caused by Devel::Cycle
On 11/22/19 7:03 PM, Thomas Lamprecht wrote:
> On 11/22/19 6:21 PM, Stoiko Ivanov wrote:
>> When validating the parameters (or return values) of an API call against the
>> schema we run the parameter hash through Devel::Cycle::find_cycle.
>>
>> find_cycle has no handler for GLOB types (e.g. filehandles).
>> Recently we introduced passing a filehandle as return attribute in
>> PMG::API2::Quarantine::download
>>
>> When find_cycle encounters such an object it emits a warning:
>> `Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
>> which ends up in the journal of pmgproxy (in that example).
>>
>> Since the line might mislead someone to think that this is a more serious bug
>> we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for
>> the
>> time when find_cycle runs, to ignore messages starting with
>> 'Unhandled type: GLOB at'. The particular location of and in in Devel::Cycle
>> might change and is left out of the match.
>>
>> [0] man perlvar
>> [1] perldoc -f warn
>>
>> Signed-off-by: Stoiko Ivanov
>> ---
>>
>> sending as RFC, since the comment above the patch already indicate that we
>> could do away with the cycle-search on every call
>
> I'd really rather to that, this is a build check, and thus we
> do not want to do it on every validation - besides those errors
> you ran into, it's also a bit costly..
>
> I'd check for an environment variable, e.g. for "DEB_BUILD_ARCH",
> which we always have set when building. As some may want to have
> validation also in other cases we could also check a opt-in one,
> for example:
>
> return if $ENV{DEB_BUILD_ARCH} && !$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
argh, missed the ! from the $ENV{DEB_BUILD_ARCH}, could be written as
# only check when building a package or told to do so
return if !($ENV{DEB_BUILD_ARCH} || $ENV{PROXMOX_FORCE_SCHEMA_VALIDATION});
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC common] fix misleading warning caused by Devel::Cycle
On Fri, 22 Nov 2019 19:05:44 +0100
Thomas Lamprecht wrote:
> On 11/22/19 7:03 PM, Thomas Lamprecht wrote:
> > On 11/22/19 6:21 PM, Stoiko Ivanov wrote:
> >> When validating the parameters (or return values) of an API call against
> >> the
> >> schema we run the parameter hash through Devel::Cycle::find_cycle.
> >>
> >> find_cycle has no handler for GLOB types (e.g. filehandles).
> >> Recently we introduced passing a filehandle as return attribute in
> >> PMG::API2::Quarantine::download
> >>
> >> When find_cycle encounters such an object it emits a warning:
> >> `Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
> >> which ends up in the journal of pmgproxy (in that example).
> >>
> >> Since the line might mislead someone to think that this is a more serious
> >> bug
> >> we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for
> >> the
> >> time when find_cycle runs, to ignore messages starting with
> >> 'Unhandled type: GLOB at'. The particular location of and in in
> >> Devel::Cycle
> >> might change and is left out of the match.
> >>
> >> [0] man perlvar
> >> [1] perldoc -f warn
> >>
> >> Signed-off-by: Stoiko Ivanov
> >> ---
> >>
> >> sending as RFC, since the comment above the patch already indicate that we
> >> could do away with the cycle-search on every call
> >
> > I'd really rather to that, this is a build check, and thus we
> > do not want to do it on every validation - besides those errors
> > you ran into, it's also a bit costly..
> >
> > I'd check for an environment variable, e.g. for "DEB_BUILD_ARCH",
> > which we always have set when building. As some may want to have
> > validation also in other cases we could also check a opt-in one,
> > for example:
> >
> > return if $ENV{DEB_BUILD_ARCH} && !$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
>
> argh, missed the ! from the $ENV{DEB_BUILD_ARCH}, could be written as
>
> # only check when building a package or told to do so
> return if !($ENV{DEB_BUILD_ARCH} || $ENV{PROXMOX_FORCE_SCHEMA_VALIDATION});
Sounds sensible to me - will send a v2 with your proposal - Thanks!
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH common] schema: only check for cycles during build or if forced
This is costly and only really required during package build.
Signed-off-by: Thomas Lamprecht
---
whipped up a patch, works fine here, did a build and manual force check by
adding big fat warn "..." below the early-return.
src/PVE/JSONSchema.pm | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 51c3881..591fb62 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -6,13 +6,19 @@ use Storable; # for dclone
use Getopt::Long;
use Encode::Locale;
use Encode;
-use Devel::Cycle -quiet; # todo: remove?
use PVE::Tools qw(split_list $IPV6RE $IPV4RE);
use PVE::Exception qw(raise);
use HTTP::Status qw(:constants);
use Net::IP qw(:PROC);
use Data::Dumper;
+BEGIN {
+if ($ENV{DEB_BUILD_ARCH} || $ENV{PROXMOX_FORCE_SCHEMA_VALIDATION}) {
+ require Devel::Cycle;
+ import Devel::Cycle -quiet;
+}
+}
+
use base 'Exporter';
our @EXPORT_OK = qw(
@@ -1035,9 +1041,9 @@ sub validate {
my $errors = {};
$errmsg = "Parameter verification failed.\n" if !$errmsg;
-# todo: cycle detection is only needed for debugging, I guess
-# we can disable that in the final release
-# todo: is there a better/faster way to detect cycles?
+# only check when building a package or told to do so, this is costly
+return 1 if !($ENV{DEB_BUILD_ARCH} ||
$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION});
+
my $cycles = 0;
find_cycle($instance, sub { $cycles = 1 });
if ($cycles) {
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH common v2] schema: only check for cycles during build
Do not check for cycles or for self-validation if not in a build
environment.
The slight drawback is that we also avoid this cycle checks when we
do (development) testing through the API and don't remeber to set the
PROXMOX_FORCE_SCHEMA_VALIDATION environment variable.
But honestyl, I never had a cycle in the API and got noticed by
*this* check, as then to_json and the behavior were all the pointers
needed to get this.
Signed-off-by: Thomas Lamprecht
---
This time a bit more thought out (thanks Stoiko!)
src/PVE/JSONSchema.pm | 38 +-
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 51c3881..916f703 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -6,13 +6,21 @@ use Storable; # for dclone
use Getopt::Long;
use Encode::Locale;
use Encode;
-use Devel::Cycle -quiet; # todo: remove?
use PVE::Tools qw(split_list $IPV6RE $IPV4RE);
use PVE::Exception qw(raise);
use HTTP::Status qw(:constants);
use Net::IP qw(:PROC);
use Data::Dumper;
+my $do_build_checks;
+BEGIN {
+$do_build_checks = $ENV{DEB_BUILD_ARCH} ||
$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
+if ($do_build_checks) {
+ require Devel::Cycle;
+ import Devel::Cycle -quiet;
+}
+}
+
use base 'Exporter';
our @EXPORT_OK = qw(
@@ -1035,14 +1043,16 @@ sub validate {
my $errors = {};
$errmsg = "Parameter verification failed.\n" if !$errmsg;
-# todo: cycle detection is only needed for debugging, I guess
-# we can disable that in the final release
-# todo: is there a better/faster way to detect cycles?
-my $cycles = 0;
-find_cycle($instance, sub { $cycles = 1 });
-if ($cycles) {
- add_error($errors, undef, "data structure contains recursive cycles");
-} elsif ($schema) {
+# only check when building a package or told to do so, this is costly
+if ($do_build_checks) {
+ my $cycles = 0;
+ find_cycle($instance, sub { $cycles = 1 });
+ if ($cycles) {
+ add_error($errors, undef, "data structure contains recursive
cycles");
+ }
+}
+
+if ($schema) {
check_prop($instance, $schema, '', $errors);
}
@@ -1400,10 +1410,12 @@ sub validate_method_info {
validate_schema($info->{returns}) if $info->{returns};
}
-# run a self test on load
-# make sure we can verify the default schema
-validate_schema($default_schema_noref);
-validate_schema($method_schema);
+if ($do_build_checks) {
+# run a self test on load when building
+# make sure we can verify the default schema
+validate_schema($default_schema_noref);
+validate_schema($method_schema);
+}
# and now some utility methods (used by pve api)
sub method_get_child_link {
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common v2] schema: only check for cycles during build
On Fri, 22 Nov 2019 19:37:20 +0100
Thomas Lamprecht wrote:
> Do not check for cycles or for self-validation if not in a build
> environment.
>
> The slight drawback is that we also avoid this cycle checks when we
> do (development) testing through the API and don't remeber to set the
> PROXMOX_FORCE_SCHEMA_VALIDATION environment variable.
>
> But honestyl, I never had a cycle in the API and got noticed by
> *this* check, as then to_json and the behavior were all the pointers
> needed to get this.
do agree - fwiw - this check helps in the CLIHandler e.g. for
`pmgconfig dkim_record` (after manually adding a trivial cycle)
But this is pathologic and - However this is also not caught during
build-time (we check for cycles in the provided data - not the schema)
keeping the cycle check while building does have merit if someone
start plugging hash-references (without copying) in the api-definitions
and eventually a cycle forms IIUC
summing-up: looks good to me!
>
> Signed-off-by: Thomas Lamprecht
> ---
>
> This time a bit more thought out (thanks Stoiko!)
>
> src/PVE/JSONSchema.pm | 38 +-
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 51c3881..916f703 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -6,13 +6,21 @@ use Storable; # for dclone
> use Getopt::Long;
> use Encode::Locale;
> use Encode;
> -use Devel::Cycle -quiet; # todo: remove?
> use PVE::Tools qw(split_list $IPV6RE $IPV4RE);
> use PVE::Exception qw(raise);
> use HTTP::Status qw(:constants);
> use Net::IP qw(:PROC);
> use Data::Dumper;
>
> +my $do_build_checks;
> +BEGIN {
> +$do_build_checks = $ENV{DEB_BUILD_ARCH} ||
> $ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
> +if ($do_build_checks) {
> + require Devel::Cycle;
> + import Devel::Cycle -quiet;
> +}
> +}
> +
> use base 'Exporter';
>
> our @EXPORT_OK = qw(
> @@ -1035,14 +1043,16 @@ sub validate {
> my $errors = {};
> $errmsg = "Parameter verification failed.\n" if !$errmsg;
>
> -# todo: cycle detection is only needed for debugging, I guess
> -# we can disable that in the final release
> -# todo: is there a better/faster way to detect cycles?
> -my $cycles = 0;
> -find_cycle($instance, sub { $cycles = 1 });
> -if ($cycles) {
> - add_error($errors, undef, "data structure contains recursive cycles");
> -} elsif ($schema) {
> +# only check when building a package or told to do so, this is costly
> +if ($do_build_checks) {
> + my $cycles = 0;
> + find_cycle($instance, sub { $cycles = 1 });
> + if ($cycles) {
> + add_error($errors, undef, "data structure contains recursive
> cycles");
> + }
> +}
> +
> +if ($schema) {
> check_prop($instance, $schema, '', $errors);
> }
>
> @@ -1400,10 +1410,12 @@ sub validate_method_info {
> validate_schema($info->{returns}) if $info->{returns};
> }
>
> -# run a self test on load
> -# make sure we can verify the default schema
> -validate_schema($default_schema_noref);
> -validate_schema($method_schema);
> +if ($do_build_checks) {
> +# run a self test on load when building
> +# make sure we can verify the default schema
> +validate_schema($default_schema_noref);
> +validate_schema($method_schema);
> +}
>
> # and now some utility methods (used by pve api)
> sub method_get_child_link {
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
