[pve-devel] [RFC_V2 manager] Storage: remove guest images in storage contendview

[Thomas Lamprecht]

On 11/14/18 8:06 AM, Fabian Grünbichler wrote:
> On Mon, Nov 12, 2018 at 11:15:33AM +0100, Dietmar Maurer wrote:
>>> If an image on storage has not referenced to any guest or
>>> replication config, we can safely delete it on the GUI.
>>> Also, if a config exists on another node, we can delete it too.
>>
>> Only if the image is on local storage ...
>>
>>> But if an image has a <vmid> encoded in the image name and a guest
>>> exists in the cluster with this VMID then it must a lost image of the VM.
>>
>> OK
>>
>>> In this case, a rescan will add it back to the config.
>>
>> I would not rescan here - it is too complex (needs to be done inside a worker task).
>>
>> Also, I would remove the replication config code - simple remove the image
>> if the user request it.
>>
>>> This follows the logic of 'qm rescan',
>>> what assume if an image exists on a node,
>>> Images that are marked as unused in the config are referred.
>>>
>>> This call can't be in the store because of cycles dependencies.
>>> The extra Subclass is necessary for the "fragmentDelimiter"
>>> which is used for forwarding the correct volume name for dir storages.
>>
>> Without the replication/rescan logic, it should be possible to implement inside
>> pve-storage - please can you try?
> 
> I wonder if it would not make sense to make this a GUI only feature? we
> have all the components already in the API:
> 1) get volumes on a storage FOO, including vmid as determined by storage
> plugin
> 2) get config of VM/CT BAR and check if volid BAZ is referenced
> 3) user can (given appropriate permissions) just call the regular DELETE
> storage/content API path
> 
> there might be a small race (at least QemuServer's importdisk allocs an
> image and only adds the reference to it in the config after qemu-img
> convert is done), but for a manual action the admin/VM user probably
> knows when such an action is concurrently running.
> 
> any non-GUI users can already trivially build this functionality via the
> API/pvesh/CLI interfaces anyway, since all the 'logic' is just filtering
> returned values and passing them to the next API/.. call.

just as a note, we rejected such gui only features in the past as we did not
wanted to have much logic there, no general objection, just as question to
why this now? And races and volume deletions does not sounds fun at all...