Re: [pve-devel] [PATCH v4 manager 3/3] Allow CPUModelSelector to be searched by vendor as well
On 9/10/19 7:11 PM, Stefan Reiter wrote:
> Signed-off-by: Stefan Reiter
> ---
> www/manager6/form/CPUModelSelector.js | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/www/manager6/form/CPUModelSelector.js
> b/www/manager6/form/CPUModelSelector.js
> index dea6c44c..55532a0d 100644
> --- a/www/manager6/form/CPUModelSelector.js
> +++ b/www/manager6/form/CPUModelSelector.js
> @@ -4,6 +4,7 @@ Ext.define('PVE.form.CPUModelSelector', {
>
> valueField: 'value',
> displayField: 'value',
> +searchFields: [ 'value', 'vendor' ],
>
> emptyText: Proxmox.Utils.defaultText + ' (kvm64)',
> allowBlank: true,
>
instead of this and 2/3 we also could just a virtual combined field
in the model and use that as displayField, like I did for the USB
Selector [0].
But, just as idea, multi column search could be nice enough on
it's own do justify the relative big overwrite in ComboGrid..
[0]:
https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=19fa4ee333aaa193638858241651b0f9e1a40e2e
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v4 widget-toolkit 2/3] Allow searching for multiple columns in ComboGrid
On 9/10/19 7:11 PM, Stefan Reiter wrote:
> Uses code from ExtJS 6.0.1 ComboBox.js to overwrite 'doLocalQuery',
> adding a new property 'searchFields' which allows to specify fields from
> the bound store which will all be searched (OR filter, i.e. the searched
> value can appear in any column specified).
>
> Signed-off-by: Stefan Reiter
> ---
> form/ComboGrid.js | 82 +++
> 1 file changed, 82 insertions(+)
>
some style comments inline, did not tested it at all and would like
to get also a ack from Dominik for this feature.
> diff --git a/form/ComboGrid.js b/form/ComboGrid.js
> index c3e7782..d470a98 100644
> --- a/form/ComboGrid.js
> +++ b/form/ComboGrid.js
> @@ -310,6 +310,88 @@ Ext.define('Proxmox.form.ComboGrid', {
> return picker;
> },
>
> +createStoreFilter: function(property, queryString, suffix) {
> + var me = this;
> +
> + return new Ext.util.Filter({
> + id: me.id + '-filter-' + suffix,
> + anyMatch: me.anyMatch,
> + caseSensitive: me.caseSensitive,
> + root: 'data',
> + property: property,
> + value: me.enableRegEx ? new RegExp(queryString) : queryString
> + });
> +},
> +
> +doLocalQuery: function(queryPlan) {
> + var me = this,
> + queryString = queryPlan.query,
> + store = me.getStore(),
> + filter = me.queryFilter,
> + multiFilters;
> +
> + me.queryFilter = null;
> + // Must set changingFilters flag for this.checkValueOnChange.
> + // the suppressEvents flag does not affect the filterchange event
> + me.changingFilters = true;
> + if (filter) {
> + store.removeFilter(filter, true);
> + }
> +
> + // Querying by a string...
> + if (queryString) {
> + // proxmox overrides: allow filtering multiple fields
> + if (Ext.isArray(me.searchFields)) {
> + // Since me.queryFilter is used elsewhere as a single filter, we
> + // cannot assign an array. Instead, we create a function
> + // manually applying the different filters OR'd together and set
> + // that as the filterFn of a single, combined filter.
> + multiFilters = [];
> + Ext.Array.each(me.searchFields, function (v) {
> + multiFilters.push(me.createStoreFilter(
> + v,
> + queryString,
> + 'multi-' + v
> + ))
Do not like the split over many newlines syntax to much, doesn't really fits
Proxmox's or ExtJS unwritten syntax style.. Nice and OK for configuration
objects, but not so much for a simple function parameter list.
Either:
let filter = me.createStoreFilter(v, queryString, 'multi-' + v);
multiFilters.push(filter);
or just use a map, below an (untested) example which uses map +
arrow function replacing 8 lines with 1:
multiFilter = Ext.Array.map(me.searchFields, (v) => me.createStoreFilter(v,
queryString, 'multi-' + v));
or
multiFilter = Ext.Array.map(me.searchFields, (v) => {
return me.createStoreFilter(v, queryString, 'multi-' + v)
});
> + });
> +
> + filter = me.queryFilter = new Ext.util.Filter({
> + id: me.id + '-filter-multi',
> + root: 'data',
> + filterFn: function (item) {
> + for (var i = 0; i < multiFilters.length; i++) {
> + if (multiFilters[i].getFilterFn()(item)) {
> + return true;
> + }
> + }
> + return false;
> + }
> + })
> + } else {
> + filter = me.queryFilter = me.createStoreFilter(
> + me.displayField,
> + queryString,
> + 'single'
> + );
see above
> + }
> +
> + store.addFilter(filter, true);
> + }
> + me.changingFilters = false;
> +
> + // Expand after adjusting the filter if there are records or if
> emptyText is configured.
> + if (me.store.getCount() || me.getPicker().emptyText) {
> + // The filter changing was done with events suppressed, so
> + // refresh the picker DOM while hidden and it will layout on show.
> + me.getPicker().refresh();
> + me.expand();
> + } else {
> + me.collapse();
> + }
> +
> + me.afterQuery(queryPlan);
> +},
> +
> isValueInStore: function(value) {
> var me = this;
> var store = me.store;
>
___
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 04/12] Adapt CPUConfig to handle custom models
Thanks for the review, I'll prepare a v3 :)
Just some clarifications inline for this patch.
On 10/2/19 7:49 AM, Thomas Lamprecht wrote:
On 9/30/19 12:58 PM, Stefan Reiter wrote:
Turn CPUConfig into a SectionConfig with parsing/writing support for
custom CPU models. IO is handled using cfs.
The "custom" parameter provides differentiation between custom and
default types, even if the name is the same ('namespacing').
Signed-off-by: Stefan Reiter
---
PVE/QemuServer/CPUConfig.pm | 59 ++---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index c994228..3fde987 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -3,6 +3,8 @@ package PVE::QemuServer::CPUConfig;
use strict;
use warnings;
use PVE::JSONSchema;
+use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use base qw(PVE::SectionConfig);
use base 'Exporter';
our @EXPORT_OK = qw(
@@ -11,6 +13,15 @@ get_cpu_options
qemu_machine_feature_enabled
);
+my $default_filename = "cpu-models.conf";
+cfs_register_file($default_filename,
+ sub { PVE::QemuServer::CPUConfig->parse_config(@_); },
+ sub { PVE::QemuServer::CPUConfig->write_config(@_); });
+
+sub load_custom_model_conf {
+return cfs_read_file($default_filename);
+}
+
my $cpu_vendor_list = {
# Intel CPUs
486 => 'GenuineIntel',
@@ -80,11 +91,27 @@ my $cpu_flag = qr/[+-](@{[join('|',
@supported_cpu_flags)]})/;
our $cpu_fmt = {
cputype => {
- description => "Emulated CPU type.",
+ description => "Emulated CPU type. Can be default or custom name.",
type => 'string',
- enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],
+ format_description => 'string',
default => 'kvm64',
default_key => 1,
+ optional => 1,
+},
+custom => {
+ description => "True if 'cputype' is a custom model, false if
otherwise.",
maybe negate it and call it built-in? We use that as name for Permission roles
which
are generated by code, we probably want to call this also built-in in the UI,
and be
it only to re-use translations ;)
>> + type => 'boolean',
+ default => 0,
+ optional => 1,
+},
+'reported-model' => {
+ description => "CPU model and vendor to report to the guest. Must be a
QEMU/KVM supported model."
+. " Only valid for custom CPU model definitions, default models
will always report themselves to the guest OS.",
+ type => 'string',
+ enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],
we do not use \L at all in our code, AFAICT, just use the a bit more
expressive, or at least already used all over the place, lc()
TBH I just moved that part from the existing "cputype" enum, but I can
change it in v3 - it sure looks clearer with lc().
+ default => 'kvm64',
+ custom_only => 1,
. ^^^
That's a fake JSONSchema attribute, we do not do those normally, either
some mechanism gets added to JSONSchema to really represent this in general
or just omit it.
>> + optional => 1,
},
hidden => {
description => "Do not identify as a KVM virtual machine.",
@@ -102,14 +129,34 @@ our $cpu_fmt = {
flags => {
description => "List of additional CPU flags separated by ';'."
. " Use '+FLAG' to enable, '-FLAG' to disable a flag."
-. " Currently supported flags: @{[join(', ',
@supported_cpu_flags)]}.",
+. " Custom CPU models can specify any flag supported by"
+. " QEMU/KVM, VM-specific flags must be from the following"
+. " set for security reasons: @{[join(', ',
@supported_cpu_flags)]}.",
is the last part still true if the pattern restriction to $cpu_flag are lifted
below?
Yes, because it is checked in verify_vm_cpu_conf from patch 06/12. I
admit it's not entirely clear from just looking at this property
definition, but it was the only way I could think of to somewhat cleanly
handle the re-use of $cpu_fmt for VM-specific settings and custom models
- maybe I'll add a comment.
format_description => '+FLAG[;-FLAG...]',
type => 'string',
- pattern => qr/$cpu_flag(;$cpu_flag)*/,
+ pattern => qr/[+-][a-zA-Z0-9\-_\.]+(;[+-][a-zA-Z0-9\-_\.]+)*/,
optional => 1,
},
};
+# Section config settings
+my $defaultData = {
+# shallow copy, since SectionConfig modifies propertyList internally
+propertyList => { %$cpu_fmt },
+};
+
+sub private {
+return $defaultData;
+}
+
+sub options {
+return { %$cpu_fmt };
+}
+
+sub type {
+return 'cpu-model';
+}
+
# Print a QEMU device node for a given VM configuration for hotplugging CPUs
sub print_cpu_device {
my ($conf, $id) = @_;
@@ -248,4 +295,8 @@ sub qemu_machine_feature_enabled {
Re: [pve-devel] [PATCH v2 qemu-server 03/12] Add CPUConfig file and migrate some CPU helpers
On 10/1/19 6:12 PM, Thomas Lamprecht wrote: On 9/30/19 12:58 PM, Stefan Reiter wrote: The package will be used for custom CPU models as a SectionConfig, hence the name. For now we simply move some CPU related helper functions and declarations over from QemuServer to reduce clutter there. Single thing I'm not too sure is how "qemu_machine_feature_enabled" fit's into the CPUConfig here.. AFAICT, this rather looks like and attempt to avoid a cyclic module dependency? Pretty much, yes. I don't think it looks *too* amiss. Maybe we could also do a Machine module which then includes stuff like: machine_type_is_q35 get_basic_machine_info But seems not really worth it... better ideas? No great ideas, but should I leave it this way for v3 or work something out? Signed-off-by: Stefan Reiter --- PVE/QemuServer.pm | 242 +- PVE/QemuServer/CPUConfig.pm | 251 PVE/QemuServer/Makefile | 1 + 3 files changed, 256 insertions(+), 238 deletions(-) create mode 100644 PVE/QemuServer/CPUConfig.pm ___ 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 03/12] Add CPUConfig file and migrate some CPU helpers
On 10/2/19 10:24 AM, Stefan Reiter wrote: > On 10/1/19 6:12 PM, Thomas Lamprecht wrote: >> On 9/30/19 12:58 PM, Stefan Reiter wrote: >>> The package will be used for custom CPU models as a SectionConfig, hence >>> the name. For now we simply move some CPU related helper functions and >>> declarations over from QemuServer to reduce clutter there. >> >> Single thing I'm not too sure is how "qemu_machine_feature_enabled" fit's >> into the CPUConfig here.. >> AFAICT, this rather looks like and attempt to avoid a cyclic module >> dependency? > > Pretty much, yes. I don't think it looks *too* amiss. Hmm, but it's just not CPU related itself at all.. > >> >> Maybe we could also do a Machine module which then includes stuff like: >> machine_type_is_q35 >> get_basic_machine_info >> >> But seems not really worth it... better ideas? >> > > No great ideas, but should I leave it this way for v3 or work something out? Yeah, works for me, as long it stays in the same package moving it around is almost no work, so we can change this relatively easily afterwards. But please mention the "moved qemu_machine_feature_enabled to avoid cyclic module dependencies" in the commit message. ___ 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 04/12] Adapt CPUConfig to handle custom models
On 10/2/19 10:24 AM, Stefan Reiter wrote:
> Thanks for the review, I'll prepare a v3 :)
>
> Just some clarifications inline for this patch.
>
> On 10/2/19 7:49 AM, Thomas Lamprecht wrote:
>> On 9/30/19 12:58 PM, Stefan Reiter wrote:
>>> Turn CPUConfig into a SectionConfig with parsing/writing support for
>>> custom CPU models. IO is handled using cfs.
>>>
>>> The "custom" parameter provides differentiation between custom and
>>> default types, even if the name is the same ('namespacing').
>>>
>>> Signed-off-by: Stefan Reiter
>>> ---
>>> PVE/QemuServer/CPUConfig.pm | 59 ++---
>>> 1 file changed, 55 insertions(+), 4 deletions(-)
>>>
>>> + 'reported-model' => {
>>> + description => "CPU model and vendor to report to the guest. Must be a
>>> QEMU/KVM supported model."
>>> + . " Only valid for custom CPU model definitions, default
>>> models will always report themselves to the guest OS.",
>>> + type => 'string',
>>> + enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],
>>
>> we do not use \L at all in our code, AFAICT, just use the a bit more
>> expressive, or at least already used all over the place, lc()
>>
>
> TBH I just moved that part from the existing "cputype" enum, but I can change
> it in v3 - it sure looks clearer with lc().
argh, sorry for the wrong "accusations" then, didn't bother to look
it up but had no \L use in mind and that normally means that if there
are any they are outsiders.. ^^
>
>>> + default => 'kvm64',
>>> + custom_only => 1,
>> . ^^^
>> That's a fake JSONSchema attribute, we do not do those normally, either
>> some mechanism gets added to JSONSchema to really represent this in general
>> or just omit it.
>> >> + optional => 1,
>>> },
>>> hidden => {
>>> description => "Do not identify as a KVM virtual machine.",
>>> @@ -102,14 +129,34 @@ our $cpu_fmt = {
>>> flags => {
>>> description => "List of additional CPU flags separated by ';'."
>>> . " Use '+FLAG' to enable, '-FLAG' to disable a flag."
>>> - . " Currently supported flags: @{[join(', ',
>>> @supported_cpu_flags)]}.",
>>> + . " Custom CPU models can specify any flag supported by"
>>> + . " QEMU/KVM, VM-specific flags must be from the following"
>>> + . " set for security reasons: @{[join(', ',
>>> @supported_cpu_flags)]}.",
>>
>> is the last part still true if the pattern restriction to $cpu_flag are
>> lifted
>> below?
>>
>
> Yes, because it is checked in verify_vm_cpu_conf from patch 06/12. I admit
> it's not entirely clear from just looking at this property definition, but it
> was the only way I could think of to somewhat cleanly handle the re-use of
> $cpu_fmt for VM-specific settings and custom models - maybe I'll add a
> comment.
so it "breaks" temporarily between here and 06/12? Can be overlooked but
in general not OK, series should not introduce temporary breakage of
things, if possible, if only to make review a bit easier.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 container] use print_snapshot_tree guest helper for pct listsnapshot
adds feature parity between qm/pct 'listsnapshot' w.r.t. showing
snapshot tree ordered by date.
Signed-off-by: Oguz Bektas
---
src/PVE/CLI/pct.pm | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
index 35ad72f..705a015 100755
--- a/src/PVE/CLI/pct.pm
+++ b/src/PVE/CLI/pct.pm
@@ -861,16 +861,7 @@ our $cmddef = {
delsnapshot => [ "PVE::API2::LXC::Snapshot", 'delsnapshot', ['vmid',
'snapname'], { node => $nodename } , $upid_exit ],
-listsnapshot => [ "PVE::API2::LXC::Snapshot", 'list', ['vmid'], { node =>
$nodename },
- sub {
- my $res = shift;
- foreach my $e (@$res) {
- my $headline = $e->{description} ||
'no-description';
- $headline =~ s/\n.*//sg;
- my $parent = $e->{parent} // 'no-parent';
- printf("%-20s %-20s %s\n", $e->{name}, $parent,
$headline);
- }
- }],
+listsnapshot => [ "PVE::API2::LXC::Snapshot", 'list', ['vmid'], { node =>
$nodename }, \&PVE::GuestHelpers::print_snapshot_tree ],
rollback => [ "PVE::API2::LXC::Snapshot", 'rollback', ['vmid',
'snapname'], { node => $nodename } , $upid_exit ],
--
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 v2 qemu-server] use print_snapshot_tree guest helper for qm listsnapshot
moved code to GuestHelpers for feature parity with pct
Signed-off-by: Oguz Bektas
---
PVE/CLI/qm.pm | 53 +--
1 file changed, 1 insertion(+), 52 deletions(-)
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 17935d0..bc9e227 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -933,58 +933,7 @@ our $cmddef = {
delsnapshot => [ "PVE::API2::Qemu", 'delsnapshot', ['vmid', 'snapname'], {
node => $nodename } , $upid_exit ],
-listsnapshot => [ "PVE::API2::Qemu", 'snapshot_list', ['vmid'], { node =>
$nodename },
- sub {
- my $res = shift;
-
- my $snapshots = { map { $_->{name} => $_ } @$res };
-
- my @roots;
- foreach my $e (@$res) {
- my $parent;
- if (($parent = $e->{parent}) && defined
$snapshots->{$parent}) {
- push @{$snapshots->{$parent}->{children}},
$e->{name};
- } else {
- push @roots, $e->{name};
- }
- }
-
- # sort the elements by snaptime - with "current" (no
snaptime) highest
- my $snaptimesort = sub {
- return +1 if !defined $snapshots->{$a}->{snaptime};
- return -1 if !defined $snapshots->{$b}->{snaptime};
- return $snapshots->{$a}->{snaptime} <=>
$snapshots->{$b}->{snaptime};
- };
-
- # recursion function for displaying the tree
- my $snapshottree;
- $snapshottree = sub {
- my ($prefix, $root, $snapshots) = @_;
- my $e = $snapshots->{$root};
-
- my $description = $e->{description} ||
'no-description';
- ($description) = $description =~ m/(.*)$/m;
-
- my $timestring = "";
- if (defined $e->{snaptime}) {
- $timestring = strftime("%F %H:%M:%S",
localtime($e->{snaptime}));
- }
-
- my $len = 30 - length($prefix); # for aligning the
description
- printf("%s %-${len}s %-23s %s\n", $prefix, $root,
$timestring, $description);
-
- if ($e->{children}) {
- $prefix = "$prefix";
- foreach my $child (sort $snaptimesort
@{$e->{children}}) {
- $snapshottree->($prefix, $child,
$snapshots);
- }
- }
- };
-
- foreach my $root (sort $snaptimesort @roots) {
- $snapshottree->('`->', $root, $snapshots);
- }
- }],
+listsnapshot => [ "PVE::API2::Qemu", 'snapshot_list', ['vmid'], { node =>
$nodename }, \&PVE::GuestHelpers::print_snapshot_tree],
rollback => [ "PVE::API2::Qemu", 'rollback', ['vmid', 'snapname'], { node
=> $nodename } , $upid_exit ],
--
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 v2 qemu-server 05/18] use load_current_config for config GET call
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> PVE/API2/Qemu.pm | 35 +--
> 1 file changed, 1 insertion(+), 34 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 267a08e..aa1cd16 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -865,40 +865,7 @@ __PACKAGE__->register_method({
> code => sub {
> my ($param) = @_;
>
> - my $conf = PVE::QemuConfig->load_config($param->{vmid});
> -
> - if (my $snapname = $param->{snapshot}) {
> - my $snapshot = $conf->{snapshots}->{$snapname};
> - die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
> -
> - $snapshot->{digest} = $conf->{digest}; # keep file digest for API
> -
> - $conf = $snapshot;
> - }
> -
> - delete $conf->{snapshots};
> -
> - if (!$param->{current}) {
> - foreach my $opt (keys %{$conf->{pending}}) {
> - next if $opt eq 'delete';
> - my $value = $conf->{pending}->{$opt};
> - next if ref($value); # just to be sure
> - $conf->{$opt} = $value;
> - }
> - my $pending_delete_hash =
> PVE::QemuServer::split_flagged_list($conf->{pending}->{delete});
> - foreach my $opt (keys %$pending_delete_hash) {
> - delete $conf->{$opt} if $conf->{$opt};
> - }
> - }
> -
> - delete $conf->{pending};
> -
> - # hide cloudinit password
> - if ($conf->{cipassword}) {
> - $conf->{cipassword} = '**';
> - }
> -
> - return $conf;
> + return PVE::QemuConfig->load_current_config($param->{vmid},
> $param->{snapshot}, $param->{current});
> }});
>
> __PACKAGE__->register_method({
>
I'd squash 05/18 and 04/18, IMO they are a single logical change and
it helps to connect the changes and overwrite much better together,
for me at least.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 guest-common 02/18] refactor method used by config GET calls into AbstractConfig
your commit subject is so general that it is completely of no use..
after reading it one has not idea at all about which function with
was use gets refactored out..
How about:
> abstract config: add general load_current_config implementation
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> since this method will be both used by qemu and lxc config GET calls, it
how about:
> this code is already used by qemu-servers GET config API call, once
> container can also do pending changes the will use the same code. So add
> a general implementation here which uses some $class helpers allowing to
> abstract away the difference in child classes.
Besides that, looks OK
> makes sense to move it into AbstractConfig. only difference is that qemu
> also hides the cipassword when it's set. this can be handled by having
> qemu overwrite the method and add the cipassword code.
>
> Signed-off-by: Oguz Bektas
> ---
> PVE/AbstractConfig.pm | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
> index 910ca86..6d3f169 100644
> --- a/PVE/AbstractConfig.pm
> +++ b/PVE/AbstractConfig.pm
> @@ -136,6 +136,41 @@ sub cleanup_pending {
> return $changes;
> }
>
> +sub load_current_config {
> +my ($class, $vmid, $snapname, $current) = @_;
> +
> +my $conf = $class->load_config($vmid);
> +
> +if ($snapname) {
> + my $snapshot = $conf->{snapshots}->{$snapname};
> + die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
> +
> + # we need the digest of the file
> + $snapshot->{digest} = $conf->{digest};
> + $conf = $snapshot;
> +}
> +
> +# take pending changes in
> +if (!$current) {
> + foreach my $opt (keys %{$conf->{pending}}) {
> + next if $opt eq 'delete';
> + my $value = $conf->{pending}->{$opt};
> + next if ref($value); # just to be sure
> + $conf->{$opt} = $value;
> + }
> + my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> + foreach my $opt (keys %$pending_delete_hash) {
> + delete $conf->{$opt} if $conf->{$opt};
> + }
> +}
> +
> +delete $conf->{snapshots};
> +delete $conf->{pending};
> +
> +return $conf;
> +}
> +
> +
> # Lock config file using flock, run $code with @param, unlock config file.
> # $timeout is the maximum time to aquire the flock
> sub lock_config_full {
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 ha-manager 4/9] Add timeout parameter for shutdown
Introduces a timeout parameter for shutting a resource down.
If the parameter is 0, we perform a hard stop instead of a shutdown.
Signed-off-by: Fabian Ebner
---
src/PVE/HA/LRM.pm | 4 ++--
src/PVE/HA/Resources.pm | 2 +-
src/PVE/HA/Resources/PVECT.pm | 14 ++
src/PVE/HA/Resources/PVEVM.pm | 16 +++-
4 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index 3b4a572..e5eee94 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -535,7 +535,7 @@ sub manage_resources {
my $req_state = $sd->{state};
next if !defined($req_state);
next if $req_state eq 'freeze';
- $self->queue_resource_command($sid, $sd->{uid}, $req_state, {'target'
=> $sd->{target}});
+ $self->queue_resource_command($sid, $sd->{uid}, $req_state, {'target'
=> $sd->{target}, 'timeout' => $sd->{timeout}});
}
return $self->run_workers();
@@ -776,7 +776,7 @@ sub exec_resource_agent {
$haenv->log("info", "stopping service $sid");
- $plugin->shutdown($haenv, $id);
+ $plugin->shutdown($haenv, $id, $params->{timeout});
$running = $plugin->check_running($haenv, $id);
diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
index 7c373bc..835c314 100644
--- a/src/PVE/HA/Resources.pm
+++ b/src/PVE/HA/Resources.pm
@@ -126,7 +126,7 @@ sub start {
}
sub shutdown {
-my ($class, $haenv, $id) = @_;
+my ($class, $haenv, $id, $timeout) = @_;
die "implement in subclass";
}
diff --git a/src/PVE/HA/Resources/PVECT.pm b/src/PVE/HA/Resources/PVECT.pm
index a0f05f4..282f4ef 100644
--- a/src/PVE/HA/Resources/PVECT.pm
+++ b/src/PVE/HA/Resources/PVECT.pm
@@ -74,18 +74,24 @@ sub start {
}
sub shutdown {
-my ($class, $haenv, $id) = @_;
+my ($class, $haenv, $id, $timeout) = @_;
my $nodename = $haenv->nodename();
-my $shutdown_timeout = 60; # fixme: make this configurable
+my $shutdown_timeout = $timeout // 60;
+my $upid;
my $params = {
node => $nodename,
vmid => $id,
- timeout => $shutdown_timeout,
};
-my $upid = PVE::API2::LXC::Status->vm_shutdown($params);
+if ($shutdown_timeout) {
+ $params->{timeout} = $shutdown_timeout;
+ $upid = PVE::API2::LXC::Status->vm_shutdown($params);
+} else {
+ $upid = PVE::API2::LXC::Status->vm_stop($params);
+}
+
PVE::HA::Tools::upid_wait($upid, $haenv);
}
diff --git a/src/PVE/HA/Resources/PVEVM.pm b/src/PVE/HA/Resources/PVEVM.pm
index 9dcf558..45d87e8 100644
--- a/src/PVE/HA/Resources/PVEVM.pm
+++ b/src/PVE/HA/Resources/PVEVM.pm
@@ -72,19 +72,25 @@ sub start {
}
sub shutdown {
-my ($class, $haenv, $id) = @_;
+my ($class, $haenv, $id, $timeout) = @_;
my $nodename = $haenv->nodename();
-my $shutdown_timeout = 60; # fixme: make this configurable
+my $shutdown_timeout = $timeout // 60;
+my $upid;
my $params = {
node => $nodename,
vmid => $id,
- timeout => $shutdown_timeout,
- forceStop => 1,
};
-my $upid = PVE::API2::Qemu->vm_shutdown($params);
+if ($shutdown_timeout) {
+ $params->{timeout} = $shutdown_timeout;
+ $params->{forceStop} = 1;
+ $upid = PVE::API2::Qemu->vm_shutdown($params);
+} else {
+ $upid = PVE::API2::Qemu->vm_stop($params);
+}
+
PVE::HA::Tools::upid_wait($upid, $haenv);
}
--
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 v3 ha-manager 1/9] Move code updating resource config from API2::HA::Resources to HA::Config
This makes it easier to update the resource configuration from within the
CRM/LRM stack,
which is needed for the new 'stop' command.
Signed-off-by: Fabian Ebner
---
src/PVE/API2/HA/Resources.pm | 34 +
src/PVE/HA/Config.pm | 37
2 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 22d7f28..5c11d8b 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -237,39 +237,7 @@ __PACKAGE__->register_method ({
check_service_state($sid, $param->{state});
- PVE::HA::Config::lock_ha_domain(
- sub {
-
- my $cfg = PVE::HA::Config::read_resources_config();
-
- PVE::SectionConfig::assert_if_modified($cfg, $digest);
-
- my $scfg = $cfg->{ids}->{$sid} ||
- die "no such resource '$sid'\n";
-
- my $plugin = PVE::HA::Resources->lookup($scfg->{type});
- my $opts = $plugin->check_config($sid, $param, 0, 1);
-
- foreach my $k (%$opts) {
- $scfg->{$k} = $opts->{$k};
- }
-
- if ($delete) {
- my $options = $plugin->private()->{options}->{$type};
- foreach my $k (PVE::Tools::split_list($delete)) {
- my $d = $options->{$k} ||
- die "no such option '$k'\n";
- die "unable to delete required option '$k'\n"
- if !$d->{optional};
- die "unable to delete fixed option '$k'\n"
- if $d->{fixed};
- delete $scfg->{$k};
- }
- }
-
- PVE::HA::Config::write_resources_config($cfg)
-
- }, "update resource failed");
+ PVE::HA::Config::update_resources_config($sid, $param, $delete,
$digest);
return undef;
}});
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index ead1ee2..676eaaf 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -125,6 +125,43 @@ sub read_and_check_resources_config {
return $conf;
}
+sub update_resources_config {
+my ($sid, $param, $delete, $digest) = @_;
+
+lock_ha_domain(
+ sub {
+ my $cfg = read_resources_config();
+ ($sid, my $type, my $name) = parse_sid($sid);
+
+ PVE::SectionConfig::assert_if_modified($cfg, $digest);
+
+ my $scfg = $cfg->{ids}->{$sid} ||
+ die "no such resource '$sid'\n";
+
+ my $plugin = PVE::HA::Resources->lookup($scfg->{type});
+ my $opts = $plugin->check_config($sid, $param, 0, 1);
+
+ foreach my $k (%$opts) {
+ $scfg->{$k} = $opts->{$k};
+ }
+
+ if ($delete) {
+ my $options = $plugin->private()->{options}->{$type};
+ foreach my $k (PVE::Tools::split_list($delete)) {
+ my $d = $options->{$k} ||
+ die "no such option '$k'\n";
+ die "unable to delete required option '$k'\n"
+ if !$d->{optional};
+ die "unable to delete fixed option '$k'\n"
+ if $d->{fixed};
+ delete $scfg->{$k};
+ }
+ }
+
+ write_resources_config($cfg);
+ }, "update resources config failed");
+}
+
sub parse_sid {
my ($sid) = @_;
--
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 v3 ha-manager 2/9] Add update_service_config to HA environment interface
Signed-off-by: Fabian Ebner
---
src/PVE/HA/Env.pm | 6 ++
src/PVE/HA/Env/PVE2.pm | 6 ++
src/PVE/HA/Sim/Env.pm | 6 ++
3 files changed, 18 insertions(+)
diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
index bb37486..ac569a9 100644
--- a/src/PVE/HA/Env.pm
+++ b/src/PVE/HA/Env.pm
@@ -87,6 +87,12 @@ sub read_service_config {
return $self->{plug}->read_service_config();
}
+sub update_service_config {
+my ($self, $sid, $param) = @_;
+
+return $self->{plug}->update_service_config($sid, $param);
+}
+
sub parse_sid {
my ($self, $sid) = @_;
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 796acd9..83145dc 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -120,6 +120,12 @@ sub read_service_config {
return PVE::HA::Config::read_and_check_resources_config();
}
+sub update_service_config {
+my ($self, $sid, $param) = @_;
+
+return PVE::HA::Config::update_resources_config($sid, $param);
+}
+
sub parse_sid {
my ($self, $sid) = @_;
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index 22e13e6..b286708 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -203,6 +203,12 @@ sub read_service_config {
return $self->{hardware}->read_service_config();
}
+sub update_service_config {
+my ($self, $sid, $param) = @_;
+
+return $self->{hardware}->update_service_config($sid, $param);
+}
+
sub parse_sid {
my ($self, $sid) = @_;
--
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 v3 ha-manager 8/9] Log timeout parameter when present
Signed-off-by: Fabian Ebner
---
src/PVE/HA/LRM.pm | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index e5eee94..95ec351 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -774,7 +774,11 @@ sub exec_resource_agent {
return SUCCESS if !$running;
- $haenv->log("info", "stopping service $sid");
+ if (defined($params->{timeout})) {
+ $haenv->log("info", "stopping service $sid
(timeout=$params->{timeout})");
+ } else {
+ $haenv->log("info", "stopping service $sid");
+ }
$plugin->shutdown($haenv, $id, $params->{timeout});
--
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 v3 ha-manager 5/9] Introduce crm-command to CLI and add stop as a subcommand
This should reduce confusion between the old 'set --state stopped' and
the new 'stop' command by making it explicit that it is sent as a crm command.
Signed-off-by: Fabian Ebner
---
src/PVE/CLI/ha_manager.pm | 46 +--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/src/PVE/CLI/ha_manager.pm b/src/PVE/CLI/ha_manager.pm
index 5ce4c30..b087319 100644
--- a/src/PVE/CLI/ha_manager.pm
+++ b/src/PVE/CLI/ha_manager.pm
@@ -9,6 +9,7 @@ use JSON;
use PVE::JSONSchema qw(get_standard_option);
use PVE::CLIHandler;
use PVE::Cluster;
+use PVE::Tools qw(extract_param);
use PVE::RPCEnvironment;
use PVE::HA::Config; # needed for bash completion in PVE::HA::Tools!
@@ -73,6 +74,41 @@ __PACKAGE__->register_method ({
return undef;
}});
+__PACKAGE__->register_method ({
+name => 'stop',
+path => 'stop',
+method => 'POST',
+description => "Request the service to be stopped.",
+permissions => {
+ check => ['perm', '/', [ 'Sys.Console' ]],
+},
+parameters => {
+ additionalProperties => 0,
+ properties => {
+ sid => get_standard_option('pve-ha-resource-or-vm-id',
+ { completion =>
\&PVE::HA::Tools::complete_sid }),
+ timeout => {
+ description => "Timeout in seconds. If set to 0 a hard stop
will be performed.",
+ type => 'integer',
+ minimum => 0,
+ },
+ },
+},
+returns => { type => 'null' },
+code => sub {
+ my ($param) = @_;
+
+ my $sid = PVE::HA::Config::parse_sid(extract_param($param, 'sid'));
+
+ PVE::HA::Config::service_is_ha_managed($sid);
+
+ PVE::API2::HA::Resources::check_service_state($sid);
+
+ PVE::HA::Config::queue_crm_commands("stop $sid $param->{timeout}");
+
+ return undef;
+}});
+
our $cmddef = {
status => [ __PACKAGE__, 'status'],
config => [ 'PVE::API2::HA::Resources', 'index', [], {}, sub {
@@ -111,8 +147,14 @@ our $cmddef = {
remove => [ "PVE::API2::HA::Resources", 'delete', ['sid'] ],
set => [ "PVE::API2::HA::Resources", 'update', ['sid'] ],
-migrate => [ "PVE::API2::HA::Resources", 'migrate', ['sid', 'node'] ],
-relocate => [ "PVE::API2::HA::Resources", 'relocate', ['sid', 'node'] ],
+migrate => { alias => 'crm-command migrate' },
+relocate => { alias => 'crm-command relocate' },
+
+'crm-command' => {
+ migrate => [ "PVE::API2::HA::Resources", 'migrate', ['sid', 'node'] ],
+ relocate => [ "PVE::API2::HA::Resources", 'relocate', ['sid', 'node'] ],
+ stop => [ __PACKAGE__, 'stop', ['sid', 'timeout'] ],
+}
};
--
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 v3 ha-manager 9/9] Add test for the stop command
Signed-off-by: Fabian Ebner
---
src/test/test-stop-command1/README | 2 +
src/test/test-stop-command1/cmdlist | 8 +++
src/test/test-stop-command1/hardware_status | 5 ++
src/test/test-stop-command1/log.expect | 69 +
src/test/test-stop-command1/manager_status | 1 +
src/test/test-stop-command1/service_config | 6 ++
6 files changed, 91 insertions(+)
create mode 100644 src/test/test-stop-command1/README
create mode 100644 src/test/test-stop-command1/cmdlist
create mode 100644 src/test/test-stop-command1/hardware_status
create mode 100644 src/test/test-stop-command1/log.expect
create mode 100644 src/test/test-stop-command1/manager_status
create mode 100644 src/test/test-stop-command1/service_config
diff --git a/src/test/test-stop-command1/README
b/src/test/test-stop-command1/README
new file mode 100644
index 000..d725380
--- /dev/null
+++ b/src/test/test-stop-command1/README
@@ -0,0 +1,2 @@
+Test the stop command with different timeouts
+and what happens with a failed service.
diff --git a/src/test/test-stop-command1/cmdlist
b/src/test/test-stop-command1/cmdlist
new file mode 100644
index 000..c7c9e67
--- /dev/null
+++ b/src/test/test-stop-command1/cmdlist
@@ -0,0 +1,8 @@
+[
+[ "power node1 on", "power node2 on", "power node3 on" ],
+[ "service vm:101 stop 0" ],
+[ "service vm:101 stop 1" ],
+[ "service vm:102 stop 37" ],
+[ "service vm:103 stop 60" ],
+[ "service fa:1001 stop 0" ]
+]
diff --git a/src/test/test-stop-command1/hardware_status
b/src/test/test-stop-command1/hardware_status
new file mode 100644
index 000..451beb1
--- /dev/null
+++ b/src/test/test-stop-command1/hardware_status
@@ -0,0 +1,5 @@
+{
+ "node1": { "power": "off", "network": "off" },
+ "node2": { "power": "off", "network": "off" },
+ "node3": { "power": "off", "network": "off" }
+}
diff --git a/src/test/test-stop-command1/log.expect
b/src/test/test-stop-command1/log.expect
new file mode 100644
index 000..05f9712
--- /dev/null
+++ b/src/test/test-stop-command1/log.expect
@@ -0,0 +1,69 @@
+info 0 hardware: starting simulation
+info 20 cmdlist: execute power node1 on
+info 20node1/crm: status change startup => wait_for_quorum
+info 20node1/lrm: status change startup => wait_for_agent_lock
+info 20 cmdlist: execute power node2 on
+info 20node2/crm: status change startup => wait_for_quorum
+info 20node2/lrm: status change startup => wait_for_agent_lock
+info 20 cmdlist: execute power node3 on
+info 20node3/crm: status change startup => wait_for_quorum
+info 20node3/lrm: status change startup => wait_for_agent_lock
+info 20node1/crm: got lock 'ha_manager_lock'
+info 20node1/crm: status change wait_for_quorum => master
+info 20node1/crm: node 'node1': state changed from 'unknown' =>
'online'
+info 20node1/crm: node 'node2': state changed from 'unknown' =>
'online'
+info 20node1/crm: node 'node3': state changed from 'unknown' =>
'online'
+info 20node1/crm: adding new service 'fa:1001' on node 'node3'
+info 20node1/crm: adding new service 'vm:101' on node 'node1'
+info 20node1/crm: adding new service 'vm:102' on node 'node2'
+info 20node1/crm: adding new service 'vm:103' on node 'node3'
+info 21node1/lrm: got lock 'ha_agent_node1_lock'
+info 21node1/lrm: status change wait_for_agent_lock => active
+info 21node1/lrm: starting service vm:101
+info 21node1/lrm: service status vm:101 started
+info 22node2/crm: status change wait_for_quorum => slave
+info 23node2/lrm: got lock 'ha_agent_node2_lock'
+info 23node2/lrm: status change wait_for_agent_lock => active
+info 23node2/lrm: starting service vm:102
+info 23node2/lrm: service status vm:102 started
+info 24node3/crm: status change wait_for_quorum => slave
+info 25node3/lrm: got lock 'ha_agent_node3_lock'
+info 25node3/lrm: status change wait_for_agent_lock => active
+info 25node3/lrm: starting service fa:1001
+info 25node3/lrm: service status fa:1001 started
+info 25node3/lrm: starting service vm:103
+info 25node3/lrm: service status vm:103 started
+info120 cmdlist: execute service vm:101 stop 0
+info120node1/crm: got crm command: stop vm:101 0
+info120node1/crm: stop service with timeout '0'
+info120node1/crm: service 'vm:101': state changed from 'started' to
'request_stop' (timeout = 0)
+info121node1/lrm: stopping service vm:101 (timeout=0)
+info121node1/lrm: service status vm:101 stopped
+info140node1/crm: service 'vm:101': state changed from 'request_stop'
to 'stopped'
+info220 cmdlist: execute service vm:101 stop 1
+info220node1/crm: got crm command: stop vm:101 1
+info220node1/crm: ignore service 'vm:101' stop request - serv
[pve-devel] [PATCH v3 ha-manager 6/9] Add stop command to simulation
Signed-off-by: Fabian Ebner
---
src/PVE/HA/Sim/Hardware.pm | 8
1 file changed, 8 insertions(+)
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 3cdc85b..121cd1b 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -541,6 +541,7 @@ sub get_cfs_state {
# restart-lrm
# service
# service
+# service stop
# service lock/unlock [lockname]
sub sim_hardware_cmd {
@@ -658,6 +659,13 @@ sub sim_hardware_cmd {
$self->queue_crm_commands_nolock("$action $sid $param");
+ } elsif ($action eq 'stop') {
+
+ die "sim_hardware_cmd: missing timeout for '$action' command"
+ if !defined($param);
+
+ $self->queue_crm_commands_nolock("$action $sid $param");
+
} elsif ($action eq 'add') {
$self->add_service($sid, {state => 'started', node => $param});
--
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 v3 ha-manager 0/9] Implement a stop command for HA
This patch series introduces a new 'stop' command for ha-manager. The command takes a timeout parameter and in case it is 0, it performs a hard stop. The series also includes a test for the new command. A few changes to how parameters were handled in CRM/LRM were necessary as well as allowing the service config to be updated from within the manager. Changes from v2: * Reorder parameters in 'update_service_config' to make it cleaner * Squash old 05/06 which introduce the timeout for shutdown * Do not expose the stop command to the API, instead introduce a crm-command command in the CLI and implement stop as a subcommand locally Fabian Ebner (9): Move code updating resource config from API2::HA::Resources to HA::Config Add update_service_config to HA environment interface Implement update_service_config for simulation Add timeout parameter for shutdown Introduce crm-command to CLI and add stop as a subcommand Add stop command to simulation Add crm command 'stop' Log timeout parameter when present Add test for the stop command src/PVE/API2/HA/Resources.pm| 34 +- src/PVE/CLI/ha_manager.pm | 46 +- src/PVE/HA/Config.pm| 37 +++ src/PVE/HA/Env.pm | 6 ++ src/PVE/HA/Env/PVE2.pm | 6 ++ src/PVE/HA/LRM.pm | 10 ++- src/PVE/HA/Manager.pm | 27 ++-- src/PVE/HA/Resources.pm | 2 +- src/PVE/HA/Resources/PVECT.pm | 14 +++-- src/PVE/HA/Resources/PVEVM.pm | 16 +++-- src/PVE/HA/Sim/Env.pm | 6 ++ src/PVE/HA/Sim/Hardware.pm | 22 +++ src/test/test-stop-command1/README | 2 + src/test/test-stop-command1/cmdlist | 8 +++ src/test/test-stop-command1/hardware_status | 5 ++ src/test/test-stop-command1/log.expect | 69 + src/test/test-stop-command1/manager_status | 1 + src/test/test-stop-command1/service_config | 6 ++ 18 files changed, 265 insertions(+), 52 deletions(-) create mode 100644 src/test/test-stop-command1/README create mode 100644 src/test/test-stop-command1/cmdlist create mode 100644 src/test/test-stop-command1/hardware_status create mode 100644 src/test/test-stop-command1/log.expect create mode 100644 src/test/test-stop-command1/manager_status create mode 100644 src/test/test-stop-command1/service_config -- 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 v3 ha-manager 3/9] Implement update_service_config for simulation
Signed-off-by: Fabian Ebner
---
src/PVE/HA/Sim/Hardware.pm | 14 ++
1 file changed, 14 insertions(+)
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 9c0ad05..3cdc85b 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -109,6 +109,20 @@ sub read_service_config {
return $conf;
}
+sub update_service_config {
+my ($self, $sid, $param) = @_;
+
+my $conf = $self->read_service_config();
+
+my $sconf = $conf->{$sid} || die "no such resource '$sid'\n";
+
+foreach my $k (%$param) {
+ $sconf->{$k} = $param->{$k};
+}
+
+$self->write_service_config($conf);
+}
+
sub write_service_config {
my ($self, $conf) = @_;
--
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 v3 ha-manager 7/9] Add crm command 'stop'
Not every command parameter is 'target' anymore, so
it was necessary to modify the parsing of $sd->{cmd}.
Just changing the state to request_stop is not enough,
we need to actually update the service configuration as well.
Signed-off-by: Fabian Ebner
---
src/PVE/HA/Manager.pm | 27 +++
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 5137de8..617e369 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -349,6 +349,14 @@ sub update_crm_commands {
$haenv->log('err', "crm command error - no such service: $cmd");
}
+ } elsif ($cmd =~ m/^stop\s+(\S+)\s+(\S+)$/) {
+ my ($sid, $timeout) = ($1, $2);
+ if (my $sd = $ss->{$sid}) {
+ $haenv->log('info', "got crm command: $cmd");
+ $ss->{$sid}->{cmd} = [ 'stop', $timeout ];
+ } else {
+ $haenv->log('err', "crm command error - no such service: $cmd");
+ }
} else {
$haenv->log('err', "unable to parse crm command: $cmd");
}
@@ -561,10 +569,10 @@ sub next_state_stopped {
}
if ($sd->{cmd}) {
- my ($cmd, $target) = @{$sd->{cmd}};
- delete $sd->{cmd};
+ my $cmd = shift @{$sd->{cmd}};
if ($cmd eq 'migrate' || $cmd eq 'relocate') {
+ my $target = shift @{$sd->{cmd}};
if (!$ns->node_is_online($target)) {
$haenv->log('err', "ignore service '$sid' $cmd request - node
'$target' not online");
} elsif ($sd->{node} eq $target) {
@@ -574,9 +582,12 @@ sub next_state_stopped {
target => $target);
return;
}
+ } elsif ($cmd eq 'stop') {
+ $haenv->log('info', "ignore service '$sid' $cmd request -
service already stopped");
} else {
$haenv->log('err', "unknown command '$cmd' for service '$sid'");
}
+ delete $sd->{cmd};
}
if ($cd->{state} eq 'disabled') {
@@ -638,10 +649,10 @@ sub next_state_started {
if ($cd->{state} eq 'started') {
if ($sd->{cmd}) {
- my ($cmd, $target) = @{$sd->{cmd}};
- delete $sd->{cmd};
+ my $cmd = shift @{$sd->{cmd}};
if ($cmd eq 'migrate' || $cmd eq 'relocate') {
+ my $target = shift @{$sd->{cmd}};
if (!$ns->node_is_online($target)) {
$haenv->log('err', "ignore service '$sid' $cmd request -
node '$target' not online");
} elsif ($sd->{node} eq $target) {
@@ -650,9 +661,17 @@ sub next_state_started {
$haenv->log('info', "$cmd service '$sid' to node
'$target'");
&$change_service_state($self, $sid, $cmd, node =>
$sd->{node}, target => $target);
}
+ } elsif ($cmd eq 'stop') {
+ my $timeout = shift @{$sd->{cmd}};
+ $haenv->log('info', "$cmd service with timeout '$timeout'");
+ &$change_service_state($self, $sid, 'request_stop', timeout =>
$timeout);
+ $haenv->update_service_config($sid, {'state' => 'stopped'});
} else {
$haenv->log('err', "unknown command '$cmd' for service '$sid'");
}
+
+ delete $sd->{cmd};
+
} else {
my $try_next = 0;
--
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 v2 guest-common 03/18] refactor code from qm/pct 'pending' call into AbstractConfig
container hasn't any pending calls to factor out at this point,
subject makes no sense?
Also hows "refactor code from somwhere" a telling subject?
how about:
> helpers: add pending-aware guest config printer from qemu-server
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> PVE/GuestHelpers.pm | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/PVE/GuestHelpers.pm b/PVE/GuestHelpers.pm
> index ebe2781..a16433f 100644
> --- a/PVE/GuestHelpers.pm
> +++ b/PVE/GuestHelpers.pm
> @@ -60,4 +60,30 @@ sub exec_hookscript {
> }
> }
>
> +sub format_pending {
> +my ($data) = @_;
add new line here
> +foreach my $item (sort { $a->{key} cmp $b->{key}} @$data) {
> + my $k = $item->{key};
> + next if $k eq 'digest';
> + my $v = $item->{value};
> + my $p = $item->{pending};
newline here and while I now that the origin of this in the qm CLI
module uses the same variable names, I'd still do:
s/k/key/
s/v/value/
s/p/pending/
> + if ($k eq 'description') {
> + $v = PVE::Tools::encode_text($v) if defined($v);
> + $p = PVE::Tools::encode_text($p) if defined($p);
> + }
> + if (defined($v)) {
> + if ($item->{delete}) {
> + print "del $k: $v\n";
> + } elsif (defined($p)) {
> + print "cur $k: $v\n";
> + print "new $k: $p\n";
> + } else {
> + print "cur $k: $v\n";
> + }
> + } elsif (defined($p)) {
> + print "new $k: $p\n";
> + }
> +}
> +}
> +
> 1;
>
___
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 04/18] overwrite load_current_config from AbstractConfig
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> we overwrite the load_current_config method from AbstractConfig, in
> order to handle cipassword.
>
see my reply to 05/18 regarding squashing, as when thinking about a
sense full commit message for this the felling that this does not
deserves to be a stand alone patch just got much much stronger.
> Signed-off-by: Oguz Bektas
> ---
> PVE/QemuConfig.pm | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index edbf1a7..7f229b3 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -397,6 +397,20 @@ sub __snapshot_foreach_volume {
>
> PVE::QemuServer::foreach_drive($conf, $func);
> }
> +
> +sub load_current_config {
> +my ($class, $vmid, $snapname, $current) = @_;
> +
> +my $conf = $class->SUPER::load_current_config($vmid, $snapname,
> $current);
> +
> +if ($conf->{cipassword}) {
> + $conf->{cipassword} = '**';
> +}
> +
> +return $conf;
> +}
> +
> +
> # END implemented abstract methods from PVE::AbstractConfig
>
> 1;
>
___
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 06/18] refactor pending changes related code
subject:
> use new config helpers from guest-common for pending changes
from a quick glance: OK besides that
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> most of the pending changes related code has been moved into
> AbstractConfig, so we have to call them as class methods from QemuConfig
> instead.
>
> Signed-off-by: Oguz Bektas
> ---
> PVE/API2/Qemu.pm | 14 -
> PVE/QemuServer.pm | 79 +--
> 2 files changed, 15 insertions(+), 78 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index aa1cd16..9dc07a6 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -919,7 +919,7 @@ __PACKAGE__->register_method({
>
> my $conf = PVE::QemuConfig->load_config($param->{vmid});
>
> - my $pending_delete_hash =
> PVE::QemuServer::split_flagged_list($conf->{pending}->{delete});
> + my $pending_delete_hash =
> PVE::QemuConfig->parse_pending_delete($conf->{pending}->{delete});
>
> my $res = [];
>
> @@ -1170,7 +1170,7 @@ my $update_vm_api = sub {
> $rpcenv->check_vm_perm($authuser, $vmid, undef,
> ['VM.Config.Disk']);
> PVE::QemuServer::vmconfig_register_unused_drive($storecfg,
> $vmid, $conf, PVE::QemuServer::parse_drive($opt, $val))
> if $is_pending_val;
> - PVE::QemuServer::vmconfig_delete_pending_option($conf,
> $opt, $force);
> + PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> PVE::QemuConfig->write_config($vmid, $conf);
> } elsif ($opt =~ m/^serial\d+$/) {
> if ($val eq 'socket') {
> @@ -1178,7 +1178,7 @@ my $update_vm_api = sub {
> } elsif ($authuser ne 'root@pam') {
> die "only root can delete '$opt' config for real
> devices\n";
> }
> - PVE::QemuServer::vmconfig_delete_pending_option($conf,
> $opt, $force);
> + PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> PVE::QemuConfig->write_config($vmid, $conf);
> } elsif ($opt =~ m/^usb\d+$/) {
> if ($val =~ m/spice/) {
> @@ -1186,10 +1186,10 @@ my $update_vm_api = sub {
> } elsif ($authuser ne 'root@pam') {
> die "only root can delete '$opt' config for real
> devices\n";
> }
> - PVE::QemuServer::vmconfig_delete_pending_option($conf,
> $opt, $force);
> + PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> PVE::QemuConfig->write_config($vmid, $conf);
> } else {
> - PVE::QemuServer::vmconfig_delete_pending_option($conf,
> $opt, $force);
> + PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> PVE::QemuConfig->write_config($vmid, $conf);
> }
> }
> @@ -1230,13 +1230,13 @@ my $update_vm_api = sub {
> } else {
> $conf->{pending}->{$opt} = $param->{$opt};
> }
> - PVE::QemuServer::vmconfig_undelete_pending_option($conf, $opt);
> + PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
> PVE::QemuConfig->write_config($vmid, $conf);
> }
>
> # remove pending changes when nothing changed
> $conf = PVE::QemuConfig->load_config($vmid); # update/reload
> - my $changes = PVE::QemuServer::vmconfig_cleanup_pending($conf);
> + my $changes = PVE::QemuConfig->cleanup_pending($conf);
> PVE::QemuConfig->write_config($vmid, $conf) if $changes;
>
> return if !scalar(keys %{$conf->{pending}});
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 70ed910..6c499c9 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2345,40 +2345,6 @@ sub vm_is_volid_owner {
> return undef;
> }
>
> -sub split_flagged_list {
> -my $text = shift || '';
> -$text =~ s/[,;]/ /g;
> -$text =~ s/^\s+//;
> -return { map { /^(!?)(.*)$/ && ($2, $1) } ($text =~ /\S+/g) };
> -}
> -
> -sub join_flagged_list {
> -my ($how, $lst) = @_;
> -join $how, map { $lst->{$_} . $_ } keys %$lst;
> -}
> -
> -sub vmconfig_delete_pending_option {
> -my ($conf, $key, $force) = @_;
> -
> -delete $conf->{pending}->{$key};
> -my $pending_delete_hash = split_flagged_list($conf->{pending}->{delete});
> -$pending_delete_hash->{$key} = $force ? '!' : '';
> -$conf->{pending}->{delete} = join_flagged_list(',',
> $pending_delete_hash);
> -}
> -
> -sub vmconfig_undelete_pending_option {
> -my ($conf, $key) = @_;
> -
> -my $pending_delete_hash = split_flagged_list($conf->{pending}->{delete});
> -delete $pending_delete_hash->{$key};
> -
> -if (%$pending_delete_hash) {
> - $conf->{pending}->{delete} = join_flagged_list(',',
> $pending_delete_hash);
> -} else {
> - delete
Re: [pve-devel] [PATCH v2 qemu-server 07/18] use format_pending from GuestHelpers for 'qm pending' command
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
Acked-by: Thomas Lamprecht
Reviewed-by: Thomas Lamprecht
> ---
> PVE/CLI/qm.pm | 28 +---
> 1 file changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 17935d0..2f1969a 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -898,33 +898,7 @@ our $cmddef = {
> }
> }],
>
> -pending => [ "PVE::API2::Qemu", 'vm_pending', ['vmid'],
> - { node => $nodename }, sub {
> - my $data = shift;
> - foreach my $item (sort { $a->{key} cmp $b->{key}} @$data) {
> - my $k = $item->{key};
> - next if $k eq 'digest';
> - my $v = $item->{value};
> - my $p = $item->{pending};
> - if ($k eq 'description') {
> - $v = PVE::Tools::encode_text($v) if defined($v);
> - $p = PVE::Tools::encode_text($p) if defined($p);
> - }
> - if (defined($v)) {
> - if ($item->{delete}) {
> - print "del $k: $v\n";
> - } elsif (defined($p)) {
> - print "cur $k: $v\n";
> - print "new $k: $p\n";
> - } else {
> - print "cur $k: $v\n";
> - }
> - } elsif (defined($p)) {
> - print "new $k: $p\n";
> - }
> - }
> - }],
> -
> +pending => [ "PVE::API2::Qemu", 'vm_pending', ['vmid'], { node =>
> $nodename }, \&PVE::GuestHelpers::format_pending ],
> showcmd => [ __PACKAGE__, 'showcmd', ['vmid']],
>
> status => [ __PACKAGE__, 'status', ['vmid']],
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 10/18] adapt CT config parser for pending changes
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> config parser can now read/write [pve:pending] section. this was named
> such, instead of [PENDING], after on- and offline discussion regarding
> namespacing the pending section and snapshots.
>
> this also adds an optional non-capturing regex group into the parser for
> [snap: snapname] entries which can be supported in PVE 7.0
1. completely nothing to do with pending changes itself -> separate patch
2. they could be supported in 6.x already? PVE cluster host need to be on the
same version of software, we guarantee only that old -> new and new <-> new
works, so doing this now is no issue... Else we never could add any new
feature in a stable release..
Also this is missing from qemu-server?
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC/Config.pm | 37 -
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 9790345..47bd4bb 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -751,6 +751,7 @@ sub parse_pct_config {
> my $res = {
> digest => Digest::SHA::sha1_hex($raw),
> snapshots => {},
> + pending => {},
> };
>
> $filename =~ m|/lxc/(\d+).conf$|
> @@ -766,7 +767,14 @@ sub parse_pct_config {
> foreach my $line (@lines) {
> next if $line =~ m/^\s*$/;
>
> - if ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
> + if ($line =~ m/^\[pve:pending\]\s*$/i) {
why not add the new stuff to the end of the elsif block? less churn..
> + $section = 'pending';
> + $conf->{description} = $descr if $descr;
> + $descr = '';
> + $conf = $res->{$section} = {};
> + next;
> + } elsif ($line =~ m/^\[(?:snap:)?([a-z][a-z0-9_\-]+)\]\s*$/i) {
> + # extended regex for namespacing snapshots in PVE 7.0
> $section = $1;
> $conf->{description} = $descr if $descr;
> $descr = '';
> @@ -794,6 +802,13 @@ sub parse_pct_config {
> $descr .= PVE::Tools::decode_text($2);
> } elsif ($line =~ m/snapstate:\s*(prepare|delete)\s*$/) {
> $conf->{snapstate} = $1;
> + } elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
> + my $value = $1;
> + if ($section eq 'pending') {
> + $conf->{delete} = $value;
> + } else {
> + warn "vm $vmid - property 'delete' is only allowed in
> [pve:pending]\n";
> + }
> } elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S.*)\s*$/) {
> my $key = $1;
> my $value = $2;
> @@ -832,14 +847,19 @@ sub write_pct_config {
> }
>
> my $generate_raw_config = sub {
> - my ($conf) = @_;
> + my ($conf, $pending) = @_;
>
> my $raw = '';
>
> # add description as comment to top of file
> - my $descr = $conf->{description} || '';
> - foreach my $cl (split(/\n/, $descr)) {
> - $raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
> + if (defined(my $descr = $conf->{description})) {
> + if ($descr) {
> + foreach my $cl (split(/\n/, $descr)) {
> + $raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
> + }
> + } else {
> + $raw .= "#\n" if $pending;
> + }
> }
>
> foreach my $key (sort keys %$conf) {
> @@ -864,7 +884,14 @@ sub write_pct_config {
>
> my $raw = &$generate_raw_config($conf);
>
> +if (scalar(keys %{$conf->{pending}})){
> + $raw .= "\n[pve:pending]\n";
> + $raw .= &$generate_raw_config($conf->{pending}, 1);
> +}
> +
> foreach my $snapname (sort keys %{$conf->{snapshots}}) {
> + # TODO: namespace snapshots for PVE 7.0
> + #$raw .= "\n[snap:$snapname]\n";
> $raw .= "\n[$snapname]\n";
> $raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
> }
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 11/18] use load_current_config for config GET call
subject proposal:
> api: config: add 'current' param to get config without pending changes
commit message:
> container can now also have pending changes, thus we want to have a method
> to get the current applied config and the one with pending changes. This
> makes the GET config API more consistent with the qemu-server one by reusing
> the load_current_config through the AbstractConfig interface.
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/API2/LXC/Config.pm | 22 --
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> index 769fc3b..7eaef74 100644
> --- a/src/PVE/API2/LXC/Config.pm
> +++ b/src/PVE/API2/LXC/Config.pm
> @@ -34,6 +34,12 @@ __PACKAGE__->register_method({
> properties => {
> node => get_standard_option('pve-node'),
> vmid => get_standard_option('pve-vmid', { completion =>
> \&PVE::LXC::complete_ctid }),
> + current => {
> + description => "Get current values (instead of pending
> values).",
> + optional => 1,
> + default => 0,
> + type => 'boolean',
> + },
> snapshot => get_standard_option('pve-snapshot-name', {
> description => "Fetch config values from given snapshot.",
> optional => 1,
> @@ -62,22 +68,10 @@ __PACKAGE__->register_method({
> code => sub {
> my ($param) = @_;
>
> - my $conf = PVE::LXC::Config->load_config($param->{vmid});
> -
> - if (my $snapname = $param->{snapshot}) {
> - my $snapshot = $conf->{snapshots}->{$snapname};
> - die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
> -
> - # we need the digest of the file
> - $snapshot->{digest} = $conf->{digest};
> - $conf = $snapshot;
> - }
> -
> - delete $conf->{snapshots};
> -
> - return $conf;
> + return PVE::LXC::Config->load_current_config($param->{vmid},
> $param->{snapshot}, $param->{current})
> }});
>
> +
> my $vm_config_perm_list = [
> 'VM.Config.Disk',
> 'VM.Config.CPU',
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 12/18] skip pending changes while cloning
some rationale here would be great, e.g.:
> we only can clone from the current applied state, as else
> the on-disk state may not match the one from the config. Further
> this makes it more consistent with the qemu-server behavior.
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/API2/LXC.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 9c040d1..2cb9f9d 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1441,6 +1441,7 @@ __PACKAGE__->register_method({
> # Replace the 'disk' lock with a 'create' lock.
> $newconf->{lock} = 'create';
>
> + delete $newconf->{pending};
> delete $newconf->{template};
> if ($param->{hostname}) {
> $newconf->{hostname} = $param->{hostname};
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 14/18] apply pending changes during container start
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC.pm | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 475d9be..65c41f5 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1930,6 +1930,13 @@ sub userns_command {
> sub vm_start {
> my ($vmid, $conf, $skiplock) = @_;
>
> +# apply pending changes while starting
> +my $storecfg = PVE::Storage::config();
only get $storecfg if required, i.e., in the if branch?
> +if (scalar(keys %{$conf->{pending}})) {
> + PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storecfg);
> + $conf = PVE::LXC::Config->load_config($vmid); # update/reload
> +}
> +
> update_lxc_config($vmid, $conf);
>
> my $skiplock_flag_fn = "/run/lxc/skiplock-$vmid";
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 15/18] add revert parameter to config PUT
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/API2/LXC/Config.pm | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> index 7eaef74..2c036f5 100644
> --- a/src/PVE/API2/LXC/Config.pm
> +++ b/src/PVE/API2/LXC/Config.pm
> @@ -102,6 +102,11 @@ __PACKAGE__->register_method({
> description => "A list of settings you want to delete.",
> optional => 1,
> },
> + revert => {
> + type => 'string', format => 'pve-configid-list',
> + description => "Revert a pending change.",
> + optional => 1,
> + },
> digest => {
> type => 'string',
> description => 'Prevent changes if current configuration
> file has different SHA1 digest. This can be used to prevent concurrent
> modifications.',
>
does nothing here, and no rationale why this works now (if it worked) or
when (in a future patch, or?) this will work or do something at all, in the
commit message...
I'd squash it in 16/18 or at least order it after it..
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 17/18] rework update_pct_config to write into pending section
On 9/30/19 2:44 PM, Oguz Bektas wrote:
> use vmconfig_hotplug_pending or vmconfig_apply_pending to apply the
> pending changes, depending on whether the CT is on- or offline.
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC/Config.pm | 334 ++
> 1 file changed, 106 insertions(+), 228 deletions(-)
>
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 47bd4bb..14c26bc 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -4,11 +4,13 @@ use strict;
> use warnings;
>
> use PVE::AbstractConfig;
> +use PVE::RPCEnvironment;
> use PVE::Cluster qw(cfs_register_file);
> use PVE::GuestHelpers;
> use PVE::INotify;
> +use PVE::Exception qw(raise_param_exc);
> use PVE::JSONSchema qw(get_standard_option);
> -use PVE::Tools;
> +use PVE::Tools qw(extract_param);
>
> use base qw(PVE::AbstractConfig);
>
> @@ -900,267 +902,143 @@ sub write_pct_config {
> }
>
> sub update_pct_config {
> -my ($class, $vmid, $conf, $running, $param, $delete) = @_;
> +my ($class, $vmid, $conf, $running, $param) = @_;
you already use the new $param behavior and remove the $delete
one from the api/config PUT call in 16/18 but only make it work
here? NAK!
Breaks things, even if only temporarily, and makes review unnecessary
harder.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 guest-common 01/18] refactor pending changes related config code into AbstractConfig
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> also use a better naming scheme for methods:
>
> split_flagged_list -> parse_pending_delete
> join_flagged_list -> print_pending_delete
> vmconfig_delete_pending_option -> add_to_pending_delete
> vmconfig_undelete_pending_option -> remove_from_pending_delete
> vmconfig_cleanup_pending -> cleanup_pending
>
> Signed-off-by: Oguz Bektas
> ---
> PVE/AbstractConfig.pm | 68 +++
> 1 file changed, 68 insertions(+)
>
> diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
> index e0d0f10..910ca86 100644
> --- a/PVE/AbstractConfig.pm
> +++ b/PVE/AbstractConfig.pm
> @@ -68,6 +68,74 @@ sub write_config {
> PVE::Cluster::cfs_write_file($cfspath, $conf);
> }
>
> +# Pending changes related
> +
> +sub parse_pending_delete {
> +my ($class, $text) = @_;
conventionally, $text is called $data in our parse_ subs
> +$text ||= '';
> +$text =~ s/[,;]/ /g;
> +$text =~ s/^\s+//;
> +return { map { /^(!?)(.*)$/ && ($2, $1) } ($text =~ /\S+/g) };
like I said in v1 of this part, why not use the opportunity to give this
a sane perl representation?
e.g.,
{
'opt1' => { 'force' => 0 },
'opt2' => { 'force' => 1 },
'opt3' => { 'force' => 0 },
}
then you could still iterate over all pending deletions, and where you
need that info, you can check if $pending_delete_hash->{$opt}->{force}?
right now, if you accidentally do
if ($pending_delete_hash->{$opt})
instead of
if (defined($pending_delete_hash->{$opt}))
or
if (exists($pending_delete_hash->{$opt}))
you probably do the opposite of what you want ;) there is only a single
such caller atm, and that one is correct, but it's very easy to get
wrong IMHO.
> +}
> +
> +sub print_pending_delete {
> +my ($class, $how, $lst) = @_;
> +join $how, map { $lst->{$_} . $_ } keys %$lst;
we always want to join with ',', so why make it configurable? also, this
ain't no l(i)st anymore ;) if we change the $pending_delete_hash
structure, this obviously needs to be adapted to encode the
'forcefulness' correctly.
> +}
> +
> +sub add_to_pending_delete {
> +my ($class, $conf, $key, $force) = @_;
> +
> +delete $conf->{pending}->{$key};
> +my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> +$pending_delete_hash->{$key} = $force ? '!' : '';
> +$conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> +}
also would need adaption if we don't store '!' in the hash.
> +
> +sub remove_from_pending_delete {
> +my ($class, $conf, $key) = @_;
> +
> +my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> +delete $pending_delete_hash->{$key};
> +
> +if (%$pending_delete_hash) {
> + $conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> +} else {
> + delete $conf->{pending}->{delete};
> +}
> +}
> +
> +sub cleanup_pending {
> +my ($class, $conf) = @_;
> +
> +# remove pending changes when nothing changed
> +my $changes;
> +foreach my $opt (keys %{$conf->{pending}}) {
> + if (defined($conf->{$opt}) && ($conf->{pending}->{$opt} eq
> $conf->{$opt})) {
> + $changes = 1;
> + delete $conf->{pending}->{$opt};
> + }
> +}
> +
> +my $current_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> +my $pending_delete_hash = {};
> +while (my ($opt, $force) = each %$current_delete_hash) {
> + if (defined($conf->{$opt})) {
> + $pending_delete_hash->{$opt} = $force;
> + } else {
> + $changes = 1;
> + }
> +}
> +
> +if (%$pending_delete_hash) {
> + $conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> +} else {
> + delete $conf->{pending}->{delete};
> +}
> +
> +return $changes;
> +}
> +
> # Lock config file using flock, run $code with @param, unlock config file.
> # $timeout is the maximum time to aquire the flock
> sub lock_config_full {
> --
> 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 v2 guest-common 02/18] refactor method used by config GET calls into AbstractConfig
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> since this method will be both used by qemu and lxc config GET calls, it
> makes sense to move it into AbstractConfig. only difference is that qemu
> also hides the cipassword when it's set. this can be handled by having
> qemu overwrite the method and add the cipassword code.
>
> Signed-off-by: Oguz Bektas
> ---
> PVE/AbstractConfig.pm | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
> index 910ca86..6d3f169 100644
> --- a/PVE/AbstractConfig.pm
> +++ b/PVE/AbstractConfig.pm
> @@ -136,6 +136,41 @@ sub cleanup_pending {
> return $changes;
> }
>
> +sub load_current_config {
like I said when we discussed this offline, that name is at most a
working title :-P the method either returns
A the top level config, ignoring any pending changes (called 'current'
in the API schema)
B the top level config, with any pending changes pseudo-applied (the
opposite of 'current')
C the config of a specific snapshot + digest of whole file (also not very
'current').
we can either split this up into
sub load_config_snapshot => returns C
sub load_ ? => returns A or B, depending on parameter
or make a
sub load_config_filtered {
my ($class, $vmid, $filter) = @_;
where $filter is a hash, currently with
{ 'snapshot' => $snapname }
or
{ 'current' => 1 }
both at once don't make sense, and should probably lead to death.
I'd prefer the split, since the first half for case C, and the second
half for cases A and B don't have much in common anyway. if we have some
other filters in mind that we might add in the future, then the $filter
variant might be more attractive.
> +my ($class, $vmid, $snapname, $current) = @_;
> +
> +my $conf = $class->load_config($vmid);
> +
> +if ($snapname) {
> + my $snapshot = $conf->{snapshots}->{$snapname};
> + die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
> +
> + # we need the digest of the file
> + $snapshot->{digest} = $conf->{digest};
> + $conf = $snapshot;
> +}
> +
> +# take pending changes in
> +if (!$current) {
> + foreach my $opt (keys %{$conf->{pending}}) {
> + next if $opt eq 'delete';
> + my $value = $conf->{pending}->{$opt};
> + next if ref($value); # just to be sure
> + $conf->{$opt} = $value;
> + }
> + my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> + foreach my $opt (keys %$pending_delete_hash) {
> + delete $conf->{$opt} if $conf->{$opt};
> + }
> +}
> +
> +delete $conf->{snapshots};
> +delete $conf->{pending};
> +
> +return $conf;
> +}
> +
> +
> # Lock config file using flock, run $code with @param, unlock config file.
> # $timeout is the maximum time to aquire the flock
> sub lock_config_full {
> --
> 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 v2 container 08/18] add lxc/pending API path
you completely ignored my comments for v1 of this patch? the whole code
is identical to qemu-server's, except for cipassword handling. pending
changes are also encoded identically for both pve-container and
qemu-server, so it makes sense to move this to AbstractConfig or
GuestHelpers.pm with an override for cipassword in QemuConfig.pm/here..
this basically just converts $conf into a different representation for
easier client consumption..
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/API2/LXC.pm | 88 +
> 1 file changed, 88 insertions(+)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 07280fb..9c040d1 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -515,6 +515,7 @@ __PACKAGE__->register_method({
>
> my $res = [
> { subdir => 'config' },
> + { subdir => 'pending' },
> { subdir => 'status' },
> { subdir => 'vncproxy' },
> { subdir => 'termproxy' },
> @@ -1865,4 +1866,91 @@ __PACKAGE__->register_method({
> return $task;
>}});
>
> +__PACKAGE__->register_method({
> +name => 'vm_pending',
> +path => '{vmid}/pending',
> +method => 'GET',
> +proxyto => 'node',
> +description => 'Get container configuration, including pending changes.',
> +permissions => {
> + check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
> +},
> +parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + vmid => get_standard_option('pve-vmid', { completion =>
> \&PVE::LXC::complete_ctid }),
> + },
> +},
> +returns => {
> + type => "array",
> + items => {
> + type => "object",
> + properties => {
> + key => {
> + description => 'Configuration option name.',
> + type => 'string',
> + },
> + value => {
> + description => 'Current value.',
> + type => 'string',
> + optional => 1,
> + },
> + pending => {
> + description => 'Pending value.',
> + type => 'string',
> + optional => 1,
> + },
> + delete => {
> + description => "Indicates a pending delete request if
> present and not 0.",
> + type => 'integer',
> + minimum => 0,
> + maximum => 1,
> + optional => 1,
> + },
> + },
> + },
> +},
> +code => sub {
> + my ($param) = @_;
> +
> + my $conf = PVE::LXC::Config->load_config($param->{vmid});
> +
> + my $pending_delete_hash =
> PVE::LXC::Config->parse_pending_delete($conf->{pending}->{delete});
> +
> + my $res = [];
> +
> + foreach my $opt (keys %$conf) {
> + next if ref($conf->{$opt});
> + next if $opt eq 'pending';
> + my $item = { key => $opt } ;
> + $item->{value} = $conf->{$opt} if defined($conf->{$opt});
> + $item->{pending} = $conf->{pending}->{$opt} if
> defined($conf->{pending}->{$opt});
> + $item->{delete} = ($pending_delete_hash->{$opt} ? 2 : 1) if exists
> $pending_delete_hash->{$opt};
> +
> + push @$res, $item;
> + }
> +
> + foreach my $opt (keys %{$conf->{pending}}) {
> + next if $opt eq 'delete';
> + next if ref($conf->{pending}->{$opt});
> + next if defined($conf->{$opt});
> + my $item = { key => $opt };
> + $item->{pending} = $conf->{pending}->{$opt};
> +
> + push @$res, $item;
> + }
> +
> + # FIXME: $force delete is not implemented for CTs
> + while (my ($opt, undef) = each %$pending_delete_hash) {
> + next if $conf->{pending}->{$opt};
> + next if $conf->{$opt};
> + my $item = { key => $opt, delete => 1 };
> + push @$res, $item;
> + }
> +
> + return $res;
> +
> +}});
> +
> 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 mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 container 10/18] adapt CT config parser for pending changes
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> config parser can now read/write [pve:pending] section. this was named
> such, instead of [PENDING], after on- and offline discussion regarding
> namespacing the pending section and snapshots.
>
> this also adds an optional non-capturing regex group into the parser for
> [snap: snapname] entries which can be supported in PVE 7.0
like Thomas said, please split these two (the latter is not even related
to this series at all! while I agree it's good to do it now, together
with the same change on qemu-server's part, let's not make this series
any bigger than it needs to be ;))
one question inline
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC/Config.pm | 37 -
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 9790345..47bd4bb 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -751,6 +751,7 @@ sub parse_pct_config {
> my $res = {
> digest => Digest::SHA::sha1_hex($raw),
> snapshots => {},
> + pending => {},
> };
>
> $filename =~ m|/lxc/(\d+).conf$|
> @@ -766,7 +767,14 @@ sub parse_pct_config {
> foreach my $line (@lines) {
> next if $line =~ m/^\s*$/;
>
> - if ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
> + if ($line =~ m/^\[pve:pending\]\s*$/i) {
> + $section = 'pending';
> + $conf->{description} = $descr if $descr;
> + $descr = '';
> + $conf = $res->{$section} = {};
> + next;
> + } elsif ($line =~ m/^\[(?:snap:)?([a-z][a-z0-9_\-]+)\]\s*$/i) {
> + # extended regex for namespacing snapshots in PVE 7.0
> $section = $1;
> $conf->{description} = $descr if $descr;
> $descr = '';
> @@ -794,6 +802,13 @@ sub parse_pct_config {
> $descr .= PVE::Tools::decode_text($2);
> } elsif ($line =~ m/snapstate:\s*(prepare|delete)\s*$/) {
> $conf->{snapstate} = $1;
> + } elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
> + my $value = $1;
> + if ($section eq 'pending') {
> + $conf->{delete} = $value;
> + } else {
> + warn "vm $vmid - property 'delete' is only allowed in
> [pve:pending]\n";
> + }
> } elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S.*)\s*$/) {
> my $key = $1;
> my $value = $2;
> @@ -832,14 +847,19 @@ sub write_pct_config {
> }
>
> my $generate_raw_config = sub {
> - my ($conf) = @_;
> + my ($conf, $pending) = @_;
>
> my $raw = '';
>
> # add description as comment to top of file
> - my $descr = $conf->{description} || '';
> - foreach my $cl (split(/\n/, $descr)) {
> - $raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
> + if (defined(my $descr = $conf->{description})) {
> + if ($descr) {
> + foreach my $cl (split(/\n/, $descr)) {
> + $raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
> + }
> + } else {
> + $raw .= "#\n" if $pending;
> + }
that seems wrong.. why handle a pending description like that?
> }
>
> foreach my $key (sort keys %$conf) {
> @@ -864,7 +884,14 @@ sub write_pct_config {
>
> my $raw = &$generate_raw_config($conf);
>
> +if (scalar(keys %{$conf->{pending}})){
> + $raw .= "\n[pve:pending]\n";
> + $raw .= &$generate_raw_config($conf->{pending}, 1);
> +}
> +
> foreach my $snapname (sort keys %{$conf->{snapshots}}) {
> + # TODO: namespace snapshots for PVE 7.0
> + #$raw .= "\n[snap:$snapname]\n";
> $raw .= "\n[$snapname]\n";
> $raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
> }
> --
> 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 v2 container 10/18] adapt CT config parser for pending changes
On October 2, 2019 12:22 pm, Thomas Lamprecht wrote:
> On 9/30/19 2:44 PM, Oguz Bektas wrote:
>> config parser can now read/write [pve:pending] section. this was named
>> such, instead of [PENDING], after on- and offline discussion regarding
>> namespacing the pending section and snapshots.
>>
>> this also adds an optional non-capturing regex group into the parser for
>> [snap: snapname] entries which can be supported in PVE 7.0
>
> 1. completely nothing to do with pending changes itself -> separate patch
>
> 2. they could be supported in 6.x already? PVE cluster host need to be on the
> same version of software, we guarantee only that old -> new and new <-> new
> works, so doing this now is no issue... Else we never could add any new
> feature in a stable release..
>
> Also this is missing from qemu-server?
the problem is that this is not just a case of new -> old does not work,
but it's a case of new -> old silently drops parts (/most) of your guest
config.. since it's just nice to have, but not blocking anything
important, we should postpone it to 7.0 IMHO (where we can assume that
the 'old' 6.x parser already supports it, so the fallout is basically
non-existent at that point).
>
>>
>> Signed-off-by: Oguz Bektas
>> ---
>> src/PVE/LXC/Config.pm | 37 -
>> 1 file changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
>> index 9790345..47bd4bb 100644
>> --- a/src/PVE/LXC/Config.pm
>> +++ b/src/PVE/LXC/Config.pm
>> @@ -751,6 +751,7 @@ sub parse_pct_config {
>> my $res = {
>> digest => Digest::SHA::sha1_hex($raw),
>> snapshots => {},
>> +pending => {},
>> };
>>
>> $filename =~ m|/lxc/(\d+).conf$|
>> @@ -766,7 +767,14 @@ sub parse_pct_config {
>> foreach my $line (@lines) {
>> next if $line =~ m/^\s*$/;
>>
>> -if ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
>> +if ($line =~ m/^\[pve:pending\]\s*$/i) {
>
> why not add the new stuff to the end of the elsif block? less churn..
for qemu-server, we need to add it up-front to parse it earlier then a
snapshot called pending, this would keep the two parsers (that we
hopefully unify some day) closer together. OTOH, we'd drop all of this
if we unify it anyway, so..
>
>> +$section = 'pending';
>> +$conf->{description} = $descr if $descr;
>> +$descr = '';
>> +$conf = $res->{$section} = {};
>> +next;
>> +} elsif ($line =~ m/^\[(?:snap:)?([a-z][a-z0-9_\-]+)\]\s*$/i) {
>> +# extended regex for namespacing snapshots in PVE 7.0
>> $section = $1;
>> $conf->{description} = $descr if $descr;
>> $descr = '';
>> @@ -794,6 +802,13 @@ sub parse_pct_config {
>> $descr .= PVE::Tools::decode_text($2);
>> } elsif ($line =~ m/snapstate:\s*(prepare|delete)\s*$/) {
>> $conf->{snapstate} = $1;
>> +} elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
>> +my $value = $1;
>> +if ($section eq 'pending') {
>> +$conf->{delete} = $value;
>> +} else {
>> +warn "vm $vmid - property 'delete' is only allowed in
>> [pve:pending]\n";
>> +}
>> } elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S.*)\s*$/) {
>> my $key = $1;
>> my $value = $2;
>> @@ -832,14 +847,19 @@ sub write_pct_config {
>> }
>>
>> my $generate_raw_config = sub {
>> -my ($conf) = @_;
>> +my ($conf, $pending) = @_;
>>
>> my $raw = '';
>>
>> # add description as comment to top of file
>> -my $descr = $conf->{description} || '';
>> -foreach my $cl (split(/\n/, $descr)) {
>> -$raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
>> +if (defined(my $descr = $conf->{description})) {
>> +if ($descr) {
>> +foreach my $cl (split(/\n/, $descr)) {
>> +$raw .= '#' . PVE::Tools::encode_text($cl) . "\n";
>> +}
>> +} else {
>> +$raw .= "#\n" if $pending;
>> +}
>> }
>>
>> foreach my $key (sort keys %$conf) {
>> @@ -864,7 +884,14 @@ sub write_pct_config {
>>
>> my $raw = &$generate_raw_config($conf);
>>
>> +if (scalar(keys %{$conf->{pending}})){
>> +$raw .= "\n[pve:pending]\n";
>> +$raw .= &$generate_raw_config($conf->{pending}, 1);
>> +}
>> +
>> foreach my $snapname (sort keys %{$conf->{snapshots}}) {
>> +# TODO: namespace snapshots for PVE 7.0
>> +#$raw .= "\n[snap:$snapname]\n";
>> $raw .= "\n[$snapname]\n";
>> $raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
>> }
>>
>
>
> ___
> 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 v2 container 16/18] adapt config PUT method for the new update_pct_config
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> we don't need to extract 'delete' here, instead we pass it all as $param
> and extract 'delete', 'revert' and most other things in
> update_pct_config
I already asked that in v1 - wouldn't it make sense to keep the
parameter checks in the API call, and pass the already "verified"
$delete and $revert to update_pct_config? this would avoid the need of
adding new module dependencies from PVE::LXC::Config to
PVE::RPCEnvironment and PVE::Exception in #17..
if you see a good reason for moving all that to update_pct_config,
please say so (either in the changelog of the patch, or as reply to the
original review) and don't silently ignore open questions ;)
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/API2/LXC/Config.pm | 55 --
> 1 file changed, 5 insertions(+), 50 deletions(-)
>
> diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> index 2c036f5..46d8e2f 100644
> --- a/src/PVE/API2/LXC/Config.pm
> +++ b/src/PVE/API2/LXC/Config.pm
> @@ -119,58 +119,10 @@ __PACKAGE__->register_method({
> code => sub {
> my ($param) = @_;
>
> - my $rpcenv = PVE::RPCEnvironment::get();
> - my $authuser = $rpcenv->get_user();
> -
> my $node = extract_param($param, 'node');
> my $vmid = extract_param($param, 'vmid');
> -
> my $digest = extract_param($param, 'digest');
>
> - die "no options specified\n" if !scalar(keys %$param);
> -
> - my $delete_str = extract_param($param, 'delete');
> - my @delete = PVE::Tools::split_list($delete_str);
> -
> - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef,
> {}, [@delete]);
> -
> - foreach my $opt (@delete) {
> - raise_param_exc({ delete => "you can't use '-$opt' and -delete
> $opt' at the same time" })
> - if defined($param->{$opt});
> -
> - if (!PVE::LXC::Config->option_exists($opt)) {
> - raise_param_exc({ delete => "unknown option '$opt'" });
> - }
> - }
> -
> - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef,
> $param, []);
> -
> - my $storage_cfg = cfs_read_file("storage.cfg");
> -
> - my $repl_conf = PVE::ReplicationConfig->new();
> - my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
> - if ($is_replicated) {
> - PVE::LXC::Config->foreach_mountpoint_full($param, 0, sub {
> - my ($opt, $mountpoint) = @_;
> - my $volid = $mountpoint->{volume};
> - return if !$volid || !($mountpoint->{replicate}//1);
> - if ($mountpoint->{type} eq 'volume') {
> - my ($storeid, $format);
> - if ($volid =~ $PVE::LXC::NEW_DISK_RE) {
> - $storeid = $1;
> - $format = $mountpoint->{format} ||
> PVE::Storage::storage_default_format($storage_cfg, $storeid);
> - } else {
> - ($storeid, undef) =
> PVE::Storage::parse_volume_id($volid, 1);
> - $format = (PVE::Storage::parse_volname($storage_cfg,
> $volid))[6];
> - }
> - return if PVE::Storage::storage_can_replicate($storage_cfg,
> $storeid, $format);
> - my $scfg = PVE::Storage::storage_config($storage_cfg,
> $storeid);
> - return if $scfg->{shared};
> - }
> - die "cannot add non-replicatable volume to a replicated VM\n";
> - });
> - }
> -
> my $code = sub {
>
> my $conf = PVE::LXC::Config->load_config($vmid);
> @@ -180,10 +132,13 @@ __PACKAGE__->register_method({
>
> my $running = PVE::LXC::check_running($vmid);
>
> - PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param,
> \@delete);
> + die "no options specified\n" if !scalar(keys %$param);
> +
> + PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param);
> + $conf = PVE::LXC::Config->load_config($vmid);
>
> - PVE::LXC::Config->write_config($vmid, $conf);
> PVE::LXC::update_lxc_config($vmid, $conf);
> +
> };
>
> PVE::LXC::Config->lock_config($vmid, $code);
> --
> 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 v2 container 17/18] rework update_pct_config to write into pending section
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> use vmconfig_hotplug_pending or vmconfig_apply_pending to apply the
> pending changes, depending on whether the CT is on- or offline.
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC/Config.pm | 334 ++
> 1 file changed, 106 insertions(+), 228 deletions(-)
>
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 47bd4bb..14c26bc 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -4,11 +4,13 @@ use strict;
> use warnings;
>
> use PVE::AbstractConfig;
> +use PVE::RPCEnvironment;
> use PVE::Cluster qw(cfs_register_file);
> use PVE::GuestHelpers;
> use PVE::INotify;
> +use PVE::Exception qw(raise_param_exc);
> use PVE::JSONSchema qw(get_standard_option);
> -use PVE::Tools;
> +use PVE::Tools qw(extract_param);
>
> use base qw(PVE::AbstractConfig);
>
> @@ -900,267 +902,143 @@ sub write_pct_config {
> }
>
> sub update_pct_config {
> -my ($class, $vmid, $conf, $running, $param, $delete) = @_;
> +my ($class, $vmid, $conf, $running, $param) = @_;
>
> -my @nohotplug;
> +my $rpcenv = PVE::RPCEnvironment::get();
> +my $authuser = $rpcenv->get_user();
>
> -my $new_disks = 0;
> -my @deleted_volumes;
> +my $delete_str = extract_param($param, 'delete');
> +my @delete = PVE::Tools::split_list($delete_str);
> +my $revert_str = extract_param($param, 'revert');
> +my @revert = PVE::Tools::split_list($revert_str);
I am still not convinced that hashes for delete and revert would not be
more readable down below.
>
> -my $rootdir;
> -if ($running) {
> - my $pid = PVE::LXC::find_lxc_pid($vmid);
> - $rootdir = "/proc/$pid/root";
> +PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef,
> {}, [@delete]);
still no permission checks for reverting?
> +
> +foreach my $opt (PVE::Tools::split_list($revert_str)) {
should be @revert..
> + raise_param_exc({ revert => "unknown option '$opt'" })
> + if !$class->option_exists($opt);
> +
> + raise_param_exc({ delete => "you can't use '-$opt' and " .
wrong parameter to raise the exception for
> + "'-revert $opt' at the same time" })
> + if defined($param->{$opt});
> +
> + push @revert, $opt;
> }
@revert now contains each reverted option twice ;)
>
> -my $hotplug_error = sub {
> - if ($running) {
> - push @nohotplug, @_;
> - return 1;
> - } else {
> - return 0;
> - }
> -};
> +foreach my $opt (@revert) {
> + delete $conf->{pending}->{$opt};
> + # FIXME: maybe check before doing this?
check what?
> + $class->remove_from_pending_delete($conf, $opt); # remove from deletion
> queue
> +}
> +$class->write_config($vmid, $conf);
do we really need to write_config here? this was just a logical change,
and if we die afterwards the API call has failed and the changes so
far where side-effect free anyway..
>
> -if (defined($delete)) {
> - foreach my $opt (@$delete) {
> - if (!exists($conf->{$opt})) {
> - # silently ignore
> - next;
> - }
> +foreach my $opt (@delete) {
> + raise_param_exc({ delete => "you can't use '-$opt' and -delete $opt' at
> the same time" })
> + if defined($param->{$opt});
>
> - if ($opt eq 'memory' || $opt eq 'rootfs') {
> - die "unable to delete required option '$opt'\n";
> - } elsif ($opt eq 'hostname') {
> - delete $conf->{$opt};
> - } elsif ($opt eq 'swap') {
> - delete $conf->{$opt};
> - PVE::LXC::write_cgroup_value("memory", $vmid,
> - "memory.memsw.limit_in_bytes", -1);
> - } elsif ($opt eq 'description' || $opt eq 'onboot' || $opt eq
> 'startup' || $opt eq 'hookscript') {
> - delete $conf->{$opt};
> - } elsif ($opt eq 'nameserver' || $opt eq 'searchdomain' ||
> - $opt eq 'tty' || $opt eq 'console' || $opt eq 'cmode') {
> - next if $hotplug_error->($opt);
> - delete $conf->{$opt};
> - } elsif ($opt eq 'cores') {
> - delete $conf->{$opt}; # rest is handled by pvestatd
> - } elsif ($opt eq 'cpulimit') {
> - PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.cfs_quota_us",
> -1);
> - delete $conf->{$opt};
> - } elsif ($opt eq 'cpuunits') {
> - PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.shares",
> $confdesc->{cpuunits}->{default});
> - delete $conf->{$opt};
> - } elsif ($opt =~ m/^net(\d)$/) {
> - delete $conf->{$opt};
> - next if !$running;
> - my $netid = $1;
> - PVE::Network::veth_delete("veth${vmid}i$netid");
> - } elsif ($opt eq 'protection') {
> - delete $conf->{$opt};
> - } elsif ($opt =
Re: [pve-devel] [PATCH v2 container 18/18] add vmconfig_hotplug_pending and vmconfig_apply_pending
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> vmconfig_hotplug_pending is responsible for checking if a key/value pair in
> the
> pending section can be hotplugged, if yes; perform a generic replace,
> or perform specific actions for hotplugging the special cases.
>
> vmconfig_apply_pending is only supposed to be called when ct isn't live.
>
> Signed-off-by: Oguz Bektas
> ---
> src/PVE/LXC.pm| 14 +--
> src/PVE/LXC/Config.pm | 199 --
> 2 files changed, 203 insertions(+), 10 deletions(-)
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 65c41f5..f91e27d 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1632,7 +1632,7 @@ sub alloc_disk {
>
> our $NEW_DISK_RE = qr/^([^:\s]+):(\d+(\.\d+)?)$/;
> sub create_disks {
> -my ($storecfg, $vmid, $settings, $conf) = @_;
> +my ($storecfg, $vmid, $settings, $conf, $pending) = @_;
>
> my $vollist = [];
>
> @@ -1659,10 +1659,14 @@ sub create_disks {
> push @$vollist, $volid;
> $mountpoint->{volume} = $volid;
> $mountpoint->{size} = $size_kb * 1024;
> - $conf->{$ms} =
> PVE::LXC::Config->print_ct_mountpoint($mountpoint, $ms eq 'rootfs');
> + if ($pending) {
> + $conf->{pending}->{$ms} =
> PVE::LXC::Config->print_ct_mountpoint($mountpoint, $ms eq 'rootfs');
> + } else {
> + $conf->{$ms} =
> PVE::LXC::Config->print_ct_mountpoint($mountpoint, $ms eq 'rootfs');
> + }
> } else {
> -# use specified/existing volid/dir/device
> -$conf->{$ms} =
> PVE::LXC::Config->print_ct_mountpoint($mountpoint, $ms eq 'rootfs');
> + # use specified/existing volid/dir/device
> + $conf->{$ms} =
> PVE::LXC::Config->print_ct_mountpoint($mountpoint, $ms eq 'rootfs');
> }
> });
>
> @@ -1676,7 +1680,7 @@ sub create_disks {
> # free allocated images on error
> if (my $err = $@) {
> destroy_disks($storecfg, $vollist);
> -die $err;
> + die $err;
> }
> return $vollist;
> }
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 14c26bc..10dfc75 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -1177,6 +1177,194 @@ sub option_exists {
> }
> # END JSON config code
>
> +my $LXC_FASTPLUG_OPTIONS= {
> +'description' => 1,
> +'onboot' => 1,
> +'startup' => 1,
> +'protection' => 1,
> +'hostname' => 1,
> +'hookscript' => 1,
> +'cores' => 1,
> +'tags' => 1,
> +};
> +
> +sub vmconfig_hotplug_pending {
> +my ($class, $vmid, $conf, $storecfg, $selection, $errors) = @_;
> +
> +my $pid = PVE::LXC::find_lxc_pid($vmid);
> +my $rootdir = "/proc/$pid/root";
> +
> +my $add_error = sub {
> + my ($opt, $msg) = @_;
> + $errors->{$opt} = "hotplug problem - $msg";
> +};
> +
> +my $changes;
> +foreach my $opt (keys %{$conf->{pending}}) { # add/change
> + next if $selection && !$selection->{$opt};
> + if ($LXC_FASTPLUG_OPTIONS->{$opt}) {
> + $conf->{$opt} = delete $conf->{pending}->{$opt};
> + $changes = 1;
> + }
> +}
> +
> +if ($changes) {
> + $class->write_config($vmid, $conf);
> +}
> +
> +# There's no separate swap size to configure, there's memory and "total"
> +# memory (iow. memory+swap). This means we have to change them together.
> +my $hotplug_memory_done;
> +my $hotplug_memory = sub {
> + my ($wanted_memory, $wanted_swap) = @_;
> + my $old_memory = ($conf->{memory} || $confdesc->{memory}->{default});
> + my $old_swap = ($conf->{swap} || $confdesc->{swap}->{default});
> +
> + $wanted_memory //= $old_memory;
> + $wanted_swap //= $old_swap;
> +
> + my $total = $wanted_memory + $wanted_swap;
> + my $old_total = $old_memory + $old_swap;
> +
> + if ($total > $old_total) {
> + PVE::LXC::write_cgroup_value("memory", $vmid,
> + "memory.memsw.limit_in_bytes",
> + int($total*1024*1024));
> + PVE::LXC::write_cgroup_value("memory", $vmid,
> + "memory.limit_in_bytes",
> + int($wanted_memory*1024*1024));
> + } else {
> + PVE::LXC::write_cgroup_value("memory", $vmid,
> + "memory.limit_in_bytes",
> + int($wanted_memory*1024*1024));
> + PVE::LXC::write_cgroup_value("memory", $vmid,
> + "memory.memsw.limit_in_bytes",
> + int($total*1024*1024));
> + }
> + $hotplug_memory_done = 1;
> +};
> +
> +my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> +# FIXME: $force deletion is not implemented for CTs
> +while (my ($opt, unde
Re: [pve-devel] [PATCH v2 00/18] lxc pending changes
On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> this series makes it possible to add/delete/revert pending changes in
> the backend for containers.
>
> this v2 took longer than expected, mainly because there were small bugs
> popping up everywhere, everytime i tried to change anything :)
>
> big thanks to fabian for the extensive review on v1, and for putting up
> with me :D
thanks for the v2! some detailed feedback on invidivual patches. except
for the naming/order/splitting of commits (see below) and some open
questions, this already looks quite good!
I'll give v3 a more detailed test run again, since some stuff will
probably be moved/refactored/rewritten again.
>
>
> v1 -> v2:
> * better refactoring into guest-common, as suggested by fabian and
> thomas
> * fixed up some bugs (probably added some too):
> * backup/restore
> * cloning
> * unlimited swap bug
> * mountpoint handling (more on that)
> * other stuff that i can't remember right now :D
> * small changes with style
> * in v1, mountpoints were a special-special case, since they were
> handled earlier in code before going into the hotplug/apply pending
> routines. after much discussion, they're now created/deleted in these
> phases instead of update_pct_config. unused disks can still be removed
> directly.
> * some other small changes around helper functions, mainly adding them
> support for handling $conf->{pending}
>
> pve-container:
>
> Oguz Bektas (11):
> add lxc/pending API path
> add 'pct pending'
> adapt CT config parser for pending changes
> use load_current_config for config GET call
> skip pending changes while cloning
> skip pending changes while taking backup
> apply pending changes during container start
> add revert parameter to config PUT
> adapt config PUT method for the new update_pct_config
> rework update_pct_config to write into pending section
> add vmconfig_hotplug_pending and vmconfig_apply_pending
the order here is a bit wrong IMHO
#12 and #13 can go further up (they just make the later commits safe,
and don't change anything without the later commits - good for bisecting
;))
#14 relies on a method only introduced in #18
#15 adds an API parameter, but no code to handle it (I already told you
this in v1 ;))
#16 reworks that API call further to adapt for changes that only come in
patch #17
#17 relies on methods only introduced in #18
so, #18 needs to come first (if nobody calls those methods yet, it is
clear that that change alone cannot break anything!)
#14 can come after, since it only has an effect if pending changes can
exist (which, barring manual editing, is impossible with just the
patches up to this point)
#15-17 are a single logical change (switching from "directly apply what
is possible, die otherwise" to "write to pending, and either apply or
hotplug depending on whether the CT is running"). no part of that works
without the other, so just squash them into a single commit unless you
find a *meaningful* way to split them up. sometimes commits are big, and
that is okay as long as they are not big because lots of un- or
semi-related stuff got thrown together.
>
> src/PVE/API2/LXC.pm| 89 ++
> src/PVE/API2/LXC/Config.pm | 82 ++
> src/PVE/CLI/pct.pm | 3 +
> src/PVE/LXC.pm | 21 +-
> src/PVE/LXC/Config.pm | 566 +
> src/PVE/VZDump/LXC.pm | 1 +
> 6 files changed, 457 insertions(+), 305 deletions(-)
>
> qemu-server:
>
> Oguz Bektas (4):
> overwrite load_current_config from AbstractConfig
> use load_current_config for config GET call
> refactor pending changes related code
> use format_pending from GuestHelpers for 'qm pending' command
those could almost be one commit, since they all do the same:
code moved to AbstractConfig/GuestHelpers, drop own copy and use shared
one. at least the first two should be squashed for sure.
>
> PVE/API2/Qemu.pm | 49 +
> PVE/CLI/qm.pm | 28 +
> PVE/QemuConfig.pm | 14 +
> PVE/QemuServer.pm | 79 +--
> 4 files changed, 31 insertions(+), 139 deletions(-)
>
> pve-guest-common:
> Oguz Bektas (3):
> refactor pending changes related config code into AbstractConfig
> refactor method used by config GET calls into AbstractConfig
> refactor code from qm/pct 'pending' call into AbstractConfig
>
> PVE/AbstractConfig.pm | 103 ++
> PVE/GuestHelpers.pm | 26 +++
> 2 files changed, 129 insertions(+)
>
> --
> 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
[pve-devel] applied: [PATCH v4 manager 1/3] Make CPU Model Selector a searchable grid view
On 9/10/19 7:11 PM, Stefan Reiter wrote:
> Uses a ComboGrid with search feature and a column for vendor. Can be
> sorted by both columns.
>
> Default sort is as given in this file, I tried to align it as
> * AMD
> * Intel
> * Other
> alphabetically and in QEMU order (as before, seems to be release date?)
> within those "groups".
>
> Doesn't work with value set in widget definition (would need to be
> preferredValue), but we always call setValue() anyway (and if we don't,
> value will be '', aka the default, which is correct too), so just remove
> that from ProcessorEdit.js.
>
> Signed-off-by: Stefan Reiter
> ---
> www/manager6/form/CPUModelSelector.js | 193 +-
> www/manager6/qemu/ProcessorEdit.js| 1 -
> 2 files changed, 159 insertions(+), 35 deletions(-)
>
applied with solving the trivial merge conflict from Alexandre's recent
model additions[0].
Also did a follow up which changed the kvm{32,64} qemu{32,64} vendors
from Other to QEMU and the one from host to Host.
thanks!
[0]: https://pve.proxmox.com/pipermail/pve-devel/2019-September/039228.html
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] Failing to build pve-kernel 5.0.x from git
Hi,
I'm trying to build the current pve-kernel from git, but apparently
this needs some extra tarballs? Am I holding it wrong?
Log (after git clone, git submodule update --init):
$ git describe --always
9e3f73d
$ make
test -f "submodules/ubuntu-disco/README" || git submodule update --init
submodules/ubuntu-disco
test -f "submodules/zfsonlinux/Makefile" || git submodule update --init
--recursive submodules/zfsonlinux
rm -rf build/ubuntu-disco ubuntu-disco.prepared
mkdir -p build
cp -a submodules/ubuntu-disco build/ubuntu-disco
cat build/ubuntu-disco/debian.master/config/config.common.ubuntu
build/ubuntu-disco/debian.master/config/amd64/config.common.amd64
build/ubuntu-disco/debian.master/config/amd64/config.flavour.generic >
config-5.0.21.org
cp config-5.0.21.org build/ubuntu-disco/.config
sed -i build/ubuntu-disco/Makefile -e 's/^EXTRAVERSION.*$/EXTRAVERSION=-3-pve/'
rm -rf build/ubuntu-disco/debian build/ubuntu-disco/debian.master
set -e; cd build/ubuntu-disco; for patch in ../../patches/kernel/*.patch; do
echo "applying patch '$patch'" && patch -p1 < ${patch}; done
applying patch
'../../patches/kernel/0001-Make-mkcompile_h-accept-an-alternate-timestamp-strin.patch'
patching file scripts/mkcompile_h
applying patch
'../../patches/kernel/0002-bridge-keep-MAC-of-first-assigned-port.patch'
patching file net/bridge/br_stp_if.c
applying patch
'../../patches/kernel/0003-pci-Enable-overrides-for-missing-ACS-capabilities-4..patch'
patching file Documentation/admin-guide/kernel-parameters.txt
patching file drivers/pci/quirks.c
applying patch
'../../patches/kernel/0004-kvm-disable-default-dynamic-halt-polling-growth.patch'
patching file virt/kvm/kvm_main.c
applying patch
'../../patches/kernel/0005-Revert-KVM-VMX-enable-nested-virtualization-by-defau.patch'
patching file arch/x86/kvm/vmx/vmx.c
applying patch
'../../patches/kernel/0006-rbd-don-t-assert-on-writes-to-snapshots.patch'
patching file drivers/block/rbd.c
applying patch
'../../patches/kernel/0007-x86-fpu-backport-copy_kernel_to_XYZ_err-helpers.patch'
patching file arch/x86/include/asm/fpu/internal.h
touch ubuntu-disco.prepared
rm -rf build/modules/pkg-zfs build/modules/tmp pkg-zfs.prepared
mkdir -p build/modules/tmp
cp -a submodules/zfsonlinux/* build/modules/tmp
cd build/modules/tmp; make kernel
make[1]: Entering directory '/home/ch/Source/pve-kernel/build/modules/tmp'
test -f "upstream/README.md" || git submodule update --init
rm -rf zfs-linux_0.8.2
mkdir zfs-linux_0.8.2
cp -a upstream/* zfs-linux_0.8.2/
cp: cannot stat 'upstream/*': No such file or directory
make[1]: *** [Makefile:59: zfs-linux_0.8.2] Error 1
make[1]: Leaving directory '/home/ch/Source/pve-kernel/build/modules/tmp'
make: *** [Makefile:99: pkg-zfs.prepared] Error 2
$ ls submodules/zfsonlinux/upstream/
$
Any hints would be appreciated.
Thanks,
Chris
PS: the README still says things about bionic, while this is
building a disco-based kernel.
--
Chris Hofstaedtler / Deduktiva GmbH (FN 418592 b, HG Wien)
www.deduktiva.com / +43 1 353 1707
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Failing to build pve-kernel 5.0.x from git
On 10/2/19 4:46 PM, Chris Hofstaedtler | Deduktiva wrote:
> Hi,
>
> I'm trying to build the current pve-kernel from git, but apparently
> this needs some extra tarballs? Am I holding it wrong?
we have no tarballs anymore, it's submodules all the way down.
>
> Log (after git clone, git submodule update --init):
Either don't do the initial "git submodule update --init" yourself and
let the makefile handle that or add the "--recursive" flag..
But a fresh
# git clone git://git.proxmox.com/git/pve-kernel.git
# make
works just fine, just retried here.
>
> $ git describe --always
> 9e3f73d
> $ make
> test -f "submodules/ubuntu-disco/README" || git submodule update --init
> submodules/ubuntu-disco
> test -f "submodules/zfsonlinux/Makefile" || git submodule update --init
> --recursive submodules/zfsonlinux
> rm -rf build/ubuntu-disco ubuntu-disco.prepared
> mkdir -p build
> cp -a submodules/ubuntu-disco build/ubuntu-disco
> cat build/ubuntu-disco/debian.master/config/config.common.ubuntu
> build/ubuntu-disco/debian.master/config/amd64/config.common.amd64
> build/ubuntu-disco/debian.master/config/amd64/config.flavour.generic >
> config-5.0.21.org
> cp config-5.0.21.org build/ubuntu-disco/.config
> sed -i build/ubuntu-disco/Makefile -e
> 's/^EXTRAVERSION.*$/EXTRAVERSION=-3-pve/'
> rm -rf build/ubuntu-disco/debian build/ubuntu-disco/debian.master
> set -e; cd build/ubuntu-disco; for patch in ../../patches/kernel/*.patch; do
> echo "applying patch '$patch'" && patch -p1 < ${patch}; done
> applying patch
> '../../patches/kernel/0001-Make-mkcompile_h-accept-an-alternate-timestamp-strin.patch'
> patching file scripts/mkcompile_h
> applying patch
> '../../patches/kernel/0002-bridge-keep-MAC-of-first-assigned-port.patch'
> patching file net/bridge/br_stp_if.c
> applying patch
> '../../patches/kernel/0003-pci-Enable-overrides-for-missing-ACS-capabilities-4..patch'
> patching file Documentation/admin-guide/kernel-parameters.txt
> patching file drivers/pci/quirks.c
> applying patch
> '../../patches/kernel/0004-kvm-disable-default-dynamic-halt-polling-growth.patch'
> patching file virt/kvm/kvm_main.c
> applying patch
> '../../patches/kernel/0005-Revert-KVM-VMX-enable-nested-virtualization-by-defau.patch'
> patching file arch/x86/kvm/vmx/vmx.c
> applying patch
> '../../patches/kernel/0006-rbd-don-t-assert-on-writes-to-snapshots.patch'
> patching file drivers/block/rbd.c
> applying patch
> '../../patches/kernel/0007-x86-fpu-backport-copy_kernel_to_XYZ_err-helpers.patch'
> patching file arch/x86/include/asm/fpu/internal.h
> touch ubuntu-disco.prepared
> rm -rf build/modules/pkg-zfs build/modules/tmp pkg-zfs.prepared
> mkdir -p build/modules/tmp
> cp -a submodules/zfsonlinux/* build/modules/tmp
> cd build/modules/tmp; make kernel
> make[1]: Entering directory '/home/ch/Source/pve-kernel/build/modules/tmp'
> test -f "upstream/README.md" || git submodule update --init
> rm -rf zfs-linux_0.8.2
> mkdir zfs-linux_0.8.2
> cp -a upstream/* zfs-linux_0.8.2/
> cp: cannot stat 'upstream/*': No such file or directory
your zfs submodule is not correctly checked out, it uses a submodule itself
the
# git submodule update --init --recursive submodules/zfsonlinux
command normally takes care of that, but as you manually did one
without the --recursive flag the
# test -f "submodules/zfsonlinux/Makefile" || git submodule update --init
--recursive submodules/zfsonlinux
makefield command doesn't execute any more as the first
condition is already true.
Try
# git submodule update --init --recursive submodules/zfsonlinux
to fiy this up without a fresh clone or other things
> make[1]: *** [Makefile:59: zfs-linux_0.8.2] Error 1
> make[1]: Leaving directory '/home/ch/Source/pve-kernel/build/modules/tmp'
> make: *** [Makefile:99: pkg-zfs.prepared] Error 2
> $ ls submodules/zfsonlinux/upstream/
> $
>
> Any hints would be appreciated.
>
> Thanks,
> Chris
>
> PS: the README still says things about bionic, while this is
> building a disco-based kernel.
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Failing to build pve-kernel 5.0.x from git
On 10/2/19 4:46 PM, Chris Hofstaedtler | Deduktiva wrote: > PS: the README still says things about bionic, while this is > building a disco-based kernel. > Thanks for the hint, fixed. Note that we switched over to Eoan based Kernel on master just today, you may want to use the pve-kernel-5.0 branch for now, as our 5.3 based kernel from master is not yet to be considered stable. cheers, Thomas ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Failing to build pve-kernel 5.0.x from git
* Thomas Lamprecht [191002 16:58]: > On 10/2/19 4:46 PM, Chris Hofstaedtler | Deduktiva wrote: > Either don't do the initial "git submodule update --init" yourself and > let the makefile handle that or add the "--recursive" flag.. > > But a fresh > # git clone git://git.proxmox.com/git/pve-kernel.git > # make > > works just fine, just retried here. Ah! Thanks for the --recursive hint. Looking better now. Cheers, Chris ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] scsi-hd vs scsi-generic with iSCSI
- Le 30 Sep 19, à 11:52, Thomas Lamprecht t.lampre...@proxmox.com a écrit : > > Depends on the outcome of above, but effectively we would like to > not have the choice between "working or not", so ideally we can > just either fix it in QEMU or set the respective (working) option > for all new started VMs (QEMU machine versioned, to not break live > migration due to changed VM HW). Just thinking about it, as scsi-hd, scsi-block scsi-generic won't present the same HW to the VM, moving disk from one storage to another is already broken if both source and dest are not the same type (eg if source uses scsi-hd but dest scsi-block or scsi-generic). Without even considering the issues I have when ZFS is used as a backend. Cheers, Daniel -- [ https://www.firewall-services.com/ ] Daniel Berteaud FIREWALL-SERVICES SAS, La sécurité des réseaux Société de Services en Logiciels Libres Tél : +33.5 56 64 15 32 Matrix: @dani:fws.fr [ https://www.firewall-services.com/ | https://www.firewall-services.com ] ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
