Hello Aaron,

On 11/19/14, 15:11 PM, Aaron Karper wrote:
> Hi Constantinos,
>
> Thanks for replying for Alex and helping with this :) Actually shortly
> after writing that, I got push rights, so there actually will be no need
> for a second review.

That sounds good. Then, Alex will resend the patch series incorporating
your comments once he gets back.

Thanks,
Constantinos

> Cheerio,
> Aaron
>
>
> On Mon, Nov 17, 2014 at 08:07:52PM +0200, Constantinos Venetsanopoulos wrote:
>> Hello Aaron,
>>
>> I read all your comments and most of them seem reasonable.
>> I guess the only one that I could argue against is the one on
>> patch 9 that I've already replied upon. This means that Alex
>> will be happy to do all the changes you propose. The only
>> problem is that Alex will be out of work until the end of the
>> month and won't be able to reply to your comments (that's
>> why I'm doing it :) ). So would it be possible to have the second
>> reviewer review the patch now, so that Alex can incorporate the
>> comments of both once he gets back? I think we'll save a lot of
>> time this way.
>>
>> Thanks a lot,
>> Constantinos
>>
>> On 11/10/14, 17:48 PM, 'Aaron Karper' via ganeti-devel wrote:
>>> On Thu, Nov 06, 2014 at 12:30:34AM +0200, Alex Pyrgiotis wrote:
>>>> Hi everybody,
>>>>
>>>> This patch series implements the necessary logic for the attach/detach
>>>> operation on disks. This functionality will allow users to remove a disk
>>>> from an instance and attach it later on to another one, or keep it for
>>>> forensic reasons. The latter part means that the Ganeti configuration
>>>> now supports having detached disks.
>>>>
>>>> In order to document this new functionality, we have extended the man
>>>> page for gnt-instance, as well as the design doc for disks. Also, to
>>>> make sure that we don't introduce any new bugs, we have added many new
>>>> unit tests and extended older ones. Moreover, we have extended the
>>>> burnin tool in order to live-test this new functionality on an existing
>>>> cluster.
>>>>
>>>> Finally, a related patch about file-based instances is in the works and
>>>> will be sent in a couple of days.
>>>>
>>>> Cheers,
>>>> Alex
>>>>
>>>> Alex Pyrgiotis (19):
>>>>   config: Use disk name to get its info
>>>>   config: Fix docstring of _UnlockedGetInstanceNodes
>>>>   cmdlib: Turn index code to functions
>>>>   cmdlib: Return RPC result in AssembleInstanceDisks
>>>>   config: Create wrappers for attach/detach ops
>>>>   constants: Add attach/detach and UUID constant
>>>>   cmdlib: Add checks for attach/detach
>>>>   cmdlib: Add core attach/detach logic
>>>>   config: Remove checks for detached disks
>>>>   burnin: Test if attach/detach works properly
>>>>   tests.config: Add test for GetDiskInfoByName
>>>>   tests.cmdlib: Test index operations
>>>>   tests.opcodes/client: Add test for constants
>>>>   tests.config: Test attach/detach wrappers
>>>>   tests.cmdlib: Check if attach/detach checks work
>>>>   tests.cmdlib: Extend tests for _ApplyContainerMods
>>>>   tests.cluster: Add test for VerifyConfig
>>>>   doc: Extend the design document for disks
>>>>   man: Update the gnt-instance man page
>>>>
>>>> Ilias Tsitsimpis (2):
>>>>   Implement GetAllDisksInfo in config
>>>>   AllocateDRBDMinor per disk, not per instance
>>>>
>>>>  doc/design-disks.rst                           |   49 ++-
>>>>  lib/client/gnt_instance.py                     |   54 +++-
>>>>  lib/cmdlib/cluster.py                          |   35 ++-
>>>>  lib/cmdlib/instance.py                         |   10 +-
>>>>  lib/cmdlib/instance_create.py                  |    3 +-
>>>>  lib/cmdlib/instance_migration.py               |    4 +-
>>>>  lib/cmdlib/instance_set_params.py              |  185 +++++++++--
>>>>  lib/cmdlib/instance_storage.py                 |   47 +--
>>>>  lib/cmdlib/instance_utils.py                   |   97 ++++--
>>>>  lib/config.py                                  |  138 ++++++--
>>>>  lib/tools/burnin.py                            |  122 ++++++--
>>>>  man/gnt-instance.rst                           |   25 +-
>>>>  src/Ganeti/Config.hs                           |    1 +
>>>>  src/Ganeti/Constants.hs                        |    8 +-
>>>>  src/Ganeti/Types.hs                            |    4 +
>>>>  src/Ganeti/WConfd/Core.hs                      |   14 +-
>>>>  src/Ganeti/WConfd/TempRes.hs                   |   40 +--
>>>>  test/py/cmdlib/cluster_unittest.py             |    6 +
>>>>  test/py/cmdlib/instance_unittest.py            |  399
>>> ++++++++++++++++++++----
>>>>  test/py/cmdlib/testsupport/config_mock.py      |   18 +-
>>>>  test/py/ganeti.client.gnt_instance_unittest.py
>>> <http://ganeti.client.gnt_instance_unittest.py> |   20 +-
>>>>  test/py/ganeti.config_unittest.py
>>> <http://ganeti.config_unittest.py>              |   96 ++++++
>>>>  test/py/ganeti.opcodes_unittest.py
>>> <http://ganeti.opcodes_unittest.py>             |    3 +
>>>>  23 files changed, 1097 insertions(+), 281 deletions(-)
>>>>
>>>> --
>>>> 1.7.10.4
>>>>
>>> Hi Alex,
>>> Thanks for adding this feature. I will be doing a first pass of review,
>>> but since I don't have push rights, there will be a second reviewer.
>>>
>>>
>>> --
>>> Google Germany GmbH
>>> Dienerstr. 12
>>> 80331 München
>>>
>>> Registergericht und -nummer: Hamburg, HRB 86891
>>> Sitz der Gesellschaft: Hamburg
>>> Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to