[pve-devel] [PATCH-SERIES qemu/storage/qemu-server/container/manager v7 00/37] backup provider API

[Wolfgang Bumiller]

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...