This patch solves the very slow (~8-9 seconds) gnt-instance modify
behaviour. Well, it solves in general the slow RPC behaviour, but it was
most visible in that LU.

It seems that curl's behaviour with regard to file uploads (via PUT) and
the 'Expect' header are interacting badly with our http server.

First, our http server doesn't properly handle this header. According to
RFC 2616:

  Requirements for HTTP/1.1 origin servers: Upon receiving a request
  which includes an Expect request-header field with the "100-continue"
  expectation, an origin server MUST either respond with 100 (Continue)
  status and continue to read from the input stream, or respond with a
  final status code.

Our server doesn't do this, and hence it triggers this behaviour in curl
(from the curl FAQ):

  4.16 My HTTP POST or PUT requests are slow!

  libcurl makes all POST and PUT requests (except for POST requests with a
  very tiny request body) use the "Expect: 100-continue" header. This header
  allows the server to deny the operation early so that libcurl can bail out
  already before having to send any data. This is useful in authentication
  cases and others.

  However, many servers don't implement the Expect: stuff properly and if the
  server doesn't respond (positively) within 1 second libcurl will continue
  and send off the data anyway.

  You can disable libcurl's use of the Expect: header the same way you disable
  any header, using -H / CURLOPT_HTTPHEADER, or by forcing it to use HTTP 1.0.

This behaviour was detected by watching the captured traffic (in non-SSL
mode), where between the initial HTTP headers (ending with the Expect
one), there was a ~1-2 second pause until curl was sending the body.
Properly RTFM-ing would have saved ~1 day of digging around, but hey…

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This simplifies the code a bit--the status is only checked once.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

I've seen cases where the result from str(sys.exc_info()[1]) is ""; this
breaks the error reporting as the parent relies on non-empty error
messages to properly detect child status (otherwise it will try to read
the pid and fail, so on).

While this was always in case of asserts, we need to ensure this doesn't
happen. Therefore we abstract this functionality (writing the error
message) and ensure we write a non-empty string in the new function.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Since daemon startup error will be often related to socket errors, so it
makes sense to change the original reporting:

  Error when starting daemon process: "(98, 'Address already in use')"

Into:

  Error when starting daemon process: 'Socket-related error: Address
  already in use (errno=98)'

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch copies the pipe-based error reporting functionality from
utils.StartDaemon (I gave up for now on tryin to merge the two).

This patch will fix two longstanding bugs:

- if we fork, we lose all error reporting from the child to the original
  parent
- if we fork, the original parent exits before the child is ready to
  "work" (whatever the work might be)

Both these are fixed once the users of daemon.GenericMain are converted
to the three-state setup, as we'll get error reporting via the pipe and
also not exit until the PrepFn is done.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Currently, GenericMain does a two-staged workflow:

- Check, before forking
- then Exec, after forking

This means we don't have any possibility to treat preparation work
(before the daemon is ready for work) different from the actual work.

The patch adds another PreExec function that is run just before Exec,
and which should ensure that the daemon is ready for serving client
before it returns. Its result is then sent as the third argument to
Exec.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch merges the pid file handling used for ganeti-* daemons and
impexp daemons. The latter version is used, since it's more reliable:
uses locked pid files as opposed to checking 'live' processes.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This does some slight changes:

- Daemonize() doesn't explicitly close the file-descriptors anymore, but
  only implicitly via the usage of dup2
- StartDaemonChild uses separate devnull for stdin (rdonly) and
  stdout/stderr (wronly), or if using a log file, it uses it in append
  mode

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch abstracts the chdir/umask/setsid functionality, which is
identical in the code functions, just that Daemonize did the chdir/umask
in the second child; with this change it does it in the first, as
StartDaemon.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Use utils.CommaJoin to add spaces after comma, clean up code a bit.

Before: Tag(s) 'bar','baz','foo','moo' not found
After: Tag(s) 'bar', 'baz', 'foo', 'moo' not found

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Retrieving tags can be done while the lock is shared. Only writing
needs to be exclusive.

Also add a FIXME for cluster tags, where the code currently doesn't
use any locks except the config lock.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This will be used by LUXI client programs to display the VG name.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This function will also be used in tools/move-instance.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Using hugepages, KVM instances can get a good performance boost. To
activate that, we need to pass the -mem-path argument to KVM along with
the mount point of the hugetlbfs file system on the node.

For the sake of memory availability computation, we use the -mem-prealloc
argument when enabling hugepages, so KVM will reserve all hugepages it
needs when it starts. This avoids allocating an instance on a node that
will not have enough pages in case other instance needs more than what
is available after it boots.

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Per the mailing list discussion, rename _oss to _os, both in cluster parameters
and in the rest of the code.

This is just an s/_oss/_os, with the exception of a small bit of cleanup
around the helper_os function in cmdlib.py.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Committ 3fe11ba3 broke the instance rename as we don't use the FQDN
anymore. This fixes it.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Committ 3fe11ba3 broke the instance rename as we don't use the FQDN
anymore. This fixes it.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

The OS list and variants as returned from LUDiagnoseOS is not sorted,
and gnt-instance reinstall doesn't sort it either. This means that it
the menu that users are present with is inconsistent across clusters,
and that is confusing.

To make this consistent across all users of the LU, we sort the names in
the LU itself.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch changes the behaviour of OpDiagnoseOS with regards to the
'valid' field to be similar to the one for the hidden/blacklisted
fields: unless this field is requested, invalid OSes are filtered out.

The rationale is that, except for the gnt-os info/diagnose, all other
users of this opcode are requesting the valid field just to filter out
invalid OSes, and not for any other use. Thus, changing this behaviour
makes these callers simpler.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

These are useful for more in-depth checking of some kinds of arguments.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

This changes the behaviour of LUDiagnoseOS significantly.

The addition of hidden/blacklisted OSes would mean that each user-facing
client would have to filter intentionally such OSes from display, which
is not a good choice. Rather, the patch makes LUDiagnoseOS not return
any hidden or blacklisted OSes unless the hidden or respectively the
blacklisted status is requested.

While unconventional, this makes `gnt-instance reinstall --select-os`
work as intended without any changes; similar for gnt-os list. gnt-os
diagnose/gnt-os info are changed to query for, and display the new
fields.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

The new variables are:

- a list of hidden OSes, that should not be displayed to the users in
  interactive selection (e.g. reinstall); however, if they are selected, they
  can be used
- a list of OSes that should be hidden and blocked from install-time selection

The filtering will apply at pure OS name level, not OS+variant level.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Currently, the computation of the 'pure' name or the variant is
hardcoded and spread around the functions that need it. This is not
nice, and in the future we'd spread it even more with more usage of
variants/pure os names.

This patch abstracts these functions into the OS class, and then
replaces the hardcoded uses with the new functions.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Forbid nodegroups to be called with a name that matches the UUID regular
expression. Uppercase versions are forbidden as well.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

epydoc complained:
“File …/ganeti/jqueue.py, line 886, in
ganeti.jqueue._JobProcessor._MarkWaitlock
  Warning: Redefinition of type for job”

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Until now the priority for lock acquires couldn't be passed
when running opcodes.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

This allows users of these classes to simply pass None if they want to use the
default value (the actual default is an internal constant), instead of
dynamically constructing the keyword arguments.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

As already noted in the design document, an opcode's priority is
increased when the lock(s) can't be acquired within a certain amount of
time, except at the highest priority, where in such a case a blocking
acquire is used.

A unittest is provided. Priorities are not yet used for acquiring the
lock(s)—this will need further changes on mcpu.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

This patch removes duplicate code found in backend which also needs to
get VG infos. To make it simpler we moved to bdev.LogicalVolume.GetVGInfo.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This is better to group per-opcode data.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

The changes to job queue processing require some changes on this class'
interface. LockAttemptTimeoutStrategy might move to another place, but that'll
be done in a later patch.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Right now the timeout is not passed by any caller, making the code
effectively go back to blocking acquires. Since the timeout is always
None, no caller needs to be changed in this patch.

This change also means that any LUXI query handled by ganeti-masterd
will use blocking acquires if they need locks (only the case for getting
tags).

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>