From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 storage 3/5] add disk reassign feature
Date: Thu, 03 Sep 2020 09:55:51 +0200 [thread overview]
Message-ID: <1599119469.zi7lf79gc3.astroid@nora.none> (raw)
In-Reply-To: <20200901124421.25901-4-a.lauterer@proxmox.com>
some small nits inline
On September 1, 2020 2:44 pm, Aaron Lauterer wrote:
> Functionality has been added for the following storage types:
>
> * dir based ones
> * directory
> * NFS
> * CIFS
> * ZFS
> * (thin) LVM
> * Ceph
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> rfc -> v1 -> v2: nothing changed
>
> PVE/Storage.pm | 10 ++++++++++
> PVE/Storage/LVMPlugin.pm | 15 +++++++++++++++
> PVE/Storage/Plugin.pm | 21 +++++++++++++++++++++
> PVE/Storage/RBDPlugin.pm | 13 +++++++++++++
> PVE/Storage/ZFSPoolPlugin.pm | 9 +++++++++
> 5 files changed, 68 insertions(+)
>
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 4a60615..919c606 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -1759,6 +1759,16 @@ sub complete_volume {
> return $res;
> }
>
> +sub reassign_volume {
> + my ($cfg, $volid, $target_vmid) = @_;
> +
> + my ($storeid, $volname) = parse_volume_id($volid);
> + my $scfg = storage_config($cfg, $storeid);
> + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> + return $plugin->reassign_volume($scfg, $storeid, $volname, $target_vmid);
> +}
> +
> # Various io-heavy operations require io/bandwidth limits which can be
> # configured on multiple levels: The global defaults in datacenter.cfg, and
> # per-storage overrides. When we want to do a restore from storage A to storage
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
> index c0740d4..28ff6c8 100644
> --- a/PVE/Storage/LVMPlugin.pm
> +++ b/PVE/Storage/LVMPlugin.pm
> @@ -337,6 +337,13 @@ sub lvcreate {
> run_command($cmd, errmsg => "lvcreate '$vg/$name' error");
> }
>
> +sub lvrename {
> + my ($vg, $oldname, $newname) = @_;
> +
> + my $cmd = ['/sbin/lvrename', $vg, $oldname, $newname];
> + run_command($cmd, errmsg => "lvrename '${vg}/${oldname}' to '${newname}' error");
> +}
> +
> sub alloc_image {
> my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>
> @@ -681,4 +688,12 @@ sub volume_import_write {
> input => '<&'.fileno($input_fh));
> }
>
> +sub reassign_volume {
> + my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> + my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> + lvrename($scfg->{vgname}, $volname, $target_volname);
> + return "${storeid}:${target_volname}";
> +}
> +
> 1;
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 8a58ff4..770a482 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -1411,4 +1411,25 @@ sub volume_import_formats {
> return ();
> }
>
> +sub reassign_volume {
> + my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> + my $basedir = $class->get_subdir($scfg, 'images');
> + my $imagedir = "${basedir}/${target_vmid}";
> +
> + mkpath $imagedir;
> +
> + my @parsed_volname = $class->parse_volname($volname);
> + my $format = $parsed_volname[6];
> + my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1);
> +
$volname vs $target_volname, but one includes the vmid the other does
not? rename 'target_volname' to diskname (like the sub you call), then
you can do
my $target_volname = "$target_vmid/$diskname";
> + my $old_path = "${basedir}/${volname}";
> + my $new_path = "${imagedir}/${target_volname}";
and construct both paths in the same fashion
> +
> + rename($old_path, $new_path) ||
> + die "rename '$old_path' to '$new_path' failed - $!\n";
> +
> + return "${storeid}:${target_vmid}/${target_volname}";
and return $storeid:$target_volname here
> +}
> +
> 1;
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 38f2b46..c0bd74c 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -719,4 +719,17 @@ sub volume_has_feature {
> return undef;
> }
>
> +sub reassign_volume {
> + my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> + my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> + my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $volname, $target_volname);
> +
> + run_rbd_command(
> + $cmd,
> + errmsg => "could not rename image '$volname' to '$target_volname'",
> + );
> + return "${storeid}:${target_volname}";
> +}
> +
> 1;
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index 10354b3..5baa5c2 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -749,4 +749,13 @@ sub volume_import_formats {
> return $class->volume_export_formats($scfg, $storeid, $volname, undef, $base_snapshot, $with_snapshots);
> }
>
> +sub reassign_volume {
> + my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> + my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> + $class->zfs_request($scfg, 5, 'rename', "$scfg->{pool}/$volname", "$scfg->{pool}/$target_volname");
> +
> + return "${storeid}:${target_volname}";
> +}
> +
> 1;
> --
> 2.20.1
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
next prev parent reply other threads:[~2020-09-03 7:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 12:44 [pve-devel] [PATCH v2 series 0/5] disk reassign: add new feature Aaron Lauterer
2020-09-01 12:44 ` [pve-devel] [PATCH v2 qemu-server 1/5] disk reassign: add API endpoint Aaron Lauterer
2020-09-03 7:46 ` Fabian Grünbichler
2020-09-03 8:30 ` Aaron Lauterer
2020-09-03 9:07 ` Fabian Grünbichler
2020-09-01 12:44 ` [pve-devel] [PATCH v2 qemu-server 2/5] cli: disk reassign: add reassign_disk to qm command Aaron Lauterer
2020-09-01 12:44 ` [pve-devel] [PATCH v2 storage 3/5] add disk reassign feature Aaron Lauterer
2020-09-03 7:55 ` Fabian Grünbichler [this message]
2020-09-01 12:44 ` [pve-devel] [PATCH v2 storage 4/5] disk reassign: add not implemented yet message to storages Aaron Lauterer
2020-09-03 7:58 ` Fabian Grünbichler
2020-09-03 9:01 ` Aaron Lauterer
2020-09-03 9:06 ` Aaron Lauterer
2020-09-03 9:19 ` Fabian Grünbichler
2020-09-01 12:44 ` [pve-devel] [PATCH v2 widget-toolkit 5/5] utils: task_desc_table: add qmreassign Aaron Lauterer
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=1599119469.zi7lf79gc3.astroid@nora.none \
--to=f.gruenbichler@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