Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices
On Thu, Jan 12, 2017 at 05:09:48PM +0300, Dmitry Petuhov wrote: > 12.01.2017 16:18, Fabian Grünbichler пишет: > > On Thu, Jan 12, 2017 at 03:33:48PM +0300, Dmitry Petuhov wrote: > > > Set zero size for backing block devices in qmp call. In that case qemu > > > sets size of device in guest to current size of backing device, which > > > was resized earlier. Otherwise, any non-zero value causes error here. > > this is not clearly documented, but if PVE::Storage::volume_resize > > returns 1, we assume the storage layer did not resize and qemu should do > > the resizing > > > > if it returns undef, we assume the storage resized the volume and we are > > done. > There's problem: even if storage has resized blockdev, this change is not > propagated to running qemu until we send block_resize to it. > So we need to resize BOTH in storage AND in qemu. yes, I was describing the current code here, and proposed a change further down ;) > > > so in the latter case, instead of returning in line 3983 we could set > > $size to 0 (based on the return code of course ;)) and continue with the > > running check and block_resize via monitor. this should only improve the > > situation for all storages, without any wonky checks based on block > > device or not. > ZFS plugins are showing interesting behaviour here: return $new_size; > Maybe we could do that in all plugins, that support resizing and just > $size = PVE::Storage::volume_resize($storecfg, $volid, $size, $running); > and then continue with any defined value (includig 0)? see my other response to your patch series, IMHO we don't gain anything by this vs. returning undef and passing 0 to qemu? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices
Just tested it,it's working fine with ceph+krbd
- Mail original -
De: "Fabian Grünbichler"
À: "pve-devel"
Envoyé: Jeudi 12 Janvier 2017 14:18:29
Objet: Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for
block devices
On Thu, Jan 12, 2017 at 03:33:48PM +0300, Dmitry Petuhov wrote:
> Set zero size for backing block devices in qmp call. In that case qemu
> sets size of device in guest to current size of backing device, which
> was resized earlier. Otherwise, any non-zero value causes error here.
this is not clearly documented, but if PVE::Storage::volume_resize
returns 1, we assume the storage layer did not resize and qemu should do
the resizing
if it returns undef, we assume the storage resized the volume and we are
done.
so in the latter case, instead of returning in line 3983 we could set
$size to 0 (based on the return code of course ;)) and continue with the
running check and block_resize via monitor. this should only improve the
situation for all storages, without any wonky checks based on block
device or not.
the above semantic should be valid for all our plugins - I assume your
plugin returns 1 even if it did the resize operation (because otherwise
you would not send the monitor command..)? you'd need to change that..
(also please include the signoff if you send a new version!)
>
> ---
> PVE/QemuServer.pm | 6 ++
> 1 file changed, 6 insertions(+)
>
> v2: fix typo -d instead of -b
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bc26da2..3f53ac1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3984,6 +3984,12 @@ sub qemu_block_resize {
>
> return if !$running;
>
> + my $path = PVE::Storage::path($cfg, $volid, undef);
> + if ($path =~ m|^/|) {
> + $path = abs_path($path);
> + $size = 0 if -b $path;
> + }
> +
> vm_mon_cmd($vmid, "block_resize", device => $deviceid, size => int($size));
>
> }
> --
> 2.1.4
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices
>>There's problem: even if storage has resized blockdev, this change is >>not propagated to running qemu until we send block_resize to it. >>So we need to resize BOTH in storage AND in qemu. Oh,I was not aware of this. In the past, I was using iscsi luns, and I thinked that resizing the lun exposed the new size to guest. (maybe because of iscsi passthrough with virtio-scsi ?) - Mail original - De: "Dmitry Petuhov" À: "Fabian Grünbichler" , "pve-devel" Envoyé: Jeudi 12 Janvier 2017 15:09:48 Objet: Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices 12.01.2017 16:18, Fabian Grünbichler пишет: > On Thu, Jan 12, 2017 at 03:33:48PM +0300, Dmitry Petuhov wrote: >> Set zero size for backing block devices in qmp call. In that case qemu >> sets size of device in guest to current size of backing device, which >> was resized earlier. Otherwise, any non-zero value causes error here. > this is not clearly documented, but if PVE::Storage::volume_resize > returns 1, we assume the storage layer did not resize and qemu should do > the resizing > > if it returns undef, we assume the storage resized the volume and we are > done. There's problem: even if storage has resized blockdev, this change is not propagated to running qemu until we send block_resize to it. So we need to resize BOTH in storage AND in qemu. > so in the latter case, instead of returning in line 3983 we could set > $size to 0 (based on the return code of course ;)) and continue with the > running check and block_resize via monitor. this should only improve the > situation for all storages, without any wonky checks based on block > device or not. ZFS plugins are showing interesting behaviour here: return $new_size; Maybe we could do that in all plugins, that support resizing and just $size = PVE::Storage::volume_resize($storecfg, $volid, $size, $running); and then continue with any defined value (includig 0)? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices
12.01.2017 16:18, Fabian Grünbichler пишет: On Thu, Jan 12, 2017 at 03:33:48PM +0300, Dmitry Petuhov wrote: Set zero size for backing block devices in qmp call. In that case qemu sets size of device in guest to current size of backing device, which was resized earlier. Otherwise, any non-zero value causes error here. this is not clearly documented, but if PVE::Storage::volume_resize returns 1, we assume the storage layer did not resize and qemu should do the resizing if it returns undef, we assume the storage resized the volume and we are done. There's problem: even if storage has resized blockdev, this change is not propagated to running qemu until we send block_resize to it. So we need to resize BOTH in storage AND in qemu. so in the latter case, instead of returning in line 3983 we could set $size to 0 (based on the return code of course ;)) and continue with the running check and block_resize via monitor. this should only improve the situation for all storages, without any wonky checks based on block device or not. ZFS plugins are showing interesting behaviour here: return $new_size; Maybe we could do that in all plugins, that support resizing and just $size = PVE::Storage::volume_resize($storecfg, $volid, $size, $running); and then continue with any defined value (includig 0)? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server] Fix block_resize qmp call for block devices
On Thu, Jan 12, 2017 at 03:33:48PM +0300, Dmitry Petuhov wrote:
> Set zero size for backing block devices in qmp call. In that case qemu
> sets size of device in guest to current size of backing device, which
> was resized earlier. Otherwise, any non-zero value causes error here.
this is not clearly documented, but if PVE::Storage::volume_resize
returns 1, we assume the storage layer did not resize and qemu should do
the resizing
if it returns undef, we assume the storage resized the volume and we are
done.
so in the latter case, instead of returning in line 3983 we could set
$size to 0 (based on the return code of course ;)) and continue with the
running check and block_resize via monitor. this should only improve the
situation for all storages, without any wonky checks based on block
device or not.
the above semantic should be valid for all our plugins - I assume your
plugin returns 1 even if it did the resize operation (because otherwise
you would not send the monitor command..)? you'd need to change that..
(also please include the signoff if you send a new version!)
>
> ---
> PVE/QemuServer.pm | 6 ++
> 1 file changed, 6 insertions(+)
>
> v2: fix typo -d instead of -b
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bc26da2..3f53ac1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3984,6 +3984,12 @@ sub qemu_block_resize {
>
> return if !$running;
>
> +my $path = PVE::Storage::path($cfg, $volid, undef);
> +if ($path =~ m|^/|) {
> +$path = abs_path($path);
> +$size = 0 if -b $path;
> +}
> +
> vm_mon_cmd($vmid, "block_resize", device => $deviceid, size =>
> int($size));
>
> }
> --
> 2.1.4
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
