[pve-devel] [PATCH qemu-server] fix #2173: use list_images to check for cloudinit disk

[Thomas Lamprecht]

Am 4/25/19 um 12:10 PM schrieb Mira Limbeck:
> 
> On 4/24/19 3:58 PM, Thomas Lamprecht wrote:
>> Am 4/15/19 um 1:50 PM schrieb Mira Limbeck:
>>> use list_images to check for existence of cloudinit disk instead of
>>> '-e'. this should solve the problem with rbd where the path returned by
>>> PVE::Storage::path is not checkable with '-e'.
>> "should" (guessing) or "does solve" (tested) it??
> I tested it of course. Bad wording
>> how does the 'qemu-img dd' then writes to it? does it map it itself,
>> or connects via rbd? (if so with which config?)
> The path contains config, id and keyring, so all required information is passed to 'qemu-img dd'. The string returned by PVE::Storage::path for one of my test-vms is: 'rbd:pool1/vm-102-cloudinit:conf=/etc/pve/ceph.conf:id=admin:keyring=/etc/pve/priv/ceph/pool1.keyring'

Thanks for clarifying!

>>
>> Why not just doing a map_volume before? then you can keep the rest as
>> is and not do a full content listing (albeit some plugins cache it,
>> it's not exactly cheap, especially with big storages)...
> I'll take a look at map_volume. If I understand it correctly it returns the absolute path (and if necessary maps the volume so one exists (e.g. rbd))?

Yes, the single issue is that the default implementation does not
defaults to returning the path, but we could change that just fine,
and actually Wolfgang and I talked about this a few times and, IIRC,
agreed that this change would be nicer ABI wise (and not break old
ABI)..

As currently our map vol use go like:

my $path = PVE:Storage::map_vol(...);
$path = PVE:Storage::path(...) if !defined($path);



>>
>>> Signed-off-by: Mira Limbeck<m.limbeck at proxmox.com>
>>> ---
>>>   PVE/QemuServer/Cloudinit.pm | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
>>> index 445c777..da08470 100644
>>> --- a/PVE/QemuServer/Cloudinit.pm
>>> +++ b/PVE/QemuServer/Cloudinit.pm
>>> @@ -31,13 +31,17 @@ sub commit_cloudinit_disk {
>>>       my $iso_path = PVE::Storage::path($storecfg, $drive->{file});
>>>       my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>>>       my $format = PVE::QemuServer::qemu_img_format($scfg, $volname);
>>> -    if (! -e $iso_path) {
>>> +
>>> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>>> +    my $vollist = $plugin->list_images($storeid, $scfg, $vmid);
>>> +    my $exists = grep { $_->{volid} eq $drive->{file} } @$vollist;
>>> +
>>> +    if (!$exists) {
>>>       $volname =~ m/(vm-$vmid-cloudinit(.(qcow2|raw))?)/;
>>>       my $name = $1;
>>>       my $d = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, $name, 4 * 1024);
>>>       }
>>>   -    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>>>       $plugin->activate_volume($storeid, $scfg, $volname);
>>>         my $size = PVE::Storage::file_size_info($iso_path);
>>>