[pve-devel] [PATCH 5/5] cloudinit : add support for generic storage + dedicated sata drive

Wolfgang Bumiller w.bumiller at proxmox.com
Mon Jul 6 13:15:39 CEST 2015


> ahciX : local:vm-xxx-cloudinit 

That would work. Just need to find a way to implement it without
changing too much.

> >>maybe could we add some kind of api to generate the configdrive once ?
> >>something like : ( enable|disable) cloudinit api :  create|delete the config drive on selected interface.

If there's such an interface though it might be worth adding the ability
to set a specific uuid? I'm not sure it's an ideal setup, though.

On Mon, Jul 06, 2015 at 12:56:16PM +0200, Alexandre DERUMIER wrote:
> >>maybe could we add some kind of api to generate the configdrive once ?
> >>something like : ( enable|disable) cloudinit api :  create|delete the config drive on selected interface.
> 
> maybe simply extend  : qm set xxx -(sata|ide|virtio)X  storeid:configdrive
> 
> 
> 
> 
> 
> ----- Mail original -----
> De: "aderumier" <aderumier at odiso.com>
> À: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
> Cc: "pve-devel" <pve-devel at pve.proxmox.com>
> Envoyé: Lundi 6 Juillet 2015 11:49:30
> Objet: Re: [pve-devel] [PATCH 5/5] cloudinit : add support for generic storage + dedicated sata drive
> 
> >>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` 
> 
> maybe could we add some kind of api to generate the configdrive once ? 
> something like : ( enable|disable) cloudinit api : create|delete the config drive on selected interface. 
> 
> 
> also, I think it could be great to have something matching the other drives 
> 
> ahciX : local:vm-xxx-cloudinit 
> 
> 
> Like this, we can keep current live migration,snapshot code without changing anything. (just need to change the parser) 
> 
> 
> 
> 
> 
> 
> ----- Mail original ----- 
> De: "Wolfgang Bumiller" <w.bumiller at proxmox.com> 
> À: "aderumier" <aderumier at odiso.com> 
> Cc: "pve-devel" <pve-devel at pve.proxmox.com> 
> Envoyé: Lundi 6 Juillet 2015 11:33:26 
> Objet: Re: [pve-devel] [PATCH 5/5] cloudinit : add support for generic storage + dedicated sata drive 
> 
> > + 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 <aderumier at 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 at pve.proxmox.com 
> > http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> > 
> _______________________________________________ 
> pve-devel mailing list 
> pve-devel at pve.proxmox.com 
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> 
> 



More information about the pve-devel mailing list