From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>,
PVE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage] fix-4272: btrfs: add rename feature
Date: Wed, 3 Jul 2024 15:08:00 +0200 [thread overview]
Message-ID: <20846ad2-3932-41ea-bf16-1bb7270db426@proxmox.com> (raw)
In-Reply-To: <s8o1q4a3bbj.fsf@proxmox.com>
On 2024-07-03 14:32, Maximiliano Sandoval wrote:
> Aaron Lauterer <a.lauterer@proxmox.com> writes:
>
>> Works overall. I did not test edge cases like working around the BTRFS plugin to
>> have qcow2 files instead of the raw files in a subvol.
>
> Thanks for testing!
>
[...]
>> I am not too familiar with how BTRFS and our plugin works, but looking at other
>> functions, like 'alloc_image' or 'clone_image' it seems that we do have checks
>> regarding the format in place. If it is not a 'raw' or 'subvol' we call the
>> SUPER from the main Plugin.pm instead of continuing in the BTRFS specific
>> implementations that use subvols.
>>
>> This seems to be missing here, but it might be that it is fine if the "move" is
>> working in any way.
>
> While it is true that others methods call the SUPER method, I didn't
> find any other implementation using SUPER::rename_volume.
Not surprising, as this behavior seems to be a BTRFS specific one for
the edge case that we are dealing with a regular file (non .raw) instead
of how the plugin usually wants it. AFAICT this is a safety mechanism,
just in case.
For example (IIRC): on BTRFS the layout if usually:
images/{vmid}/vm-{vmid}-disk-X/disk.raw
where the `vm-{vmid}-disk-X` part is a BTRFS subvolume. But it would
also be possible that someone (manually) builds it like a regular
directory based storage plugin:
images/{vmid}/vm-{vmid}-disk-X.qcow2
where everything is just a path/file. How well does the rename_volume
code handle this? Or should it call the SUPER method?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2024-07-03 13:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-27 11:42 Maximiliano Sandoval
2024-07-02 13:16 ` Maximiliano Sandoval
2024-07-02 15:24 ` Aaron Lauterer
[not found] ` <s8o1q4a3bbj.fsf@proxmox.com>
2024-07-03 13:08 ` Aaron Lauterer [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=20846ad2-3932-41ea-bf16-1bb7270db426@proxmox.com \
--to=a.lauterer@proxmox.com \
--cc=m.sandoval@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.