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.
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 > -- 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
