Re: [pve-devel] [RFC PATCH common] cli: prettify tables even more

2019-08-22 Thread Wolfgang Bumiller
On Wed, Aug 21, 2019 at 09:35:56PM +0200, Dietmar Maurer wrote:
> Are your sure common terminals support those characters?
> 
> They did not when I tested ...

Curious, when using utf-8 encoding for those characters I wouldn't see
how these characters would be any more special than any other?
In any case, tested urxvt, terminator, xfce4-terminal, gnome-terminal,
lxterminal.
Also note that all those symbols already existed in the extended ascii set,
codes 181, 198 and 216, and yes, those do show up correctly in dosbox ;-)

> > On 21 August 2019 14:33 Wolfgang Bumiller  wrote:
> > 
> >  
> > Separate the header with a double line.
> > 
> > Signed-off-by: Wolfgang Bumiller 
> > ---
> >  src/PVE/CLIFormatter.pm | 18 +-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/CLIFormatter.pm b/src/PVE/CLIFormatter.pm
> > index 84dbed1..0e9cbe6 100644
> > --- a/src/PVE/CLIFormatter.pm
> > +++ b/src/PVE/CLIFormatter.pm
> > @@ -186,6 +186,7 @@ sub print_text_table {
> >  my $borderstring_m = '';
> >  my $borderstring_b = '';
> >  my $borderstring_t = '';
> > +my $borderstring_h = '';
> >  my $formatstring = '';
> >  
> >  my $column_count = scalar(@$props_to_print);
> > @@ -255,41 +256,49 @@ sub print_text_table {
> > if ($utf8) {
> > $formatstring .= "│ %$alignstr${cutoff}s │";
> > $borderstring_t .= "┌─" . ('─' x $cutoff) . "─┐";
> > +   $borderstring_h .= "╞═" . ('═' x $cutoff) . '═╡';
> > $borderstring_m .= "├─" . ('─' x $cutoff) . "─┤";
> > $borderstring_b .= "└─" . ('─' x $cutoff) . "─┘";
> > } else {
> > $formatstring .= "| %$alignstr${cutoff}s |";
> > $borderstring_m .= "+-" . ('-' x $cutoff) . "-+";
> > +   $borderstring_h .= "+=" . ('=' x $cutoff) . '=';
> > }
> > } elsif ($i == 0) {
> > if ($utf8) {
> > $formatstring .= "│ %$alignstr${cutoff}s ";
> > $borderstring_t .= "┌─" . ('─' x $cutoff) . '─';
> > +   $borderstring_h .= "╞═" . ('═' x $cutoff) . '═';
> > $borderstring_m .= "├─" . ('─' x $cutoff) . '─';
> > $borderstring_b .= "└─" . ('─' x $cutoff) . '─';
> > } else {
> > $formatstring .= "| %$alignstr${cutoff}s ";
> > $borderstring_m .= "+-" . ('-' x $cutoff) . '-';
> > +   $borderstring_h .= "+=" . ('=' x $cutoff) . '=';
> > }
> > } elsif ($i == ($column_count - 1)) {
> > if ($utf8) {
> > $formatstring .= "│ %$alignstr${cutoff}s │";
> > $borderstring_t .= "┬─" . ('─' x $cutoff) . "─┐";
> > +   $borderstring_h .= "╪═" . ('═' x $cutoff) . '═╡';
> > $borderstring_m .= "┼─" . ('─' x $cutoff) . "─┤";
> > $borderstring_b .= "┴─" . ('─' x $cutoff) . "─┘";
> > } else {
> > $formatstring .= "| %$alignstr${cutoff}s |";
> > $borderstring_m .= "+-" . ('-' x $cutoff) . "-+";
> > +   $borderstring_h .= "+=" . ('=' x $cutoff) . "=+";
> > }
> > } else {
> > if ($utf8) {
> > $formatstring .= "│ %$alignstr${cutoff}s ";
> > $borderstring_t .= "┬─" . ('─' x $cutoff) . '─';
> > +   $borderstring_h .= "╪═" . ('═' x $cutoff) . '═';
> > $borderstring_m .= "┼─" . ('─' x $cutoff) . '─';
> > $borderstring_b .= "┴─" . ('─' x $cutoff) . '─';
> > } else {
> > $formatstring .= "| %$alignstr${cutoff}s ";
> > $borderstring_m .= "+-" . ('-' x $cutoff) . '-';
> > +   $borderstring_h .= "+=" . ('=' x $cutoff) . '=';
> > }
> > }
> > } else {
> > @@ -313,15 +322,22 @@ sub print_text_table {
> >  
> >  $writeln->($borderstring_t) if $border;
> >  
> > +my $borderstring_sep;
> >  if ($header) {
> > my $text = sprintf $formatstring, map { $colopts->{$_}->{title} } 
> > @$props_to_print;
> > $writeln->($text);
> > +   $borderstring_sep = $borderstring_h;
> > +} else {
> > +   $borderstring_sep = $borderstring_m;
> >  }
> >  
> >  for (my $i = 0; $i < scalar(@$tabledata); $i++) {
> > my $coldata = $tabledata->[$i];
> >  
> > -   $writeln->($borderstring_m) if $border && ($i != 0 || $header);
> > +   if ($border && ($i != 0 || $header)) {
> > +   $writeln->($borderstring_sep);
> > +   $borderstring_sep = $borderstring_m;
> > +   }
> >  
> > for (my $i = 0; $i < $coldata->{height}; $i++) {
> >  
> > -- 
> > 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.proxmo

[pve-devel] [PATCH manager 1/2] fix #2340: gui: ceph: handle 'null' versions for hosts

2019-08-22 Thread Dominik Csapak
the api returns 'null' for a host that is in the crushmap, but
without actual version information, so just check for falsyness instead
of 'undefined', else we run later into javascript exceptions and no
content on the osd page

Signed-off-by: Dominik Csapak 
---
i am not so sure if the api return 'null' is a bug too, but this change
here is easy and does the right thing also

we can also change the 'compare_ceph_version' method so that it
can properly handle undefined and null values, if wanted

 www/manager6/ceph/OSD.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index ed1b660c..24f6f9d6 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -311,7 +311,7 @@ Ext.define('PVE.node.CephOsdTree', {
};
traverse(data.root, node => {
// compatibility for old api call
-   if (node.type === 'host' && node.version === undefined) 
{
+   if (node.type === 'host' && !node.version) {
node.version = data.versions[node.name];
}
 
-- 
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 manager 2/2] fix #2341: ceph: osd create: allow db/wal on partioned disks

2019-08-22 Thread Dominik Csapak
It was intended that for partitioned disks, we create one and use it.
Instead the code died always when the disk was used and not of type 'LVM'

We now check correctly the 2 cases:
* used for partitions and has gpt
* used and lvm

The remaining api call handles those two cases correctly

Signed-off-by: Dominik Csapak 
---
 PVE/API2/Ceph/OSD.pm | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 78ad3734..5f70cf58 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -313,10 +313,13 @@ __PACKAGE__->register_method ({
my $name = $d->{name};
my $info = $disklist->{$name};
die "unable to get device info for '$d->{dev}' for type $type\n" if 
!$disklist->{$name};
-   die "device '$d->{dev}' is not GPT partitioned\n"
-   if $info->{used} && $info->{used} eq 'partitions' && 
!$info->{gpt};
-   die "device '$d->{dev}' is already in use and has no LVM on it\n"
-   if $info->{used} && $info->{used} ne 'LVM';
+   if (my $usage = $info->{used}) {
+   if ($usage eq 'partitions') {
+   die "device '$d->{dev}' is not GPT partitioned\n" if 
!$info->{gpt};
+   } elsif ($usage ne 'LVM') {
+   die "device '$d->{dev}' is already in use and has no LVM on 
it\n";
+   }
+   }
}
 
# get necessary ceph infos
-- 
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 storage 1/2] fix #2216: Allow .img files in 'iso' type storages

2019-08-22 Thread Stefan Reiter
To maintain full (backwards) compatibility, leave the type name as
'iso' - this makes this patch work without changing every consumer of
storage APIs.

Note that currently these files can only be attached as a CDROM/DVD
drive, so USB-only imgs can be uploaded but might not work in VMs.

Signed-off-by: Stefan Reiter 
---

Note that starting a VM with an img file attached breaks live migration to
machines without this patch - not sure how relevant, considering this is
a new addition anyway.


 PVE/API2/Storage/Status.pm | 6 +++---
 PVE/Storage.pm | 2 +-
 PVE/Storage/Plugin.pm  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index ce7e040..09cf8b7 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -350,7 +350,7 @@ __PACKAGE__->register_method ({
 name => 'upload',
 path => '{storage}/upload', 
 method => 'POST',
-description => "Upload templates and ISO images.",
+description => "Upload templates and ISO/IMG images.",
 permissions => { 
check => ['perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
 },
@@ -408,8 +408,8 @@ __PACKAGE__->register_method ({
my $path;
 
if ($content eq 'iso') {
-   if ($filename !~ m![^/]+\.[Ii][Ss][Oo]$!) {
-   raise_param_exc({ filename => "missing '.iso' extension" });
+   if ($filename !~ m![^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg])$!) {
+   raise_param_exc({ filename => "missing '.iso' or '.img' 
extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 755eca8..de117a5 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -501,7 +501,7 @@ sub path_to_volume_id {
return ('images', $info->{volid});
}
}
-   } elsif ($path =~ m!^$isodir/([^/]+\.[Ii][Ss][Oo])$!) {
+   } elsif ($path =~ m!^$isodir/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!) {
my $name = $1;
return ('iso', "$sid:iso/$name");
} elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 27f832f..87a6b9a 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -415,7 +415,7 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
-} elsif ($volname =~ m!^iso/([^/]+\.[Ii][Ss][Oo])$!) {
+} elsif ($volname =~ m!^iso/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!) {
return ('iso', $1);
 } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
return ('vztmpl', $1);
@@ -915,7 +915,7 @@ my $get_subdir_files = sub {
my $info;
 
if ($tt eq 'iso') {
-   next if $fn !~ m!/([^/]+\.iso)$!i;
+   next if $fn !~ m!/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!i;
 
$info = { volid => "$sid:iso/$1", format => 'iso' };
 
-- 
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 manager 2/2] Show supported file types in upload file selector

2019-08-22 Thread Stefan Reiter
By default, all file types are shown, but the user now has the option of
filtering only by supported types in the file selector dialog.

Signed-off-by: Stefan Reiter 
---
 www/manager6/storage/ContentView.js | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/www/manager6/storage/ContentView.js 
b/www/manager6/storage/ContentView.js
index 3677f8ad..b72fc88d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -218,7 +218,14 @@ Ext.define('PVE.storage.Upload', {
xtype: 'filefield',
name: 'filename',
buttonText: gettext('Select File...'),
-   allowBlank: false
+   allowBlank: false,
+   listeners: {
+   afterrender: function(cmp) {
+   cmp.fileInputEl.set({
+   accept: '.img, .iso'
+   });
+   }
+   }
},
pbar
]
-- 
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 storage 1/2] fix #2216: Allow .img files in 'iso' type storages

2019-08-22 Thread Dominik Csapak

comments inline

On 8/22/19 1:35 PM, Stefan Reiter wrote:

To maintain full (backwards) compatibility, leave the type name as
'iso' - this makes this patch work without changing every consumer of
storage APIs.

Note that currently these files can only be attached as a CDROM/DVD
drive, so USB-only imgs can be uploaded but might not work in VMs.

Signed-off-by: Stefan Reiter 
---

Note that starting a VM with an img file attached breaks live migration to
machines without this patch - not sure how relevant, considering this is
a new addition anyway.


  PVE/API2/Storage/Status.pm | 6 +++---
  PVE/Storage.pm | 2 +-
  PVE/Storage/Plugin.pm  | 4 ++--
  3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index ce7e040..09cf8b7 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -350,7 +350,7 @@ __PACKAGE__->register_method ({
  name => 'upload',
  path => '{storage}/upload',
  method => 'POST',
-description => "Upload templates and ISO images.",
+description => "Upload templates and ISO/IMG images.",
  permissions => {
check => ['perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
  },
@@ -408,8 +408,8 @@ __PACKAGE__->register_method ({
my $path;
  
  	if ($content eq 'iso') {

-   if ($filename !~ m![^/]+\.[Ii][Ss][Oo]$!) {
-   raise_param_exc({ filename => "missing '.iso' extension" });
+   if ($filename !~ m![^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg])$!) {


why not

m![^/]+\.(?:iso|img)$!i

in my opinion, much more readable


+   raise_param_exc({ filename => "missing '.iso' or '.img' 
extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 755eca8..de117a5 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -501,7 +501,7 @@ sub path_to_volume_id {
return ('images', $info->{volid});
}
}
-   } elsif ($path =~ m!^$isodir/([^/]+\.[Ii][Ss][Oo])$!) {
+   } elsif ($path =~ m!^$isodir/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!) {


same here, but we would have to be carefull with $isodir, so maybe not?


my $name = $1;
return ('iso', "$sid:iso/$name");
} elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 27f832f..87a6b9a 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -415,7 +415,7 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
-} elsif ($volname =~ m!^iso/([^/]+\.[Ii][Ss][Oo])$!) {
+} elsif ($volname =~ m!^iso/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!) {


same here but now with 'iso'...


return ('iso', $1);
  } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
return ('vztmpl', $1);
@@ -915,7 +915,7 @@ my $get_subdir_files = sub {
my $info;
  
  	if ($tt eq 'iso') {

-   next if $fn !~ m!/([^/]+\.iso)$!i;
+   next if $fn !~ m!/([^/]+\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg]))$!i;


here we even have the 'i' already

maybe the '[Ii][Ss]...' part can be refactored into a variable, so that
we do not have to update 4 places everytime we want to add an extension?
(also makes it more readable)

  
  	$info = { volid => "$sid:iso/$1", format => 'iso' };
  




___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH manager 2/2] Show supported file types in upload file selector

2019-08-22 Thread Dominik Csapak
looks ok, a bit weird, but researching showed that there are no extjs 
builtin ways for this, so...


Acked-by: Dominik Csapak 

On 8/22/19 1:35 PM, Stefan Reiter wrote:

By default, all file types are shown, but the user now has the option of
filtering only by supported types in the file selector dialog.

Signed-off-by: Stefan Reiter 
---
  www/manager6/storage/ContentView.js | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/www/manager6/storage/ContentView.js 
b/www/manager6/storage/ContentView.js
index 3677f8ad..b72fc88d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -218,7 +218,14 @@ Ext.define('PVE.storage.Upload', {
xtype: 'filefield',
name: 'filename',
buttonText: gettext('Select File...'),
-   allowBlank: false
+   allowBlank: false,
+   listeners: {
+   afterrender: function(cmp) {
+   cmp.fileInputEl.set({
+   accept: '.img, .iso'
+   });
+   }
+   }
},
pbar
]




___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] applied: [PATCH manager] ui: vm memory: correctly set initial maxValue for balloon

2019-08-22 Thread Thomas Lamprecht
Signed-off-by: Thomas Lamprecht 
---
 www/manager6/qemu/MemoryEdit.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/qemu/MemoryEdit.js b/www/manager6/qemu/MemoryEdit.js
index 4c4d1815..d1284716 100644
--- a/www/manager6/qemu/MemoryEdit.js
+++ b/www/manager6/qemu/MemoryEdit.js
@@ -60,6 +60,7 @@ Ext.define('PVE.qemu.MemoryInputPanel', {
xtype: 'pveMemoryField',
name: 'balloon',
minValue: 1,
+   maxValue: 512,
step: 32,
fieldLabel: gettext('Minimum memory') + ' (MiB)',
hotplug: me.hotplug,
-- 
2.20.1


___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] applied: [PATCH manager 1/2] fix #2340: gui: ceph: handle 'null' versions for hosts

2019-08-22 Thread Thomas Lamprecht
On 8/22/19 12:26 PM, Dominik Csapak wrote:
> the api returns 'null' for a host that is in the crushmap, but
> without actual version information, so just check for falsyness instead
> of 'undefined', else we run later into javascript exceptions and no
> content on the osd page
> 
> Signed-off-by: Dominik Csapak 
> ---
> i am not so sure if the api return 'null' is a bug too, but this change
> here is easy and does the right thing also
> 
> we can also change the 'compare_ceph_version' method so that it
> can properly handle undefined and null values, if wanted
> 

could be an good idea anyway, but for now: applied, thanks!

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] applied: [PATCH manager 2/2] fix #2341: ceph: osd create: allow db/wal on partioned disks

2019-08-22 Thread Thomas Lamprecht
On 8/22/19 12:26 PM, Dominik Csapak wrote:
> It was intended that for partitioned disks, we create one and use it.
> Instead the code died always when the disk was used and not of type 'LVM'
> 
> We now check correctly the 2 cases:
> * used for partitions and has gpt
> * used and lvm
> 
> The remaining api call handles those two cases correctly
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/Ceph/OSD.pm | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

applied, thanks!

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH v2 manager 2/2] Show supported file types in upload file selector

2019-08-22 Thread Stefan Reiter
By default, all file types are shown, but the user now has the option of
filtering only by supported types in the file selector dialog.

Signed-off-by: Stefan Reiter 
Acked-by: Dominik Csapak 
---

I found it weird too, but also couldn't find a different way to go about it...

 www/manager6/storage/ContentView.js | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/www/manager6/storage/ContentView.js 
b/www/manager6/storage/ContentView.js
index 3677f8ad..b72fc88d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -218,7 +218,14 @@ Ext.define('PVE.storage.Upload', {
xtype: 'filefield',
name: 'filename',
buttonText: gettext('Select File...'),
-   allowBlank: false
+   allowBlank: false,
+   listeners: {
+   afterrender: function(cmp) {
+   cmp.fileInputEl.set({
+   accept: '.img, .iso'
+   });
+   }
+   }
},
pbar
]
-- 
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 storage 1/2] fix #2216: Allow .img files in 'iso' type storages

2019-08-22 Thread Stefan Reiter
To maintain full (backwards) compatibility, leave the type name as
'iso' - this makes this patch work without changing every consumer of
storage APIs.

Note that currently these files can only be attached as a CDROM/DVD
drive, so USB-only images can be uploaded but might not work in VMs.

Signed-off-by: Stefan Reiter 
---

v1 -> v2:
* Refactored regex into variable

I left the [Ii][Ss]... since we don't know if the variable is used with //i
or not.

 PVE/API2/Storage/Status.pm | 4 ++--
 PVE/Storage.pm | 4 +++-
 PVE/Storage/Plugin.pm  | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index ce7e040..91946ac 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -408,8 +408,8 @@ __PACKAGE__->register_method ({
my $path;
 
if ($content eq 'iso') {
-   if ($filename !~ m![^/]+\.[Ii][Ss][Oo]$!) {
-   raise_param_exc({ filename => "missing '.iso' extension" });
+   if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+   raise_param_exc({ filename => "missing '.iso' or '.img' 
extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 755eca8..08b6e14 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -99,6 +99,8 @@ PVE::Storage::Plugin->init();
 
 my $UDEVADM = '/sbin/udevadm';
 
+my $iso_extension_re = qr/\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg])/;
+
 #  PVE::Storage utility functions
 
 sub config {
@@ -501,7 +503,7 @@ sub path_to_volume_id {
return ('images', $info->{volid});
}
}
-   } elsif ($path =~ m!^$isodir/([^/]+\.[Ii][Ss][Oo])$!) {
+   } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
my $name = $1;
return ('iso', "$sid:iso/$name");
} elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 27f832f..9a419f1 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -415,7 +415,7 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
-} elsif ($volname =~ m!^iso/([^/]+\.[Ii][Ss][Oo])$!) {
+} elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
return ('iso', $1);
 } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
return ('vztmpl', $1);
@@ -915,7 +915,7 @@ my $get_subdir_files = sub {
my $info;
 
if ($tt eq 'iso') {
-   next if $fn !~ m!/([^/]+\.iso)$!i;
+   next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
 
$info = { volid => "$sid:iso/$1", format => 'iso' };
 
-- 
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] Fix #2041, #2272 Add Spice enhancements

2019-08-22 Thread Dominik Csapak

looks good, but 2 comments inline

On 8/20/19 6:02 PM, Aaron Lauterer wrote:

This adds a new config option called `spice_enhancements` with two
optional settings:

* videostreaming
* foldersharing

Signed-off-by: Aaron Lauterer 

v1 -> v2:
format changes suggested by dominik:
* changed descriptions
* changed `videostreaming` to enum

refactoring suggested by thomas:

I went with moving the `has_spice_enhancements` to the
`config_to_command` function and reducing the LOC.

Added a `spice` hash to store spice settings. Intended to be used in
further cleanup patches in this part of the code.

An empty `spice_enhancements` in the config file can be set (thanks
Dominik for pointing it out). That case is now handled to avoid errors.

The `streaming-video` option is always present now in the `-spice`
device. My tests showed no problems when migrating from a PVE node
without this patch to one with it. Migrating from newer PVE to an older
version without the patch resultet in a "resume failed" error.


normally we do not put commit changelogs in the commit message



---


but here <---

this way, they do not show up in the git history


  PVE/QemuServer.pm | 32 +++-
  1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 9f5bf56..4701193 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -295,6 +295,20 @@ my $audio_fmt = {
  },
  };
  
+my $spice_enhancements_fmt = {

+foldersharing => {
+   type => 'boolean',
+   optional => 1,
+   description =>  "Enable folder sharing via SPICE. Needs Spice-WebDAV daemon 
installed in the VM."
+},
+videostreaming =>  {
+   type => 'string',
+   enum => ['off', 'all', 'filter'],
+   optional => 1,
+   description => "Enable video streaming. Uses compression for detected video 
streams."
+},
+};
+
  my $confdesc = {
  onboot => {
optional => 1,
@@ -672,6 +686,12 @@ EODESCR
description => "Configure a audio device, useful in combination with 
QXL/Spice.",
optional => 1
  },
+spice_enhancements => {
+   type => 'string',
+   format => $spice_enhancements_fmt,
+   description => "Configure additional enhancements for SPICE.",
+   optional => 1
+},
  };
  
  my $cicustom_fmt = {

@@ -3992,11 +4012,21 @@ sub config_to_command {
my $localhost = PVE::Network::addr_to_ip($nodeaddrs[0]->{addr});
$spice_port = PVE::Tools::next_spice_port($pfamily, $localhost);
  
-	push @$devices, '-spice', "tls-port=${spice_port},addr=$localhost,tls-ciphers=HIGH,seamless-migration=on";

+   my $spice = {};
+   my $spice_enhancements = $conf->{spice_enhancements} ? 
PVE::JSONSchema::parse_property_string($spice_enhancements_fmt, 
$conf->{spice_enhancements}) : {} ;
+   $spice->{videostreaming} = $spice_enhancements->{videostreaming} // 
'off';


i would prefer to keep it backwards compatible, like so:

$spice->{videostreaming} = $spice_enhancements->{videostreaming} ? 
",streaming-video=$spice_enhancements->{videostreaming}" : "";


and simply add this string to the '-spice' parameters


+   $spice->{foldersharing} = $spice_enhancements->{foldersharing} // 0;


the '// 0' is unecessary, but it does not really hurt...


+
+   push @$devices, '-spice', 
"tls-port=${spice_port},addr=$localhost,tls-ciphers=HIGH,seamless-migration=on,streaming-video=$spice->{videostreaming}";
  
  	push @$devices, '-device', "virtio-serial,id=spice$pciaddr";

push @$devices, '-chardev', "spicevmc,id=vdagent,name=vdagent";
push @$devices, '-device', 
"virtserialport,chardev=vdagent,name=com.redhat.spice.0";
+
+   if ($spice_enhancements->{foldersharing}) {
+   push @$devices, '-chardev', 
"spiceport,id=foldershare,name=org.spice-space.webdav.0";
+   push @$devices, '-device', 
"virtserialport,chardev=foldershare,name=org.spice-space.webdav.0";
+   }
  }
  
  # enable balloon by default, unless explicitly disabled





___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH docs 1/2] Mention GUI for creating a cluster and adding nodes

2019-08-22 Thread Stefan Reiter
Signed-off-by: Stefan Reiter 
---
 pvecm.adoc | 81 --
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/pvecm.adoc b/pvecm.adoc
index e986a75..5379c3f 100644
--- a/pvecm.adoc
+++ b/pvecm.adoc
@@ -103,25 +103,33 @@ to the other with SSH via the easier to remember node 
name (see also
 xref:pvecm_corosync_addresses[Link Address Types]). Note that we always
 recommend to reference nodes by their IP addresses in the cluster 
configuration.
 
-
-[[pvecm_create_cluster]]
 Create the Cluster
 --
 
-Login via `ssh` to the first {pve} node. Use a unique name for your cluster.
-This name cannot be changed later. The cluster name follows the same rules as
-node names.
+Use a unique name for your cluster. This name cannot be changed later. The
+cluster name follows the same rules as node names.
+
+Create via Web GUI
+~~
+
+Under __Datacenter -> Cluster__, click on *Create Cluster*. Type your cluster
+name and select a network connection from the dropdown to serve as your main
+cluster network (Link 0, default is what the node's hostname resolves to).
+
+Optionally, you can select the 'Advanced' check box and choose an additional
+network interface for fallback purposes (Link 1, see also
+xref:pvecm_redundancy[Corosync Redundancy]).
+
+Create via Command Line
+~~~
+
+Login via `ssh` to the first {pve} node and run the following command:
 
 
  hp1# pvecm create CLUSTERNAME
 
 
-NOTE: It is possible to create multiple clusters in the same physical or 
logical
-network. Use unique cluster names if you do so. To avoid human confusion, it is
-also recommended to choose different names even if clusters do not share the
-cluster network.
-
-To check the state of your cluster use:
+To check the state of your new cluster use:
 
 
  hp1# pvecm status
@@ -131,9 +139,9 @@ Multiple Clusters In Same Network
 ~
 
 It is possible to create multiple clusters in the same physical or logical
-network. Each such cluster must have a unique name, this does not only helps
-admins to distinguish on which cluster they currently operate, it is also
-required to avoid possible clashes in the cluster communication stack.
+network. Each such cluster must have a unique name, to not only help admins
+distinguish which cluster they are currently operating on, but also to avoid
+possible clashes in the cluster communication stack.
 
 While the bandwidth requirement of a corosync cluster is relatively low, the
 latency of packages and the package per second (PPS) rate is the limiting
@@ -145,6 +153,39 @@ infrastructure for bigger clusters.
 Adding Nodes to the Cluster
 ---
 
+CAUTION: A new node cannot hold any VMs, because you would get
+conflicts about identical VM IDs. Also, all existing configuration in
+`/etc/pve` is overwritten when you join a new node to the cluster. To
+workaround, use `vzdump` to backup and restore to a different VMID after
+adding the node to the cluster.
+
+Add Node via GUI
+
+
+If you want to use "assisted join", where most parameters will be filled in for
+you, first login to the web interface on a node already in the cluster. Under
+__Datacenter -> Cluster__, click on *Join Information* at the top. Click on
+*Copy Information* or manually copy the string from the 'Information' field.
+
+To add the new node, login to the web interface on the node you want to add.
+Under __Datacenter -> Cluster__, click on *Join Cluster*. Fill in the
+'Information' field with the text you copied earlier.
+
+For security reasons, the password is not included, so you have to fill that in
+manually.
+
+NOTE: The Join Information is not necessarily required, you can also uncheck 
the
+'Assisted Join' checkbox and fill in the required fields manually.
+
+After clicking on *Join* your node will immediately be added to the cluster.
+You might need to reload the web page, and re-login with the cluster
+credentials.
+
+Confirm that your node is visible under __Datacenter -> Cluster__.
+
+Add Node via Command Line
+~
+
 Login via `ssh` to the node you want to add.
 
 
@@ -154,11 +195,6 @@ Login via `ssh` to the node you want to add.
 For `IP-ADDRESS-CLUSTER` use the IP or hostname of an existing cluster node.
 An IP address is recommended (see xref:pvecm_corosync_addresses[Link Address 
Types]).
 
-CAUTION: A new node cannot hold any VMs, because you would get
-conflicts about identical VM IDs. Also, all existing configuration in
-`/etc/pve` is overwritten when you join a new node to the cluster. To
-workaround, use `vzdump` to backup and restore to a different VMID after
-adding the node to the cluster.
 
 To check the state of the cluster use:
 
@@ -229,6 +265,8 @@ pvecm add IP-ADDRESS-CLUSTER -link0 LOCAL-IP-ADDRESS-LINK0
 If you want to use the built-in xref:pvecm_redundancy[redundancy] of the
 kronosnet transport layer

Re: [pve-devel] [PATCH docs 1/2] Mention GUI for creating a cluster and adding nodes

2019-08-22 Thread Thomas Lamprecht
On 8/22/19 4:53 PM, Stefan Reiter wrote:
> Signed-off-by: Stefan Reiter 
> ---
>  pvecm.adoc | 81 --
>  1 file changed, 60 insertions(+), 21 deletions(-)
> 
> diff --git a/pvecm.adoc b/pvecm.adoc
> index e986a75..5379c3f 100644
> --- a/pvecm.adoc
> +++ b/pvecm.adoc
> @@ -103,25 +103,33 @@ to the other with SSH via the easier to remember node 
> name (see also
>  xref:pvecm_corosync_addresses[Link Address Types]). Note that we always
>  recommend to reference nodes by their IP addresses in the cluster 
> configuration.
>  
> -
> -[[pvecm_create_cluster]]

NAK! You remove this but never re-add it in this patch. After installing 
pve-docs
with that patch applied building pve-manager would be broken, as this reference
is used there by the cluster GUI parts...

>  Create the Cluster
>  --
>  
> -Login via `ssh` to the first {pve} node. Use a unique name for your cluster.
> -This name cannot be changed later. The cluster name follows the same rules as
> -node names.
> +Use a unique name for your cluster. This name cannot be changed later. The
> +cluster name follows the same rules as node names.
> +
> +Create via Web GUI
> +~~
> +
> +Under __Datacenter -> Cluster__, click on *Create Cluster*. Type your cluster
> +name and select a network connection from the dropdown to serve as your main
> +cluster network (Link 0, default is what the node's hostname resolves to).
> +
> +Optionally, you can select the 'Advanced' check box and choose an additional
> +network interface for fallback purposes (Link 1, see also
> +xref:pvecm_redundancy[Corosync Redundancy]).
> +
> +Create via Command Line
> +~~~
> +
> +Login via `ssh` to the first {pve} node and run the following command:
>  
>  
>   hp1# pvecm create CLUSTERNAME
>  
>  
> -NOTE: It is possible to create multiple clusters in the same physical or 
> logical
> -network. Use unique cluster names if you do so. To avoid human confusion, it 
> is
> -also recommended to choose different names even if clusters do not share the
> -cluster network.
> -
> -To check the state of your cluster use:
> +To check the state of your new cluster use:
>  
>  
>   hp1# pvecm status
> @@ -131,9 +139,9 @@ Multiple Clusters In Same Network
>  ~
>  
>  It is possible to create multiple clusters in the same physical or logical
> -network. Each such cluster must have a unique name, this does not only helps
> -admins to distinguish on which cluster they currently operate, it is also
> -required to avoid possible clashes in the cluster communication stack.
> +network. Each such cluster must have a unique name, to not only help admins
> +distinguish which cluster they are currently operating on, but also to avoid
> +possible clashes in the cluster communication stack.
>  
>  While the bandwidth requirement of a corosync cluster is relatively low, the
>  latency of packages and the package per second (PPS) rate is the limiting
> @@ -145,6 +153,39 @@ infrastructure for bigger clusters.
>  Adding Nodes to the Cluster
>  ---
>  
> +CAUTION: A new node cannot hold any VMs, because you would get
> +conflicts about identical VM IDs. Also, all existing configuration in
> +`/etc/pve` is overwritten when you join a new node to the cluster. To
> +workaround, use `vzdump` to backup and restore to a different VMID after
> +adding the node to the cluster.
> +
> +Add Node via GUI
> +
> +
> +If you want to use "assisted join", where most parameters will be filled in 
> for
> +you, first login to the web interface on a node already in the cluster. Under
> +__Datacenter -> Cluster__, click on *Join Information* at the top. Click on
> +*Copy Information* or manually copy the string from the 'Information' field.
> +
> +To add the new node, login to the web interface on the node you want to add.
> +Under __Datacenter -> Cluster__, click on *Join Cluster*. Fill in the
> +'Information' field with the text you copied earlier.
> +
> +For security reasons, the password is not included, so you have to fill that 
> in
> +manually.
> +
> +NOTE: The Join Information is not necessarily required, you can also uncheck 
> the
> +'Assisted Join' checkbox and fill in the required fields manually.
> +
> +After clicking on *Join* your node will immediately be added to the cluster.
> +You might need to reload the web page, and re-login with the cluster
> +credentials.
> +
> +Confirm that your node is visible under __Datacenter -> Cluster__.
> +
> +Add Node via Command Line
> +~
> +
>  Login via `ssh` to the node you want to add.
>  
>  
> @@ -154,11 +195,6 @@ Login via `ssh` to the node you want to add.
>  For `IP-ADDRESS-CLUSTER` use the IP or hostname of an existing cluster node.
>  An IP address is recommended (see xref:pvecm_corosync_addresses[Link Address 
> Types]).
>  
> -CAUTION: A new node cannot hold any VMs, bec

[pve-devel] [PATCH v3 qemu-server] Fix #2041, #2272 Add Spice enhancements

2019-08-22 Thread Aaron Lauterer
This adds a new config option called `spice_enhancements` with two
optional settings:

* videostreaming
* foldersharing

Signed-off-by: Aaron Lauterer 
---

v2 -> v3:

Move the video-streaming string creation out of the '-spice' string for
backwards compatibility.
Remove the `// 0` from setting the foldersharing option.

v1 -> v2:
format changes suggested by dominik:
* changed descriptions
* changed `videostreaming` to enum

refactoring suggested by thomas:

I went with moving the `has_spice_enhancements` to the
`config_to_command` function and reducing the LOC.

Added a `spice` hash to store spice settings. Intended to be used in
further cleanup patches in this part of the code.

An empty `spice_enhancements` in the config file can be set (thanks
Dominik for pointing it out). That case is now handled to avoid errors.

The `streaming-video` option is always present now in the `-spice`
device. My tests showed no problems when migrating from a PVE node
without this patch to one with it. Migrating from newer PVE to an older
version without the patch resultet in a "resume failed" error.


 PVE/QemuServer.pm | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 9f5bf56..890a27c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -295,6 +295,20 @@ my $audio_fmt = {
 },
 };
 
+my $spice_enhancements_fmt = {
+foldersharing => {
+   type => 'boolean',
+   optional => 1,
+   description =>  "Enable folder sharing via SPICE. Needs Spice-WebDAV 
daemon installed in the VM."
+},
+videostreaming =>  {
+   type => 'string',
+   enum => ['off', 'all', 'filter'],
+   optional => 1,
+   description => "Enable video streaming. Uses compression for detected 
video streams."
+},
+};
+
 my $confdesc = {
 onboot => {
optional => 1,
@@ -672,6 +686,12 @@ EODESCR
description => "Configure a audio device, useful in combination with 
QXL/Spice.",
optional => 1
 },
+spice_enhancements => {
+   type => 'string',
+   format => $spice_enhancements_fmt,
+   description => "Configure additional enhancements for SPICE.",
+   optional => 1
+},
 };
 
 my $cicustom_fmt = {
@@ -3992,11 +4012,21 @@ sub config_to_command {
my $localhost = PVE::Network::addr_to_ip($nodeaddrs[0]->{addr});
$spice_port = PVE::Tools::next_spice_port($pfamily, $localhost);
 
-   push @$devices, '-spice', 
"tls-port=${spice_port},addr=$localhost,tls-ciphers=HIGH,seamless-migration=on";
+   my $spice = {};
+   my $spice_enhancements = $conf->{spice_enhancements} ? 
PVE::JSONSchema::parse_property_string($spice_enhancements_fmt, 
$conf->{spice_enhancements}) : {};
+   $spice->{videostreaming} = $spice_enhancements->{videostreaming} ? 
",streaming-video=$spice_enhancements->{videostreaming}" : '';
+   $spice->{foldersharing} = $spice_enhancements->{foldersharing};
+
+   push @$devices, '-spice', 
"tls-port=${spice_port},addr=$localhost,tls-ciphers=HIGH,seamless-migration=on$spice->{videostreaming}";
 
push @$devices, '-device', "virtio-serial,id=spice$pciaddr";
push @$devices, '-chardev', "spicevmc,id=vdagent,name=vdagent";
push @$devices, '-device', 
"virtserialport,chardev=vdagent,name=com.redhat.spice.0";
+
+   if ($spice_enhancements->{foldersharing}) {
+   push @$devices, '-chardev', 
"spiceport,id=foldershare,name=org.spice-space.webdav.0";
+   push @$devices, '-device', 
"virtserialport,chardev=foldershare,name=org.spice-space.webdav.0";
+   }
 }
 
 # enable balloon by default, unless explicitly disabled
-- 
2.20.1


___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel