Re: [pve-devel] [PATCH qemu-server pve-storage] Allow storage plugins to modify volume sizes reported to qemu

2017-01-13 Thread Wolfgang Bumiller
On Fri, Jan 13, 2017 at 11:39:59AM +0100, Fabian Grünbichler wrote:
> On Fri, Jan 13, 2017 at 10:11:31AM +0300, Dmitry Petuhov wrote:
> > [PATCH qemu-server] Honour volume size returned by storage plugin.
> > [PATCH pve-storage 1/3] Make volume_resize() return new volume size
> > [PATCH pve-storage 2/3] Allow Ceph volume resize with krbd enabled.
> > [PATCH pve-storage 3/3] Increment storage plugin API version
> > 
> > This patch series allows plugins to set size reported to qemu for
> > online VMs andusing this feature, fixes online resizing of Ceph
> > volumes with krbd option enabled.
> 
> I don't think this makes a lot of sense. We basically have three cases:
> - guest is not running, storage layer can simply do a resize
> - guest is running, storage layer does the resize, qemu needs to re-read
>   the size (block devices which qemu cannot resize)
> - guest is running, qemu does the resize because storage layer reported
>   "I can't/didn't resize this" (everything that qemu can resize 'live')
> 
> in the first case, nothing needs to change
> 
> in the second case, the storage layer should return undef (the
> ZFSPoolPlugin needs fixing here) which should trigger qemu block_resize
> with 0 as size, which triggers a re-read of the actual size
> 
> in the third case, the storage layer should return 1, and qemu
> block_resize should be called with the requested size, which means that
> qemu tries to resize the device
> 
> so there are only two things that need fixing:
> - storage plugins that report something other than undef when they
>   successfully did a resize
> - passing 0 to block_resize instead of returning in
>   PVE::QemuServer::qemu_block_resize when PVE::Storage::volume_resize
>   returned undef
> 
> that would IMHO not need a API bump because this was the old
> (undocumented) expected behaviour, but if desired we can bump the ABI to
> mark the change from "expected, not documented" to "explicitly
> documented", hopefully making some of the external plugin authors aware
> of potential problems in their plugins?

Sounds good. This fix and the bump should probably happen together with
the documentation in order to properly define the API. For now it's
still sort of an undocumented "internal but extensible" interface, so
if we make a bump for a change which has no real side effects anyway,
effectively getting the users attention, it would be good if we could
present them with a little more information in general.

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH qemu-server pve-storage] Allow storage plugins to modify volume sizes reported to qemu

2017-01-13 Thread Fabian Grünbichler
On Fri, Jan 13, 2017 at 10:11:31AM +0300, Dmitry Petuhov wrote:
> [PATCH qemu-server] Honour volume size returned by storage plugin.
> [PATCH pve-storage 1/3] Make volume_resize() return new volume size
> [PATCH pve-storage 2/3] Allow Ceph volume resize with krbd enabled.
> [PATCH pve-storage 3/3] Increment storage plugin API version
> 
> This patch series allows plugins to set size reported to qemu for
> online VMs andusing this feature, fixes online resizing of Ceph
> volumes with krbd option enabled.

I don't think this makes a lot of sense. We basically have three cases:
- guest is not running, storage layer can simply do a resize
- guest is running, storage layer does the resize, qemu needs to re-read
  the size (block devices which qemu cannot resize)
- guest is running, qemu does the resize because storage layer reported
  "I can't/didn't resize this" (everything that qemu can resize 'live')

in the first case, nothing needs to change

in the second case, the storage layer should return undef (the
ZFSPoolPlugin needs fixing here) which should trigger qemu block_resize
with 0 as size, which triggers a re-read of the actual size

in the third case, the storage layer should return 1, and qemu
block_resize should be called with the requested size, which means that
qemu tries to resize the device

so there are only two things that need fixing:
- storage plugins that report something other than undef when they
  successfully did a resize
- passing 0 to block_resize instead of returning in
  PVE::QemuServer::qemu_block_resize when PVE::Storage::volume_resize
  returned undef

that would IMHO not need a API bump because this was the old
(undocumented) expected behaviour, but if desired we can bump the ABI to
mark the change from "expected, not documented" to "explicitly
documented", hopefully making some of the external plugin authors aware
of potential problems in their plugins?

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel