On 03/04/2025 09:52, Wolfgang Bumiller wrote: > 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).
Agreed, I don't see it as a blocker either. > [...] >> - 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... 😦 Ah, I see ... so this is expected behavior for the example plugin, makes sense. >> - 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(... > Yes, this works as expected -- shows empty notes and when attempting to save notes, errors out with > attribute 'notes' is not supported on backup-provider-dir-example > volume at /usr/share/perl5/PVE/Storage/Custom/ > BackupProviderDirExamplePlugin.pm line 246. (500) > 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... Heh, fun! :) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
