Hi,

This is a very useful feature, thanks for digging into it.

I don't know if you have seen a relative thread a year ago [1], but
since I was also looking into it, please allow me some comments:

1) Instead of introducing new RPC/Backend/Hypervisor methods I would
   extend and use the already existing ones (i.e HotplugSupported() for
   DiskHotResizeSupported(), HotplugDevice() for HotResizeDisk(),
   _VerifyHotplugCommand() for _VerifyHotResizeCommand()).

2) hyper.HotModDevice() is currently not implemented for disk modifications
   so I guess it is a perfect place for implementing online resize. I
   also agree with you that grow-disk functionallity should be merged into
   gnt-instance modify.

3) The hypervisor methods CheckDiskHotResizeSupport() and HotResizeDisk()
   could stay as is but invoked by HotModDevice().

4) You introduce _GetBlockDevices(). I think this is the right time to
   unify it with GetPCIDevices() under a more generic GetDevices() with
   proper arguments (device type, bus type).

5) Inside DiskHotResizeSupported() you call the 'info version' monitor command
   to check if the "Instance is probably down". This should be done by the
   _with_qmp() decorator (connect() would fail and a proper message should be
   displayed)

Hope the above helps as a first round of thoughts,
dimara

[1] https://groups.google.com/forum/#!msg/ganeti-devel/ldueQpLEmpg/7-oHGmh-D3gJ

* Yoann Laissus <[email protected]> [2015-06-08 02:10:28 -0700]:

> Hello,
> 
> It's my first patch for Ganeti.
> It allows to hot resize disks on KVM instances by running the block_resize 
> QMP command.
> A new parameter (--hot-resize) has been added to gnt-instance grow-disk.
> It's disabled by default, what do you think about that ?
> 
> I would like to have your feedback about this patch before writing some 
> unit tests.
> 
> I'm also wondering if the grow mechanic should be added to gnt-instance 
> modify.
> It would be much more natural than a specific command for growing disk

Attachment: signature.asc
Description: Digital signature

Reply via email to