From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <aderumier@odiso.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 4CA8E6A6B2
 for <pve-devel@lists.proxmox.com>; Thu, 25 Mar 2021 17:29:16 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 3D0951F5C9
 for <pve-devel@lists.proxmox.com>; Thu, 25 Mar 2021 17:29:16 +0100 (CET)
Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com
 [IPv6:2a00:1450:4864:20::329])
 (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id C6B491F5B1
 for <pve-devel@lists.proxmox.com>; Thu, 25 Mar 2021 17:29:12 +0100 (CET)
Received: by mail-wm1-x329.google.com with SMTP id 12so1513730wmf.5
 for <pve-devel@lists.proxmox.com>; Thu, 25 Mar 2021 09:29:12 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=odiso-com.20150623.gappssmtp.com; s=20150623;
 h=message-id:subject:from:to:date:in-reply-to:references:user-agent
 :mime-version; bh=QUJm0vN5LC6ldM4y1UQFXnQKPK76gnrji2uYkoAcLjY=;
 b=17ysCbBQKZP4BL93KOEBSVdbVVT0cYplHWz46GOnnkyGco6EIZZtE8AdB32NdGEtq6
 cSbcthXKFM2Y3bOYmEt0KKouuzakljd+vU+Cia1XaiJwrFwKEhs47U5Olvi/Pae4Rp7j
 6KnE6WI8b3rw3tDRHgyuMML3ZS44Q6xPtBxutyUn/s0UyiL5j2TD+xAXouz2sWwmgy24
 Dmu7S2C8iBX3hXAKkQ+1qTHPqSyIJ/72Vu2Iynt2pVvjd+YOG2ZnLRYr5W6kGgnro98H
 R1iYfX323XU4tFiUjD4q4MU6vVMQ0ZrIlUAfoeN9LF14S3wkfa3KlZYpHzAe94x1APCA
 /agw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to
 :references:user-agent:mime-version;
 bh=QUJm0vN5LC6ldM4y1UQFXnQKPK76gnrji2uYkoAcLjY=;
 b=VEH1gQJRlURVJuUpm5PnLmrvW2ZDv7sDchTfztY2phy6YlUdlVzIjBcpw0vvw73u03
 RpYysbFpOC/8Vn0uL8MWOgXAtbfDimEi924xtmx0HbYdUkHgK0Ad/Ty0r/dq/DrRWoxU
 Yje1SsGxau9BbjwzC8GnSwm8fu8vxiCnU/JFjLMXInr1Cbrr9BJO058BPAI6JWqV7Xl9
 Uivj5dq1PCVNnM0cdx4uOBrEMp+ATT3ZUlYNepikP0uPnqSHxUx/xk2GtO2vp6DhfW55
 cumTwdaKG/nkvxB6RNa+IMTOJ6kiZcfguCGpMnNlJjfFA5zwvSXnyZEwY0fSWqOmCVO9
 RxMg==
X-Gm-Message-State: AOAM5329RlJcCbgDUKzS92eNjmUeFdOoWrNe8IFkVXtLq6iKo3BcAGJd
 M7cKLTwxuJ+xZ229KqRuXCjueSQaJtcegGWdLPQ=
X-Google-Smtp-Source: ABdhPJwZrxaByOeSYCCXtWjqnlwPvCMd2SdAUv9QQmYOyR/N93VADoTnjk7DKno5ByjIirnxYP0+kA==
X-Received: by 2002:a1c:2016:: with SMTP id g22mr8828883wmg.137.1616689746436; 
 Thu, 25 Mar 2021 09:29:06 -0700 (PDT)
Received: from [192.168.178.50] ([79.132.236.200])
 by smtp.gmail.com with ESMTPSA id t14sm7924861wru.64.2021.03.25.09.29.05
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Thu, 25 Mar 2021 09:29:06 -0700 (PDT)
Message-ID: <e270af1e0e5670adc2d53e683eb432cf3dc398dd.camel@odiso.com>
From: aderumier@odiso.com
To: Thomas Lamprecht <t.lamprecht@proxmox.com>, Proxmox VE development
 discussion <pve-devel@lists.proxmox.com>
Date: Thu, 25 Mar 2021 17:29:05 +0100
In-Reply-To: <7f406812-b102-37ea-d967-1d43d4a759cf@proxmox.com>
References: <20210319120656.86740-1-aderumier@odiso.com>
 <20210319120656.86740-2-aderumier@odiso.com>
 <7f406812-b102-37ea-d967-1d43d4a759cf@proxmox.com>
User-Agent: Evolution 3.38.4 
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.155 Adjusted score from AWL reputation of From: address
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 HTML_MESSAGE            0.001 HTML included in message
 RCVD_IN_DNSWL_NONE     -0.0001 Sender listed at https://www.dnswl.org/,
 no trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [cloudinit.pm, qemuserver.pm]
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
X-Content-Filtered-By: Mailman/MimeDel 2.1.29
Subject: Re: [pve-devel] [PATCH qemu-server 1/4] cloudinit: add cloudinit
 section for current generated config.
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 25 Mar 2021 16:29:16 -0000

> But that collides with snapshots? E.g., if I make one named CLOUDINIT
> I get
> also a [CLOUDINIT] section in the config?
> 
oh, you are right indeed. I have added a check for new snasphots, but
user could
have already a snapshot called CLOUDINIT. 

> If we go that way you may want to use a unique identifier,
> [special:cloudinit]
> or the like (colons are not allowed for snapshot names, IIRC).

ok, I'll rework my patch .

Thanks for the review !

Le mercredi 24 mars 2021 à 06:58 +0100, Thomas Lamprecht a écrit :
> On 19.03.21 13:06, Alexandre Derumier wrote:
> > Instead using vm pending options for pending cloudinit generated
> > config,
> > 
> > write current generated cloudinit config in a new [CLOUDINIT]
> > SECTION.
> 
> But that collides with snapshots? E.g., if I make one named CLOUDINIT
> I get
> also a [CLOUDINIT] section in the config?
> 
> If we go that way you may want to use a unique identifier,
> [special:cloudinit]
> or the like (colons are not allowed for snapshot names, IIRC).
> 
> > 
> > Currently, some options like vm name, nic mac address can be
> > hotplugged,
> > so they are not way to know if the cloud-init disk is already
> > updated.
> > ---
> >  PVE/QemuServer.pm           | 22 +++++++++++++++++-----
> >  PVE/QemuServer/Cloudinit.pm | 32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 49 insertions(+), 5 deletions(-)
> > 
> > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> > index 57cfe62..f47ae87 100644
> > --- a/PVE/QemuServer.pm
> > +++ b/PVE/QemuServer.pm
> > @@ -2135,6 +2135,7 @@ sub parse_vm_config {
> >         digest => Digest::SHA::sha1_hex($raw),
> >         snapshots => {},
> >         pending => {},
> > +       cloudinit => {},
> >      };
> >  
> >      $filename =~ m|/qemu-server/(\d+)\.conf$|
> > @@ -2159,6 +2160,11 @@ sub parse_vm_config {
> >             $descr = undef;
> >             $conf = $res->{$section} = {};
> >             next;
> > +       } elsif ($line =~ m/^\[CLOUDINIT\]\s*$/i) {
> > +           $section = 'cloudinit';
> > +           $descr = undef;
> > +           $conf = $res->{$section} = {};
> > +           next;
> >  
> >         } elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
> >             $section = $1;
> > @@ -2219,7 +2225,6 @@ sub parse_vm_config {
> >             warn "vm $vmid - unable to parse config: $line\n";
> >         }
> >      }
> > -
> >      if (defined($descr)) {
> >         $descr =~ s/\s+$//;
> >         $conf->{description} = $descr;
> > @@ -2256,7 +2261,7 @@ sub write_vm_config {
> >  
> >         foreach my $key (keys %$cref) {
> >             next if $key eq 'digest' || $key eq 'description' ||
> > $key eq 'snapshots' ||
> > -               $key eq 'snapstate' || $key eq 'pending';
> > +               $key eq 'snapstate' || $key eq 'pending' || $key eq
> > 'cloudinit';
> >             my $value = $cref->{$key};
> >             if ($key eq 'delete') {
> >                 die "propertry 'delete' is only allowed in
> > [PENDING]\n"
> > @@ -2280,8 +2285,10 @@ sub write_vm_config {
> >  
> >      &$cleanup_config($conf->{pending}, 1);
> >  
> > +    &$cleanup_config($conf->{cloudinit}, 1);
> > +
> >      foreach my $snapname (keys %{$conf->{snapshots}}) {
> > -       die "internal error: snapshot name '$snapname' is
> > forbidden" if lc($snapname) eq 'pending';
> > +       die "internal error: snapshot name '$snapname' is
> > forbidden" if lc($snapname) =~ m/^(pending|cloudinit)$/; 
> >         &$cleanup_config($conf->{snapshots}->{$snapname}, undef,
> > $snapname);
> >      }
> >  
> > @@ -2310,7 +2317,7 @@ sub write_vm_config {
> >         }
> >  
> >         foreach my $key (sort keys %$conf) {
> > -           next if $key =~
> > /^(digest|description|pending|snapshots)$/;
> > +           next if $key =~
> > /^(digest|description|pending|cloudinit|snapshots)$/;
> >             $raw .= "$key: $conf->{$key}\n";
> >         }
> >         return $raw;
> > @@ -2323,6 +2330,11 @@ sub write_vm_config {
> >         $raw .= &$generate_raw_config($conf->{pending}, 1);
> >      }
> >  
> > +    if (scalar(keys %{$conf->{cloudinit}})){
> > +       $raw .= "\n[CLOUDINIT]\n";
> > +       $raw .= &$generate_raw_config($conf->{cloudinit}, 1);
> > +    }
> > +
> >      foreach my $snapname (sort keys %{$conf->{snapshots}}) {
> >         $raw .= "\n[$snapname]\n";
> >         $raw .= &$generate_raw_config($conf->{snapshots}-
> > >{$snapname});
> > @@ -4702,9 +4714,9 @@ sub vmconfig_apply_pending {
> >             $conf->{$opt} = delete $conf->{pending}->{$opt};
> >         }
> >      }
> > -
> >      # write all changes at once to avoid unnecessary i/o
> >      PVE::QemuConfig->write_config($vmid, $conf);
> > +
> >  }
> >  
> >  sub vmconfig_update_net {
> > diff --git a/PVE/QemuServer/Cloudinit.pm
> > b/PVE/QemuServer/Cloudinit.pm
> > index c464bf3..f4bf925 100644
> > --- a/PVE/QemuServer/Cloudinit.pm
> > +++ b/PVE/QemuServer/Cloudinit.pm
> > @@ -570,6 +570,38 @@ sub generate_cloudinitconfig {
> >  
> >         $generator->($conf, $vmid, $drive, $volname, $storeid);
> >      });
> > +
> > +    my $cloudinitconf = delete $conf->{cloudinit};
> > +    $cloudinitconf = {};
> > +
> > +    my @cloudinit_opts = keys
> > %{PVE::QemuServer::cloudinit_config_properties()};
> > +    push @cloudinit_opts, 'name';
> > +
> > +    foreach my $opt (@cloudinit_opts) {
> > +
> > +       if ($opt =~ m/^ipconfig(\d+)/) {
> > +           my $netid = "net$1";
> > +           next if !defined($conf->{$netid});
> > +           $conf->{cloudinit}->{$netid} = $conf->{$netid};
> > +       } 
> > +
> > +       $conf->{cloudinit}->{$opt} = $conf->{$opt} if $conf-
> > >{$opt}; 
> > +    }
> > +
> > +    $conf->{cloudinit}->{name} = "VM$vmid" if !$conf->{cloudinit}-
> > >{name};
> > +
> > +    foreach my $opt (keys %{$conf}) {
> > +       if (PVE::QemuServer::is_valid_drivename($opt)) {
> > +           my $drive = PVE::QemuServer::parse_drive($opt, $conf-
> > >{$opt});
> > +           if (PVE::QemuServer::drive_is_cloudinit($drive)) {
> > +               $conf->{cloudinit}->{$opt} = $conf->{$opt};
> > +
> > +           }
> > +       }
> > +    }
> > +
> > +    PVE::QemuConfig->write_config($vmid, $conf);
> > +
> >  }
> >  
> >  sub dump_cloudinit_config {
> > 
> 
>