On Apr 04 17:39, Hrvoje Ribicic wrote:
> A completely new patch, instead! Only the content, both the description and
> the patch name stay the same.
>
> diff --git a/lib/cli.py b/lib/cli.py
> index d31411d..be87b01 100644
> --- a/lib/cli.py
> +++ b/lib/cli.py
> @@ -230,6 +230,8 @@ __all__ = [
> "YES_DOIT_OPT",
> "ZEROING_IMAGE_OPT",
> "ZERO_FREE_SPACE_OPT",
> + "ZEROING_TIMEOUT_FIXED_OPT",
> + "ZEROING_TIMEOUT_PER_MIB_OPT",
> "DISK_STATE_OPT",
> "HV_STATE_OPT",
> "IGNORE_IPOLICY_OPT",
> @@ -1748,6 +1750,18 @@ ZERO_FREE_SPACE_OPT = \
> help="Whether to zero the free space on the disks of the "
> "instance prior to the export")
>
> +ZEROING_TIMEOUT_FIXED_OPT = \
> + cli_option("--zeroing-timeout-fixed",
> + dest="zeroing_timeout_fixed", action="store", type="int",
> + help="The fixed amount of time to wait before assuming that
> the "
> + "zeroing failed")
> +
> +ZEROING_TIMEOUT_PER_MIB_OPT = \
> + cli_option("--zeroing-timeout-per-mib",
> + dest="zeroing_timeout_per_mib", action="store",
> type="float",
> + help="The amount of time to wait per MiB of data to zero,
> in "
> + "addition to the fixed timeout")
> +
> #: Options provided by all commands
> COMMON_OPTS = [DEBUG_OPT, REASON_OPT]
>
> diff --git a/lib/client/gnt_backup.py b/lib/client/gnt_backup.py
> index ae11163..91251a4 100644
> --- a/lib/client/gnt_backup.py
> +++ b/lib/client/gnt_backup.py
> @@ -90,14 +90,19 @@ def ExportInstance(opts, args):
> raise errors.OpPrereqError("Target node must be specified",
> errors.ECODE_INVAL)
>
> - op = opcodes.OpBackupExport(instance_name=args[0],
> - target_node=opts.node,
> - compress=opts.transport_compression,
> - shutdown=opts.shutdown,
> - shutdown_timeout=opts.shutdown_timeout,
> - remove_instance=opts.remove_instance,
> -
> ignore_remove_failures=ignore_remove_failures,
> - zero_free_space=opts.zero_free_space)
> +
> + op = opcodes.OpBackupExport(
> + instance_name=args[0],
> + target_node=opts.node,
> + compress=opts.transport_compression,
> + shutdown=opts.shutdown,
> + shutdown_timeout=opts.shutdown_timeout,
> + remove_instance=opts.remove_instance,
> + ignore_remove_failures=ignore_remove_failures,
> + zero_free_space=opts.zero_free_space,
> + zeroing_timeout_fixed=opts.zeroing_timeout_fixed,
> + zeroing_timeout_per_mib=opts.zeroing_timeout_per_mib
> + )
>
> SubmitOrSend(op, opts)
> return 0
> @@ -153,7 +158,8 @@ commands = {
> ExportInstance, ARGS_ONE_INSTANCE,
> [FORCE_OPT, SINGLE_NODE_OPT, TRANSPORT_COMPRESSION_OPT, NOSHUTDOWN_OPT,
> SHUTDOWN_TIMEOUT_OPT, REMOVE_INSTANCE_OPT, IGNORE_REMOVE_FAILURES_OPT,
> - DRY_RUN_OPT, PRIORITY_OPT, ZERO_FREE_SPACE_OPT] + SUBMIT_OPTS,
> + DRY_RUN_OPT, PRIORITY_OPT, ZERO_FREE_SPACE_OPT,
> ZEROING_TIMEOUT_FIXED_OPT,
> + ZEROING_TIMEOUT_PER_MIB_OPT] + SUBMIT_OPTS,
> "-n <target_node> [opts...] <name>",
> "Exports an instance to an image"),
> "import": (
> diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
> index 2ca67cf..55dfd50 100644
> --- a/lib/cmdlib/backup.py
> +++ b/lib/cmdlib/backup.py
> @@ -298,6 +298,18 @@ class LUBackupExport(LogicalUnit):
> raise errors.OpPrereqError("A zeroing image must be set for
> zeroing to"
> " work")
>
> + if self.op.zeroing_timeout_fixed is None:
> + self.op.zeroing_timeout_fixed = constants.HELPER_VM_STARTUP
> +
> + if self.op.zeroing_timeout_per_mib is None:
> + self.op.zeroing_timeout_per_mib = constants.ZEROING_TIMEOUT_PER_MIB
> +
> + else:
> + if (self.op.zeroing_timeout_fixed is not None or
elif
Rest LGTM. No need to resend.
Thanks,
Jose
> + self.op.zeroing_timeout_per_mib is not None):
> + raise errors.OpPrereqError("Zeroing timeout options can only be
> used"
> + " only with the --zero-free-space
> option")
> +
> def _CleanupExports(self, feedback_fn):
> """Removes exports of current instance from all other nodes.
>
> @@ -382,6 +394,9 @@ class LUBackupExport(LogicalUnit):
> @param feedback_fn: Function used to log progress
>
> """
> + assert self.op.zeroing_timeout_fixed is not None
> + assert self.op.zeroing_timeout_per_mib is not None
> +
> zeroing_image = self.cfg.GetZeroingImage()
> src_node_uuid = self.instance.primary_node
> disk_size = self._DetermineImageSize(zeroing_image, src_node_uuid)
> diff --git a/man/gnt-backup.rst b/man/gnt-backup.rst
> index 4ce2664..f42a415 100644
> --- a/man/gnt-backup.rst
> +++ b/man/gnt-backup.rst
> @@ -28,7 +28,8 @@ EXPORT
> | [\--shutdown-timeout=*N*] [\--noshutdown] [\--remove-instance]
> | [\--ignore-remove-failures] [\--submit] [\--print-job-id]
> | [\--transport-compression=*compression-mode*]
> -| [\--zero-free-space]
> +| [\--zero-free-space] [\--zeroing-timeout-fixed]
> +| [\--zeroing-timeout-per-mib]
> | {*instance*}
>
> Exports an instance to the target node. All the instance data and
> @@ -55,7 +56,10 @@ removing the instance.
>
> The ``--zero-free-space`` option can be used to zero the free space
> of the instance prior to exporting it, saving space if compression
> -is used.
> +is used. The ``--zeroing-timeout-fixed`` and
> +``--zeroing-timeout-per-mib`` options control the timeout, the former
> +determining the minimum time to wait, and the latter how much longer
> +to wait per MiB of data the instance has.
>
> The exit code of the command is 0 if all disks were backed up
> successfully, 1 if no data was backed up or if the configuration
>
>
> On Wed, Apr 2, 2014 at 4:56 PM, Jose A. Lopes <[email protected]> wrote:
>
> > On Apr 02 13:30, Hrvoje Ribicic wrote:
> > > This patch adds the two parameters to gnt-backup export, documenting
> > > their meanings in the manual file as well.
> > >
> > > Signed-off-by: Hrvoje Ribicic <[email protected]>
> > > ---
> > > lib/cli.py | 14 ++++++++++++++
> > > lib/client/gnt_backup.py | 39 ++++++++++++++++++++++++++++++---------
> > > man/gnt-backup.rst | 8 ++++++--
> > > 3 files changed, 50 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/lib/cli.py b/lib/cli.py
> > > index 9e572a0..8ce7996 100644
> > > --- a/lib/cli.py
> > > +++ b/lib/cli.py
> > > @@ -230,6 +230,8 @@ __all__ = [
> > > "YES_DOIT_OPT",
> > > "ZEROING_IMAGE_OPT",
> > > "ZERO_FREE_SPACE_OPT",
> > > + "ZEROING_TIMEOUT_FIXED_OPT",
> > > + "ZEROING_TIMEOUT_PER_MIB_OPT",
> > > "DISK_STATE_OPT",
> > > "HV_STATE_OPT",
> > > "IGNORE_IPOLICY_OPT",
> > > @@ -1748,6 +1750,18 @@ ZERO_FREE_SPACE_OPT = \
> > > help="Whether to zero the free space on the disk of the "
> > > "instance prior to the export")
> > >
> > > +ZEROING_TIMEOUT_FIXED_OPT = \
> > > + cli_option("--zeroing-timeout-fixed",
> > > + dest="zeroing_timeout_fixed", action="store", type="int",
> > > + help="The fixed amount of time to wait before assuming
> > that the "
> > > + "zeroing failed")
> > > +
> > > +ZEROING_TIMEOUT_PER_MIB_OPT = \
> > > + cli_option("--zeroing-timeout-per-mib",
> > > + dest="zeroing_timeout_per_mib", action="store",
> > type="float",
> > > + help="The amount of time to wait per MiB of data to
> > zero, in "
> > > + "addition to the fixed timeout")
> > > +
> > > #: Options provided by all commands
> > > COMMON_OPTS = [DEBUG_OPT, REASON_OPT]
> > >
> > > diff --git a/lib/client/gnt_backup.py b/lib/client/gnt_backup.py
> > > index ae11163..4e5a9a8 100644
> > > --- a/lib/client/gnt_backup.py
> > > +++ b/lib/client/gnt_backup.py
> > > @@ -90,14 +90,34 @@ def ExportInstance(opts, args):
> > > raise errors.OpPrereqError("Target node must be specified",
> > > errors.ECODE_INVAL)
> > >
> > > - op = opcodes.OpBackupExport(instance_name=args[0],
> > > - target_node=opts.node,
> > > - compress=opts.transport_compression,
> > > - shutdown=opts.shutdown,
> > > - shutdown_timeout=opts.shutdown_timeout,
> > > - remove_instance=opts.remove_instance,
> > > -
> > ignore_remove_failures=ignore_remove_failures,
> > > - zero_free_space=opts.zero_free_space)
> > > + if not opts.zero_free_space and \
> > > + (opts.zeroing_timeout_fixed is not None or
> > > + opts.zeroing_timeout_per_mib is not None):
> > > + raise errors.OpPrereqError("Zeroing timeout options can only be
> > used only "
> > > + "with the --zero-free-space option")
> >
> > Shouldn't this be checked in the LU?
> > Otherwise, does the RAPI check this?
> >
> > > +
> > > + # We could pass None's into the opcode, but that duplicates the
> > defaults and
> > > + # results in an ugly opcode definition and is evil. Better to
> > sacrifice the
> > > + # static pylint check here, although not the best solution.
> > > + optional_numeric_params = dict((k, getattr(opts, k)) for k in
> > > + ["zeroing_timeout_fixed",
> > > + "zeroing_timeout_per_mib"]
> > > + if hasattr(opts, k))
> >
> > Don't like the reflection part too much.
> > I see you have murdered pylint... tsc tsc.
> > Anyway, just my two cents...
> >
> > > + # Disabling warning about ** magic
> > > + # pylint: disable=W0142
> > > + op = opcodes.OpBackupExport(
> > > + instance_name=args[0],
> > > + target_node=opts.node,
> > > + compress=opts.transport_compression,
> > > + shutdown=opts.shutdown,
> > > + shutdown_timeout=opts.shutdown_timeout,
> > > + remove_instance=opts.remove_instance,
> > > + ignore_remove_failures=ignore_remove_failures,
> > > + zero_free_space=opts.zero_free_space,
> > > + **optional_numeric_params
> > > + )
> > > + # pylint: enable=W0142
> > >
> > > SubmitOrSend(op, opts)
> > > return 0
> > > @@ -153,7 +173,8 @@ commands = {
> > > ExportInstance, ARGS_ONE_INSTANCE,
> > > [FORCE_OPT, SINGLE_NODE_OPT, TRANSPORT_COMPRESSION_OPT,
> > NOSHUTDOWN_OPT,
> > > SHUTDOWN_TIMEOUT_OPT, REMOVE_INSTANCE_OPT,
> > IGNORE_REMOVE_FAILURES_OPT,
> > > - DRY_RUN_OPT, PRIORITY_OPT, ZERO_FREE_SPACE_OPT] + SUBMIT_OPTS,
> > > + DRY_RUN_OPT, PRIORITY_OPT, ZERO_FREE_SPACE_OPT,
> > ZEROING_TIMEOUT_FIXED_OPT,
> > > + ZEROING_TIMEOUT_PER_MIB_OPT] + SUBMIT_OPTS,
> > > "-n <target_node> [opts...] <name>",
> > > "Exports an instance to an image"),
> > > "import": (
> > > diff --git a/man/gnt-backup.rst b/man/gnt-backup.rst
> > > index 4ce2664..f42a415 100644
> > > --- a/man/gnt-backup.rst
> > > +++ b/man/gnt-backup.rst
> > > @@ -28,7 +28,8 @@ EXPORT
> > > | [\--shutdown-timeout=*N*] [\--noshutdown] [\--remove-instance]
> > > | [\--ignore-remove-failures] [\--submit] [\--print-job-id]
> > > | [\--transport-compression=*compression-mode*]
> > > -| [\--zero-free-space]
> > > +| [\--zero-free-space] [\--zeroing-timeout-fixed]
> > > +| [\--zeroing-timeout-per-mib]
> > > | {*instance*}
> > >
> > > Exports an instance to the target node. All the instance data and
> > > @@ -55,7 +56,10 @@ removing the instance.
> > >
> > > The ``--zero-free-space`` option can be used to zero the free space
> > > of the instance prior to exporting it, saving space if compression
> > > -is used.
> > > +is used. The ``--zeroing-timeout-fixed`` and
> > > +``--zeroing-timeout-per-mib`` options control the timeout, the former
> > > +determining the minimum time to wait, and the latter how much longer
> > > +to wait per MiB of data the instance has.
> > >
> > > The exit code of the command is 0 if all disks were backed up
> > > successfully, 1 if no data was backed up or if the configuration
> > > --
> > > 1.9.1.423.g4596e3a
> > >
> >
> > --
> > Jose Antonio Lopes
> > 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
> >
--
Jose Antonio Lopes
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
