> +    if( $scfg->{path}) {
> +     $name .= ".qcow2";
> +     $fmt = 'qcow2';
> +    }else{
> +     $fmt = 'raw';
> +    }

This looks like a weird condition?

> +    $conf->{cloudinitdrive0} = PVE::QemuServer::print_drive($vmid, $drive);
> +    update_config_nolock($vmid, $conf);

The idea of changing the config when starting a VM is weird. It makes
everything more complicated to handle.
It would be nicer if the user was presented with an interface similar to
when adding CDROMs: They'd choose a controler (IDE/SATA/...) and the
disk gets added to it.
>From the backend side this would look similar to what your patches did
initially: react to a cloudinit storage type, except without modifying
the config.
`ahciX: cloudinit,storage=local`

> +    #fix me : remove old drive of if cloudinitdrive0 storeid change

Unless snapshots still reference it. Not sure if it makes sense to
support more than one cloud-init drive anyway, so there would only be
one, and this one contains snapshots and current data.

On Tue, Jun 30, 2015 at 04:01:50PM +0200, Alexandre Derumier wrote:
> This patch add support to create the cloudinit drive on any storage
> 
> I introduce an
> 
> cloudinitdrive0: local:100/vm-100-cloudinit.qcow2
> 
> to store the generate iso reference.
> 
> This is to avoid to scan everytime the storage at vm start,
> to see if the drive has already been generated.
> (and also if we change storeid from cloudinit option, we can remove the old 
> drive easily).
> 
> This drive is on a dedicated sata controller,so no conflict possible with 
> current users config.
> sata will be migratable in qemu 2.4 (already ok in master).
> 
> This drive works like other drives, so live migration will works out of the 
> box
> 
> Signed-off-by: Alexandre Derumier <aderum...@odiso.com>
> ---
>  PVE/QemuServer.pm | 80 
> ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 47 insertions(+), 33 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 82905ad..15fb471 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -636,6 +636,9 @@ for (my $i = 0; $i < $MAX_SATA_DISKS; $i++)  {
>      $confdesc->{"sata$i"} = $satadesc;
>  }
>  
> +$drivename_hash->{"cloudinitdrive0"} = 1;
> +$confdesc->{"cloudinitdrive0"} = $satadesc;
> +
>  for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
>      $drivename_hash->{"scsi$i"} = 1;
>      $confdesc->{"scsi$i"} = $scsidesc ;
> @@ -703,7 +706,8 @@ sub disknames {
>      return ((map { "ide$_" } (0 .. ($MAX_IDE_DISKS - 1))),
>              (map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
>              (map { "virtio$_" } (0 .. ($MAX_VIRTIO_DISKS - 1))),
> -            (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))));
> +            (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))),
> +         'cloudinitdrive');
>  }
>  
>  sub valid_drivename {
> @@ -1163,6 +1167,10 @@ sub print_drivedevice_full {
>       my $controller = int($drive->{index} / $MAX_SATA_DISKS);
>       my $unit = $drive->{index} % $MAX_SATA_DISKS;
>       $device = 
> "ide-drive,bus=ahci$controller.$unit,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}$drive->{index}";
> +    } elsif ($drive->{interface} eq 'cloudinitdrive'){
> +     my $controller = 1;
> +     my $unit = 0;
> +     $device = 
> "ide-cd,bus=ahci$controller.$unit,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}$drive->{index}";
>      } elsif ($drive->{interface} eq 'usb') {
>       die "implement me";
>       #  -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0
> @@ -2068,11 +2076,6 @@ sub write_vm_config {
>       delete $conf->{cdrom};
>      }
>  
> -    if ($conf->{cloudinit} && $conf->{ide3}) {
> -     die "option cloudinit conflicts with ide3\n";
> -     delete $conf->{cloudinit};
> -    }
> -
>      # we do not use 'smp' any longer
>      if ($conf->{sockets}) {
>       delete $conf->{smp};
> @@ -2615,21 +2618,6 @@ sub foreach_drive {
>  
>       &$func($ds, $drive);
>      }
> -
> -    if (my $storeid = $conf->{cloudinit}) {
> -     my $storecfg = PVE::Storage::config();
> -     my $imagedir = PVE::Storage::get_image_dir($storecfg, $storeid, $vmid);
> -     my $iso_name = "vm-$vmid-cloudinit.qcow2";
> -     my $iso_path = "$imagedir/$iso_name";
> -     # Only snapshot it if it has already been created.
> -     # (Which is not the case if the VM has never been started before with
> -     # cloud-init enabled.)
> -     if (-e $iso_path) {
> -         my $ds = 'ide3';
> -         my $drive = parse_drive($ds, 
> "$storeid:$vmid/vm-$vmid-cloudinit.qcow2");
> -         &$func($ds, $drive) if $drive;
> -     }
> -    }
>  }
>  
>  sub foreach_volid {
> @@ -3203,6 +3191,13 @@ sub config_to_command {
>             $ahcicontroller->{$controller}=1;
>          }
>  
> +        if ($drive->{interface} eq 'cloudinitdrive') {
> +           my $controller = 1;
> +           $pciaddr = print_pci_addr("ahci$controller", $bridges);
> +           push @$devices, '-device', 
> "ahci,id=ahci$controller,multifunction=on$pciaddr" if 
> !$ahcicontroller->{$controller};
> +           $ahcicontroller->{$controller}=1;
> +        }
> +
>       my $drive_cmd = print_drive_full($storecfg, $vmid, $drive);
>       push @$devices, '-drive',$drive_cmd;
>       push @$devices, '-device', print_drivedevice_full($storecfg, $conf, 
> $vmid, $drive, $bridges);
> @@ -4851,6 +4846,7 @@ sub print_pci_addr {
>       'net29' => { bus => 1, addr => 24 },
>       'net30' => { bus => 1, addr => 25 },
>       'net31' => { bus => 1, addr => 26 },
> +     'ahci1' => { bus => 1, addr => 27 },
>       'virtio6' => { bus => 2, addr => 1 },
>       'virtio7' => { bus => 2, addr => 2 },
>       'virtio8' => { bus => 2, addr => 3 },
> @@ -6381,17 +6377,25 @@ sub scsihw_infos {
>  my $cloudinit_iso_size = 5; # in MB
>  
>  sub prepare_cloudinit_disk {
> -    my ($vmid, $storeid) = @_;
> +    my ($vmid, $conf, $storeid) = @_;
>  
>      my $storecfg = PVE::Storage::config();
> -    my $imagedir = PVE::Storage::get_image_dir($storecfg, $storeid, $vmid);
> -    my $iso_name = "vm-$vmid-cloudinit.qcow2";
> -    my $iso_path = "$imagedir/$iso_name";
> -    return $iso_path if -e $iso_path;
> +    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> +    my $name = "vm-$vmid-cloudinit";
> +    my $fmt = undef;
> +    if( $scfg->{path}) {
> +     $name .= ".qcow2";
> +     $fmt = 'qcow2';
> +    }else{
> +     $fmt = 'raw';
> +    }
> +    my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, 
> $name, $cloudinit_iso_size*1024);
>  
> -    # vdisk_alloc size is in K
> -    my $iso_volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, 
> 'qcow2', $iso_name, $cloudinit_iso_size*1024);
> -    return $iso_path;
> +    my $drive = {};
> +    $drive->{file} = $volid;
> +    $drive->{media} = 'cdrom';
> +    $conf->{cloudinitdrive0} = PVE::QemuServer::print_drive($vmid, $drive);
> +    update_config_nolock($vmid, $conf);
>  }
>  
>  # FIXME: also in LXCCreate.pm => move to pve-common
> @@ -6407,10 +6411,10 @@ sub next_free_nbd_dev {
>  }
>  
>  sub commit_cloudinit_disk {
> -    my ($file_path, $iso_path) = @_;
> +    my ($file_path, $iso_path, $format) = @_;
>  
>      my $nbd_dev = next_free_nbd_dev();
> -    run_command(['qemu-nbd', '-c', $nbd_dev, $iso_path]);
> +    run_command(['qemu-nbd', '-c', $nbd_dev, $iso_path, '-f', $format]);
>  
>      eval {
>       run_command(['genisoimage',
> @@ -6439,8 +6443,18 @@ sub generate_cloudinitconfig {
>                   . generate_cloudinit_network($conf, $path);
>      generate_cloudinit_metadata($conf, $path, $digest_data);
>  
> -    my $iso_path = prepare_cloudinit_disk($vmid, $storeid);
> -    commit_cloudinit_disk("$path/drive", $iso_path);
> +    #fix me : remove old drive of if cloudinitdrive0 storeid change
> +    prepare_cloudinit_disk($vmid, $conf, $storeid) if 
> !$conf->{cloudinitdrive0};
> +
> +    my $drive = parse_drive('cloudinitdrive0', $conf->{cloudinitdrive0});
> +
> +    my $storecfg = PVE::Storage::config();
> +    my (undef, $volname) = PVE::Storage::parse_volume_id($drive->{file}, 1);
> +    my $iso_path = PVE::Storage::path($storecfg, $drive->{file});
> +    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> +    my $format = qemu_img_format($scfg, $volname);
> +
> +    commit_cloudinit_disk("$path/drive", $iso_path, $format);
>      rmtree("$path/drive");
>  }
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to