Re: [pve-devel] [PATCH manager 2/3] Allow setting targetstorage for offline migration

2020-05-02 Thread Alexandre DERUMIER 
>>The problem is that offline migration with target storage might not
>>always work depending on supported export/import formats. Then users
>>might start an offline migration, which then fails after a few disks
>>have already been copied.

Hi,

Technically, it could be possible to support any kind of src/dst format for 
offline migration,
with creating an nbd server on target (with qemu-nbd for example).

Maybe it could be implemented as fallback, if any other method exist ?




- Mail original -
De: "Fabian Ebner" 
À: "pve-devel" 
Envoyé: Jeudi 30 Avril 2020 13:15:59
Objet: Re: [pve-devel] [PATCH manager 2/3] Allow setting targetstorage for 
offline migration

Let's make this one an RFC ;) 
The problem is that offline migration with target storage might not 
always work depending on supported export/import formats. Then users 
might start an offline migration, which then fails after a few disks 
have already been copied. 
If we had a check for the export/import formats before starting 
migration, it would improve. But currently the erroring out happens on a 
per disk basis inside storage_migrate. 

So I'm not sure anymore if this is an improvement. If not, and if patch 
#3 is fine, I'll send a v2 without this one. 

On 30.04.20 12:59, Fabian Ebner wrote: 
> Signed-off-by: Fabian Ebner  
> --- 
> www/manager6/window/Migrate.js | 12 ++-- 
> 1 file changed, 2 insertions(+), 10 deletions(-) 
> 
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js 
> index 9fc66a9b..20e057ad 100644 
> --- a/www/manager6/window/Migrate.js 
> +++ b/www/manager6/window/Migrate.js 
> @@ -44,13 +44,6 @@ Ext.define('PVE.window.Migrate', { 
> return gettext('Offline'); 
> } 
> }, 
> - setStorageselectorHidden: function(get) { 
> - if (get('migration.with-local-disks') && get('running')) { 
> - return false; 
> - } else { 
> - return true; 
> - } 
> - }, 
> setLocalResourceCheckboxHidden: function(get) { 
> if (get('running') || !get('migration.hasLocalResources') || 
> Proxmox.UserName !== 'root@pam') { 
> @@ -126,8 +119,7 @@ Ext.define('PVE.window.Migrate', { 
> if (vm.get('migration.with-local-disks')) { 
> params['with-local-disks'] = 1; 
> } 
> - //only submit targetstorage if vm is running, storage migration to 
> different storage is only possible online 
> - if (vm.get('migration.with-local-disks') && vm.get('running')) { 
> + if (vm.get('migration.with-local-disks')) { 
> params.targetstorage = values.targetstorage; 
> } 
> 
> @@ -352,7 +344,7 @@ Ext.define('PVE.window.Migrate', { 
> fieldLabel: gettext('Target storage'), 
> storageContent: 'images', 
> bind: { 
> - hidden: '{setStorageselectorHidden}' 
> + hidden: '{!migration.with-local-disks}', 
> } 
> }, 
> { 
> 

___ 
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 v2 firewall] fix #2686: don't add arp-ip-src filter for dhcp

2020-05-02 Thread Thomas Lamprecht 
On 4/30/20 12:26 PM, Mira Limbeck wrote:
> When the IPFilter setting is enabled and the container has DHCP
> configured on an interface no 'arp-ip-src' filter should be added as we
> don't have an IP address.
> Previously '--arp-ip-src dhcp' was passed to ebtables which led to an error.
> 
> Signed-off-by: Mira Limbeck 
> ---
> v2:
>  - changed regex to a simple string comparison
> 

applied, thanks!

>  src/PVE/Firewall.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index d22b15a..250a642 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -3904,7 +3904,9 @@ sub compile_ebtables_filter {
>   # ebtables changes this to a .0/MASK network but we just
>   # want the address here, no network - see #2193
>   $ip =~ s|/(\d+)$||;
> - push @$arpfilter, $ip;
> + if ($ip ne 'dhcp') {
> + push @$arpfilter, $ip;
> + }
>   }
>   generate_tap_layer2filter($ruleset, $iface, $macaddr, 
> $vmfw_conf, $vmid, $arpfilter);
>   }
> 


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

[pve-devel] applied-series: [PATCH firewall/cluster 0/9] add locking to firewall config changes

2020-05-02 Thread Thomas Lamprecht 
On 4/29/20 10:52 AM, Fabian Grünbichler wrote:
> the second cluster patch is optional, but improves usability of
> non-worker API calls that do
> 
> cfs_lock_foo(..., sub {
> 
>   raise_foo
> 
> });
> 
> the last three firewall patches are unrelated bug fixes that I found
> while testing.
> 
> pve-cluster:
> 
> Fabian Grünbichler (2):
>   cfs_lock: add firewall lock helper
>   cfs_lock: re-raise exceptions
> 
>  data/PVE/Cluster.pm | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> pve-firewall:
> 
> Fabian Grünbichler (7):
>   configs: add locking helpers
>   api: add locking helpers
>   api: lock configs
>   clone_vmfw_conf: lock new config
>   api/ipsets: parse_cidr before checking for duplicates
>   configs: warn about duplicate ipset entries
>   rules: verify referenced security group exists
> 

applied series, thanks!

>  src/PVE/API2/Firewall/Aliases.pm | 104 ++--
>  src/PVE/API2/Firewall/Cluster.pm |  36 +++---
>  src/PVE/API2/Firewall/Groups.pm  |  52 
>  src/PVE/API2/Firewall/Host.pm|  42 ---
>  src/PVE/API2/Firewall/IPSet.pm   | 204 +--
>  src/PVE/API2/Firewall/Rules.pm   | 130 ++--
>  src/PVE/API2/Firewall/VM.pm  |  43 +++
>  src/PVE/Firewall.pm  |  65 --
>  8 files changed, 449 insertions(+), 227 deletions(-)
> 



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