GitLab

The “acquired_locks” attribute in LUs is used to keep a list of acquired
locks at each lock level. This information is already known in the lock
manager, which also happens to be the authoritative source. Removing the
attribute and directly talking to the lock manager saves us from having
to maintain the duplicate information when releasing locks.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Saves some typing and we'll use it more often in the future.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Depending on the opcode and its parameters, the existing “Summary”
function can give a rater long summary. For displaying the summary in
logs and in the lock monitor, it should be shorter. Hence this new
function is added to just use the opcode ID with common prefixes
replaced (e.g. “INSTANCE_” becomes “I_”). Opcode values are not used.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

When all locks contained in a set are acquired, the lockset's internal
lock is acquired with the same mode. With this patch the internal lock
will show up on the lock monitor, named e.g. “instances/[lockset]”.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

It is always used in the locking code. Unittests are updated.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

In some rare cases new shared acquires could sneak in through the
condition cached in “__pending_shared” while the code was still
notifying acquires. This was only working because such a condition
would be on top of the queue.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Commit 1c6e5787

 removed the iallocator and target_node keyword
parameters from TLMigrateInstance, but I didn't update their use in
LUInstanceFailover and (not fully) in LUInstanceMigrate.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Commit faaabe3c

 fixed failover behaviour for DTS_INT_MIRROR instances, however
it broke migration for DTS_EXT_MIRROR instances, by moving iallocator and node
checks from LUInstanceMigrate to TLMigrateInstance. This has the side-effect
that the LU called the TL with None for both, node and iallocator when the
default iallocator was being used.

This patch maintains the iallocator checks in TLMigrateInstance and fixes the
LU-TL integration.
Signed-off-by: Iustin Pop <iustin@google.com>
[iustin@google.com: rebased patch on current HEAD]
Reviewed-by: Iustin Pop <iustin@google.com>

This is one of the first opcodes to make use of node group locking. To
get an instance's node groups, the instance's nodes need to be looked
at. Due to a previous design decision nodes are locked after the group,
hence there's no clean locking order. This patch works around that by
first getting the instance's groups without locks, and then verifying
them after actually getting all locks. Rough overview:

- Lock instance
- Get groups of instance's nodes
- Lock groups
- Lock all nodes in groups
- Verify node groups
- Run iallocator
- Release group and unused nodes
- Replace disks, etc.

There are probably too many assertions in the code, but it's locking and
we've been bitten in the past.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This will be used for locking only the necessary node group(s)
for per-instance operations.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Commit 77fcff4a

 unintentionally incorporated code from
TLMigrateInstance.CheckPrereq into TLMigrateInstance._RunAllocator, presumably
during a rebase from earlier versions of the patch to the 2.5 codebase. As a
result all migrations running without an iallocator were performed non-live :-(

This patch moves the affected code back to CheckPrereq.
Signed-off-by: Apollon Oikonomopoulos <apollon@noc.grnet.gr>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

- Clarify some error messages
- Remove unnecessary punctuation
- Merge two if conditions in one place
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Hi all,
this patch will add 3 new KVM parameters and a new option.

New Parameters:
 - floppy_image_path = "" -> Specify the floppy image to load as
floppy disk.
 - cdrom2_image_path = "" -> Specify a second cdrom image to load on
the system (note: this in not intended to be used as a boot device. To
boot the system from cdrom you must use the "cdrom_image_path"
parameter as always).
 - cdrom_disk_type = "" -> it can be one of the kvm supported types as
"ide,scsi,paravirtual,ecc". I introduced this optional parameter to
make possible to specify a different virtual device for cdroms. It is
useful if you want to install a windows system

New option for "boot_device" parameter:
 -  "floppy": with this value you should be able to boot a KVM
instance from floppy image.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

There will be more lock releasing with upcoming changes, so this will
centralize the logic behind it (what locks to keep, which variables to
update, etc.).
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

* devel-2.4:
  TLReplaceDisks: Use implicit loop for dictionary
  Release unneeded locks while replacing disks
  locking: Export “list_owned” from lock manager
  gnt-instance: Fix typo in error message
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: Iustin Pop <iustin@google.com>

If an iallocator is used, “gnt-instance replace-disks” would acquire the
locks of all nodes (only the allocator will decide which node to use).
Unfortunately the unneeded locks were not released during the operation,
causing unnecessary delays for other jobs.

This patch changes the LU to release unneeded locks and adds assertions.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This is analog to “is_owned” and will be used for assertions.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

The iallocator parameter is “-I”, not “-i”.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

* devel-2.4: (24 commits)
  mlock: fail gracefully if libc.so.6 cannot be loaded
  Allow creating the DRBD metadev in a different VG
  Make _GenerateDRBD8Branch accept different VG names
  Fix WriteFile with unicode data
  Replace disks: keep the meta device in the same VG
  Fix for multiple VGs - PlainToDrbd and replace-disks
  Fix potential data-loss in utils.WriteFile
  Improve error messages in cluster verify/OS
  Prevent readding of the master node
  Fix punctuation in an error message
  cli: Fix wrong argument kind for groups
  Quote filename in gnt-instance.8
  Fix typo in LUGroupAssignNodes
  gnt-instance info: automatically request locking
  Document the dependency on OOB for gnt-node power
  Fix master IP activation in failover with no-voting
  disk wiping: fix bug in chunk size computation
  Fix bug in watcher
  Release locks before wiping disks during instance creation
  utils.WriteFile: Close file before renaming
  ...

Conflicts:
	lib/cmdlib.py: Disk parameter constants
	man/gnt-instance.rst: Trivial
	man/gnt-node.rst: Trivial
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This allows noded to continue instead of blowing up if the libc major
number changes.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

It is no longer used.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Commit d5cafd31

 changed this error message, swapping the
text parts in the process.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

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

Quoting from iallocator.rst: “[…] ``relocate`` request is used when an
existing instance needs to be moved within its node group […]”.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

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

Currently RemoveEtcHostsEntry keeps the ordering, but SetEtcHostsEntry
not, as it will always write the new entry at the end of file. I
personally dislike this as it "uglifies" my custom host files, so this
patch makes it update the record instead in-place so to say instead of
moving it.

The patch also simplifies the construction of the new line (we were
doing duplicate work for no gain).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

It makes no sense to iteratively write the new etc/hosts file, as we
can pre-compute the desired contents (neither the old nor the new
versions are safe against concurrent changes anyway).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This is a simple change to allow specifying a different VG for the
meta device during the creation of instances and addition of disks via
gnt-instance modify.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This is a small change to make this function take a list of VG names,
instead of a single one.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Unicode is fun, indeed:

>>> len(buffer("abc"))
3
>>> len(buffer(u"abc"))
12

So we can't pass unicode data to buffer(), as the result will be to
write the in-memory (usually UTF-32) representation to disk.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch enhances the multi-VG support in replace disks, by keeping
the meta device in the same VG, as opposed to moving it to the data
device VG (note that we don't have a way to create the meta in a
different VG in the first place, but at least we correctly handle a
custom config).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Converting an instance from 'plain' to 'drbd'.  The old code would
create the drbd volumes in the default VG and then the renames would
fail.  This fix pulls the plain VG names from the existing volumes and
places it into the new disk template.

Running 'replace-disks' has a similar issue with the new disks going
into the wrong VG and then the rename failing.

Their might be a similar issue with 'recreate-disks', but I actually
have no idea what recreate-disks does, so did not look into it.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

os.write can do incomplete writes, as long as at least some bytes have
been written (like write(2)):

>>> os.write(fd, " " * 1300)
1300
>>> os.write(fd, " " * 1300)
1300
>>> os.write(fd, " " * 1300)
1300
>>> os.write(fd, " " * 1300)
980
>>> os.write(fd, " " * 1300)
Traceback (most recent call last):
 File "<stdin>", line 1, in ?
OSError: [Errno 28] No space left on device

Note that incomplete write that only wrote 980 bytes, before the
exception.

To workaround this, we simply iterate until all data is
written. Unittests could be written by using a parameter instead of
hardcoding os.write and checking for incomplete writes.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

A few issues in the clarity of the error messages are fixed:

- "ERROR: node node3: OS API version lenny-image": no preposition
  between the parameter type and the OS name, changed to "for
  lenny-image"

- "API version lenny-image differs from reference node node1: 10, 5
  vs. 10, 20, 5, 15": parameters not sorted in display

- "OS variants list lenny-image differs from reference node node1:
  vs. default, i386": empty sets are not clearly delimited, changed to
  add [] around the sets: "node node1: [] vs. [default, i386]"

- "OS parameters lenny-image differs from reference node node1:
  vs. (u'dhcp', u'Whether to enable (yes) or disable (dhcp)')": ugly
  formatting in the OS parameters list, as we used to just "%s" the
  tuple; now it is "reference node node1: [] vs. [dhcp: Whether to
  enable (yes) or disable (dhcp)]"
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This breaks Ganeti in multiple ways. If we don't make the check in
gnt-node itself, then bootstrap.SetupNodeDaemon will restart the
master daemon, making the operation fail:

  node1# gnt-node add --readd node1
  Cannot communicate with the master daemon.
  Is it running and listening for connections?

The check in cmdlib is more of a safety check, as we shouldn't reach
it. If we do (via a bad client), then it will prevent breakage in the
job queue/config handling.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

IIRC we don't use punctuation at the end of error messages.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

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: Iustin Pop <iustin@google.com>