[pve-devel] [PATCH Qemu-Server Adapt Cloudbase-init for windows 1/1] Signed-off-by: Austin Chan <sptrsca at gmail.com>

[Thomas Lamprecht]

On 4/11/19 10:46 AM, Austin Chan wrote:
>  On branch cloudbase-init
>  Changes to be committed:
> 	modified:   PVE/API2/Qemu.pm
> 	modified:   PVE/QemuServer/Cloudinit.pm
> ---
>  PVE/API2/Qemu.pm            |  8 ++++++--
>  PVE/QemuServer/Cloudinit.pm | 13 +++++++++----
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index d8c9726..b1b7191 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1012,10 +1012,14 @@ my $update_vm_api  = sub {
>  
>      my $background_delay = extract_param($param, 'background_delay');
>  
> +    my $ostype = PVE::QemuConfig->load_config($vmid)->{ostype};
> +
>      if (defined(my $cipassword = $param->{cipassword})) {
>  	# Same logic as in cloud-init (but with the regex fixed...)
> -	$param->{cipassword} = PVE::Tools::encrypt_pw($cipassword)
> -	    if $cipassword !~ /^\$(?:[156]|2[ay])(\$.+){2}/;
> +	if (!(PVE::QemuServer::windows_version($ostype))) {

nitpick: coding style, the inner parenthesis can be omitted, short comment
why not doing this with windows could be great for other readers.

> +	    $param->{cipassword} = PVE::Tools::encrypt_pw($cipassword)
> +		if $cipassword !~ /^\$(?:[156]|2[ay])(\$.+){2}/;
> +	}
>      }
>  
>      my @paramarr = (); # used for log message
> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
> index 445c777..4f39a2b 100644
> --- a/PVE/QemuServer/Cloudinit.pm
> +++ b/PVE/QemuServer/Cloudinit.pm
> @@ -43,7 +43,7 @@ sub commit_cloudinit_disk {
>      my $size = PVE::Storage::file_size_info($iso_path);
>  
>      eval {
> -	run_command([['genisoimage', '-R', '-V', $label, $path],
> +	run_command([['genisoimage','-iso-level', '3', '-R', '-V', $label, $path],

why do you make this change, don't get me wrong, I'm not opposed in general
but I'd like to have reasons for such changes.

>  		     ['qemu-img', 'dd', '-n', '-f', 'raw', '-O', $format,
>  		      'isize=0', "osize=$size", "of=$iso_path"]]);
>      };
> @@ -159,11 +159,11 @@ sub configdrive2_network {
>      my ($searchdomains, $nameservers) = get_dns_conf($conf);
>      if ($nameservers && @$nameservers) {
>  	$nameservers = join(' ', @$nameservers);
> -	$content .= "        dns_nameservers $nameservers\n";
> +	$content .= "        dns-nameservers $nameservers\n";
>      }
>      if ($searchdomains && @$searchdomains) {
>  	$searchdomains = join(' ', @$searchdomains);
> -	$content .= "        dns_search $searchdomains\n";
> +	$content .= "        dns-search $searchdomains\n";

do I get any advantage, or why do you change this? DO you have a link to a spec
of this (I know this being cloudinit, that may not be easy to have), but a quick
check in the cloud-init repo gives me a lot of hits with both, the underscore
and the hyphen version, so...

>      }
>  
>      my @ifaces = grep(/^net(\d+)$/, keys %$conf);
> @@ -203,9 +203,14 @@ sub configdrive2_network {
>  
>  sub configdrive2_metadata {
>      my ($uuid) = @_;
> +	my ($conf, $vmid, $uuid) = @_;

hmm, now we have two = @_ param assignments here, and a redefinition
of $uuid? maybe remove the old one?

> +	my ($hostname, $fqdn) = get_hostname_fqdn($conf, $vmid);
> +	my $password = $conf->{cipassword};
>      return <<"EOF";
>  {
>       "uuid": "$uuid",
> +	 "hostname": "$hostname",
> +	 "admin_pass": "$password",

again, please split the changes up and provide a reason for adding this,
even if it may seem clear to you it makes it easier to review and to look
back at in the future, thanks!

>       "network_config": { "content_path": "/content/0000" }
>  }
>  EOF
> @@ -222,7 +227,7 @@ sub generate_configdrive2 {
>  	my $digest_data = $user_data . $network_data;
>  	my $uuid_str = Digest::SHA::sha1_hex($digest_data);
>  
> -	$meta_data = configdrive2_metadata($uuid_str);
> +	$meta_data = configdrive2_metadata($conf, $vmid, $uuid_str);
>      }
>      my $files = {
>  	'/openstack/latest/user_data' => $user_data,
>