public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Friedrich Weber <f.weber@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH-SERIES qemu/storage/qemu-server/container/manager v7 00/37] backup provider API
Date: Thu, 3 Apr 2025 09:52:47 +0200	[thread overview]
Message-ID: <hd4zgmjagdir5y3qu3fhcnalbxfajyvo43apzsuyvnpf22wkuq@mxyxdosgkakv> (raw)
In-Reply-To: <587b5ac2-1f8b-437b-91e6-52a4b5142235@proxmox.com>

On Wed, Apr 02, 2025 at 05:15:24PM +0200, Friedrich Weber wrote:
> Hi, I tried v7 very quickly with the backup-provider-dir-example
> storage. Some minor things I noticed below, IMO nothing that can't be
> fixed in follow-ups.
> 
> Note that I didn't look too closely how the backup provider API really
> works, so some of the following may just be consequences of the example
> backup-provider-dir-example plugin being an example plugin, in this case
> feel free to ignore:
> 
> - naively triggering a one-off VM backup fails with "ERROR: cannot setup
> backup access without fleecing". I understand why and this is nothing
> critical, but I can imagine this could cause some confusion (e.g. if
> users don't immediately read the relevant ERROR line of the task log).
> Might be nice to either allow enabling fleecing also for one-off
> backups, or disable the "Backup Now" button for external storages.

Agreed, it would certainly be a nicer UX if we could select the fleecing
storage and/or expose to the UI whether a fleecing storage default is
set for this.
But I don't see this as a blocker right now (particularly if the example
plugin cannot be set-up via the UI anyway, such UI consequences IMO
don't need to block the roll-out of this series).

> 
> - I needed to install nbdfuse and modprobe nbd -- might be nice to do
> both automatically somehow (though packages of external backup plugins
> could probably add nbdfuse as a dependency?)

That's a good point. It's only required for the `file-handle` mechanism,
so I'm not sure a hard-dependency directly in qemu-server makes sense.

> 
> - by accident I let the destination filesystem run full, which resulted
> in a stuck backup task and the following message being spammed to the
> journal:
> 
> > Apr 02 16:46:34 pve-backupprovider kernel: block nbd0: Other side
> returned error (28)
> 
> Maybe there is a way to abort the backup in such a case?

Sounds like an issue in the example provider, but will have to check,
it's not very obvious where this would happen, as the read/write access
to/from the nbd devices is checked for errors.
The provider would have to bubble up the errors. If it keeps rolling
instead there's not much we can do on the API side.

> 
> - the VM backup wrote out the full 32G disk image including zeroes to
> the qcow2, but from a quick look at the source this is probably
> expected? Restoring this backup to e.g. lvm-thin gets rid of the zeroes
> again, though.

The example plugin is a simple read/write loop which only ever uses the
bitmap.
I have another test plugin which, for when the bitmap is *new* (or does
not exist), uses qemu's "base:allocation" information to query for
holes via nbd. Also, an NBD client using structured replies will get
told about zeroes/holes rather than receiving zeroes over the network.
It is a bit tricky, though, as one cannot blindly ask for the allocation
info via NBD as qemu will just return errors when trying to query
the block status of parts which aren't dirty... 😦

> 
> - navigating to a VM backup on the backup-provider-dir-example storage
> and clicking on "Edit Notes" fails with
> 
> > failed to stat 'PVE::Storage::Custom::BackupProviderDirExamplePlugin'
> (500)
> 
> and the following message in the journal:
> 
> > Apr 02 17:04:38 pve-backupprovider pvedaemon[960]: file_size_info:
> detected call without $file_format parameter

Can you retry after fixing up the `BackupProviderDirExamplePlugin.pm` at
line 260:

    - $class->file_size_info(...
    + PVE::Storage::Plugin::file_size_info(...

This was apparently mistakenly called as a "class method" which means
it used the class name as a file name, and the `undef` timeout ended up
in the `file_format` parameter...


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

  reply	other threads:[~2025-04-03  7:53 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-01 17:33 Fiona Ebner
2025-04-01 17:33 ` [pve-devel] [PATCH qemu v7 01/37] PVE backup: clean up directly in setup_snapshot_access() when it fails Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 02/37] PVE backup: factor out helper to clear backup state's bitmap list Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 03/37] PVE backup: factor out helper to initialize backup state stat struct Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 04/37] PVE backup: add target ID in backup state Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 05/37] PVE backup: get device info: allow caller to specify filter for which devices use fleecing Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 06/37] PVE backup: implement backup access setup and teardown API for external providers Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 07/37] PVE backup: factor out get_single_device_info() helper Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 08/37] PVE backup: implement bitmap support for external backup access Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu v7 09/37] PVE backup: backup-access api: indicate situation where a bitmap was recreated Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 10/37] add storage_has_feature() helper function Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 11/37] common: add deallocate " Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 12/37] plugin: introduce new_backup_provider() method Fiona Ebner
2025-04-02 10:01   ` Max Carrara
2025-04-02 10:44     ` Thomas Lamprecht
2025-04-02 14:02       ` Max Carrara
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 13/37] config api/plugins: let plugins define sensitive properties themselves Fiona Ebner
2025-04-02 10:01   ` Max Carrara
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 14/37] plugin api: bump api version and age Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH storage v7 15/37] extract backup config: delegate to backup provider for storages that support it Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [POC storage v7 16/37] add backup provider example Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [POC storage v7 17/37] Borg example plugin Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 18/37] backup: keep track of block-node size for fleecing Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 19/37] backup: fleecing: use exact size when allocating non-raw fleecing images Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 20/37] backup: allow adding fleecing images also for EFI and TPM Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 21/37] backup: implement backup for external providers Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 22/37] test: qemu img convert: add test cases for snapshots Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 23/37] image convert: collect options in hash argument Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 24/37] image convert: allow caller to specify the format of the source path Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 25/37] backup: implement restore for external providers Fiona Ebner
2025-04-02  9:10   ` Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 26/37] backup: future-proof checks for QEMU feature support Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 27/37] backup: support 'missing-recreated' bitmap action Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH qemu-server v7 28/37] backup: bitmap action to human: lie about TPM state Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 29/37] add LXC::Namespaces module Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 30/37] backup: implement backup for external providers Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 31/37] backup: implement restore " Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 32/37] external restore: don't use 'one-file-system' tar flag when restoring from a directory Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 33/37] create: factor out compression option helper Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 34/37] restore tar archive: check potentially untrusted archive Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH container v7 35/37] api: add early check against restoring privileged container from external source Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH manager v7 36/37] ui: backup: also check for backup subtype to classify archive Fiona Ebner
2025-04-01 17:34 ` [pve-devel] [PATCH manager v7 37/37] backup: implement backup for external providers Fiona Ebner
2025-04-02 15:15 ` [pve-devel] [PATCH-SERIES qemu/storage/qemu-server/container/manager v7 00/37] backup provider API Friedrich Weber
2025-04-03  7:52   ` Wolfgang Bumiller [this message]
2025-04-03 13:55     ` Friedrich Weber
2025-04-04  7:30   ` Fiona Ebner
2025-04-03 13:11 ` [pve-devel] superseded: " Wolfgang Bumiller

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=hd4zgmjagdir5y3qu3fhcnalbxfajyvo43apzsuyvnpf22wkuq@mxyxdosgkakv \
    --to=w.bumiller@proxmox.com \
    --cc=f.weber@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