Hi Dimara, thanks a lot for your valuable comments! I have placed some questions/comments inline.
On Wed, Jan 13, 2016 at 2:32 PM, Dimitris Aragiorgis < [email protected]> wrote: > Hi Lisa! > > Thanks a lot for this design doc! This was really a pleasant surprise! > > Please see some comments inline. > > * 'Lisa Velden' via ganeti-devel <[email protected]> > [2016-01-13 12:08:26 +0100]: > > > This design document introduces the gnt-disk command, so that disks can > > be treated as top-level entities. > > > > Signed-off-by: Lisa Velden <[email protected]> > > --- > > Makefile.am | 3 +- > > doc/design-draft.rst | 1 + > > doc/design-standalone-disks.rst | 130 > ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 133 insertions(+), 1 deletion(-) > > create mode 100644 doc/design-standalone-disks.rst > > > > diff --git a/Makefile.am b/Makefile.am > > index 0080f00..f93aac6 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -546,7 +546,7 @@ hypervisor_hv_kvm_PYTHON = \ > > > > jqueue_PYTHON = \ > > lib/jqueue/__init__.py \ > > - lib/jqueue/exec.py \ > > + lib/jqueue/exec.py \ > > lib/jqueue/post_hooks_exec.py > > > > storage_PYTHON = \ > > @@ -733,6 +733,7 @@ docinput = \ > > doc/design-shared-storage.rst \ > > doc/design-shared-storage-redundancy.rst \ > > doc/design-ssh-ports.rst \ > > + doc/design-standalone-disks.rst \ > > doc/design-storagetypes.rst \ > > doc/design-sync-rate-throttling.rst \ > > doc/design-systemd.rst \ > > diff --git a/doc/design-draft.rst b/doc/design-draft.rst > > index e4c9f85..18d9ca9 100644 > > --- a/doc/design-draft.rst > > +++ b/doc/design-draft.rst > > @@ -28,6 +28,7 @@ Design document drafts > > design-global-hooks.rst > > design-scsi-kvm.rst > > design-disks.rst > > + design-standalone-disks.rst > > > > .. vim: set textwidth=72 : > > .. Local Variables: > > diff --git a/doc/design-standalone-disks.rst > b/doc/design-standalone-disks.rst > > new file mode 100644 > > index 0000000..71cca81 > > --- /dev/null > > +++ b/doc/design-standalone-disks.rst > > @@ -0,0 +1,130 @@ > > +========================= > > +Disks as Separate Objects > > +========================= > > + > > +.. contents:: :depth: 3 > > + > > +This design document improves the former :doc:`design-disks` document. > > +It introduces the gnt-disk command, so that disks can really be treated > > +as top-level entities. However, the focus is on disks as separate > objects, > > +that is, we do not introduce other new features, such as instances with > > +mixed disk templates. > > + > > + > > +Current state and shortcomings > > +============================== > > + > > +What can already be done? > > +------------------------- > > + > > +Currently, disks can only be created within an instance. But it is > > +already possible to have disks reside in the configuration without being > > +tied to an instance if that disk was previously detached from an > > +instance with a modification command like > > +``gnt-instance modify --disk *UUID*:detach *INSTANCE*``. > > + > > +However, detaching a disk from an instance results in a warning in > > +``gnt-cluster verify`` about orphan volumes for most of the disk > > +templates. > > + > > +Besides detaching, it is also possible to attach a pre-existing disk to > > +an instance and to adopt disks into the Ganeti configuration which have > > +been created outside of Ganeti, e.g. on plain KVM with LVM. Currently, > > +disk adoption is done with ``gnt-instance add --disk=*N*:adopt=*LV*``, > > +which means it is adopt and attach in one step. > > + > > +It is possible to list all disks on a node with their associated > > +instances, if any. The command for that is ``gnt-node volumes``. > However, > > +only LVM volumes are listed with that command. > > + > > +Furthermore, instances can be destroyed while its disks are left as is. > > +But therefore disks have to be detached first and > > +``gnt-instance remove`` has to be called afterwards. > > + > > +What cannot be done at the current state? > > +----------------------------------------- > > + > > +Standalone disks cannot be properly accessed. There is no command that > > +lists all disks for all disk templates, nor is it easy to find the > > +respective instance, if any, for a given disk. > > + > > +It is also not convenient that detached disks can only be removed when > > +they are attached to an instance again and that disk adoption only works > > +for lvm volumes and block devices at the current state. > > + > > +Currently, disk adoption only works with plain and blockdev disk > > +templates. It should also work with the ext disk template, so that in a > > +shared storage environment we can do cross-cluster failovers, where we > > +instantaneously move an instance from one cluster to another. > > + > > +Proposed changes > > +================ > > + > > +As the current way to do disk adoption as well as removing detached > > +disks from the Ganeti configuration is not intuitive, we propose to > > +introduce a new command ``gnt-disk`` that supports the following > > +subcommands: > > + > > +* ``gnt-disk info *DISK_UUID|DISK_NAME*``: all relevant information for > > + a disk is listed, e.g. size, connected instance, position among other > > + disks > > + > > +* ``gnt-disk list``: disks that should be listed can be specified with > > + ``--attached``, ``--detached`` or ``--all`` where all disks are also > > + shown as default if no option was given > > + > > +* ``gnt-disk adopt *VOLUME*``: adopts a volume into the Ganeti > > + configuration. This should work not only for plain and block devices, > > + but also for shared storage > > + > > This is something that we discussed in depth in GanetiCon'15. Please > note that the *VOLUME* should be a template specific value. In other > words it should be an arbitrary string that the upper layers of Ganeti > (e.g. cmdlib, backend) should not be able to understand. Only bdev > should be able to understand/translate/use it. > > The existing adoption implementation for plain/blockdev should me moved > down to bdev. Specifically the bdev.adopt() method is actually a > bdev.create() that will check the given arbitrary string, and do > whatever is needed (e.g. for plain will rename this LV) so that at the > end of the day a disk with the given logical id gets created. > > The above asumes that the logical id is generated by Ganeti in advance > in the exact way it happens now. With the above aproach, adoption > becomes uniform for all disk templates, and if a template supports > adoption it should just implement the bdev.adopt() method. Then > supporting adoption in ExtStorage would be completely straightforward. > > > +* ``gnt-disk create *DISK_TEMPLATE* *SIZE* options``: creates a disk, so > > + that it appears in the Ganeti configuration. This command will take > > + the same options as ``gnt-instance add --disk`` except for the > > + position value and the adopt option > > + > > +* ``gnt-disk remove *DISK_UUID|DISK_NAME*``: removes the given disk from > > + the Ganeti configuration > > + > > Please make sure that there will be an option so that we can remove a > disk from the Ganeti configuration without actually removing the disk's > data. Oh, I forgot to update this. It was actually already discussed to rename this to ``gnt-disk abandon`` and to only remove the disk from the Ganeti configuration, but not removing the disk's data as a default. Hence, there would be an option to also remove the disk's data. This would make the disk adoptable in the future. So a disk > can be in one of the four following states: > > 1) Attached to an instance (done) > 2) Detached from an instance but present in config.data and known > to Ganeti (done) > 3) Present on the host, unknown to Ganeti, but potentially adoptable > (not yet implemented) > 4) Non existent/visible neither to Ganeti nor to the host (done) > > > +Besides the ``gnt-disk`` command, we propose the following changes: > > + > > +* Make the logical id from disks that is present in > > + ``gnt-instance info *INSTANCE*`` also available through the basic > > + instance information from the RAPI > > + > > +* Disallow all disk operations via the RAPI, except for moves > > + > > +* Add a flag to preserve disks to the ``gnt-instance remove`` command > > + > > +Further possible changes > > +======================== > > + > > +If there is need for it, it would be also possible to move disk > > +detachment and attachment to the new ``gnt-disk`` command, so that we > > +would have the following simplified commands, instead of the respective > > +``gnt-instance modify --disk`` commands: > > + > > +* ``gnt-disk attach *DISK_UUID|DISK_NAME* *INSTANCE*``: attaches a disk > > + to an instance. If the disk was not in the configuration before, an > > + error is raised > > + > > +* ``gnt-disk detach *DISK_UUID|DISK_NAME|DISK_POSITION* *INSTANCE*``: > > + detaches a disk from an instance and leaves it in the Ganeti > > + configuration > > + > > +Note that this does not add new functionality, because it will behave > > +just as the current solution for disk attachment and detachment. For > > +example, disk attachment might still fail due to misaligned DRBD disks. > > + > > Since cmdlib/bdev is going to be refactored I would suggest that we push > all template specific actions down to bdev and remove them from other > parts of the code. I know that the existence of drbd makes this task > really hard, because it involves two nodes but it should be possible for > all other templates without huge an effort. > > Also since we are going to have gnt-disk, it would be great to have > a uniform snapshot/clone functionality as well in bdev and the > corresponding commands. > Could you remind me what the difference would be between snapshot and clone? > > Finally, gnt-disk copy/convert could be implemented on top of existing > disk-template convertion logic. > And also what the difference between copy and snapshot is supposed to be? > > > +Implementation details > > +====================== > > + > > +Especially for the ``gnt-disk create`` part it will be handy to > re-factor > > +the current ``LUInstanceCreate`` in such a way that we can reuse the > disk > > +creation part. That can be achieved with tasklets like they are used > > +in ``LUInstanceMigrate`` and ``LUInstanceReplaceDisks``. The same holds > > +for ``LUInstanceSetParams``, because we want to be able to make disk > > +changes also to standalone disks. > > + > > One question here: Is gnt-instance create --disk going to create two jobs > (i.e. LUDiskCreate + LUInstanceCreate)? Will it involve Tasklets/job > chains? > It would still be ``gnt-instance add`` and the idea is to re-use the disk creation tasklet that we will have for ``gnt-disk create``, so yes, we would have tasklet chains then. > > Hope the above makes sense :) > > dimara > > > +Except for creating diskless instances on purpose, the diskless disk > > +template should not be required if an instance becomes diskless, but the > > +instance should be considered to be in the state ``diskless``. > > -- > > 2.6.0.rc2.230.g3dd15c0 > -- Lisa Velden Software Engineer [email protected] Google Germany GmbH Erika-Mann-Straße 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
