Intruduce new function _InstanceRunning().
Fix *PCIInfo config methods to hold exclusively _config_lock and
take instance object as an argument.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Signed-off-by: Dimitris Aragriorgs <dimara@grnet.gr>

Signed-off-by: Dimitris Aragriorgs <dimara@grnet.gr>

Introduce _GenerateKVMBlockDevicesOptions()

In case hotplug is enabled per instance then the runtime file
contains one more field: block_devices. In this case kvm_cmd
is extended with block device options during _ExecuteKVMRuntime().

If no hotplug is enabled then runtime file has block_devices info
encapsulated in kvm_cmd and block device options are exteded
during _GenerateKVMRuntime.

Signed-off-by: Dimitris Aragriorgs <dimara@grnet.gr>

Add --hotplug option.
Only used by OpInstanceCreate()/OpInstanceSetParams.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Hotplugging is done by functions invoked by ApplyContainerMods().  In
order hotplugging to take place a --hotplug option must be passed.
Hotplug can enabled for a VM if --hotplug is passed during
gnt-instance add (hotplug_info will be added).

In order initial devices of a VM to be hotpluggable hotplug_info must be
early available. So _GetPCIInfo() gets either lu.hotplug_info in case of
LUInstanceCreate() or invokes lu.cfg.GetPCIInfo() in case of
LUInstanceSetParams(). To return a pci slot into the pool
lu.cfg.UpdatePCIInfo() must be invoked.

If no --hotplug is passed during gnt-instance modify --net add the nic
won't get a pci value and will be placed automaticaly in a free pci slot
after reboot. This means that is not going to be hotpluggable for the
rest of the instance's life.

To keep pci pool consistent we do not let a device that has been
hotplugged (i.e. has pci value) to be removed without --hotplug.

Before hotplugging a Disk it must be assembled. During
LuInstanceSetParams() newly created disks are not added to the instance
so _ExpandCheckDisks() in _AssembleInstanceDisks() will fail. So we make
this check optional only for this case.

In order to remove a disk (with blockdev_remove) it must be shutdown. So
after unplugging the disk _ShutdownDiskInstanceDisks() must be invoked.

Everything else is just like nic hotplugging.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Introduce new RPCs that eventually invoke hypervisor specific
hotplug functions.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

KVM >= 1.0 is needed.

Introduce new methods: HotAddNic/HotDelNic

Both qemu monitor interface for hotplugging. If a device has
hotplug info (idx, pci) then:
 - it gets named after it (e.g. virtio-net-pci.%d % idx ).
 - it gets placed to the pci slot (hex(pci)) of bus 0.
If no hotplug info exist or kvm version is old then no hotplug takes
place and modifications will take place after reboot:
 - devices will get traditionaly named
 - and placed automaticaly in empty pci slots

NIC hotplug:
 - open a tap and get its file descriptor.
 - pass fd with SCM rights (using python-fdsend) via monitor socket
 - create netdev and device with proper qemu interface

Disk hotplug:
 - create drive with drive%d % idx name
 - create device virtio-blk-pci.%d % idx

In order to migrate a VM that has devices hotplugged, an identical
VM should be booted with -incoming. To this end every time a hotplug
takes place runtime file must be updated.

For disk hotplug support we removeds disk from kvm_cmd in
runtime file add added them separately just like nics. So code building
disk related options has been moved out from _GenerateKVMRuntime()
and added to _ExecuteKVMRuntime().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Introduce new HotplugInfo object as part of Instance object.

hotplug_info { 'nics': 0, 'disks': 0, 'pci_pool': [16, 17, 18] }

nics/disks is an index used for device identification and naming.
Everytime a new device (disk/nic) is hotadded these values get incremented.

pci_pool is a list of pci slots where a device can be placed. Every
time a device gets hot-plugged/unplugged a pci slot gets popped/appended.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

Currently, hail fails with FailDisk when trying to add an instance
of type: 'file', 'sharedfile' and 'rbd'.

This is due to a "0" or None value in the corresponding dict inside
_ComputeDiskSize, which results in a "O" or non Int value of the
exported 'disk_space_total' parameter. This in turn makes hail fail,
when trying to process the value:

 - with "Unable to read Int" if value is None (file)
 - with FailDisk if value is 0 (sharedfile, rbd)

The latter happens because the 0 value doesn't match the instance's
IPolicy, since it is lower than the minimum disk size.

The second problem still exists when using adoption with 'plain'
and 'blockdev' template and will be addressed in another commit.

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Due to an oversight, the pause/resume sync RPC calls in _WipeDisks
lack the verification of the overall RPC status, and directly iterate
over the payload. The code actually doing the wipe does verify
correctly the results. This can result in jobs failing with a hard to
diagnose:

OpExecError ['NoneType' object is not iterable]

instead of proper "RPC failed" message.

This patch adds a hard check on the pause call, but for the resume
call it just logs a warning if the RPC failed; the rationale being
that if we can't contact the node for pausing the sync, it's likely
wiping will fail too, but after the wipe has been done, we can
continue.

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

This breaks the command, as optparse considers that an error.

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

It seems that 'explicitely' is wrong, and that the right form is
'explicitly'. This is just fixing the typo plus adjusting affected
paragraphs.

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

In case LUClusterVerifyGroup is run on a group which doesn't contain
the master node, the following could happen:

- master node is selected due to the explicit check
- if the order of nodes in the 'absent_nodes' list is such that the
  master node is the first in it, then we'll select (again) the master
  node
- passing duplicate nodes to RPC calls will break due to RPC
  internals; this should be fixed separately, but in the meantime we
  just refrain from passing such duplicates

This patch should not change the semantics of the code, since it
wasn't guaranteed even before that we find a vm_capable node.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

Commit 904b3bfe tried to fix the deletion of custom ndparams from
group, but instead broke both modification and deletion: because we
run ForceDictType on self.op.ndparams instead of the updated
new_ndparams, we can neither delete nor set properly spindle_count
(since it won't be coerced to int).

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

This generalises commit 4304964a to cdroms too, since they have
somewhat the same logic. We just abstract the needs_boot_flag into a
separate variable, and then reuse it in the cdrom section.

Note that the logic of what 'if=' type to pass to KVM was very
convoluted, and (I think) incorrect; I went and cleaned it to be more
consistent.

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

This addresses issue 230: passing two methods of booting to KVM can,
depending on the KVM version, confuse it.

Note that commit 4304964a introduced a partial fix for this (but only
for disks, and keyed on KVM versions). However, it didn't fix cdrom
booting, which still fails with the same error, so let's fix it more
generically; we still leave the per-disk check since that is about
-boot c versus -drive …,boot=on rather than two boot methods.

Patch is based on the one submitted by Vladimir Mencl, many thanks!

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

On node groups we don't have the std field. However, the InstancePolicy
object always verifies that the std value is within a given range. As we
fill it up with defaults if not set (as it happens to be on node groups)
and the min value is higher than the default std value (taken from
constants.py) we fail.

We overcome this situation by simply let the function know if we want to
verify the std value at all. If we don't want to verify std, we just set
it to a compliant value (min_v) and continue.

We also slightly adapt the error message provided, as we don't have std
values on groups.

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

Sorry, I forgot that with GenerateTable this needs to be handled
manually. Fixed now and tested in both ways.

(But to be honest, this should be abstracted in GenerateTable, instead
of the 'if' test in all its callers.)

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

This uses confd to query the DRBD minors, which is very special; no
other command currently does so.

Since the backend is only implemented in the Haskell version of confd,
we have checks that 1) confd is enable, and 2) hs confd is also
enabled. If by mistake people do manage to query Python confd, the
error message will be clean:

  Query gave non-ok status '2': not implemented

So nothing breaks in an "ugly" way.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

This is not implemented currently.

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

It seems this was not updated since the move to static minors…

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Commit cce46164 fixed upgrading from other 2.6 versions, but
accidentally broke upgrading from 2.5 (disk.params was left as None,
which breaks FillDict). The simplest way to handle params is to always
set them to an empty dict (disregarding what they currently contain).

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

While looking at the testability of this piece of code, I found another
consistency problem. We have two if branches instead of one, with
elif's.

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

This inconsistency was found during rebalancing. Hbal failed because,
Ganeti couldn't load the opcode. After digging through the cause, an
inconsistency with the "args" field in the LUXI protocol was triggered
by the TemplateHaskell side where it's done uniformed.

For SubmitJob and SubmitManyJobs we treat args as one argument,
containing the job definition. In every other LUXI call args is actually
a list of arguments. This patch fixes this consistency.

This change is NOT backwards compatible.

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

This bug was found during disk parameter debugging. While looking at the
config some values seem present on the disk parameters, but that's not
expected behaviour. This patch fixes this, and also fixes the "broken"
configs automatically upon masterd restart.

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

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

When we delete DRBD disks from some instance, we do not want to get
errors due to nodes other than that instance's primary being offline.

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

Currently, this is not allowed, so one can't run a replace-disks; this
breaks any non-invasive method of recovering the redundancy of the
instance if its disks are already stopped (but it still works if the
disks on the primary are active). So let's fix this inconsistency.

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

For compatibility with the old Ganeti version, we want to keep the
beparams/memory field around for another release. This patch fixes this
regression.

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

Currently, _RedistributeAncillaryFiles computes two lists: the list of
online nodes (for all files redistribution), and the list of
vm_capable nodes, for hypervisor-specific files. However, the
vm_capable list includes offline nodes too, leading to warning
messages:

  WARNING: Copy of file /etc/xen/xend-config.sxp to node node13.example.com failed: Node is marked offline

We fix this by trivially intersecting the vm_capable list with the
online one.

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

This error does not show up until we exceed the pool of master
candidates and have nodes which are not master candidates.

The background is that we check for master-ip-setup script on master
candidates and expect them not to be on the other nodes. However, we
distribute a default master-ip-script which break this assumption.
Furthermore, there's no reason why the file should just exists on the
master candidates.

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

With the new gnt-instance modify syntax for addition and removal of
disks/NICs on arbitrary indexes, we hit an assertion if the user
passes `modify' as one of the disk's parameters. E.g::

 gnt-instance modify --disk 2:modify,size=3G instance1
 gnt-instance modify --disk 3:add,size=1G,modify instance2

This patch fixes the bug, by allowing `modify' to be passed as a
parameter (as happens with `add' and `remove'), as long as it is
not done alongside `add' or `remove'. If so, it is treated in the
same way as if none of modify/add/remove is passed --> modify.

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

We call _OpenRealBD during the process and this needs disk parameters to
work. This was reported by Constantinos.

The fix is very ugly though.

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

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

This annotates the disks for the blockdev_remove where it is
appropriate. It leaves out 2 cases were we can't reliably annotate disk
parameters due to lack of knowledge what we should annotate. Those cases
affects only lvs used for drbd, so it doesn't affect the bug reported by
Constantinos.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

This is also related to the bug reported by Constantinos,
as we've only one getmirrorstatus_multi call in whole cmdlib, we just
annotate them while we are building the disk list.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>