* [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs
@ 2021-05-27 12:23 Lorenz Stechauner
2021-05-27 12:23 ` [pve-devel] [PATCH v3 container 1/1] " Lorenz Stechauner
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Lorenz Stechauner @ 2021-05-27 12:23 UTC (permalink / raw)
To: pve-devel
changes to v2:
* typo s/supoort/support/
* more detailed error messages
* implemented check also for vms
pve-container:
Lorenz Stechauner (1):
fix #3421: allow custom storage plugins to support rootfs
src/PVE/LXC.pm | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
qemu-server:
Lorenz Stechauner (1):
vm_start: check if storages of volumes support content images
PVE/QemuServer.pm | 7 +++++++
1 file changed, 7 insertions(+)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH v3 container 1/1] fix #3421: allow custom storage plugins to support rootfs
2021-05-27 12:23 [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Lorenz Stechauner
@ 2021-05-27 12:23 ` Lorenz Stechauner
2021-06-21 8:52 ` [pve-devel] applied: " Thomas Lamprecht
2021-05-27 12:23 ` [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images Lorenz Stechauner
2021-06-02 7:29 ` [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Fabian Ebner
2 siblings, 1 reply; 7+ messages in thread
From: Lorenz Stechauner @ 2021-05-27 12:23 UTC (permalink / raw)
To: pve-devel
it is now necessary for storages to support the 'rootdir' content in
order to start containers on them. all native storage plugins
already report the rootdir content correctly.
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
src/PVE/LXC.pm | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 7e6f378..556f7fa 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1682,15 +1682,16 @@ sub __mountpoint_mount {
}
};
my $use_loopdev = 0;
- if ($scfg->{path}) {
- $mounted_dev = run_with_loopdev($domount, $path, $readonly);
- $use_loopdev = 1;
- } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm' ||
- $scfg->{type} eq 'rbd' || $scfg->{type} eq 'lvmthin') {
- $mounted_dev = $path;
- &$domount($path);
+ if ($scfg->{content}->{rootdir}) {
+ if ($scfg->{path}) {
+ $mounted_dev = run_with_loopdev($domount, $path, $readonly);
+ $use_loopdev = 1;
+ } else {
+ $mounted_dev = $path;
+ &$domount($path);
+ }
} else {
- die "unsupported storage type '$scfg->{type}'\n";
+ die "storage '$storage' does not support containers\n";
}
return wantarray ? ($path, $use_loopdev, $mounted_dev) : $path;
} else {
@@ -1871,7 +1872,7 @@ sub alloc_disk {
eval {
my $do_format = 0;
- if ($scfg->{type} eq 'dir' || $scfg->{type} eq 'nfs' || $scfg->{type} eq 'cifs' ) {
+ if ($scfg->{content}->{rootdir} && $scfg->{path}) {
if ($size_kb > 0) {
$volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw',
undef, $size_kb);
@@ -1882,21 +1883,14 @@ sub alloc_disk {
$needs_chown = 1;
}
} elsif ($scfg->{type} eq 'zfspool') {
-
$volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'subvol',
undef, $size_kb);
$needs_chown = 1;
- } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm' || $scfg->{type} eq 'lvmthin') {
-
- $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw', undef, $size_kb);
- $do_format = 1;
-
- } elsif ($scfg->{type} eq 'rbd') {
-
+ } elsif ($scfg->{content}->{rootdir}) {
$volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw', undef, $size_kb);
$do_format = 1;
} else {
- die "unable to create containers on storage type '$scfg->{type}'\n";
+ die "storage '$storage' does not support containers\n";
}
format_disk($storecfg, $volid, $rootuid, $rootgid) if $do_format;
};
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images
2021-05-27 12:23 [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Lorenz Stechauner
2021-05-27 12:23 ` [pve-devel] [PATCH v3 container 1/1] " Lorenz Stechauner
@ 2021-05-27 12:23 ` Lorenz Stechauner
2021-06-21 9:11 ` Thomas Lamprecht
2021-06-02 7:29 ` [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Fabian Ebner
2 siblings, 1 reply; 7+ messages in thread
From: Lorenz Stechauner @ 2021-05-27 12:23 UTC (permalink / raw)
To: pve-devel
it is now necessary for storages to support the 'images' content in
order to start vms on them. all native storage plugins already
report the images content correctly.
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/QemuServer.pm | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0bfbca4..3b656a1 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5124,6 +5124,13 @@ sub vm_start_nolock {
my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
+ for my $vol (@$vollist) {
+ if (my ($storeid, $volname) = PVE::Storage::parse_volume_id($vol, 1)) {
+ my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+ die "storage '$storeid' does not support vm disks\n" if !$scfg->{content}->{images};
+ }
+ }
+
my $migration_ip;
my $get_migration_ip = sub {
my ($nodename) = @_;
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs
2021-05-27 12:23 [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Lorenz Stechauner
2021-05-27 12:23 ` [pve-devel] [PATCH v3 container 1/1] " Lorenz Stechauner
2021-05-27 12:23 ` [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images Lorenz Stechauner
@ 2021-06-02 7:29 ` Fabian Ebner
2021-06-02 11:51 ` Fabian Ebner
2 siblings, 1 reply; 7+ messages in thread
From: Fabian Ebner @ 2021-06-02 7:29 UTC (permalink / raw)
To: pve-devel, l.stechauner
There's an edge case with 'restart' migration for containers that breaks
because of the new content type on startup checks:
If there is an already running container with a volume on storage A, and
now storage A is reconfigured to not support 'rootdir' anymore, then
migration itself does work, but there'll be an error on startup on the
remote node. It would be nicer if the error would appear at the start of
the migration already.
For VMs (with 'online' migration) the situation is not as bad, because
the remote start happens earlier, so the VM will still be running on the
original node after the error.
And one can offline migrate such unstartable guests around ;)
IMHO, if we add the checks for content type on startup, it's all the
more reason to have content type checks for migration as well. For VM
migration with the targetstorage option, there already are such checks.
It's a tangential problem of course, your patches look fine to me:
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
Am 27.05.21 um 14:23 schrieb Lorenz Stechauner:
> changes to v2:
> * typo s/supoort/support/
> * more detailed error messages
> * implemented check also for vms
>
> pve-container:
>
> Lorenz Stechauner (1):
> fix #3421: allow custom storage plugins to support rootfs
>
> src/PVE/LXC.pm | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
>
> qemu-server:
>
> Lorenz Stechauner (1):
> vm_start: check if storages of volumes support content images
>
> PVE/QemuServer.pm | 7 +++++++
> 1 file changed, 7 insertions(+)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs
2021-06-02 7:29 ` [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Fabian Ebner
@ 2021-06-02 11:51 ` Fabian Ebner
0 siblings, 0 replies; 7+ messages in thread
From: Fabian Ebner @ 2021-06-02 11:51 UTC (permalink / raw)
To: pve-devel, l.stechauner
Since I just ran into it: It also breaks (at least container) backups
when there is a volume on a misconfigured storage.
Am 02.06.21 um 09:29 schrieb Fabian Ebner:
> There's an edge case with 'restart' migration for containers that breaks
> because of the new content type on startup checks:
> If there is an already running container with a volume on storage A, and
> now storage A is reconfigured to not support 'rootdir' anymore, then
> migration itself does work, but there'll be an error on startup on the
> remote node. It would be nicer if the error would appear at the start of
> the migration already.
>
> For VMs (with 'online' migration) the situation is not as bad, because
> the remote start happens earlier, so the VM will still be running on the
> original node after the error.
>
> And one can offline migrate such unstartable guests around ;)
>
> IMHO, if we add the checks for content type on startup, it's all the
> more reason to have content type checks for migration as well. For VM
> migration with the targetstorage option, there already are such checks.
>
>
> It's a tangential problem of course, your patches look fine to me:
>
> Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
>
> Am 27.05.21 um 14:23 schrieb Lorenz Stechauner:
>> changes to v2:
>> * typo s/supoort/support/
>> * more detailed error messages
>> * implemented check also for vms
>>
>> pve-container:
>>
>> Lorenz Stechauner (1):
>> fix #3421: allow custom storage plugins to support rootfs
>>
>> src/PVE/LXC.pm | 30 ++++++++++++------------------
>> 1 file changed, 12 insertions(+), 18 deletions(-)
>>
>>
>> qemu-server:
>>
>> Lorenz Stechauner (1):
>> vm_start: check if storages of volumes support content images
>>
>> PVE/QemuServer.pm | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] applied: [PATCH v3 container 1/1] fix #3421: allow custom storage plugins to support rootfs
2021-05-27 12:23 ` [pve-devel] [PATCH v3 container 1/1] " Lorenz Stechauner
@ 2021-06-21 8:52 ` Thomas Lamprecht
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2021-06-21 8:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Lorenz Stechauner
On 27.05.21 14:23, Lorenz Stechauner wrote:
> it is now necessary for storages to support the 'rootdir' content in
> order to start containers on them. all native storage plugins
> already report the rootdir content correctly.
>
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> src/PVE/LXC.pm | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
>
applied, with Fabi's R-b tag, thanks!
Edited the error message a bit, to convey that the "rootdir" content-type may
just not be configured
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images
2021-05-27 12:23 ` [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images Lorenz Stechauner
@ 2021-06-21 9:11 ` Thomas Lamprecht
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2021-06-21 9:11 UTC (permalink / raw)
To: Proxmox VE development discussion, Lorenz Stechauner
On 27.05.21 14:23, Lorenz Stechauner wrote:
> it is now necessary for storages to support the 'images' content in
> order to start vms on them. all native storage plugins already
> report the images content correctly.
>
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> PVE/QemuServer.pm | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 0bfbca4..3b656a1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5124,6 +5124,13 @@ sub vm_start_nolock {
> my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
> $conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
>
> + for my $vol (@$vollist) {
> + if (my ($storeid, $volname) = PVE::Storage::parse_volume_id($vol, 1)) {
> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> + die "storage '$storeid' does not support vm disks\n" if !$scfg->{content}->{images};
> + }
> + }
we'd already loop overall volumes in cfg2cmd, so why not check there (with a small
helper method to avoid adding to much bloat there)?
Also, are you aware that efidisk and VM state images are included by this check
ensured that it's consistent regarding the handling of those special "files" elsewhere?
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-21 9:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 12:23 [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Lorenz Stechauner
2021-05-27 12:23 ` [pve-devel] [PATCH v3 container 1/1] " Lorenz Stechauner
2021-06-21 8:52 ` [pve-devel] applied: " Thomas Lamprecht
2021-05-27 12:23 ` [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if storages of volumes support content images Lorenz Stechauner
2021-06-21 9:11 ` Thomas Lamprecht
2021-06-02 7:29 ` [pve-devel] [PATCH-SERIES v3 container/qemu-server] fix #3421: allow custom storage plugins to support rootfs Fabian Ebner
2021-06-02 11:51 ` Fabian Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox