Hi Dimitris, This is ok as is - I will just just consider this patch a part of the previous patch series. I will run QA on the patches just in case due to the general refactoring changes, and then finish off reviewing everything and push if all is well.
Btw, this patch does not fix RBD snapshots (issue 545) in case the Ganeti RBD disk template was used. The rbd snap create comment in that issue suggests that external storage was not used. Do you mind if I follow up with a patch to fix this or do you want to do it? I have a QA set up that uses this disk template, so it might be more convenient for me. Cheers, Riba On Mon, Jul 14, 2014 at 8:14 AM, Dimitris Aragiorgis <[email protected]> wrote: > > Hi! > > You are right about breaking compatibility. Are you OK with these fixes? > If yes I can squash the relevant diffs with the previous commits, rebase > the whole patch set with current master and resend it. > > Let me know, thanks, > dimara > > > > * Dimitris Aragiorgis <[email protected]> [2014-07-14 09:10:46 +0300]: > > > ..for the ExtStorage providers. This way we do not break > > compatibility with existing providers that do not implement > > such a functionality. > > > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > > --- > > doc/design-shared-storage.rst | 6 +++++- > > lib/masterd/instance.py | 4 +++- > > lib/storage/extstorage.py | 7 ++++++- > > man/ganeti-extstorage-interface.rst | 4 ++++ > > 4 files changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/doc/design-shared-storage.rst > b/doc/design-shared-storage.rst > > index c1b3c01..793c522 100644 > > --- a/doc/design-shared-storage.rst > > +++ b/doc/design-shared-storage.rst > > @@ -198,7 +198,7 @@ provider is expected to provide the following > scripts: > > - ``detach`` > > - ``setinfo`` > > - ``verify`` > > -- ``snapshot`` > > +- ``snapshot`` (optional) > > > > All scripts will be called with no arguments and get their input via > > environment variables. A common set of variables will be exported for > > @@ -233,6 +233,10 @@ error, accompanied by an appropriate error message > on stderr. The > > the block device's full path, after it has been successfully attached to > > the host node. On error it should return non-zero. > > > > +To keep backwards compatibility we let the ``snapshot`` script be > > +optional. If present then the provider will support instance backup > > +export as well. > > + > > Implementation > > -------------- > > > > diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py > > index c1639e3..ebd563c 100644 > > --- a/lib/masterd/instance.py > > +++ b/lib/masterd/instance.py > > @@ -1157,7 +1157,9 @@ class ExportInstanceHelper(object): > > self._removed_snaps = [False] * len(instance.disks) > > > > def CreateSnapshots(self): > > - """Creates an LVM snapshot for every disk of the instance. > > + """Creates a snapshot for every disk of the instance. > > + > > + Currently support drbd, plain and ext disk templates. > > > > """ > > assert not self._snap_disks > > diff --git a/lib/storage/extstorage.py b/lib/storage/extstorage.py > > index 19692a3..71b4f1b 100644 > > --- a/lib/storage/extstorage.py > > +++ b/lib/storage/extstorage.py > > @@ -381,14 +381,19 @@ def ExtStorageFromDisk(name, base_dir=None): > > # an optional one > > es_files = dict.fromkeys(constants.ES_SCRIPTS, True) > > > > + # Let the snapshot script be optional > > + es_files[constants.ES_SCRIPT_SNAPSHOT] = False > > + > > es_files[constants.ES_PARAMETERS_FILE] = True > > > > - for (filename, _) in es_files.items(): > > + for (filename, required) in es_files.items(): > > es_files[filename] = utils.PathJoin(es_dir, filename) > > > > try: > > st = os.stat(es_files[filename]) > > except EnvironmentError, err: > > + if not required: > > + continue > > return False, ("File '%s' under path '%s' is missing (%s)" % > > (filename, es_dir, utils.ErrnoOrStr(err))) > > > > diff --git a/man/ganeti-extstorage-interface.rst > b/man/ganeti-extstorage-interface.rst > > index 44a1adb..97ce14f 100644 > > --- a/man/ganeti-extstorage-interface.rst > > +++ b/man/ganeti-extstorage-interface.rst > > @@ -220,6 +220,10 @@ respectively (see above). > > > > The script returns ``0`` on success. > > > > +Please note that this script is optional and not all providers should > > +implement it. Of course if it is not present, instance backup export > > +will not be supported for the given provider. > > + > > TEXT FILES > > ---------- > > > > -- > > 1.7.10.4 > Hrvoje Ribicic Ganeti Engineering 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 Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
