public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Aaron Lauterer <a.lauterer@proxmox.com>,
	Fabian Ebner <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH v6 storage 1/1] add disk reassign feature
Date: Thu, 15 Apr 2021 14:20:40 +0200	[thread overview]
Message-ID: <bc4ad6ef-6eb8-7770-366f-9381f69b8130@proxmox.com> (raw)
In-Reply-To: <3a122331-1f39-5e52-bcff-8ed28f763a2c@proxmox.com>

On 15.04.21 13:53, Aaron Lauterer wrote:
> Just adding the functionality on the top level Plugin.pm could have some
> potential ugly side effects for 3rd party plugins that do not yet handle that
> call themselves. So to be on the safe side, by default we rather fail right
> there (was discussed a versions ago).

I may have forgotten the old discussion, but I do not think that this is a problem.

External plugins can detect if they require it and implement it, and actually,
we could just check the ABI version a plugin provides on calling the base method
and error out if it's less than the one where we introduced this method.

> IMHO it would be nice though to change the structure of the storage plugins a
> bit. E.g. instead of assuming dir/file storages for Plugin.pm, having a basic
> abstraction specifically for any directory/file based storage which handles
> all the common tasks and further down the hierarchy the specific
> implementations regarding mounting and such. But that would mean a hard break
> of the current approach, especially for 3rd party plugins.

That sounds actually quite like what we already have, rather the base plugin
module should just provide the set of methods with a `die "implement me"`, and
probably only that, i.e., be a plain abstract interface.

But that's quite some change involved and requires a ABI version break as all
plugins would need to adapt to that one, and the benefit is meh, at least for
our internal ones; and after all those are the ones we actually support.




  parent reply	other threads:[~2021-04-15 12:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-02 10:19 [pve-devel] [PATCH v6 series 0/5] disk reassign: add new feature Aaron Lauterer
2021-04-02 10:19 ` [pve-devel] [PATCH v6 storage 1/1] add disk reassign feature Aaron Lauterer
2021-04-13  7:53   ` Dominic Jäger
2021-04-15 11:07   ` Fabian Ebner
2021-04-15 11:31     ` Fabian Ebner
2021-04-15 11:53       ` Aaron Lauterer
2021-04-15 12:09         ` Fabian Ebner
2021-04-15 12:21           ` Thomas Lamprecht
2021-04-15 12:20         ` Thomas Lamprecht [this message]
2021-04-02 10:19 ` [pve-devel] [PATCH v6 qemu-server 2/5] disk reassign: add API endpoint Aaron Lauterer
2021-04-15 11:52   ` Fabian Ebner
2021-04-19  9:26     ` Aaron Lauterer
2021-04-18 15:24   ` Thomas Lamprecht
2021-04-19  9:25     ` Aaron Lauterer
2021-04-02 10:19 ` [pve-devel] [PATCH v6 qemu-server 3/5] cli: disk reassign: add reassign_disk to qm command Aaron Lauterer
2021-04-18 14:50   ` Thomas Lamprecht
2021-04-02 10:19 ` [pve-devel] [PATCH v6 guest-common 4/5] Replication: mention disk reassign in comment of possible reasons Aaron Lauterer
2021-04-02 10:19 ` [pve-devel] [PATCH v6 manager 5/5] ui: tasks: add qmreassign task description Aaron Lauterer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc4ad6ef-6eb8-7770-366f-9381f69b8130@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=a.lauterer@proxmox.com \
    --cc=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal