seems good to me. (note: maybe send in again as separate mail? had to
edit the mail because commit message wasn't picked up right and apply with
--ignore-whitespace)

Tested-by: Oguz Bektas <o.bek...@proxmox.com>

On Wed, Sep 25, 2019 at 02:37:32PM +0200, Fabian Grünbichler wrote:
> On September 25, 2019 1:30 pm, Oguz Bektas wrote:
> > merging $conf->{lxc} causes lxc.idmap entries to be duplicated in the
> > restored configuration. instead, we can overwrite the contents from the
> > extracted configuration file. this way we don't duplicate these entries.
> > (having duplicate idmap entries causes container to crash during start)
> > 
> > Co-developed-by: Stefan Reiter <s.rei...@proxmox.com>
> > Signed-off-by: Oguz Bektas <o.bek...@proxmox.com>
> 
> What about the following instead (note, generated with -w for 
> readability)? seems simpler, and with less potential for future breakage 
> ;)
> 
> From 8e0679858748be369d5ddc5695376b12504daa50 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbich...@proxmox.com>
> Date: Wed, 25 Sep 2019 14:35:04 +0200
> Subject: [PATCH container] restore lxc.* entries once
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> either via recover_config, OR via restore_configuration. non-root behaviour 
> stays the same.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
> ---
>  src/PVE/API2/LXC.pm   | 4 ++--
>  src/PVE/LXC/Create.pm | 6 +-----
>  2 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 07280fb..28c9047 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -353,11 +353,11 @@ __PACKAGE__->register_method({
>                       my $orig_conf;
>                       ($orig_conf, $orig_mp_param) = 
> PVE::LXC::Create::recover_config($archive);
>                       $was_template = delete $orig_conf->{template};
> -                     # When we're root call 'restore_configuration' with 
> ristricted=0,
> +                     # When we're root call 'restore_configuration' with 
> restricted=0,
>                       # causing it to restore the raw lxc entries, among 
> which there may be
>                       # 'lxc.idmap' entries. We need to make sure that the 
> extracted contents
>                       # of the container match up with the restored 
> configuration afterwards:
> -                     $conf->{lxc} = [grep { $_->[0] eq 'lxc.idmap' } 
> @{$orig_conf->{lxc}}];
> +                     $conf->{lxc} = $orig_conf->{lxc};
>                   }
>               }
>               if ($storage_only_mode) {
> diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
> index a46a50c..241ca88 100644
> --- a/src/PVE/LXC/Create.pm
> +++ b/src/PVE/LXC/Create.pm
> @@ -176,18 +176,14 @@ sub restore_configuration {
>           # storage supports creating a template there
>           next if $key =~ /^template$/;
>  
> -         if ($key eq 'lxc') {
> +         if ($key eq 'lxc' && $restricted) {
>               my $lxc_list = $oldconf->{'lxc'};
> -             if ($restricted) {
>               warn "skipping custom lxc options, restore manually as root:\n";
>               warn "--------------------------------\n";
>               foreach my $lxc_opt (@$lxc_list) {
>                   warn "$lxc_opt->[0]: $lxc_opt->[1]\n"
>               }
>               warn "--------------------------------\n";
> -             } else {
> -                 @{$conf->{$key}} = (@$lxc_list, @{$conf->{$key}});
> -             }
>               next;
>           }
>  
> -- 
> 2.20.1
> 
> > ---
> >  src/PVE/LXC/Create.pm | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
> > index a46a50c..8b2cfc9 100644
> > --- a/src/PVE/LXC/Create.pm
> > +++ b/src/PVE/LXC/Create.pm
> > @@ -186,7 +186,9 @@ sub restore_configuration {
> >                 }
> >                 warn "--------------------------------\n";
> >             } else {
> > -               @{$conf->{$key}} = (@$lxc_list, @{$conf->{$key}});
> > +               # $conf->{lxc} can only have lxc.idmap
> > +               # so we can overwrite the current $conf from $oldconf
> > +               $conf->{$key} = $lxc_list;
> >             }
> >             next;
> >         }
> > -- 
> > 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 mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to