public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Joaquin Varela <joaquinvarela@neatech.ar>
To: "Max R. Carrara" <m.carrara@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [RFC] Native ZFS over NVMe/TCP storage backend for Proxmox VE
Date: Sun, 2 Aug 2026 00:20:41 -0300	[thread overview]
Message-ID: <CANSq6_LCXRwcvhBmV9ceDZQqBk8KOxkQBV40j6=W-qw3m+YsHg@mail.gmail.com> (raw)
In-Reply-To: <DK482XJKMBPD.36AEQU7BG2MUP@proxmox.com>

Hi Max,

sorry for the delayed reply. Gmail had grouped your response into the
original RFC conversation and I missed it at the time.

Thank you very much for taking the time to review the repositories and
for the encouraging feedback.

I signed and submitted the Proxmox Individual CLA on July 15.

I have now applied the style changes you suggested:

- the new Perl modules use `use v5.36`;
- the relevant regular expressions are declared as constants and use
  the `n` and `x`/`xx` modifiers with named capture groups;
- internal helpers use lexical `my sub` declarations where appropriate;
- the new code uses postfix dereferencing;
- the submission branches do not contain debian/changelog entries.

Regarding the storage backend used for validation, the target was a
dedicated Ubuntu 24.04 virtual machine running:

- kernel 6.8.0-136-generic;
- ZFS 2.2.2;
- Linux nvmet;
- nvme-cli 2.8;
- a ZFS pool named `tank`;
- two independent NVMe/TCP portals.

The initiators were a two-node nested Proxmox VE 9.2 cluster running
kernel 7.0.14-8-pve and nvme-cli 2.13. Each node had two independent
NVMe/TCP paths and native kernel multipath enabled.

The tests covered thin provisioning and discard, snapshots, clones,
concurrent namespace allocation, path failures, all-path loss policies,
target reboot reconstruction, bidirectional live migration under guest
I/O, migration to a degraded destination, HA fencing, a live corosync
partition and explicit dual-writer detection.

The target-reboot testing also uncovered an ACL publication race. The
backend now restores every configured Host NQN, DH-HMAC-CHAP key and
namespace before linking the subsystem to its NVMe/TCP ports.

The complete test report, machine-readable results and raw logs are
available here:

https://github.com/joaquinv98/pve-storage/tree/feature/zfs-nvme-tcp

The clean branches corresponding to the patch series are:

https://github.com/joaquinv98/pve-storage/tree/submission/zfs-nvme-tcp-rfc-v2
https://github.com/joaquinv98/pve-manager/tree/submission/zfs-nvme-tcp-rfc-v2
https://github.com/joaquinv98/pve-docs/tree/submission/zfs-nvme-tcp-rfc-v2

I have prepared the formal pve-storage, pve-manager and pve-docs patch
series and will send them to pve-devel as separate threads shortly.

Thank you again for your review.

Kind regards,
Joaquin Varela

On Tue, Jul 21, 2026 at 8:46 AM Max R. Carrara <m.carrara@proxmox.com>
wrote:

> On Wed Jul 15, 2026 at 6:23 PM CEST, Joaquin Varela wrote:
> > Hello,
> >
> > I have implemented an in-tree Proxmox VE storage backend named `zfsnvme`.
> > It follows the remote ZFS lifecycle model used by ZFS over iSCSI, but
> uses
> > Linux nvmet, NVMe/TCP and native kernel multipath.
>
> Hello!
>
> This sounds like a great contribution! Using the existing flow of the
> ZFS-over-iSCSI plugin is also a good idea.
>
> >
> > The ZFS dataset is the durable source of truth. Each owned zvol stores
> its
> > subsystem NQN, NSID and namespace UUID as ZFS user properties. Configfs
> is
> > reconciled from those properties, and the namespace UUID provides the
> stable
> > by-id path used by QEMU.
> >
> > The implementation includes:
> >
> > - one NVMe/TCP connection per configured portal and host interface;
> > - native multipath and ANA state handling;
> > - DH-HMAC-CHAP without exposing the key in process arguments;
> > - thin provisioning, discard, snapshots, rollback and linked clones;
> > - safe teardown and foreign-zvol identity checks;
> > - an explicit queue-versus-fast-fail policy for complete path loss;
> > - pve-manager and pve-docs integration.
> >
> > Validation covered the complete pve-storage test suite (593 assertions),
> > two-node live migration under guest I/O, migration to a degraded
> > destination,
> > real ANA transitions, simultaneous allocations from both cluster nodes,
> > single and dual path loss, configfs reconstruction, watchdog HA and a
> live
> > Corosync/QDevice partition. No source/destination QEMU overlap was
> observed
> > in
> > the live partition test.
> >
> > The implementation and detailed validation report are available here:
> >
> > https://github.com/joaquinv98/pve-storage/tree/feature/zfs-nvme-tcp
> > https://github.com/joaquinv98/pve-manager/tree/feature/zfs-nvme-tcp
> > https://github.com/joaquinv98/pve-docs/tree/feature/zfs-nvme-tcp
> >
> > The lab results are not presented as hardware certification. TLS, online
> > resize and target control-plane HA remain explicit limitations.
> >
> > I have prepared signed patch series for pve-storage, pve-manager and
> > pve-docs.
> > Before posting the full series, I would appreciate feedback on whether
> this
> > in-tree backend and its architecture are acceptable for upstream review.
>
> I have skimmed through the respective branches on your repositories
> above, and from what I can tell, the commits look pretty decent.
>
> I do have some remarks up front that you may want to consider before
> sending this in as a patch series -- mostly to save you some time:
>
> - If you haven't already, please sign our CLA. You can find the
>   respective section here:
> https://proxmox.com/en/about/open-source/developers
>
> - From what I can tell, you're already adhering quite well to our style
>   guide, which is great! A few remarks:
>
>   * I suggest using `use v5.36;` for new Perl modules instead of
>     `use strict; use warnings;`, as that enables subroutine signatures
>     by default. Helps modernizing the code base overall.
>     See our style guide for more details:
> https://pve.proxmox.com/wiki/Perl_Style_Guide
>
>   * While not in the style guide, I can personally recommend using the
>     `n` and `x`/`xx` modifiers for your regexes, and declaring any
>     inline regexes as constants (=> `my $RE_NVME_FOO = qr/.../nxx`)
>     in a little section at the top of your module. These flags help
>     future readers to understand your regexes a little easier, and since
>     `n` forces the use of named capture groups, they'll also more likely
>     to be "self-documenting" in a sense.
>
>   * For subroutines that are not part of the storage plugin API, i.e.
>     helpers, utilts, etc. specific to your module, prefer declaring them
>     using `my sub` instead of a plain `sub`, as that keeps them visible
>     inside the module only (=> "private" in other languages). That
>     prevents other places from using these helpers.
>
>     You can then also drop the `_` prefix in that case (if you prefer).
>
>   * I also suggest using postfix dereferencing instead of the
>     "traditional" dereferencing syntax, that is, `$hashref->%*` instead
>     of `%$hashref`, for example. It's a little more legible and makes
>     nested hash access a little easier to follow as well. Note that this
>     isn't explicitly stated in our style guide either (we only have it
>     there for subref calls, hash and array accessing), so no hard
>     feelings.
>
> - When you submit your series, please do not any entries in
>   `debian/changelog` -- though it's nice to see that you've been
>   thorough there!
>
> That's all I can think of / find at the moment without giving it a
> proper in-depth review. I'll happily review your series once you send it
> in.
>
> I'd also be interested in what kind of storage backend you were using
> for your tests, so that I can replicate your setup myself. I have
> skimmed through your docs and validation reports, but haven't found
> anything specific -- maybe I just missed it.
>
> >
> > Kind regards,
>
> Thanks a lot for your interest in contributing and your work already,
> it's much appreciated!
>
> Kind regards,
> Max
>
>
>

-- 
Joaquin Varela

      reply	other threads:[~2026-08-02  3:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 16:23 [RFC] Native ZFS over NVMe/TCP storage backend for Proxmox VE Joaquin Varela
2026-07-21 11:45 ` Max R. Carrara
2026-08-02  3:20   ` Joaquin Varela [this message]

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='CANSq6_LCXRwcvhBmV9ceDZQqBk8KOxkQBV40j6=W-qw3m+YsHg@mail.gmail.com' \
    --to=joaquinvarela@neatech.ar \
    --cc=m.carrara@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