* 'Hrvoje Ribicic' via ganeti-devel <[email protected]> [2014-07-14 
16:07:09 +0200]:

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

Ok. Perfect.

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

True story. I had snapshot support for the ext disk template implemented
a while ago and given the latest comment on the issue I submitted the patches.
I guess a Snapshot() method plus minor changes in the backend should do
the job..

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

Please, be my guest :)

Thanks a lot,
dimara

> 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

Attachment: signature.asc
Description: Digital signature

Reply via email to