Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

['Brian Foley' via ganeti-devel]

On Thu, Jan 12, 2017 at 10:59:55AM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This patch adds some more consistency in the parameter names
> displayed in various ganeti man pages and their helper text
> in gnt commands.

Yikes. A huge and tedious patch. Thanks for doing this. LGTM except for the
minor typo below.

Cheers,
Brian.

> Signed-off-by: Federico Morg Pareschi 
> ---
>  lib/client/gnt_backup.py   |  6 +++---
>  lib/client/gnt_cluster.py  | 21 +-
>  lib/client/gnt_debug.py|  6 +++---
>  lib/client/gnt_filter.py   | 10 -
>  lib/client/gnt_group.py| 24 ++---
>  lib/client/gnt_instance.py | 40 ++
>  lib/client/gnt_job.py  | 12 +--
>  lib/client/gnt_network.py  | 22 +--
>  lib/client/gnt_node.py | 35 +++---
>  man/gnt-backup.rst | 10 -
>  man/gnt-cluster.rst| 12 +--
>  man/gnt-debug.rst  | 28 
>  man/gnt-filter.rst | 24 ++---
>  man/gnt-group.rst  | 14 ++--
>  man/gnt-instance.rst   | 54 
> --
>  man/gnt-job.rst| 15 ++---
>  man/gnt-network.rst| 36 +++
>  man/gnt-node.rst   | 38 
>  man/gnt-os.rst |  5 +++--
>  19 files changed, 209 insertions(+), 203 deletions(-)

[snip]

> -**watch** {id}
> +**watch** {(*job-id*}

Accidental extra paren.

[PATCH stable-2.15] Re-add 2 imports incorrectly removed during cleanup

['Brian Foley' via ganeti-devel]

This was done in commit 7b1fd2e60e856ac to quell pylint (my bad!)

These broke ganeti-rapi and move-instance. Make sure, as a minimal
sanity check, that all the ganeti tools start up with --help.

Signed-off-by: Brian Foley 
---
 lib/server/rapi.py  | 1 +
 tools/move-instance | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lib/server/rapi.py b/lib/server/rapi.py
index a3583adf0..56c820e38 100644
--- a/lib/server/rapi.py
+++ b/lib/server/rapi.py
@@ -61,6 +61,7 @@ from ganeti.rapi import connector
 from ganeti.rapi import baserlib
 
 import ganeti.http.auth   # pylint: disable=W0611
+import ganeti.http.server # pylint: disable=W0611
 
 
 class RemoteApiRequestContext(object):
diff --git a/tools/move-instance b/tools/move-instance
index e43bd5c23..1509aada0 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -52,6 +52,7 @@ from ganeti import rapi
 from ganeti import errors
 
 from ganeti.rapi.client import UsesRapiClient
+import ganeti.rapi.client_utils # pylint: disable=W0611
 
 
 SRC_RAPI_PORT_OPT = \
-- 
2.11.0.390.gc69c2f50cf-goog

Re: [PATCH-2.15] kvm: use_guest_agent: QEMU Guest Agent support

['Brian Foley' via ganeti-devel]

On Thu, Jan 05, 2017 at 04:46:08PM -0800, Robin H. Johnson wrote:
> Implement the QEMU Guest Agent sockets, so that code/scripts on the
> hypervisors can communicate with guest operating systems easily.

Cool. I see that this is almost the same as the patch for master with some minor
tweaks. As we discussed in the previous review, we won't be able to apply this
to 2.15 because it modifies the RPC format, and we don't want to do this within
a minor version because it means 2.15.x and 2.15.x+1 daemons interoperate during
upgrades.

But it certainly makes sense to have this as an optional patch for your distro
for users who Know What They're Doing.

Thanks,
Brian.

> Signed-off-by: Robin H. Johnson 
> ---
>  lib/hypervisor/hv_kvm/__init__.py | 23 +++
>  man/gnt-instance.rst  |  7 +++
>  src/Ganeti/Constants.hs   |  5 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/lib/hypervisor/hv_kvm/__init__.py 
> b/lib/hypervisor/hv_kvm/__init__.py
> index cd29baa38..89bc18b85 100644
> --- a/lib/hypervisor/hv_kvm/__init__.py
> +++ b/lib/hypervisor/hv_kvm/__init__.py
> @@ -351,6 +351,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  constants.HV_MIGRATION_BANDWIDTH: hv_base.REQ_NONNEGATIVE_INT_CHECK,
>  constants.HV_MIGRATION_DOWNTIME: hv_base.REQ_NONNEGATIVE_INT_CHECK,
>  constants.HV_MIGRATION_MODE: hv_base.MIGRATION_MODE_CHECK,
> +constants.HV_USE_GUEST_AGENT: hv_base.NO_CHECK,
>  constants.HV_USE_LOCALTIME: hv_base.NO_CHECK,
>  constants.HV_DISK_CACHE:
>hv_base.ParamInSet(True, constants.HT_VALID_CACHE_TYPES),
> @@ -581,6 +582,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  """
>  return utils.PathJoin(cls._CTRL_DIR, "%s.qmp" % instance_name)
>  
> +  @classmethod
> +  def _InstanceQemuGuestAgentMonitor(cls, instance_name):
> +"""Returns the instance serial QEMU Guest Agent socket name
> +
> +"""
> +return utils.PathJoin(cls._CTRL_DIR, "%s.qga" % instance_name)
> +
>@classmethod
>def _InstanceKvmdMonitor(cls, instance_name):
>  """Returns the instance kvm daemon socket name
> @@ -667,6 +675,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  utils.RemoveFile(cls._InstanceMonitor(instance_name))
>  utils.RemoveFile(cls._InstanceSerial(instance_name))
>  utils.RemoveFile(cls._InstanceQmpMonitor(instance_name))
> +utils.RemoveFile(cls._InstanceQemuGuestAgentMonitor(instance_name))
>  utils.RemoveFile(cls._InstanceKVMRuntime(instance_name))
>  utils.RemoveFile(cls._InstanceKeymapFile(instance_name))
>  uid_file = cls._InstanceUidFile(instance_name)
> @@ -1376,6 +1385,20 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  if self._UUID_RE.search(kvmhelp):
>kvm_cmd.extend(["-uuid", instance.uuid])
>  
> +# Add guest agent socket
> +if hvp[constants.HV_USE_GUEST_AGENT]:
> +  qga_addr = utils.GetFreeSlot(pci_reservations, reserve=True)
> +  qga_pci_info = "bus=%s,addr=%s" % ('pci.0', hex(qga_addr))
> +  qga_path = self._InstanceQemuGuestAgentMonitor(instance.name)
> +  logging.info("KVM: Guest Agent available at %s", qga_path)
> +  # The 'qga0' identified can change, but the 'org.qemu.guest_agent.0' 
> string is
> +  # the default expected by the Guest Agent.
> +  kvm_cmd.extend([
> +"-chardev", "socket,path=%s,server,nowait,id=qga0" % qga_path,
> +"-device", "virtio-serial,id=qga0,%s" % qga_pci_info,
> +"-device", "virtserialport,chardev=qga0,name=org.qemu.guest_agent.0",
> +])
> +
>  if hvp[constants.HV_KVM_EXTRA]:
>kvm_cmd.extend(hvp[constants.HV_KVM_EXTRA].split(" "))
>  
> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index a29fd7972..433b1f3b1 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -526,6 +526,13 @@ viridian
>  viridian (Hyper-V) for this instance. The default is false,
>  disabling viridian support.
>  
> +use\_guest\_agent
> +Valid for the KVM hypervisor.
> +
> +A boolean option that specifies if the hypervisor should enable
> +the QEMU Guest Agent protocol for this instance. By default, the
> +Guest Agent is disabled.
> +
>  use\_localtime
>  Valid for the Xen HVM and KVM hypervisors.
>  
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 09783d4bf..cf5421946 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1806,6 +1806,9 @@ hvUsbMouse = "usb_mouse"
>  hvUseBootloader :: String
>  hvUseBootloader = "use_bootloader"
>  
> +hvUseGuestAgent :: String
> +hvUseGuestAgent = "use_guest_agent"
> +
>  hvUseLocaltime :: String
>  hvUseLocaltime = "use_localtime"
>  
> @@ -1938,6 +1941,7 @@ hvsParameterTypes = Map.fromList
>, (hvUsbDevices,  VTypeString)
>, (hvUsbMouse,VTypeString)
>, (hvUseBootloader,   VTypeBool)
> +  , (hvUseGuestAgent,   VTypeBool)
>

Re: [PATCH-master v2] kvm: use_guest_agent: QEMU Guest Agent support

['Brian Foley' via ganeti-devel]

On Thu, Jan 05, 2017 at 04:45:59PM -0800, Robin H. Johnson wrote:
> Implement the QEMU Guest Agent sockets, so that code/scripts on the
> hypervisors can communicate with guest operating systems easily.
> 
> Signed-off-by: Robin H. Johnson 
> ---
>  lib/hypervisor/hv_kvm/__init__.py | 23 +++
>  man/gnt-instance.rst  |  7 +++
>  src/Ganeti/Constants.hs   |  5 +
>  3 files changed, 35 insertions(+)
> 
> This version applies cleanly to 2.17 and 2.16.

Thanks for picking this up again! LGTM, so I'll push it to 2.16 shortly with a
trivial line-wrapping fix for a too-long comment, and it'll get merged forward
to 2.17 and master soon.

By the way, 2.15 and later should now lint cleanly with pylint 1.6.4.

Brian.
> diff --git a/lib/hypervisor/hv_kvm/__init__.py 
> b/lib/hypervisor/hv_kvm/__init__.py
> index 15f75ef48..c1b63140f 100644
> --- a/lib/hypervisor/hv_kvm/__init__.py
> +++ b/lib/hypervisor/hv_kvm/__init__.py
> @@ -497,6 +497,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  constants.HV_MIGRATION_BANDWIDTH: hv_base.REQ_NONNEGATIVE_INT_CHECK,
>  constants.HV_MIGRATION_DOWNTIME: hv_base.REQ_NONNEGATIVE_INT_CHECK,
>  constants.HV_MIGRATION_MODE: hv_base.MIGRATION_MODE_CHECK,
> +constants.HV_USE_GUEST_AGENT: hv_base.NO_CHECK,
>  constants.HV_USE_LOCALTIME: hv_base.NO_CHECK,
>  constants.HV_DISK_CACHE:
>hv_base.ParamInSet(True, constants.HT_VALID_CACHE_TYPES),
> @@ -751,6 +752,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  """
>  return utils.PathJoin(cls._CTRL_DIR, "%s.qmp" % instance_name)
>  
> +  @classmethod
> +  def _InstanceQemuGuestAgentMonitor(cls, instance_name):
> +"""Returns the instance serial QEMU Guest Agent socket name
> +
> +"""
> +return utils.PathJoin(cls._CTRL_DIR, "%s.qga" % instance_name)
> +
>@classmethod
>def _InstanceKvmdMonitor(cls, instance_name):
>  """Returns the instance kvm daemon socket name
> @@ -837,6 +845,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  utils.RemoveFile(cls._InstanceMonitor(instance_name))
>  utils.RemoveFile(cls._InstanceSerial(instance_name))
>  utils.RemoveFile(cls._InstanceQmpMonitor(instance_name))
> +utils.RemoveFile(cls._InstanceQemuGuestAgentMonitor(instance_name))
>  utils.RemoveFile(cls._InstanceKVMRuntime(instance_name))
>  utils.RemoveFile(cls._InstanceKeymapFile(instance_name))
>  uid_file = cls._InstanceUidFile(instance_name)
> @@ -1554,6 +1563,20 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>  if self._UUID_RE.search(kvmhelp):
>kvm_cmd.extend(["-uuid", instance.uuid])
>  
> +# Add guest agent socket
> +if hvp[constants.HV_USE_GUEST_AGENT]:
> +  qga_addr = utils.GetFreeSlot(bus_slots[_PCI_BUS], reserve=True)
> +  qga_pci_info = "bus=%s,addr=%s" % (_PCI_BUS, hex(qga_addr))
> +  qga_path = self._InstanceQemuGuestAgentMonitor(instance.name)
> +  logging.info("KVM: Guest Agent available at %s", qga_path)
> +  # The 'qga0' identified can change, but the 'org.qemu.guest_agent.0' 
> string is
> +  # the default expected by the Guest Agent.
> +  kvm_cmd.extend([
> +"-chardev", "socket,path=%s,server,nowait,id=qga0" % qga_path,
> +"-device", "virtio-serial,id=qga0,%s" % qga_pci_info,
> +"-device", "virtserialport,chardev=qga0,name=org.qemu.guest_agent.0",
> +])
> +
>  if hvp[constants.HV_KVM_EXTRA]:
>kvm_cmd.extend(hvp[constants.HV_KVM_EXTRA].split(" "))
>  
> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index 20d567e1a..14f7816f3 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -545,6 +545,13 @@ viridian
>  viridian (Hyper-V) for this instance. The default is false,
>  disabling viridian support.
>  
> +use\_guest\_agent
> +Valid for the KVM hypervisor.
> +
> +A boolean option that specifies if the hypervisor should enable
> +the QEMU Guest Agent protocol for this instance. By default, the
> +Guest Agent is disabled.
> +
>  use\_localtime
>  Valid for the Xen HVM and KVM hypervisors.
>  
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index bee8af3e1..e66ae2d28 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1844,6 +1844,9 @@ hvUsbMouse = "usb_mouse"
>  hvUseBootloader :: String
>  hvUseBootloader = "use_bootloader"
>  
> +hvUseGuestAgent :: String
> +hvUseGuestAgent = "use_guest_agent"
> +
>  hvUseLocaltime :: String
>  hvUseLocaltime = "use_localtime"
>  
> @@ -1979,6 +1982,7 @@ hvsParameterTypes = Map.fromList
>, (hvUsbDevices,  VTypeString)
>, (hvUsbMouse,VTypeString)
>, (hvUseBootloader,   VTypeBool)
> +  , (hvUseGuestAgent,   VTypeBool)
>, (hvUseLocaltime,VTypeBool)
>, (hvVga, VTypeString)
>, (hvVhostNet,

[PATCH stable-2.15] Add status and "version implemented" fields to design docs

['Brian Foley' via ganeti-devel]

Based on a combination of reading NEWS, Ganeti version design docs, and
looking through the commit logs to see when features were implemented.

Clean up a number of inconsistencies and missed entries while here, and
update a number of (partly) implemented designs as no longer being
drafts.

Signed-off-by: Brian Foley 
---
 doc/design-2.10.rst   |  1 -
 doc/design-2.11.rst   |  2 ++
 doc/design-2.12.rst   |  2 ++
 doc/design-2.14.rst   |  4 
 doc/design-2.6.rst|  1 +
 doc/design-2.7.rst|  1 +
 doc/design-allocation-efficiency.rst  |  4 
 doc/design-autorepair.rst |  4 
 doc/design-bulk-create.rst|  4 
 doc/design-ceph-ganeti-support.rst|  4 
 doc/design-chained-jobs.rst   |  4 
 doc/design-cmdlib-unittests.rst   |  4 
 doc/design-configlock.rst |  4 
 doc/design-cpu-pinning.rst|  4 
 doc/design-cpu-speed.rst  |  4 
 doc/design-daemons.rst|  4 
 doc/design-dedicated-allocation.rst   |  4 
 doc/design-device-uuid-name.rst   |  4 
 doc/design-disk-conversion.rst|  4 
 doc/design-disks.rst  |  4 
 doc/design-draft.rst  | 11 ---
 doc/design-file-based-disks-ownership.rst |  4 
 doc/design-file-based-storage.rst |  4 
 doc/design-glusterfs-ganeti-support.rst   |  4 
 doc/design-hotplug.rst|  4 
 doc/design-hroller.rst|  4 
 doc/design-hsqueeze.rst   |  4 
 doc/design-htools-2.3.rst |  4 
 doc/design-http-server.rst|  3 +++
 doc/design-hugepages-support.rst  |  6 ++
 doc/design-ifdown.rst |  3 +++
 doc/design-impexp2.rst|  5 +
 doc/design-internal-shutdown.rst  |  4 
 doc/design-kvmd.rst   |  4 
 doc/design-linuxha.rst|  4 
 doc/design-location.rst   |  4 
 doc/design-lu-generated-jobs.rst  |  4 
 doc/design-monitoring-agent.rst   |  4 
 doc/design-move-instance-improvements.rst |  4 
 doc/design-multi-reloc.rst|  4 
 doc/design-multi-storage-htools.rst   |  3 +++
 doc/design-multi-version-tests.rst|  4 
 doc/design-network.rst|  4 
 doc/design-network2.rst   |  3 +++
 doc/design-node-add.rst   |  4 
 doc/design-node-security.rst  |  4 
 doc/design-oob.rst|  4 
 doc/design-openvswitch.rst|  4 
 doc/design-opportunistic-locking.rst  |  4 
 doc/design-optables.rst   |  4 
 doc/design-os.rst |  4 
 doc/design-ovf-support.rst|  4 
 doc/design-partitioned.rst|  4 
 doc/design-performance-tests.rst  |  4 
 doc/design-query-splitting.rst|  3 +++
 doc/design-query2.rst |  4 
 doc/design-reason-trail.rst   |  4 
 doc/design-reservations.rst   |  4 
 doc/design-resource-model.rst |  3 +++
 doc/design-restricted-commands.rst|  4 
 doc/design-shared-storage-redundancy.rst  |  4 
 doc/design-shared-storage.rst |  4 
 doc/design-ssh-ports.rst  |  4 
 doc/design-storagetypes.rst   |  4 
 doc/design-sync-rate-throttling.rst   |  3 +++
 doc/design-systemd.rst|  4 
 doc/design-upgrade.rst|  4 
 doc/design-virtual-clusters.rst   |  3 +++
 doc/design-x509-ca.rst|  3 +++
 doc/index.rst |  6 ++
 70 files changed, 258 insertions(+), 12 deletions(-)

diff --git a/doc/design-2.10.rst b/doc/design-2.10.rst
index 11457ad90..efe3dde80 100644
--- a/doc/design-2.10.rst
+++ b/doc/design-2.10.rst
@@ -15,4 +15,3 @@ The following designs have been partially implemented in 
Ganeti 2.10.
 
 - :doc:`design-ceph-ganeti-support`
 - :doc:`design-internal-shutdown`
-- :doc:`design-query-splitting`
diff --git a/doc/design-2.11.rst b/doc/design-2.11.rst
index 2b2ca22d5..dd7b196f9 100644
--- a/doc/design-2.11.rst
+++ b/doc/design-2.11.rst
@@ -6,6 +6,8 @@ The following design documents have been implemented in Ganeti 
2.11.
 
 - :doc:`design-internal-shutdown`
 - :doc:`design-kvmd`
+- :doc:`design-glusterfs-ganeti-support`
+- :doc:`design-multi-version-tests`
 
 The following designs have been partially implemented in Ganeti 2.11.
 
diff --git a/doc/design-2.12.rst b/doc/design-2.12.rst
index cf18c528e..627e3b48f 100644
--- a/doc/design-2.12.rst
+++ 

Re: [PATCH master] Fix incorrect 'any' function call

['Brian Foley' via ganeti-devel]

On Wed, Jan 04, 2017 at 03:11:41PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> As introduced by commit 77807d83 and d388e30c6d
> and was not caught during the code review process.

LGTM. Thanks.

Re: [PATCH master] Fix incorrect 'any' function call

['Brian Foley' via ganeti-devel]

On Wed, Jan 04, 2017 at 02:44:40PM +, Federico Pareschi wrote:
> It happens, it escaped my review as well so it's not just your fault.
> I assume this is an LGTM? :)

An LGTM if you fix the same mistake in the other commit too :)

Thanks,
Brian.

> On 4 January 2017 at 14:38, Brian Foley  wrote:
> > On Wed, Jan 04, 2017 at 02:11:15PM +, 'Federico Morg Pareschi' via 
> > ganeti-devel wrote:
> >> As introduced by commit 77807d831976cf018ace04961820987a91c1d58a
> >> and was not caught during the code review process.
> >>
> >> Signed-off-by: Federico Morg Pareschi 
> >> ---
> >>  lib/cmdlib/instance_set_params.py | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/lib/cmdlib/instance_set_params.py 
> >> b/lib/cmdlib/instance_set_params.py
> >> index d443c9cdb..228f8aa3c 100644
> >> --- a/lib/cmdlib/instance_set_params.py
> >> +++ b/lib/cmdlib/instance_set_params.py
> >> @@ -994,9 +994,9 @@ class LUInstanceSetParams(LogicalUnit):
> >> else self.instance.os)
> >>
> >>  if compat.any(
> >> -self.op.osparams, self.op.osparams_private,
> >> -self.op.clear_osparams, self.op.clear_osparams_private,
> >> -self.op.remove_osparams, self.op.remove_osparams_private):
> >> +[self.op.osparams, self.op.osparams_private,
> >> + self.op.clear_osparams, self.op.clear_osparams_private,
> >> + self.op.remove_osparams, self.op.remove_osparams_private]):
> >>public_parms = self.op.osparams or {}
> >>private_parms = self.op.osparams_private or {}
> >>remove_osparams = self.op.remove_osparams or []
> >
> > D'oh! That's embarrassing. I made this mistake more than once, too. See also
> > d388e30c6d

Re: [PATCH master] Fix incorrect 'any' function call

['Brian Foley' via ganeti-devel]

On Wed, Jan 04, 2017 at 02:11:15PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> As introduced by commit 77807d831976cf018ace04961820987a91c1d58a
> and was not caught during the code review process.
> 
> Signed-off-by: Federico Morg Pareschi 
> ---
>  lib/cmdlib/instance_set_params.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/cmdlib/instance_set_params.py 
> b/lib/cmdlib/instance_set_params.py
> index d443c9cdb..228f8aa3c 100644
> --- a/lib/cmdlib/instance_set_params.py
> +++ b/lib/cmdlib/instance_set_params.py
> @@ -994,9 +994,9 @@ class LUInstanceSetParams(LogicalUnit):
> else self.instance.os)
>  
>  if compat.any(
> -self.op.osparams, self.op.osparams_private,
> -self.op.clear_osparams, self.op.clear_osparams_private,
> -self.op.remove_osparams, self.op.remove_osparams_private):
> +[self.op.osparams, self.op.osparams_private,
> + self.op.clear_osparams, self.op.clear_osparams_private,
> + self.op.remove_osparams, self.op.remove_osparams_private]):
>public_parms = self.op.osparams or {}
>private_parms = self.op.osparams_private or {}
>remove_osparams = self.op.remove_osparams or []

D'oh! That's embarrassing. I made this mistake more than once, too. See also
d388e30c6d

Thanks,
Brian.

Re: [PATCH stable-2.15] Fix coexistence of location tags and non-DRBD instances

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 12:04:07PM +0100, Iustin Pop wrote:
>On 4 December 2016 at 18:44, Brian Foley <[1]bpfo...@google.com> wrote:
> 
>  On Fri, Dec 02, 2016 at 11:03:55PM +0100, Iustin Pop wrote:
>  > From: Iustin Pop <[2]ius...@google.com>
>  >
>  > This addresses issue 1185, “hbal: IntMap.!: key -1 is not an
>  element of
>  > the map”. The issue is that the location tags code presumed all
>  > instances have a primary and a secondary (i.e., they are DRBD).
>  >
>  > The fix is to set the location score for non-DRBD instances to
>  zero,
>  > since this is what I understand the correct value: such an
>  instance
>  > cannot be optimized by moving, so its score is "perfect" (zero).
>  >
>  > Signed-off-by: Iustin Pop <[3]ius...@google.com>
>  LGTM, and thank you. I'm going to do a merge forward of the recent
>  (and a
>  batch of upcoming) 2.15 patches in the next few days, and this is a
>  nice fix
>  to include.
> 
>Note:  someone with edit rights will need to update the issue once this
>is committed (or released). Thanks for the review!
>iustin

(Belatedly) committed, and issue updated.

Thanks,
Brian.

Re: [PATCH stable-2.15] Fix backup export in case of ext disk template

['Brian Foley' via ganeti-devel]

On Wed, Jan 04, 2017 at 02:01:54PM +0200, Dimitris Aragiorgis wrote:
> This commit fixes backup export in case of ext disk template
> in two ways:
> 
> - Makes the generated snapshot (if any) inherit the params of the
>   origin disk.
> 
> - Partially reverts commit 712ea2f that did export the origin disk
>   instead of the snapshot one.
> 
> Fixes #1198.
> 
> Signed-off-by: Dimitris Aragiorgis 
> ---
>  lib/backend.py  | 16 
>  lib/masterd/instance.py |  1 +
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/backend.py b/lib/backend.py
> index b262de7..79455ca 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -5151,14 +5151,14 @@ def _GetImportExportIoCommand(instance, mode, ieio, 
> ieargs):
>script = inst_os.import_script
>  
>  elif mode == constants.IEM_EXPORT:
> -  disk_path_var = "DISK_%d_PATH" % disk_index
> -  if disk_path_var in env:
> -env["EXPORT_DEVICE"] = env[disk_path_var]
> -env["EXPORT_DISK_PATH"] = env[disk_path_var]
> -
> -  disk_uri_var = "DISK_%d_URI" % disk_index
> -  if disk_uri_var in env:
> -env["EXPORT_DISK_URI"] = env[disk_uri_var]
> +  real_disk = _OpenRealBD(disk)
> +  if real_disk.dev_path:
> +env["EXPORT_DEVICE"] = real_disk.dev_path
> +env["EXPORT_DISK_PATH"] = real_disk.dev_path
> +
> +  uri = _CalculateDeviceURI(instance, disk, real_disk)
> +  if uri:
> +env["EXPORT_DISK_URI"] = uri
>  
>env["EXPORT_INDEX"] = str(disk_index)
>script = inst_os.export_script
> diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
> index 9abbc69..084c332 100644
> --- a/lib/masterd/instance.py
> +++ b/lib/masterd/instance.py
> @@ -1220,6 +1220,7 @@ class ExportInstanceHelper(object):
>  else:
>dev_type = constants.DT_PLAIN
>  disk_params = constants.DISK_LD_DEFAULTS[dev_type].copy()
> +disk_params.update(disk.params)
>  new_dev = objects.Disk(dev_type=dev_type, size=disk.size,
> logical_id=disk_id, iv_name=disk.iv_name,
> params=disk_params)
> -- 
> 2.1.4
> 

LGTM! Thanks to you and Darius for debugging and fixing this. Will submit.

Brian.

Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

['Brian Foley' via ganeti-devel]

On Tue, Jan 03, 2017 at 09:35:20AM +, Federico Pareschi wrote:
> Interesting, you raise valid points for a patch that I thought was
> trivial but indeed is not.
> 
> > Per our discussion yesterday, all the gnt-group commands actually do work 
> > with
> > either group specified by name or by UUID, so this is an improvement to the
> > docs. However, the command line help in lib/client/gnt_group.py also uses 
> > the
> > string "group-name". Could you update these too please?
> 
> 
> Yes, good point.
> 
> > Similarly, could you make sure you update all the other lib/client/*.py CLI 
> > arg
> > strings to match the changes you've made?
> 
> 
> Most of the other commands, surprisingly, were already correct in
> their .py files,
> however I have fixed the few that were not.

Great. Thanks.

> > Interestingly, gnt-instance shutdown/startup work correctly with either 
> > UUIDs,
> > or prefix matches on instance FQDN. However, strangely, some of the 
> > subcommands
> > like gnt-instance info will do prefix match on instance FQDN, but don't work
> > with UUID, so I think we can't just bindly change these without checking 
> > what
> > each subcommand actually uses.
> 
> 
> This looks like a symptom of something broken/inconsistent that should
> get fixed.
> First of all, the changes I had made were already in the .py helper
> strings, interestingly enough. i.e. gnt-instance reboot --help already
> shows  and not  so I think that's a good change as is
> (and indeed it does work with UUIDs as well).
> 
> The real problem, though, is that as you said some of these commands
> are just inconsistent. For example all the instance tagging commands
> do not work with UUIDs, however the group tagging does work with UUIDs
> and they all use the same base code. This leads me to believe there is
> some inconsistency in the backend code for tagging objects through
> UUIDs vs the object name and it should get fixed, but it's probably
> low priority and outside of the scope of this patch.
> 
> Now the question is, do we just avoid touching all the tags-related
> info strings and fix the rest of the man pages as proposed in this
> patch, or do we change the tags-string for group (but not for
> instance/node objects) leaving them inconsistent in the documentation
> (but consistent in the way the code behaves)?

Good question. Before I went on holidays I tried to figure out where exactly
the arg parsing/'name resolution' was being done, and as you say it seems
inconsistent.

My inclination would be to say that it's better for the docs to be accurate and
expose the inconsistency in the implementation, than to have nice 'regular'
docs that aren't accurate.

If we intend to fix the inconsistent implementation in the near future, I'd say
leave the docs alone, and just fix the implementation; but if we don't want to
go down the rabbit-hole of fixing the impl, then just update the docs.

[snip]

> > gnt-node list-drbd, list-tags definitely looks up by prefix match on FQDN or
> > UUID. Is this definitely true for gnt-node add, and the others though?
> 
> I tried one example, gnt-node remove  and indeed it does not
> work with UUIDs. This really bothers me as leaving it as it is, with
> node_name, makes it inconsistent with a lot of the rest of the code
> where UUIDs do work. Again, we should be fixing this but it's probably
> very low priority compared to other issues.

Agreed. It's pretty low priority, and I don't recall anyone else complaining
about it.

> I'll revert the changes in those flags that do not use UUIDs. I'll
> send a new patch after we've decided what we should do with the
> tags-related code as I mentioned earlier.

Thanks,
Brian.

Re: [PATCH stable-2.15] Fix typo in cli_opts.py IGNORE_HVVERSIONS_OPT flag

['Brian Foley' via ganeti-devel]

On Tue, Jan 03, 2017 at 08:46:06AM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> Signed-off-by: Federico Morg Pareschi 
> ---
>  lib/cli_opts.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/cli_opts.py b/lib/cli_opts.py
> index 0b1bb0c..75379aa 100644
> --- a/lib/cli_opts.py
> +++ b/lib/cli_opts.py
> @@ -852,7 +852,7 @@ IGNORE_CONSIST_OPT = cli_option("--ignore-consistency",
>  IGNORE_HVVERSIONS_OPT = cli_option("--ignore-hvversions",
> dest="ignore_hvversions",
> action="store_true", default=False,
> -   help="Ignore imcompatible hypervisor"
> +   help="Ignore incompatible hypervisor"
> " versions between source and target")

LGTM thanks. Nothing like a little "imcompatibility" to ruin your day... :)

[PATCH stable-2.15] Fix miscellaneous typos found when reading design docs

['Brian Foley' via ganeti-devel]

Some of these are also in code comments, but don't touch any string
literals or error messages, so there should be no functional change.

Signed-off-by: Brian Foley 
---
 NEWS |  8 
 configure.ac |  2 +-
 doc/cluster-keys-replacement.rst |  2 +-
 doc/design-allocation-efficiency.rst |  2 +-
 doc/design-autorepair.rst|  8 
 doc/design-bulk-create.rst   |  2 +-
 doc/design-chained-jobs.rst  |  4 ++--
 doc/design-configlock.rst| 12 ++--
 doc/design-cpu-pinning.rst   |  1 +
 doc/design-cpu-speed.rst |  2 +-
 doc/design-daemons.rst   |  6 +++---
 doc/design-dedicated-allocation.rst  |  2 +-
 doc/design-file-based-storage.rst|  4 ++--
 doc/design-hugepages-support.rst |  4 ++--
 doc/design-impexp2.rst   |  4 ++--
 doc/design-linuxha.rst   |  2 +-
 doc/design-location.rst  |  2 +-
 doc/design-monitoring-agent.rst  | 14 +++---
 doc/design-multi-reloc.rst   |  6 +++---
 doc/design-network.rst   |  6 +++---
 doc/design-node-add.rst  |  1 +
 doc/design-node-security.rst | 24 
 doc/design-oob.rst   |  1 +
 doc/design-opportunistic-locking.rst |  1 +
 doc/design-optables.rst  |  2 +-
 doc/design-ovf-support.rst   |  2 +-
 doc/design-reservations.rst  |  6 +++---
 doc/design-resource-model.rst|  8 
 doc/design-restricted-commands.rst   |  1 +
 doc/design-shared-storage-redundancy.rst |  8 
 doc/design-shared-storage.rst|  6 +++---
 doc/design-ssh-ports.rst |  2 +-
 doc/design-storagetypes.rst  |  2 +-
 doc/design-sync-rate-throttling.rst  |  1 +
 doc/design-upgrade.rst   | 10 +-
 lib/client/gnt_cluster.py|  2 +-
 lib/client/gnt_job.py|  2 +-
 man/gnt-cluster.rst  |  4 ++--
 man/mon-collector.rst|  4 ++--
 src/Ganeti/Confd/Server.hs   |  4 ++--
 src/Ganeti/THH.hs|  2 +-
 src/Ganeti/WConfd/ConfigState.hs |  2 +-
 42 files changed, 97 insertions(+), 91 deletions(-)

diff --git a/NEWS b/NEWS
index 123bc99..b193c4b 100644
--- a/NEWS
+++ b/NEWS
@@ -1607,7 +1607,7 @@ Version 2.10.6
 
 *(Released Mon, 30 Jun 2014)*
 
-- Make Ganeti tolerant towards differnt openssl library
+- Make Ganeti tolerant towards different openssl library
   version on different nodes (issue 853).
 - Allow hspace to make useful predictions in multi-group
   clusters with one group overfull (isse 861).
@@ -1748,7 +1748,7 @@ New features
   specified if the destination cluster has a default iallocator.
 - Users can now change the soundhw and cpuid settings for XEN hypervisors.
 - Hail and hbal now have the (optional) capability of accessing average CPU
-  load information through the monitoring deamon, and to use it to dynamically
+  load information through the monitoring daemon, and to use it to dynamically
   adapt the allocation of instances.
 - Hotplug support. Introduce new option '--hotplug' to ``gnt-instance modify``
   so that disk and NIC modifications take effect without the need of actual
@@ -2110,7 +2110,7 @@ Incompatible/important changes
   '--file-storage-dir' and '--shared-file-storage-dir'.
 - Cluster verification now includes stricter checks regarding the
   default file and shared file storage directories. It now checks that
-  the directories are explicitely allowed in the 'file-storage-paths' file and
+  the directories are explicitly allowed in the 'file-storage-paths' file and
   that the directories exist on all nodes.
 - The list of allowed disk templates in the instance policy and the list
   of cluster-wide enabled disk templates is now checked for consistency
@@ -4283,7 +4283,7 @@ fixes/small improvements/cleanups.
 Significant features
 
 
-The node deamon now tries to mlock itself into memory, unless the
+The node daemon now tries to mlock itself into memory, unless the
 ``--no-mlock`` flag is passed. It also doesn't fail if it can't write
 its logs, and falls back to console logging. This allows emergency
 features such as ``gnt-node powercycle`` to work even in the event of a
diff --git a/configure.ac b/configure.ac
index 9b5d06f..41d1856 100644
--- a/configure.ac
+++ b/configure.ac
@@ -638,7 +638,7 @@ AM_CONDITIONAL([GHC_LE_76], [$GHC --numeric-version | grep 
-q '^7\.[[0-6]]\.'])
 
 AC_MSG_CHECKING([checking for extra GHC flags])
 GHC_BYVERSION_FLAGS=
-# check for GHC supported flags that vary accross versions
+# check for GHC supported flags that vary across versions
 for flag in -fwarn-incomplete-uni-patterns; do
   if $GHC -e '0' $flag 

Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

['Brian Foley' via ganeti-devel]

On Tue, Dec 20, 2016 at 03:12:50PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This patch adds some more consistency in the parameter names
> displayed in various ganeti man pages.
> 
> Signed-off-by: Federico Morg Pareschi 
> ---
>  man/gnt-group.rst|  8 
>  man/gnt-instance.rst | 16 
>  man/gnt-job.rst  |  6 +++---
>  man/gnt-network.rst  |  6 +++---
>  man/gnt-node.rst | 14 +++---
>  man/gnt-os.rst   |  5 +++--
>  6 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/man/gnt-group.rst b/man/gnt-group.rst
> index 1c313b2..afe01e5 100644
> --- a/man/gnt-group.rst
> +++ b/man/gnt-group.rst
> @@ -188,7 +188,7 @@ EVACUATE
>  
>  
>  | **evacuate** [\--submit] [\--print-jobid] [\--sequential] 
> [\--force-failover]
> -| [\--iallocator *NAME*] [\--to *GROUP*...] {*group*}
> +| [\--iallocator *name*] [\--to *GROUP*...] {*group*}
>  
>  This command will move all instances out of the given node group.
>  Instances are placed in a new group by an iallocator, either given on
> @@ -219,7 +219,7 @@ Tags
>  ADD-TAGS
>  
>  
> -**add-tags** [\--from *file*] {*groupname*} {*tag*...}
> +**add-tags** [\--from *file*] {*group*} {*tag*...}
>  
>  Add tags to the given node group. If any of the tags contains invalid
>  characters, the entire operation will abort.
> @@ -233,14 +233,14 @@ stdin.
>  LIST-TAGS
>  ^
>  
> -**list-tags** {*groupname*}
> +**list-tags** {*group*}
>  
>  List the tags of the given node group.
>  
>  REMOVE-TAGS
>  ^^^
>  
> -**remove-tags** [\--from *file*] {*groupname*} {*tag*...}
> +**remove-tags** [\--from *file*] {*group*} {*tag*...}
>  
>  Remove tags from the given node group. If any of the tags are not
>  existing on the node, the entire operation will abort.

Per our discussion yesterday, all the gnt-group commands actually do work with
either group specified by name or by UUID, so this is an improvement to the
docs. However, the command line help in lib/client/gnt_group.py also uses the
string "group-name". Could you update these too please?

Similarly, could you make sure you update all the other lib/client/*.py CLI arg
strings to match the changes you've made?

> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index a29fd79..94db520 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -1523,7 +1523,7 @@ RENAME
>  ^^

[snip]

Interestingly, gnt-instance shutdown/startup work correctly with either UUIDs,
or prefix matches on instance FQDN. However, strangely, some of the subcommands
like gnt-instance info will do prefix match on instance FQDN, but don't work
with UUID, so I think we can't just bindly change these without checking what
each subcommand actually uses.


[snip]

> -**wait** {id}
> +**wait** {*job-id*}
>  
>  Wait for the job by the given *id* to finish; do not produce

Change id to job-id here too.

[snip]

> -**watch** {id}
> +**watch** {(*job-id*}
>  
>  This command follows the output of the job by the given *id* and
>  prints it.

Change id to job-id here too.

[snip]

gnt-node list-drbd, list-tags definitely looks up by prefix match on FQDN or
UUID. Is this definitely true for gnt-node add, and the others though?

> diff --git a/man/gnt-node.rst b/man/gnt-node.rst
> index 0940d7f..94f762a 100644
> --- a/man/gnt-node.rst
> +++ b/man/gnt-node.rst
> @@ -30,7 +30,7 @@ ADD
>  | [\--disk-state *diskstate*]
>  | [\--hypervisor-state *hvstate*]
>  | [\--no-node-setup]
> -| {*nodename*}
> +| {*node*}
>  ADD-TAGS
>  

Thanks,
Brian.

Re: [PATCH master] FIX: Add missing LockDecls files to Makefile.am

['Brian Foley' via ganeti-devel]

On Mon, Dec 19, 2016 at 04:10:08PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> Trivial change to add the missing LockDecls.hs files to Makefile.am.
> This should remove some complaints to some of the tests that failed to
> build.
> 
> Signed-off-by: Federico Morg Pareschi 

Hmm. As we discussed off list, we maybe we should unbitrot distcheck and
commit-check, but hey, this is a good start. LGTM.

> ---
>  Makefile.am | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e18ee7e..dc8b63b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -997,6 +997,7 @@ HS_LIB_SRCS = \
>   src/Ganeti/JQScheduler/Types.hs \
>   src/Ganeti/JQueue.hs \
>   src/Ganeti/JQueue/Lens.hs \
> + src/Ganeti/JQueue/LockDecls.hs \
>   src/Ganeti/JQueue/Objects.hs \
>   src/Ganeti/JSON.hs \
>   src/Ganeti/Jobs.hs \
> @@ -1150,6 +1151,7 @@ HS_TEST_SRCS = \
>   test/hs/Test/Ganeti/Jobs.hs \
>   test/hs/Test/Ganeti/JQScheduler.hs \
>   test/hs/Test/Ganeti/JQueue.hs \
> + test/hs/Test/Ganeti/JQueue/LockDecls.hs \
>   test/hs/Test/Ganeti/JQueue/Objects.hs \
>   test/hs/Test/Ganeti/Kvmd.hs \
>   test/hs/Test/Ganeti/Luxi.hs \
> -- 
> 2.8.0.rc3.226.g39d4020
> 

[PATCH master] Quell remaining pylint 1.6.4 warnings after merge

['Brian Foley' via ganeti-devel]

These should all be harmless modifications with no functional change.

Signed-off-by: Brian Foley 
---
 lib/backend.py| 5 ++---
 lib/cmdlib/instance_set_params.py | 7 ---
 lib/hypervisor/hv_kvm/__init__.py | 3 +++
 lib/rapi/testutils.py | 2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 9a4d205..58f4ebd 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1683,8 +1683,8 @@ def AddNodeSshKeyBulk(node_list,
 
 
 # TODO: will be fixed with pending patch series.
-# pylint: disable=R0913
-def RemoveNodeSshKey(node_uuid, node_name,
+def RemoveNodeSshKey(node_uuid, # pylint: disable=R0913
+ node_name,
  master_candidate_uuids,
  potential_master_candidates,
  master_uuid=None,
@@ -2002,7 +2002,6 @@ def RemoveNodeSshKeyBulk(node_list,
   ssh.RemovePublicKey(node_uuid, key_file=pub_key_file)
 
   return result_msgs
-# pylint: enable=R0913
 
 
 def RemoveSshKeyFromPublicKeyFile(node_name,
diff --git a/lib/cmdlib/instance_set_params.py 
b/lib/cmdlib/instance_set_params.py
index fdcd877..d443c9c 100644
--- a/lib/cmdlib/instance_set_params.py
+++ b/lib/cmdlib/instance_set_params.py
@@ -993,9 +993,10 @@ class LUInstanceSetParams(LogicalUnit):
if self.op.os_name and not self.op.force
else self.instance.os)
 
-if (self.op.osparams or self.op.osparams_private or
-self.op.clear_osparams or self.op.clear_osparams_private or
-self.op.remove_osparams or self.op.remove_osparams_private):
+if compat.any(
+self.op.osparams, self.op.osparams_private,
+self.op.clear_osparams, self.op.clear_osparams_private,
+self.op.remove_osparams, self.op.remove_osparams_private):
   public_parms = self.op.osparams or {}
   private_parms = self.op.osparams_private or {}
   remove_osparams = self.op.remove_osparams or []
diff --git a/lib/hypervisor/hv_kvm/__init__.py 
b/lib/hypervisor/hv_kvm/__init__.py
index 2e57080..15f75ef 100644
--- a/lib/hypervisor/hv_kvm/__init__.py
+++ b/lib/hypervisor/hv_kvm/__init__.py
@@ -907,6 +907,9 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   target_process.set_cpu_affinity(range(psutil.cpu_count()))
 else:
   target_process.set_cpu_affinity(cpus)
+  # psutil 2.x deprecated (but still supports) get_children and the wrapper
+  # method sig doesn't show the keyword args, so quell the pylint warning.
+  # pylint: disable=E1123
   for p in target_process.get_children(recursive=True):
 p.set_cpu_affinity(cpus)
 
diff --git a/lib/rapi/testutils.py b/lib/rapi/testutils.py
index 0020c86..6b5c29d 100644
--- a/lib/rapi/testutils.py
+++ b/lib/rapi/testutils.py
@@ -362,7 +362,7 @@ class InputTestClient(object):
 password = utils.GenerateSecret()
 
 # pylint: disable=W0232
-class SimpleAuthenticator():
+class SimpleAuthenticator(object):
   # pylint: disable=R0201
   def ValidateRequest(self, req, _handler_access, _realm):
 """Called to verify user credentials given in HTTP request.
-- 
2.8.0.rc3.226.g39d4020

[MERGE] stable-2.17 to master

['Brian Foley' via ganeti-devel]

commit e18785cdd56cf614fe43b603db3a2a51c8c9c2aa
Merge: e763a23 7f2183e
Author: Brian Foley 
Date:   Mon Dec 19 11:31:06 2016 +

Merge branch 'stable-2.17'

* stable-2.17
  1143: Add gnt-cluster modify --modify-ssh-setup option

* stable-2.15
  Fix gnt-instance console instance unpausing for xl toolstack
  Disable pylint too-many-nested-blocks in _RunCmdPipe
  Reduce nesting in import-export ProcessChildIO
  Reduce nesting in LUOobCommand.Exec
  Reduce nesting in LUInstanceCreate.RunOsScripts
  Reduce nesting in RemoveNodeSshKeyBulk key calculation
  Reduce nesting in RemoveNodeSshKeyBulk ssh logic
  Reduce nesting in gnt-cluster VerifyDisks missing disk loop
  Reduce nesting in _CheckVLANArguments
  Reduce nesting in StartDaemon
  Disable pylint bad-continuation warning
  Disable pylint superfluous-parens warning
  Disable pylint redefined-variable-type warning
  Disable pylint too-many-branches warnings
  Disable pylint broad-except warnings
  Disable incorrect pylint assigning-non-slot warning
  Quell pylint unbalanced-tuple-unpacking warning
  Cleanup: Use new-style classes everywhere
  Quell pylint socket.timeout warning
  Quell the pylint wrong-import-order warnings
  Quell cell-var-from-loop warning
  Use default value lambda param to avoid cell-var-from-loop
  Quell too-many-boolean-expressions
  Remove pylint tests removed in pylint 2.0
  Quell trailing newline
  Quell bad-whitespace warning
  Quell consider-using-enumerate warning
  Disable pylint unsubscriptable-object warning
  Disable pylint bare-except warning
  Disable unwanted pylint wrong-import-position warnings
  Disable pylint unused-wildcard-import warning
  Disable incorrect pylint not-callable warning
  Disable pylint unpacking-non-sequence warning
  Disable pylint misplaced-comparison-constant warning
  Disable incorect pylint simplify-if-statement warning
  Disable pylint eval-used warning
  Disable pylint invalid-name warning
  Disable pylint import-self warning
  Disable some pylint unused-import warnings
  Replace deprecated pylint >=0.27 pragma with new form
  Delete old warning disables removed from pylint 1.6
  Fix pylint >1.4 pycurl no-member warnings
  Cleanup: Remove unused/duplicate module/fn import
  Cleanup: Fix unidiomatic-typecheck
  Cleanup: Remove some unneeded pylint disables
  Cleanup: Iterate dict rather than key list
  Cleanup: Remove unused format key
  Cleanup: StartInstance and RebootInstance return None
  Cleanup: Fix for/else with no break in AddAuthorizedKeys
  Cleanup: Replace map/filters with list comprehensions
  Cleanup: del is a statement not a function
  Cleanup: Use FOO not in BAR instead of not FOO in BAR
  Cleanup: Simplify boolean assignment
  Cleanup: Remove some unnecessary if (...) parens
  Fix invalid variable error for file-based disks
  FIX: Refactor DiagnoseOS to use a loop, not an inner fn
  FIX: Set INSTANCE_NICn_NETWORK_NAME only if net is defined
  StartInstance restores instance state if running
  Allow migrate --cleanup to adopt an instance
  Make migrate --cleanup more robust
  Make finalize_migration_{src,dst} a single op
  Make FinalizeMigration{Src,Dst} more robust
  Fix instance state detection in _Shutdowninstance
  Code cleanup in hypervisor backend
  Fix for incorrect parsing of DRBD versions
  Fix for instance reinstall not updating config
  Change a few errors to report names, not UUIDs
  Give atomicWriteFile temp filenames a more distinct pattern
  LV check failure should print instance name
  Add ganeti-noded and ganeti-rapi --max-clients options
  Disable logging CallRPCMethod timings in non-debug configs
  568 Update hv_kvm to handle output from qemu >= 1.6.0
  Improve cluster verify ssh key errors

Resolve merge conflicts:
  lib/cmdlib/common.py
caused by 81a68a3 use names not uuids in warnings

  lib/rapi/testutils.py
trivial module import tweaks

  lib/server/rapi.py
caused by c1e18c4 --max-clients and pam options

  lib/storage/drbd_info.py
  test/py/ganeti.storage.drbd_unittest.py
caused by 49980c6 DRBD version parsing -- fixed differently (and
less flexibly) on master. Use 2.17's version

  test/hs/Test/Ganeti/OpCodes.hs
caused by e763a23 improve test data generation touching code
code in 2.16, and modify_ssh_setup cluster param

Signed-off-by: Brian Foley 

diff --cc lib/cmdlib/common.py
index 95a1186,b6e673e..08c6e4a
--- a/lib/cmdlib/common.py
+++ b/lib/cmdlib/common.py
@@@ -1600,10 -1595,9 +1601,10 @@@ def EnsureKvmdOnNodes(lu, feedback_fn, 
# Stop KVM where necessary
if stop_nodes:
  

[MERGE] stable-2.16 to stable 2.17

['Brian Foley' via ganeti-devel]

commit ac63745b0ee268fa267b1f29cea0aaa79587c7e6
Merge: f52c6e1 d287130
Author: Brian Foley 
Date:   Fri Dec 16 17:23:46 2016 +

Merge branch 'stable-2.16' into stable-2.17

* stable-2.15
  Fix gnt-instance console instance unpausing for xl toolstack
  Disable pylint too-many-nested-blocks in _RunCmdPipe
  Reduce nesting in import-export ProcessChildIO
  Reduce nesting in LUOobCommand.Exec
  Reduce nesting in LUInstanceCreate.RunOsScripts
  Reduce nesting in RemoveNodeSshKeyBulk key calculation
  Reduce nesting in RemoveNodeSshKeyBulk ssh logic
  Reduce nesting in gnt-cluster VerifyDisks missing disk loop
  Reduce nesting in _CheckVLANArguments
  Reduce nesting in StartDaemon
  Disable pylint bad-continuation warning
  Disable pylint superfluous-parens warning
  Disable pylint redefined-variable-type warning
  Disable pylint too-many-branches warnings
  Disable pylint broad-except warnings
  Disable incorrect pylint assigning-non-slot warning
  Quell pylint unbalanced-tuple-unpacking warning
  Cleanup: Use new-style classes everywhere
  Quell pylint socket.timeout warning
  Quell the pylint wrong-import-order warnings
  Quell cell-var-from-loop warning
  Use default value lambda param to avoid cell-var-from-loop
  Quell too-many-boolean-expressions
  Remove pylint tests removed in pylint 2.0
  Quell trailing newline
  Quell bad-whitespace warning
  Quell consider-using-enumerate warning
  Disable pylint unsubscriptable-object warning
  Disable pylint bare-except warning
  Disable unwanted pylint wrong-import-position warnings
  Disable pylint unused-wildcard-import warning
  Disable incorrect pylint not-callable warning
  Disable pylint unpacking-non-sequence warning
  Disable pylint misplaced-comparison-constant warning
  Disable incorect pylint simplify-if-statement warning
  Disable pylint eval-used warning
  Disable pylint invalid-name warning
  Disable pylint import-self warning
  Disable some pylint unused-import warnings
  Replace deprecated pylint >=0.27 pragma with new form
  Delete old warning disables removed from pylint 1.6
  Fix pylint >1.4 pycurl no-member warnings
  Cleanup: Remove unused/duplicate module/fn import
  Cleanup: Fix unidiomatic-typecheck
  Cleanup: Remove some unneeded pylint disables
  Cleanup: Iterate dict rather than key list
  Cleanup: Remove unused format key
  Cleanup: StartInstance and RebootInstance return None
  Cleanup: Fix for/else with no break in AddAuthorizedKeys
  Cleanup: Replace map/filters with list comprehensions
  Cleanup: del is a statement not a function
  Cleanup: Use FOO not in BAR instead of not FOO in BAR
  Cleanup: Simplify boolean assignment
  Cleanup: Remove some unnecessary if (...) parens
  Fix invalid variable error for file-based disks
  FIX: Refactor DiagnoseOS to use a loop, not an inner fn
  FIX: Set INSTANCE_NICn_NETWORK_NAME only if net is defined
  StartInstance restores instance state if running
  Allow migrate --cleanup to adopt an instance
  Make migrate --cleanup more robust
  Make finalize_migration_{src,dst} a single op
  Make FinalizeMigration{Src,Dst} more robust
  Fix instance state detection in _Shutdowninstance
  Code cleanup in hypervisor backend
  Fix for incorrect parsing of DRBD versions
  Fix for instance reinstall not updating config
  Change a few errors to report names, not UUIDs
  Give atomicWriteFile temp filenames a more distinct pattern
  LV check failure should print instance name
  Add ganeti-noded and ganeti-rapi --max-clients options
  Disable logging CallRPCMethod timings in non-debug configs
  568 Update hv_kvm to handle output from qemu >= 1.6.0
  Improve cluster verify ssh key errors
  Fix inconsistent spaces vs tabs indent in makefile

* stable-2.13
  Bugfix: migrate needs HypervisorClass, not an instance

Fix easy merge conflict in lib/backend.py -- dead code removed in 2.15

Signed-off-by: Brian Foley 

diff --cc lib/backend.py
index 58c8b3a,1d4d2e4..5b83290
--- a/lib/backend.py
+++ b/lib/backend.py
@@@ -1914,17 -1903,21 +1919,21 @@@ def RemoveNodeSshKeyBulk(node_list
  error_msg_final = ("When removing the key of node '%s', updating the"
 " SSH key files of node '%s' failed. Last error"
 " was: %s.")
- if node in potential_master_candidates:
-   logging.debug("Updating key setup of potential master candidate 
node"
- " %s.", node)
+ 
+ if node in potential_master_candidates or from_authorized_keys:
+   if node in potential_master_candidates:
+ node_desc = "potential master candidate"
+   else:
+ 

[MERGE] stable-2.15 to stable-2.16

['Brian Foley' via ganeti-devel]

commit 711fbc08fd895b826d63c1ffc7cb75f35dc4331e
Merge: 703e23e da3f300
Author: Brian Foley 
Date:   Fri Dec 16 16:01:48 2016 +

Merge branch 'stable-2.15' into stable-2.16

Merge forward patches from stable-2.15

* stable-2.15
  Fix gnt-instance console instance unpausing for xl toolstack
  Disable pylint too-many-nested-blocks in _RunCmdPipe
  Reduce nesting in import-export ProcessChildIO
  Reduce nesting in LUOobCommand.Exec
  Reduce nesting in LUInstanceCreate.RunOsScripts
  Reduce nesting in RemoveNodeSshKeyBulk key calculation
  Reduce nesting in RemoveNodeSshKeyBulk ssh logic
  Reduce nesting in gnt-cluster VerifyDisks missing disk loop
  Reduce nesting in _CheckVLANArguments
  Reduce nesting in StartDaemon
  Disable pylint bad-continuation warning
  Disable pylint superfluous-parens warning
  Disable pylint redefined-variable-type warning
  Disable pylint too-many-branches warnings
  Disable pylint broad-except warnings
  Disable incorrect pylint assigning-non-slot warning
  Quell pylint unbalanced-tuple-unpacking warning
  Cleanup: Use new-style classes everywhere
  Quell pylint socket.timeout warning
  Quell the pylint wrong-import-order warnings
  Quell cell-var-from-loop warning
  Use default value lambda param to avoid cell-var-from-loop
  Quell too-many-boolean-expressions
  Remove pylint tests removed in pylint 2.0
  Quell trailing newline
  Quell bad-whitespace warning
  Quell consider-using-enumerate warning
  Disable pylint unsubscriptable-object warning
  Disable pylint bare-except warning
  Disable unwanted pylint wrong-import-position warnings
  Disable pylint unused-wildcard-import warning
  Disable incorrect pylint not-callable warning
  Disable pylint unpacking-non-sequence warning
  Disable pylint misplaced-comparison-constant warning
  Disable incorect pylint simplify-if-statement warning
  Disable pylint eval-used warning
  Disable pylint invalid-name warning
  Disable pylint import-self warning
  Disable some pylint unused-import warnings
  Replace deprecated pylint >=0.27 pragma with new form
  Delete old warning disables removed from pylint 1.6
  Fix pylint >1.4 pycurl no-member warnings
  Cleanup: Remove unused/duplicate module/fn import
  Cleanup: Fix unidiomatic-typecheck
  Cleanup: Remove some unneeded pylint disables
  Cleanup: Iterate dict rather than key list
  Cleanup: Remove unused format key
  Cleanup: StartInstance and RebootInstance return None
  Cleanup: Fix for/else with no break in AddAuthorizedKeys
  Cleanup: Replace map/filters with list comprehensions
  Cleanup: del is a statement not a function
  Cleanup: Use FOO not in BAR instead of not FOO in BAR
  Cleanup: Simplify boolean assignment
  Cleanup: Remove some unnecessary if (...) parens
  Fix invalid variable error for file-based disks
  FIX: Refactor DiagnoseOS to use a loop, not an inner fn
  FIX: Set INSTANCE_NICn_NETWORK_NAME only if net is defined
  Fix for incorrect parsing of DRBD versions
  Change a few errors to report names, not UUIDs
  Give atomicWriteFile temp filenames a more distinct pattern
  LV check failure should print instance name
  Disable logging CallRPCMethod timings in non-debug configs
  568 Update hv_kvm to handle output from qemu >= 1.6.0

Trivial merge conflicts:
  lib/cli.py   -- whitespace
  lib/cmdlib/cluster/verify.py -- code previously removed from 2.16
  lib/hypervisor/hv_xen.py -- tuple vs list
  lib/luxi.py  -- whitespace
  lib/server/masterd.py-- code cleaned up in 2.15 and
  previously removed in 2.16
  lib/storage/filestorage.py   -- whitespace & generator instead of list
  lib/tools/node_cleanup.py-- whitespace

Signed-off-by: Brian Foley 

diff --cc lib/client/gnt_cluster.py
index f834d2b,e23fb50..8b99c2c
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@@ -299,19 -296,8 +301,16 @@@ def InitCluster(opts, args)
  
default_ialloc_params = opts.default_iallocator_params
  
-   if opts.enabled_user_shutdown:
- enabled_user_shutdown = True
-   else:
- enabled_user_shutdown = False
+   enabled_user_shutdown = bool(opts.enabled_user_shutdown)
  
 +  if opts.ssh_key_type:
 +ssh_key_type = opts.ssh_key_type
 +  else:
 +ssh_key_type = constants.SSH_DEFAULT_KEY_TYPE
 +
 +  ssh_key_bits = ssh.DetermineKeyBits(ssh_key_type, opts.ssh_key_bits, None,
 +  None)
 +
bootstrap.InitCluster(cluster_name=args[0],
  secondary_ip=opts.secondary_ip,
  vg_name=vg_name,
diff --cc lib/jqueue/__init__.py
index 9384f55,d996461..d78625c

Re: [PATCH master] Improve the test data generation algorithms

['Brian Foley' via ganeti-devel]

On Fri, Dec 16, 2016 at 03:00:41PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This commit improves some of the test data generation in the ConfigData
> dummy values of quickcheck. It generates more realistic instance, node
> and group names by sampling them from the provided ConfigData. This
> makes it possible to test for consistent data validation that might
> depend on multiple names being present in the cluster configuration at
> the same time.
> 
> Signed-off-by: Federico Morg Pareschi 

LGTM. Feel free to submit. Thanks, and sorry again for the slow review.

Cheers,
Brian.

Re: [PATCH master] Implement starvation-prevention mechanism in queue

['Brian Foley' via ganeti-devel]

On Fri, Dec 16, 2016 at 01:51:41PM +, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This patch adds a starvation prevention mechanism to the Ganeti
> predictive job queue. It calculates the age of submitted jobs
> comparing the current time to the submission time and appropriately
> adjusts the job's lock weight accordingly to avoid starvation.
> 
> Signed-off-by: Federico Morg Pareschi 

LGTM, thanks. Feel free to submit.

Re: [PATCH stable-2.15] Fix gnt-instance console instance unpausing when using the xl toolstack.

['Brian Foley' via ganeti-devel]

On Thu, Dec 15, 2016 at 12:07:22PM -0500, Nicolas Avrutin wrote:
>D'oh. Fixed in the new patch.

[snip]

Thanks. LGTM. I'll submit it in a minute.

Cheers,
Brian.

Re: [PATCH stable-2.15] Fix gnt-instance console instance unpausing when using the xl toolstack.

['Brian Foley' via ganeti-devel]

On Wed, Dec 14, 2016 at 02:21:24PM -0800, 'Nicolas Avrutin' via ganeti-devel 
wrote:
> xen-console-wrapper called the "xm" command instead of using the toolstack
> configured in Ganeti. This patch replaces those calls with calls to the
> toolstack command which is passed as the first argument to the script.
> 
> In addition, whereas "xm list --long" output a sexp, "xl list --long" outputs
> JSON. Unfortunately, the JSON data is missing the "state" field, which we need
> to determine whether the instance is paused. Thus we have to use
> "xm list"/"xl list" and chop off the header line (because there is no flag
> to omit printing the headers).
> 
> Signed-off-by: Nicolas Avrutin 
> ---
>  tools/xen-console-wrapper | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/xen-console-wrapper b/tools/xen-console-wrapper
> index fb6f986..13dd44f 100755
> --- a/tools/xen-console-wrapper
> +++ b/tools/xen-console-wrapper
> @@ -31,17 +31,17 @@ XEN_CMD="$1"
>  INSTANCE="$2"
>  
>  unpause() {
> -  ispaused=$(xm list -l "$INSTANCE" 2>/dev/null |
> - sed -n 's/^[[:blank:]]*(state ..\(.\)...)/\1/p')
> +  ispaused=$(xl list $INSTANCE 2>/dev/null | tail -n1 |
> + awk '{print $5}' | sed -n 's/..\(.\).../\1/p')

Hmm. By hardcoding 'xl' here, aren't we restricting ourselves to only work
when the xl toolstack is selected? Or does the xl list subcommand happen to
work even when toolstack == xm?

Otherwise LGTM.

BTW I thought hv_xen.py had a couple of hardcoded 'xl's in it, but it turns
out those are all related to the use of the migration daemon, which is
xl-specific.

Cheers,
Brian.

Re: [RFH] Building with GHC 8 and snap-server 1.0

['Brian Foley' via ganeti-devel]

On Sat, Dec 10, 2016 at 09:42:12PM +0200, Apollon Oikonomopoulos wrote:
> Hi all,
> 
> I'm trying to sort out the situation with Ganeti 2.15.2 in Debian 
> unstable.  Right now we have GHC 8 and snap-server 1.0 which cause 
> Ganeti's build to fail and Ganeti is due to be removed from testing. I have
> (probably) fixed all GHC 8 compatibility issues (mostly template
> Haskell-related stuff) with the attached patch, however it looks as if
> snap-server 1.0 requires substantial changes in metad's error handling to
> work[1]:

Hi Apollon,

Awesome. Thanks for this. As a side project, I started adding support for
GHC 8.0.1/cabal 1.24 chroot for Jessie a few months ago, but I put it to one
side when other work came up. At the time I was looking at finding a set of
compatible cabal packages for the chroot, and I hadn't gotten around to
fixing GHC 8 compatibility issues, but it looks like we could just take
whatever is in debian testing and go from there.

I'm on holidays at the moment, but will be back later this week and I'm
definitely interested in picking this up if someone else doesn't get to it
first.

Cheers,
Brian.

Re: [PATCH master 2/2] Implement starvation-prevention mechanism in queue

['Brian Foley' via ganeti-devel]

On Thu, Sep 08, 2016 at 04:12:56PM +0100, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This patch adds a starvation prevention mechanism to the Ganeti
> predictive job queue. It calculates the age of submitted jobs
> comparing the current time to the submission time and appropriately
> adjusts the job's lock weight accordingly to avoid starvation.

As you've said in person, this is actually pretty simple. Simple is good. :)

Mostly LGTM, some comments inline...

Cheers,
Brian.
> Signed-off-by: Federico Morg Pareschi 
> ---
>  src/Ganeti/Constants.hs| 11 +++
>  src/Ganeti/JQScheduler.hs  | 21 +++--
>  src/Ganeti/JQueue/LockDecls.hs | 14 +-
>  3 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 1e0b877..bee8af3 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -5634,3 +5634,14 @@ staticLockMaybeBlockWeight = 1.5
>  -- | Weight assigned to two locks that will surely conflict.
>  staticLockSureBlockWeight :: Double
>  staticLockSureBlockWeight = 3
> +
> +-- | How many seconds make up a "tick" in the job queue starvation prevention
> +-- system.
> +jobQueueTickInSeconds :: Double
> +jobQueueTickInSeconds = 30
> +
> +-- | Job queue starvation prevention coefficient. This means how many "ticks"
> +-- of time need to pass before a job has 100% certainty to be in front of the
> +-- queue.
> +jobQueueStarvationCoeff :: Double
> +jobQueueStarvationCoeff = 30

I suspect we might want these two as cluster parameters like max_running_jobs.

At first I thought of exposing these as luxid command line options, but
then you'd have the problem of copying updates to these options in
/etc/defaults/ganeti to all nodes, so having them as cluster params is better.

[snip]
> +-- | Adjust any static weight to its appropriate anti-starvation value. It
> +-- takes the current time and the time since the job was queued as 
> parameters.
> +adjustedWeight :: Int -> JQ.Timestamp -> Double -> Double
> +adjustedWeight currTime (recvTime, _) weight =
> +  let timeVal = max 0 (1 - ticks / C.jobQueueStarvationCoeff)
> +  ticks = timeDiff / C.jobQueueTickInSeconds
> +  timeDiff = fromIntegral $ currTime - recvTime
> +  in weight * timeVal

The type signature of this is quite odd: two timestamps, one integer and the
other JQ.Timestamp. Could you make both be JQ.Timestamp (I know it's just a
sec/usec tuple), and use JQueue.currentTimestamp to get the current time.
JQ.Timestamp seems to be used pretty heavily in the rest of the scheduler code.

Re: [PATCH master] Improve the test data generation algorithms

['Brian Foley' via ganeti-devel]

On Tue, Sep 20, 2016 at 06:01:44PM +0100, 'Federico Morg Pareschi' via 
ganeti-devel wrote:
> This commit improves some of the test data generation in the ConfigData
> dummy values of quickcheck, to have a saner and more robust testing
> infrastructure.

LGTM, but the commit message isn't terribly helpful. AFAICS what you've
done is to generate more realistic instance, node, and group names in
genOpCodeFromId by sampling them randomly from the input ConfigData if
it's provided, and changed everything else to use this.

Cheers,
Brian.

> Signed-off-by: Federico Morg Pareschi 
> ---
>  test/hs/Test/Ganeti/JQueue/LockDecls.hs |  25 ++---
>  test/hs/Test/Ganeti/Objects.hs  |  38 +---
>  test/hs/Test/Ganeti/OpCodes.hs  | 158 
> +---
>  3 files changed, 119 insertions(+), 102 deletions(-)
> 
> diff --git a/test/hs/Test/Ganeti/JQueue/LockDecls.hs 
> b/test/hs/Test/Ganeti/JQueue/LockDecls.hs
> index 6fff7c9..5a595c5 100644
> --- a/test/hs/Test/Ganeti/JQueue/LockDecls.hs
> +++ b/test/hs/Test/Ganeti/JQueue/LockDecls.hs
> @@ -38,9 +38,7 @@ module Test.Ganeti.JQueue.LockDecls (testLockDecls) where
>  
>  import Test.QuickCheck
>  import Test.HUnit
> -import qualified Data.Foldable as F
>  import Data.List
> -import qualified Data.Map as Map
>  import Data.Maybe
>  
>  import Prelude ()
> @@ -52,12 +50,9 @@ import Test.Ganeti.OpCodes (genOpCodeFromId)
>  import Test.Ganeti.TestCommon
>  
>  import qualified Ganeti.Constants as C
> -import Ganeti.Config
>  import Ganeti.JQueue.LockDecls
> -import Ganeti.JSON
>  import Ganeti.OpCodes
>  import Ganeti.Objects
> -import Ganeti.Types
>  
>  
>  prop_staticWeight :: ConfigData -> Maybe OpCode -> [OpCode] -> Property
> @@ -73,11 +68,8 @@ genExclusiveInstanceOp cfg = do
>   , "OP_INSTANCE_REBOOT"
>   , "OP_INSTANCE_RENAME"
>   ]
> -  insts = map instName . Map.elems . fromContainer . configInstances $ 
> cfg
>op_id <- elements list
> -  op <- genOpCodeFromId op_id
> -  name <- elements insts
> -  return $ op { opInstanceName = fromMaybe "" name }
> +  genOpCodeFromId op_id (Just cfg)
>  
>  prop_instNameConflictCheck :: Property
>  prop_instNameConflictCheck = do
> @@ -103,11 +95,8 @@ genExclusiveNodeOp cfg = do
>   , "OP_NODE_MODIFY_STORAGE"
>   , "OP_REPAIR_NODE_STORAGE"
>   ]
> -  nodes = map nodeName . F.toList . configNodes $ cfg
>op_id <- elements list
> -  op <- genOpCodeFromId op_id
> -  name <- elements nodes
> -  return $ op { opNodeName = fromJust $ mkNonEmpty name }
> +  genOpCodeFromId op_id (Just cfg)
>  
>  prop_nodeNameConflictCheck :: Property
>  prop_nodeNameConflictCheck = do
> @@ -130,11 +119,11 @@ prop_nodeNameConflictCheck = do
>  case_queueLockOpOrder :: Assertion
>  case_queueLockOpOrder = do
>cfg <- generate $ genConfigDataWithValues 10 50
> -  diagnoseOp <- generate . genOpCodeFromId $ "OP_OS_DIAGNOSE"
> -  networkAddOp <- generate . genOpCodeFromId $ "OP_NETWORK_ADD"
> -  groupVerifyOp <- generate . genOpCodeFromId $ "OP_GROUP_VERIFY_DISKS"
> -  nodeAddOp <- generate . genOpCodeFromId $ "OP_NODE_ADD"
> -  currentOp <- generate . genExclusiveInstanceOp $ cfg
> +  diagnoseOp <- generate $ genOpCodeFromId "OP_OS_DIAGNOSE" (Just cfg)
> +  networkAddOp <- generate $ genOpCodeFromId "OP_NETWORK_ADD" (Just cfg)
> +  groupVerifyOp <- generate $ genOpCodeFromId "OP_GROUP_VERIFY_DISKS" (Just 
> cfg)
> +  nodeAddOp <- generate $ genOpCodeFromId "OP_NODE_ADD" (Just cfg)
> +  currentOp <- generate $ genExclusiveInstanceOp cfg
>let w1 = staticWeight cfg (Just diagnoseOp) [currentOp]
>w2 = staticWeight cfg (Just networkAddOp) [currentOp]
>w3 = staticWeight cfg (Just groupVerifyOp) [currentOp]
> diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs
> index 6236133..a1a2a0a 100644
> --- a/test/hs/Test/Ganeti/Objects.hs
> +++ b/test/hs/Test/Ganeti/Objects.hs
> @@ -47,6 +47,9 @@ module Test.Ganeti.Objects
>, genInst
>, genInstWithNets
>, genValidNetwork
> +  , genValidGroupName
> +  , genValidNodeName
> +  , genValidInstanceName
>, genBitStringMaxLen
>) where
>  
> @@ -60,6 +63,7 @@ import Control.Monad (liftM, when)
>  import qualified Data.ByteString as BS
>  import qualified Data.ByteString.UTF8 as UTF8
>  import Data.Char
> +import qualified Data.Foldable as F
>  import qualified Data.List as List
>  import qualified Data.Map as Map
>  import Data.Maybe (fromMaybe)
> @@ -531,8 +535,8 @@ genConfigDataWithNetworks old_cfg = do
>  
>  genConfigDataWithValues :: Int -> Int -> Gen ConfigData
>  genConfigDataWithValues nNodes nInsts = do
> -  emptyData <- genEmptyCluster nNodes
> -  insts <- vectorOf nInsts (genInstanceFromConfigData emptyData)
> +  cfg <- genConfigDataWithNetworks =<< genEmptyCluster nNodes
> +  insts <- vectorOf nInsts (genInstanceFromConfigData cfg)
>let getInstName i
>  | RealInstance rinst <- i = UTF8.fromString $ 

Re: [PATCH stable-2.15 16/17] Reduce nesting in import-export ProcessChildIO

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 10:36:49PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:24, Ganeti Development List wrote:
> > This avoids some pylint too-many-nested-blocks warnings. Do this by
> > flattening some 'arrow antipattern code' inside the polling loop to use
> > guards instead.
> 
> Was not sure what arrow antipattern is, but after squinting at the diff
> I'll remember it :)

Woo. I get to point you at Ward's Wiki (the first wiki ever):
http://wiki.c2.com/?ArrowAntiPattern

Don't stay too long, I find it almost as much of a time sink as tvtropes.org
(see also http://tvtropes.org/pmwiki/pmwiki.php/main/tvtropeswillruinyourlife )

Cheers,
Brian.

Re: [PATCH stable-2.15 15/17] Reduce nesting in LUOobCommand.Exec

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 10:34:17PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:22, Ganeti Development List wrote:
> > This avoids a pylint too-many-nested-blocks warning.
> > 
> > NB it's hard to see from the diff because of all the whitespace, but
> > this just turns the if result.fail_msg check into a guard that continues
> > the next iteration of the loop, and dedends the rest of the loop body
> > rather than having it as an else:
> 
> Actually this is almost readable, because of the empty lines that break
> the patch into multiple chunks.

You're better at this than me then! These kinds of diffs confuse the hell out
of me. :)

Cheers,
Brian.

Re: [PATCH stable-2.15 14/17] Reduce nesting in LUInstanceCreate.RunOsScripts

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 10:32:48PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:20, Ganeti Development List wrote:
> > This avoids a pylint too-many-nested-blocks warning.
> > 
> > NB it's hard to see from the diff because of all the whitespace, but
> > this just replaces the top "if iobj.disk and not self.adopt_disks" with
> > a pair of guards that return early, and dedents the rest of the fn.
> 
> The only question is why:
> 
> > +if not iobj.disks:
> > +  return
> > +
> > +if self.adopt_disks:
> > +  return
> 
> Instead of a single if? LGTM either way (didn't validate git -w though).

No particular reason, I just thought that they were distinct cases that I might
want to document separately (and then didn't get around to commenting because I
wasn't 100% sure why the checks were there).

Re: [PATCH stable-2.15 09/17] Reduce nesting in StartDaemon

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 10:16:47PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:11, Ganeti Development List wrote:
> > This avoids a pylint too-many-nested-blocks warning.
> > 
> > The extra try: finally: os._exit(1) is unnecessary as _StartDaemonChild
> > already catches all its exceptions and if it ever finishes, calls
> > os._exit(1) anyways.
> 
> LGTM, although this wording:
> 
> > +# Child process, can't return.
> 
> Seems to me a bit misleanding - I was confused why can't return, as exec
> can fail. I'd suggest "Try to start child process, will either execve or
> exit".
> 
> But nitpicking, LGTM either way.

Entirely misleading. Fixed now.

Thanks,
Brian.

Re: [PATCH stable-2.15 07/17] Disable pylint superfluous-parens warning

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 10:12:09PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:08, Ganeti Development List wrote:
> > There are too many cases where we deliberately wrap expressions in
> > parens, either to indicate comparisons, or to allow multiline
> > expressions without line continuation chars, or to clarify confusing
> > precedence.
> > 
> > While here, clean up a few unpythonic cases.
> 
> Nice cleanup, LGTM but one bug below:
> 
> > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
> > index 3eaf292..b32e62e 100644
> > --- a/lib/client/gnt_cluster.py
> > +++ b/lib/client/gnt_cluster.py
> > @@ -195,7 +195,7 @@ def InitCluster(opts, args):
> ># check the disk template types here, as we cannot rely on the type 
> > check done
> ># by the opcode parameter types
> >diskparams_keys = set(diskparams.keys())
> > -  if not (diskparams_keys <= constants.DISK_TEMPLATES):
> > +  if not diskparams_keys > constants.DISK_TEMPLATES:
> 
> You probably wanted to drop the 'not' but missed it.

D'oh! Indeed I did. Thankyou.

Re: [PATCH stable-2.15 01/17] Cleanup: Use new-style classes everywhere

['Brian Foley' via ganeti-devel]

On Tue, Dec 06, 2016 at 09:34:39PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:29:56, Ganeti Development List wrote:
> > This quells pylint's old-style-class warning. For all classes changed by
> > this commit, none of the differences between new-style and old classes
> > matter: we don't subclass any of these classes, or use super()/type().
> 
> I don't know anymore in Python 2.6/2.7 what the status is, but very old
> Python version had the issue that the internals of the class were
> different, e.g. __slots__ only works for new-style classes. Is that
> still the case, or not?

It's still the case. __slots__ was introduced in 2.2 (along with new-style
classes), and only works with new-style classes. Assigning to __class__
breaks with Python <2.6 for classes with __slots__.

See https://docs.python.org/2/reference/datamodel.html#slots

However the classes in this patch are just used for simple encapsulation, and
there's nothing tricky going on with them -- no subclassing, no metaclass games,
no overriding of any of the __dunder__ methods, so I think we're pretty safe.

Cheers,
Brian.

[PATCH stable-2.15 09/17] Reduce nesting in StartDaemon

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning.

The extra try: finally: os._exit(1) is unnecessary as _StartDaemonChild
already catches all its exceptions and if it ever finishes, calls
os._exit(1) anyways.

Signed-off-by: Brian Foley 
---
 lib/utils/process.py | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/lib/utils/process.py b/lib/utils/process.py
index 9183e9f..bb5aed2 100644
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -360,15 +360,11 @@ def StartDaemon(cmd, env=None, cwd="/", output=None, 
output_fd=None,
   # First fork
   pid = os.fork()
   if pid == 0:
-try:
-  # Child process, won't return
-  _StartDaemonChild(errpipe_read, errpipe_write,
-pidpipe_read, pidpipe_write,
-cmd, cmd_env, cwd,
-output, output_fd, pidfile)
-finally:
-  # Well, maybe child process failed
-  os._exit(1) # pylint: disable=W0212
+# Child process, can't return.
+_StartDaemonChild(errpipe_read, errpipe_write,
+  pidpipe_read, pidpipe_write,
+  cmd, cmd_env, cwd,
+  output, output_fd, pidfile)
 finally:
   utils_wrapper.CloseFdNoError(errpipe_write)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 12/17] Reduce nesting in RemoveNodeSshKeyBulk ssh logic

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning. It also removes
some copy & paste code, showing that the master candidate and ordinary
node case are the same apart from the logging.

No funtional change.

Signed-off-by: Brian Foley 
---
 lib/backend.py | 27 +++
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 634f2e5..37ce31a 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1864,9 +1864,13 @@ def RemoveNodeSshKeyBulk(node_list,
 error_msg_final = ("When removing the key of node '%s', updating the"
" SSH key files of node '%s' failed. Last error"
" was: %s.")
-if node in potential_master_candidates:
-  logging.debug("Updating key setup of potential master candidate node"
-" %s.", node)
+
+if node in potential_master_candidates or from_authorized_keys:
+  if node in potential_master_candidates:
+node_desc = "potential master candidate"
+  else:
+node_desc = "normal"
+  logging.debug("Updating key setup of %s node %s.", node_desc, node)
   try:
 utils.RetryByNumberOfTimes(
 constants.SSHS_MAX_RETRIES,
@@ -1881,23 +1885,6 @@ def RemoveNodeSshKeyBulk(node_list,
 result_msgs.append((node, error_msg))
 logging.error(error_msg)
 
-else:
-  if from_authorized_keys:
-logging.debug("Updating key setup of normal node %s.", node)
-try:
-  utils.RetryByNumberOfTimes(
-  constants.SSHS_MAX_RETRIES,
-  errors.SshUpdateError,
-  run_cmd_fn, cluster_name, node, pathutils.SSH_UPDATE,
-  ssh_port, base_data,
-  debug=False, verbose=False, use_cluster_key=False,
-  ask_key=False, strict_host_check=False)
-except errors.SshUpdateError as last_exception:
-  error_msg = error_msg_final % (
-  node_info.name, node, last_exception)
-  result_msgs.append((node, error_msg))
-  logging.error(error_msg)
-
   for node_info in node_list:
 if node_info.clear_authorized_keys or node_info.from_public_keys or \
 node_info.clear_public_keys:
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 05/17] Disable pylint too-many-branches warnings

['Brian Foley' via ganeti-devel]

This is useful, but in some cases is a little too conservative. A fn
can have a lot of branches, but very little nesting, and can still be
easy to understand. This occurs in, eg, XenPvmHypervisor._GetConfig.

Ganeti has many functions that fail this check, so disable it for now
until we get a chance to clean up the codebase.

Signed-off-by: Brian Foley 
---
 Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 108cbba..ba99fb8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,7 +2686,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir 
$(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
-LINT_DISABLE = I0013
+# R0912 = disable too many branches warning. It's useful, but ganeti requires
+# a lot of refactoring to fix this.
+LINT_DISABLE = I0013 R0912
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 01/17] Cleanup: Use new-style classes everywhere

['Brian Foley' via ganeti-devel]

This quells pylint's old-style-class warning. For all classes changed by
this commit, none of the differences between new-style and old classes
matter: we don't subclass any of these classes, or use super()/type().

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_storage.py | 2 +-
 lib/jqueue/__init__.py | 6 +++---
 lib/rpc/node.py| 6 +++---
 lib/rpc/transport.py   | 4 ++--
 lib/ssh.py | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 0093edc..af1c2e8 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -3014,7 +3014,7 @@ class TLReplaceDisks(Tasklet):
   self._RemoveOldStorage(self.target_node_uuid, iv_names)
 
 
-class TemporaryDisk():
+class TemporaryDisk(object):
   """ Creates a new temporary bootable disk, and makes sure it is destroyed.
 
   Is a context manager, and should be used with the ``with`` statement as such.
diff --git a/lib/jqueue/__init__.py b/lib/jqueue/__init__.py
index 1aec8ae..d996461 100644
--- a/lib/jqueue/__init__.py
+++ b/lib/jqueue/__init__.py
@@ -704,7 +704,7 @@ def _EncodeOpError(err):
   return errors.EncodeException(to_encode)
 
 
-class _TimeoutStrategyWrapper:
+class _TimeoutStrategyWrapper(object):
   def __init__(self, fn):
 """Initializes this class.
 
@@ -736,7 +736,7 @@ class _TimeoutStrategyWrapper:
 return result
 
 
-class _OpExecContext:
+class _OpExecContext(object):
   def __init__(self, op, index, log_prefix, timeout_strategy_factory):
 """Initializes this class.
 
@@ -1279,7 +1279,7 @@ class _JobQueueWorkerPool(workerpool.WorkerPool):
 self.queue = queue
 
 
-class _JobDependencyManager:
+class _JobDependencyManager(object):
   """Keeps track of job dependencies.
 
   """
diff --git a/lib/rpc/node.py b/lib/rpc/node.py
index e65d998..cd2050d 100644
--- a/lib/rpc/node.py
+++ b/lib/rpc/node.py
@@ -305,7 +305,7 @@ def _SsconfResolver(ssconf_ips, node_list, _,
   return result
 
 
-class _StaticResolver:
+class _StaticResolver(object):
   def __init__(self, addresses):
 """Initializes this class.
 
@@ -363,7 +363,7 @@ def _NodeConfigResolver(single_node_fn, all_nodes_fn, 
node_uuids, opts):
 for uuid in node_uuids]
 
 
-class _RpcProcessor:
+class _RpcProcessor(object):
   def __init__(self, resolver, port, lock_monitor_cb=None):
 """Initializes this class.
 
@@ -472,7 +472,7 @@ class _RpcProcessor:
 return self._CombineResults(results, requests, procedure)
 
 
-class _RpcClientBase:
+class _RpcClientBase(object):
   def __init__(self, resolver, encoder_fn, lock_monitor_cb=None,
_req_process_fn=None):
 """Initializes this class.
diff --git a/lib/rpc/transport.py b/lib/rpc/transport.py
index 8271016..ee5516d 100644
--- a/lib/rpc/transport.py
+++ b/lib/rpc/transport.py
@@ -52,7 +52,7 @@ DEF_CTMO = constants.LUXI_DEF_CTMO
 DEF_RWTO = constants.LUXI_DEF_RWTO
 
 
-class Transport:
+class Transport(object):
   """Low-level transport class.
 
   This is used on the client side.
@@ -243,7 +243,7 @@ class Transport:
   self.socket = None
 
 
-class FdTransport:
+class FdTransport(object):
   """Low-level transport class that works on arbitrary file descriptors.
 
   Unlike L{Transport}, this doesn't use timeouts.
diff --git a/lib/ssh.py b/lib/ssh.py
index 6e16674..9ce4e1b 100644
--- a/lib/ssh.py
+++ b/lib/ssh.py
@@ -721,7 +721,7 @@ def InitPubKeyFile(master_uuid, 
key_file=pathutils.SSH_PUB_KEYS):
   AddPublicKey(master_uuid, key, key_file=key_file)
 
 
-class SshRunner:
+class SshRunner(object):
   """Wrapper for SSH commands.
 
   """
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 15/17] Reduce nesting in LUOobCommand.Exec

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning.

NB it's hard to see from the diff because of all the whitespace, but
this just turns the if result.fail_msg check into a guard that continues
the next iteration of the loop, and dedends the rest of the loop body
rather than having it as an else:

git diff -w will show this.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/misc.py | 65 +++---
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/lib/cmdlib/misc.py b/lib/cmdlib/misc.py
index 62bff52..68ba70f 100644
--- a/lib/cmdlib/misc.py
+++ b/lib/cmdlib/misc.py
@@ -147,43 +147,44 @@ class LUOobCommand(NoHooksLU):
 self.LogWarning("Out-of-band RPC failed on node '%s': %s",
 node.name, result.fail_msg)
 node_entry.append((constants.RS_NODATA, None))
+continue
+
+  try:
+self._CheckPayload(result)
+  except errors.OpExecError, err:
+self.LogWarning("Payload returned by node '%s' is not valid: %s",
+node.name, err)
+node_entry.append((constants.RS_NODATA, None))
   else:
-try:
-  self._CheckPayload(result)
-except errors.OpExecError, err:
-  self.LogWarning("Payload returned by node '%s' is not valid: %s",
-  node.name, err)
-  node_entry.append((constants.RS_NODATA, None))
-else:
-  if self.op.command == constants.OOB_HEALTH:
-# For health we should log important events
-for item, status in result.payload:
-  if status in [constants.OOB_STATUS_WARNING,
-constants.OOB_STATUS_CRITICAL]:
-self.LogWarning("Item '%s' on node '%s' has status '%s'",
-item, node.name, status)
+if self.op.command == constants.OOB_HEALTH:
+  # For health we should log important events
+  for item, status in result.payload:
+if status in [constants.OOB_STATUS_WARNING,
+  constants.OOB_STATUS_CRITICAL]:
+  self.LogWarning("Item '%s' on node '%s' has status '%s'",
+  item, node.name, status)
 
-  if self.op.command == constants.OOB_POWER_ON:
-node.powered = True
-  elif self.op.command == constants.OOB_POWER_OFF:
-node.powered = False
-  elif self.op.command == constants.OOB_POWER_STATUS:
-powered = result.payload[constants.OOB_POWER_STATUS_POWERED]
-if powered != node.powered:
-  logging.warning(("Recorded power state (%s) of node '%s' does 
not"
-   " match actual power state (%s)"), node.powered,
-  node.name, powered)
+if self.op.command == constants.OOB_POWER_ON:
+  node.powered = True
+elif self.op.command == constants.OOB_POWER_OFF:
+  node.powered = False
+elif self.op.command == constants.OOB_POWER_STATUS:
+  powered = result.payload[constants.OOB_POWER_STATUS_POWERED]
+  if powered != node.powered:
+logging.warning(("Recorded power state (%s) of node '%s' does not"
+ " match actual power state (%s)"), node.powered,
+node.name, powered)
 
-  # For configuration changing commands we should update the node
-  if self.op.command in (constants.OOB_POWER_ON,
- constants.OOB_POWER_OFF):
-self.cfg.Update(node, feedback_fn)
+# For configuration changing commands we should update the node
+if self.op.command in (constants.OOB_POWER_ON,
+   constants.OOB_POWER_OFF):
+  self.cfg.Update(node, feedback_fn)
 
-  node_entry.append((constants.RS_NORMAL, result.payload))
+node_entry.append((constants.RS_NORMAL, result.payload))
 
-  if (self.op.command == constants.OOB_POWER_ON and
-  idx < len(self.nodes) - 1):
-time.sleep(self.op.power_delay)
+if (self.op.command == constants.OOB_POWER_ON and
+idx < len(self.nodes) - 1):
+  time.sleep(self.op.power_delay)
 
 return ret
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 02/17] Quell pylint unbalanced-tuple-unpacking warning

['Brian Foley' via ganeti-devel]

Both of these functions return a list, not a tuple, and by manually
tracing the code, we can see they always return non-empty lists.

Change the callers to get the first element of the list rather than
using destructuring.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_storage.py | 4 ++--
 lib/rpc/node.py| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index af1c2e8..3abc5f8 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -596,8 +596,8 @@ def GenerateDiskTemplate(
   raise errors.ProgrammerError("Wrong template configuration")
 remote_node_uuid = secondary_node_uuids[0]
 
-(drbd_params, _, _) = objects.Disk.ComputeLDParams(template_name,
-   full_disk_params)
+drbd_params = objects.Disk.ComputeLDParams(template_name,
+   full_disk_params)[0]
 drbd_default_metavg = drbd_params[constants.LDP_DEFAULT_METAVG]
 
 names = []
diff --git a/lib/rpc/node.py b/lib/rpc/node.py
index cd2050d..f8bfa79 100644
--- a/lib/rpc/node.py
+++ b/lib/rpc/node.py
@@ -953,7 +953,7 @@ class RpcRunner(_RpcClientBase,
 """Wrapper for L{AnnotateDiskParams}.
 
 """
-(anno_disk,) = self._DisksDictDP(node, ([disk], instance))
+anno_disk = self._DisksDictDP(node, ([disk], instance))[0]
 return anno_disk
 
   def _EncodeNodeToDiskDictDP(self, node, value):
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 08/17] Disable pylint bad-continuation warning

['Brian Foley' via ganeti-devel]

pylint is much more strict than pep8, and it would be too invasive
(and arguably pointless) to update these right now.

Signed-off-by: Brian Foley 
---
 Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 88d30c6..c4a54fc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2695,7 +2695,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir 
$(built_python_sources
 # overzealous, eg where we use parens to make it clear that we're
 # deliberately doing a comparison that should yield bool, or are using
 # parens clarify precedence or to allow multi-line expressions.
-LINT_DISABLE = I0013 R0912 R0204 C0325
+# C0330 = disable wrong indentation warnings. pylint is much more strict than
+# pep8, and it would be too invasive to fix all these.
+LINT_DISABLE = I0013 R0912 R0204 C0325 C0330
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 14/17] Reduce nesting in LUInstanceCreate.RunOsScripts

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning.

NB it's hard to see from the diff because of all the whitespace, but
this just replaces the top "if iobj.disk and not self.adopt_disks" with
a pair of guards that return early, and dedents the rest of the fn.

git diff -w will show this.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py | 229 +-
 1 file changed, 117 insertions(+), 112 deletions(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index fe6c71f..1a43e10 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -1193,122 +1193,127 @@ class LUInstanceCreate(LogicalUnit):
 @param iobj: instance object
 
 """
-if iobj.disks and not self.adopt_disks:
-  disks = self.cfg.GetInstanceDisks(iobj.uuid)
-  if self.op.mode == constants.INSTANCE_CREATE:
-os_image = objects.GetOSImage(self.op.osparams)
-
-if os_image is None and not self.op.no_install:
-  pause_sync = (not self.op.wait_for_sync and
-utils.AnyDiskOfType(disks, constants.DTS_INT_MIRROR))
-  if pause_sync:
-feedback_fn("* pausing disk sync to install instance OS")
-result = self.rpc.call_blockdev_pause_resume_sync(self.pnode.uuid,
-  (disks, iobj),
-  True)
-for idx, success in enumerate(result.payload):
-  if not success:
-logging.warn("pause-sync of instance %s for disk %d failed",
- self.op.instance_name, idx)
-
-  feedback_fn("* running the instance OS create scripts...")
+if not iobj.disks:
+  return
+
+if self.adopt_disks:
+  return
+
+disks = self.cfg.GetInstanceDisks(iobj.uuid)
+if self.op.mode == constants.INSTANCE_CREATE:
+  os_image = objects.GetOSImage(self.op.osparams)
+
+  if os_image is None and not self.op.no_install:
+pause_sync = (not self.op.wait_for_sync and
+  utils.AnyDiskOfType(disks, constants.DTS_INT_MIRROR))
+if pause_sync:
+  feedback_fn("* pausing disk sync to install instance OS")
+  result = self.rpc.call_blockdev_pause_resume_sync(self.pnode.uuid,
+(disks, iobj),
+True)
+  for idx, success in enumerate(result.payload):
+if not success:
+  logging.warn("pause-sync of instance %s for disk %d failed",
+   self.op.instance_name, idx)
+
+feedback_fn("* running the instance OS create scripts...")
+# FIXME: pass debug option from opcode to backend
+os_add_result = \
+  self.rpc.call_instance_os_add(self.pnode.uuid,
+(iobj, self.op.osparams_secret),
+False,
+self.op.debug_level)
+if pause_sync:
+  feedback_fn("* resuming disk sync")
+  result = self.rpc.call_blockdev_pause_resume_sync(self.pnode.uuid,
+(disks, iobj),
+False)
+  for idx, success in enumerate(result.payload):
+if not success:
+  logging.warn("resume-sync of instance %s for disk %d failed",
+   self.op.instance_name, idx)
+
+os_add_result.Raise("Could not add os for instance %s"
+" on node %s" % (self.op.instance_name,
+ self.pnode.name))
+
+else:
+  if self.op.mode == constants.INSTANCE_IMPORT:
+feedback_fn("* running the instance OS import scripts...")
+
+transfers = []
+
+for idx, image in enumerate(self.src_images):
+  if not image:
+continue
+
+  if iobj.os:
+dst_io = constants.IEIO_SCRIPT
+dst_ioargs = ((disks[idx], iobj), idx)
+  else:
+dst_io = constants.IEIO_RAW_DISK
+dst_ioargs = (disks[idx], iobj)
+
   # FIXME: pass debug option from opcode to backend
-  os_add_result = \
-self.rpc.call_instance_os_add(self.pnode.uuid,
-  (iobj, self.op.osparams_secret),
-  False,
-  self.op.debug_level)
-  if pause_sync:
-feedback_fn("* resuming disk sync")
-result = self.rpc.call_blockdev_pause_resume_sync(self.pnode.uuid,
-  (disks, iobj),
-

[PATCH stable-2.15 10/17] Reduce nesting in _CheckVLANArguments

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning. It also has the
happy side effect of removing some duplicated code.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index 462522c..fe6c71f 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -77,6 +77,12 @@ from ganeti.cmdlib.instance_utils import \
 import ganeti.masterd.instance
 
 
+def _ValidateTrunkVLAN(vlan):
+  if not compat.all(vl.isdigit() for vl in vlan[1:].split(':')):
+raise errors.OpPrereqError("Specified VLAN parameter is invalid"
+   " : %s" % vlan, errors.ECODE_INVAL)
+
+
 class LUInstanceCreate(LogicalUnit):
   """Create an instance.
 
@@ -152,19 +158,10 @@ class LUInstanceCreate(LogicalUnit):
   # vlan starting with dot means single untagged vlan,
   # might be followed by trunk (:)
   if not vlan[1:].isdigit():
-vlanlist = vlan[1:].split(':')
-for vl in vlanlist:
-  if not vl.isdigit():
-raise errors.OpPrereqError("Specified VLAN parameter is "
-   "invalid : %s" % vlan,
- errors.ECODE_INVAL)
+_ValidateTrunkVLAN(vlan)
 elif vlan[0] == ":":
   # Trunk - tagged only
-  vlanlist = vlan[1:].split(':')
-  for vl in vlanlist:
-if not vl.isdigit():
-  raise errors.OpPrereqError("Specified VLAN parameter is invalid"
-   " : %s" % vlan, errors.ECODE_INVAL)
+  _ValidateTrunkVLAN(vlan)
 elif vlan.isdigit():
   # This is the simplest case. No dots, only single digit
   # -> Create untagged access port, dot needs to be added
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 03/17] Disable incorrect pylint assigning-non-slot warning

['Brian Foley' via ganeti-devel]

This occurs pretty heavily in lib/objects.py, where pylint isn't
correctly detecting the __slots__ assignment. This appears to be
a known issue: https://github.com/PyCQA/pylint/issues/379

Signed-off-by: Brian Foley 
---
 lib/objects.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/objects.py b/lib/objects.py
index d6e64c9..23b349c 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -35,11 +35,14 @@ pass to and from external parties.
 
 """
 
-# pylint: disable=E0203,W0201,R0902
+# pylint: disable=E0203,E0237,W0201,R0902
 
 # E0203: Access to member %r before its definition, since we use
 # objects.py which doesn't explicitly initialise its members
 
+# E0237: Assigning to attribute not defined in class slots. pylint doesn't
+# appear to notice many of the slots defined in __slots__ for several objects.
+
 # W0201: Attribute '%s' defined outside __init__
 
 # R0902: Allow instances of these objects to have more than 20 attributes
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 13/17] Reduce nesting in RemoveNodeSshKeyBulk key calculation

['Brian Foley' via ganeti-devel]

This avoids a pylint too-many-nested-blocks warning. This removes the
6th level of nesting in the function, but may also be marginally faster
by turning the calculation into the set difference operation it really
is.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/backend.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 37ce31a..b262de7 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1799,9 +1799,10 @@ def RemoveNodeSshKeyBulk(node_list,
 if master_uuid:
   master_keys = ssh.QueryPubKeyFile([master_uuid],
 key_file=pub_key_file)
-  for master_key in master_keys:
-if master_key in keys[node_info.uuid]:
-  keys[node_info.uuid].remove(master_key)
+
+  # Remove any master keys from the list of keys to remove from the 
node
+  keys[node_info.uuid] = list(
+  set(keys[node_info.uuid]) - set(master_keys))
 
   all_keys_to_remove.update(keys)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 00/17] Cleanup for pylint 1.6.4, part two

['Brian Foley' via ganeti-devel]

This second set of patches cleans up enough code and disables enough checks
to get Ganeti to run through pylint 1.6.4 with no warnings.

Most of the patches are straightforward, but I separated out each of the 'reduce
nesting' patches to make them easier to review individually, as the diff itself
is often difficult to understand. Generally it's easier to compare the before
and after side by side with diff -y and diff -w to show that the difference is
mostly whitespace.

Brian Foley (17):
  Cleanup: Use new-style classes everywhere
  Quell pylint unbalanced-tuple-unpacking warning
  Disable incorrect pylint assigning-non-slot warning
  Disable pylint broad-except warnings
  Disable pylint too-many-branches warnings
  Disable pylint redefined-variable-type warning
  Disable pylint superfluous-parens warning
  Disable pylint bad-continuation warning
  Reduce nesting in StartDaemon
  Reduce nesting in _CheckVLANArguments
  Reduce nesting in gnt-cluster VerifyDisks missing disk loop
  Reduce nesting in RemoveNodeSshKeyBulk ssh logic
  Reduce nesting in RemoveNodeSshKeyBulk key calculation
  Reduce nesting in LUInstanceCreate.RunOsScripts
  Reduce nesting in LUOobCommand.Exec
  Reduce nesting in import-export ProcessChildIO
  Disable pylint too-many-nested-blocks in _RunCmdPipe

 Makefile.am   |  13 +-
 daemons/import-export |  46 ---
 lib/backend.py|  34 ++
 lib/bootstrap.py  |   2 +-
 lib/cli.py|   2 +-
 lib/client/gnt_cluster.py |  19 +--
 lib/cmdlib/instance_create.py | 248 +++---
 lib/cmdlib/instance_set_params.py |   4 +-
 lib/cmdlib/instance_storage.py|   6 +-
 lib/cmdlib/misc.py|  65 +-
 lib/jqueue/__init__.py|   6 +-
 lib/netutils.py   |   2 +-
 lib/objects.py|   5 +-
 lib/rpc/node.py   |   8 +-
 lib/rpc/transport.py  |   4 +-
 lib/server/noded.py   |   2 +-
 lib/ssh.py|   2 +-
 lib/utils/process.py  |  19 ++-
 lib/watcher/__init__.py   |   2 +-
 tools/ganeti-listrunner   |   2 +-
 tools/move-instance   |   2 +-
 21 files changed, 253 insertions(+), 240 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 07/17] Disable pylint superfluous-parens warning

['Brian Foley' via ganeti-devel]

There are too many cases where we deliberately wrap expressions in
parens, either to indicate comparisons, or to allow multiline
expressions without line continuation chars, or to clarify confusing
precedence.

While here, clean up a few unpythonic cases.

Signed-off-by: Brian Foley 
---
 Makefile.am   | 6 +-
 lib/bootstrap.py  | 2 +-
 lib/cli.py| 2 +-
 lib/client/gnt_cluster.py | 2 +-
 lib/cmdlib/instance_set_params.py | 4 ++--
 lib/netutils.py   | 2 +-
 lib/utils/process.py  | 4 ++--
 7 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 19101ca..88d30c6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2691,7 +2691,11 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 # R0204 = disable redefined-variable-type warning. There are a large number of
 # cases where Ganeti assigns multiple types (eg set/list, float/int) to
 # the same variable, and these are benign.
-LINT_DISABLE = I0013 R0912 R0204
+# C0325 = disable superfluous-parens. There are a lot of cases where this is
+# overzealous, eg where we use parens to make it clear that we're
+# deliberately doing a comparison that should yield bool, or are using
+# parens clarify precedence or to allow multi-line expressions.
+LINT_DISABLE = I0013 R0912 R0204 C0325
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
diff --git a/lib/bootstrap.py b/lib/bootstrap.py
index 7b6fbfe..0a382c5 100644
--- a/lib/bootstrap.py
+++ b/lib/bootstrap.py
@@ -678,7 +678,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: 
disable=R0913, R0914
   for template, dt_params in diskparams.items():
 param_keys = set(dt_params.keys())
 default_param_keys = set(constants.DISK_DT_DEFAULTS[template].keys())
-if not (param_keys <= default_param_keys):
+if param_keys > default_param_keys:
   unknown_params = param_keys - default_param_keys
   raise errors.OpPrereqError("Invalid parameters for disk template %s:"
  " %s" % (template,
diff --git a/lib/cli.py b/lib/cli.py
index f01d8d9..14bd4ea 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2944,7 +2944,7 @@ def _NotAContainer(data):
   @rtype: bool
 
   """
-  return not (isinstance(data, (list, dict, tuple)))
+  return not isinstance(data, (list, dict, tuple))
 
 
 def _GetAlignmentMapping(data):
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 3eaf292..b32e62e 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -195,7 +195,7 @@ def InitCluster(opts, args):
   # check the disk template types here, as we cannot rely on the type check 
done
   # by the opcode parameter types
   diskparams_keys = set(diskparams.keys())
-  if not (diskparams_keys <= constants.DISK_TEMPLATES):
+  if not diskparams_keys > constants.DISK_TEMPLATES:
 unknown = utils.NiceSort(diskparams_keys - constants.DISK_TEMPLATES)
 ToStderr("Disk templates unknown: %s" % utils.CommaJoin(unknown))
 return 1
diff --git a/lib/cmdlib/instance_set_params.py 
b/lib/cmdlib/instance_set_params.py
index a35e95c..9ffbfdf 100644
--- a/lib/cmdlib/instance_set_params.py
+++ b/lib/cmdlib/instance_set_params.py
@@ -1260,7 +1260,7 @@ class LUInstanceSetParams(LogicalUnit):
   res_min = ComputeIPolicyInstanceSpecViolation(ipolicy, ispec_min,
 new_disk_types)
 
-  if (res_max or res_min):
+  if res_max or res_min:
 # FIXME: Improve error message by including information about whether
 # the upper or lower limit of the parameter fails the ipolicy.
 msg = ("Instance allocation to group %s (%s) violates policy: %s" %
@@ -1628,7 +1628,7 @@ class LUInstanceSetParams(LogicalUnit):
 disk = self.GenericGetDiskInfo(uuid, name)
 
 # Rename disk before attaching (if disk is filebased)
-if disk.dev_type in (constants.DTS_INSTANCE_DEPENDENT_PATH):
+if disk.dev_type in constants.DTS_INSTANCE_DEPENDENT_PATH:
   # Add disk size/mode, else GenerateDiskTemplate will not work.
   params[constants.IDISK_SIZE] = disk.size
   params[constants.IDISK_MODE] = str(disk.mode)
diff --git a/lib/netutils.py b/lib/netutils.py
index 2eada25..ab94723 100644
--- a/lib/netutils.py
+++ b/lib/netutils.py
@@ -428,7 +428,7 @@ class IPAddress(object):
 @return: True if valid, False otherwise
 
 """
-assert (isinstance(netmask, (int, long)))
+assert isinstance(netmask, (int, long))
 
 return 0 < netmask <= cls.iplen
 
diff --git a/lib/utils/process.py b/lib/utils/process.py
index 29d8b0c..9183e9f 100644
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -934,12 +934,12 @@ def Daemonize(logfile):
 
   # this might fail
   pid = os.fork()
-  if (pid == 0):  # The first child.
+  if pid == 0:  # The first child.
 

[PATCH stable-2.15 16/17] Reduce nesting in import-export ProcessChildIO

['Brian Foley' via ganeti-devel]

This avoids some pylint too-many-nested-blocks warnings. Do this by
flattening some 'arrow antipattern code' inside the polling loop to use
guards instead.

This is hard to see from diff, but git difftool -y -x 'diff -y' will
show the before and after side by side.

Sprinkle some comments while we're here.

Signed-off-by: Brian Foley 
---
 daemons/import-export | 46 --
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/daemons/import-export b/daemons/import-export
index 6c794f6..2dad4e6 100755
--- a/daemons/import-export
+++ b/daemons/import-export
@@ -323,27 +323,37 @@ def ProcessChildIO(child, socat_stderr_read_fd, 
dd_stderr_read_fd,
 
   # Read up to 1 KB of data
   data = from_.read(1024)
-  if data:
-if to:
-  to.write(data)
-elif fd == signal_notify.fileno():
-  # Signal handling
-  if signal_handler.called:
-signal_handler.Clear()
-if exit_timeout:
-  logging.info("Child process still has about %0.2f seconds"
-   " to exit", exit_timeout.Remaining())
-else:
-  logging.info("Giving child process %0.2f seconds to exit",
-   constants.CHILD_LINGER_TIMEOUT)
-  exit_timeout = \
-utils.RunningTimeout(constants.CHILD_LINGER_TIMEOUT, True)
-  else:
+
+  # On error, remove the mapping
+  if not data:
 poller.unregister(fd)
 del fdmap[fd]
+continue
 
-elif event & (select.POLLNVAL | select.POLLHUP |
-  select.POLLERR):
+  # If the data needs to be sent to another fd, write it
+  if to:
+to.write(data)
+continue
+
+  # Did we get a signal?
+  if fd != signal_notify.fileno():
+continue
+
+  # Has it been handled?
+  if not signal_handler.called:
+continue
+
+  # If so, clean up after it.
+  signal_handler.Clear()
+  if exit_timeout:
+logging.info("Child process still has about %0.2f seconds"
+ " to exit", exit_timeout.Remaining())
+  else:
+logging.info("Giving child process %0.2f seconds to exit",
+ constants.CHILD_LINGER_TIMEOUT)
+exit_timeout = \
+  utils.RunningTimeout(constants.CHILD_LINGER_TIMEOUT, True)
+elif event & (select.POLLNVAL | select.POLLHUP | select.POLLERR):
   poller.unregister(fd)
   del fdmap[fd]
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 17/17] Disable pylint too-many-nested-blocks in _RunCmdPipe

['Brian Foley' via ganeti-devel]

There doesn't appear to be any easy way of reducing the complexity
of this function without moving it elsewhere or completely reorganising
the function, so disable this warning for the time being.

Signed-off-by: Brian Foley 
---
 lib/utils/process.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/utils/process.py b/lib/utils/process.py
index bb5aed2..05553fc 100644
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -530,6 +530,7 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, 
timeout, noclose_fds,
   @return: (out, err, status)
 
   """
+  # pylint: disable=R0101
   poller = select.poll()
 
   if interactive:
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 04/17] Disable pylint broad-except warnings

['Brian Foley' via ganeti-devel]

These are all deliberate top-level catch-any-unhandled-exception cases,
used for logging and error handling so just get pylint to ignore them.

Signed-off-by: Brian Foley 
---
 lib/server/noded.py | 2 +-
 lib/watcher/__init__.py | 2 +-
 tools/ganeti-listrunner | 2 +-
 tools/move-instance | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/server/noded.py b/lib/server/noded.py
index fbf301b..cbf9626 100644
--- a/lib/server/noded.py
+++ b/lib/server/noded.py
@@ -200,7 +200,7 @@ class NodeRequestHandler(http.server.HttpServerHandler):
   # And return the error's arguments, which must be already in
   # correct tuple format
   result = err.args
-except Exception, err:
+except Exception, err: # pylint: disable=W0703
   logging.exception("Error in RPC call")
   result = (False, "Error while executing backend function: %s" % str(err))
 
diff --git a/lib/watcher/__init__.py b/lib/watcher/__init__.py
index 9c2a272..be6da0b 100644
--- a/lib/watcher/__init__.py
+++ b/lib/watcher/__init__.py
@@ -919,7 +919,7 @@ def Main():
 logging.error("Job queue is full, can't query cluster state")
   except errors.JobQueueDrainError:
 logging.error("Job queue is drained, can't maintain cluster state")
-  except Exception, err:
+  except Exception, err: # pylint: disable=W0703
 logging.exception(str(err))
 return constants.EXIT_FAILURE
 
diff --git a/tools/ganeti-listrunner b/tools/ganeti-listrunner
index d8bb2a2..7aebfca 100755
--- a/tools/ganeti-listrunner
+++ b/tools/ganeti-listrunner
@@ -420,7 +420,7 @@ def HostWorker(logdir, username, password, use_agent, 
hostname,
 print "  %s: received KeyboardInterrupt, aborting" % hostname
 WriteLog("ERROR: ABORT_KEYBOARD_INTERRUPT", logfile)
 result = 1
-  except Exception, err:
+  except Exception, err: # pylint: disable=W0703
 result = 1
 trace = traceback.format_exc()
 msg = "ERROR: UNHANDLED_EXECPTION_ERROR: %s\nTrace: %s" % (err, trace)
diff --git a/tools/move-instance b/tools/move-instance
index 84b5972..e43bd5c 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -414,7 +414,7 @@ class MoveRuntime(object):
   errmsg = None
 except Abort:
   errmsg = "Aborted"
-except Exception, err:
+except Exception, err:  # pylint: disable=W0703
   logging.exception("Caught unhandled exception")
   errmsg = str(err)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 11/17] Reduce nesting in gnt-cluster VerifyDisks missing disk loop

['Brian Foley' via ganeti-devel]

To avoid pylint's too-many-nested-blocks warning.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/client/gnt_cluster.py | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index b32e62e..ad02400 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -829,14 +829,15 @@ def VerifyDisks(opts, args):
 if all_missing:
   ToStdout("Instance %s cannot be verified as it lives on"
" broken nodes", iname)
-else:
-  ToStdout("Instance %s has missing logical volumes:", iname)
-  ival.sort()
-  for node, vol in ival:
-if node in bad_nodes:
-  ToStdout("\tbroken node %s /dev/%s", node, vol)
-else:
-  ToStdout("\t%s /dev/%s", node, vol)
+  continue
+
+ToStdout("Instance %s has missing logical volumes:", iname)
+ival.sort()
+for node, vol in ival:
+  if node in bad_nodes:
+ToStdout("\tbroken node %s /dev/%s", node, vol)
+  else:
+ToStdout("\t%s /dev/%s", node, vol)
 
   ToStdout("You need to replace or recreate disks for all the above"
" instances if this message persists after fixing broken 
nodes.")
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 06/17] Disable pylint redefined-variable-type warning

['Brian Foley' via ganeti-devel]

There are a large number of cases where Ganeti assigns multiple types
(eg set/list, float/int) to the same variable at different times, and
although these would make a type checking tool very unhappy, they are
benign here (and besides, cleaning all this up would be too invasive).

Signed-off-by: Brian Foley 
---
 Makefile.am | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index ba99fb8..19101ca 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2688,7 +2688,10 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 # I0013 = disable warnings for ignoring whole files
 # R0912 = disable too many branches warning. It's useful, but ganeti requires
 # a lot of refactoring to fix this.
-LINT_DISABLE = I0013 R0912
+# R0204 = disable redefined-variable-type warning. There are a large number of
+# cases where Ganeti assigns multiple types (eg set/list, float/int) to
+# the same variable, and these are benign.
+LINT_DISABLE = I0013 R0912 R0204
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 03:21:32PM +, Federico Pareschi wrote:
>I'm replying here for all the 37 patches, after addressing all the
>comments,
>I'd say this is an LGTM for me :)
>Thanks Brian for the good work.
>Cheers,
>Morg.

Committed, thanks. Now, onto the next batch... :)

Cheers,
Brian.

Re: [PATCH master 0/6] Clear/remove public/private OS params in gnt-instance modify/reinstall (version 2)

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 04:41:33PM +0200, Yiannis Tsiouris wrote:
> This patchset extends 'gnt-instance modify' and 'gnt-instance reinstall'
> functionality by allowing partial/total removal of current OS parameters of an
> instance. This is specifically useful for preparing or performing an instance
> reinstall to a different OS provider or the same OS provider that changed its
> OS parameter policy/supported list.
> 
> Nikos Skalkotos (1):
>   Add cmdlib tests for removing instance OS params
> 
> Yiannis Tsiouris (5):
>   Add clear OS params options in gnt-instance modify
>   Support OS params removal in gnt-instance modify
>   Test new options of gnt-instance modify in RAPI
>   Support clear OS params in gnt-instance reinstall
>   Add OS params removal in gnt-instance reinstall

LGTM. Committed with a minor tweak to make pep8 happy.

Cheers,
Brian.

Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 12:01:08PM +, Federico Pareschi wrote:
>This is an LGTM for me, however it made me wonder...
>We are adamant in not storing the secret parameters anywhere on the
>cluster,
>at least according to our design docs. Wouldn't this be going against
>such policy
>by having them printed out in an error (and, I assume, logged to disk)?
>This is probably not the correct patchset to debate this, but it'd be
>useful to maybe
>investigate this just to make sure we are not accidentally leaking data
>that we
>promised, in our design docs, we wouldn't.

I think this should be OK, because it's only showing secret OS parameter names,
not the values, which, presumably are the bit.

Cheers,
Brian.

[PATCH stable-2.15 15/36] Replace deprecated pylint >=0.27 pragma with new form

['Brian Foley' via ganeti-devel]

The disable-all pragma was deprecated in favour of the new skip-file
pragma in pylint >= 0.27. Since we're moving to 1.6.4, use this instead.

Signed-off-by: Brian Foley 
---
 src/Ganeti/THH/PyRPC.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Ganeti/THH/PyRPC.hs b/src/Ganeti/THH/PyRPC.hs
index eee1554..d25c811 100644
--- a/src/Ganeti/THH/PyRPC.hs
+++ b/src/Ganeti/THH/PyRPC.hs
@@ -181,7 +181,7 @@ genPyUDSRpcStub className constName = liftM (header $+$) .
   namesToClass className stubCode
   where
 header = text "# This file is automatically generated, do not edit!" $+$
- text "# pylint: disable-all"
+ text "# pylint: skip-file"
 stubCode =
   abstrMethod genericInvokeName [ text "method", text "*args"] $+$
   method socketPathName [] (
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 14/36] Delete old warning disables removed from pylint 1.6

['Brian Foley' via ganeti-devel]

These include star-args and r0924: badly implemented immutable.

Signed-off-by: Brian Foley 
---
 autotools/build-bash-completion   | 4 
 lib/build/sphinx_ext.py   | 3 +--
 lib/cli.py| 4 ++--
 lib/client/gnt_debug.py   | 1 -
 lib/client/gnt_instance.py| 3 +--
 lib/client/gnt_network.py | 1 -
 lib/cmdlib/cluster/verify.py  | 2 +-
 lib/cmdlib/instance_storage.py| 1 -
 lib/cmdlib/instance_utils.py  | 2 +-
 lib/cmdlib/network.py | 4 ++--
 lib/cmdlib/query.py   | 2 --
 lib/compat.py | 2 +-
 lib/config/__init__.py| 2 +-
 lib/errors.py | 1 -
 lib/hypervisor/hv_lxc.py  | 2 +-
 lib/objects.py| 2 +-
 lib/rapi/baserlib.py  | 2 +-
 lib/rapi/testutils.py | 2 +-
 lib/rpc/node.py   | 2 +-
 lib/server/noded.py   | 5 +
 lib/server/rapi.py| 2 +-
 lib/storage/container.py  | 2 +-
 lib/tools/burnin.py   | 4 ++--
 lib/utils/algo.py | 2 --
 lib/utils/retry.py| 3 ---
 lib/utils/text.py | 3 +--
 lib/workerpool.py | 2 +-
 test/py/cmdlib/testsupport/cmdlib_testcase.py | 2 --
 test/py/testutils/config_mock.py  | 2 +-
 tools/confd-client| 3 ---
 tools/ganeti-listrunner   | 2 +-
 31 files changed, 24 insertions(+), 50 deletions(-)

diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 3e35ee1..4a064ba 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -703,8 +703,6 @@ def HaskellOptToOptParse(opts, kind):
   kept in sync
 
   """
-  # pylint: disable=W0142
-  # since we pass *opts in a number of places
   opts = opts.split(",")
   if kind == "none":
 return cli.cli_option(*opts, action="store_true")
@@ -761,8 +759,6 @@ def HaskellArgToCliArg(kind, min_cnt, max_cnt):
 max_cnt = None
   else:
 max_cnt = int(max_cnt)
-  # pylint: disable=W0142
-  # since we pass **kwargs
   kwargs = {"min": min_cnt, "max": max_cnt}
 
   if kind.startswith("choices=") or kind.startswith("suggest="):
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 2150288..6b30894 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -281,9 +281,8 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   The expression's result is included as a literal.
 
   """
-  # pylint: disable=W0102,W0613,W0142
+  # pylint: disable=W0102,W0613
   # W0102: Dangerous default value as argument
-  # W0142: Used * or ** magic
   # W0613: Unused argument
 
   code = docutils.utils.unescape(text, restore_backslashes=True)
diff --git a/lib/cli.py b/lib/cli.py
index e29bc02..3c3241d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1732,8 +1732,8 @@ def GenerateTable(headers, fields, separator, data,
   if unitfields is None:
 unitfields = []
 
-  numfields = utils.FieldSet(*numfields)   # pylint: disable=W0142
-  unitfields = utils.FieldSet(*unitfields) # pylint: disable=W0142
+  numfields = utils.FieldSet(*numfields)
+  unitfields = utils.FieldSet(*unitfields)
 
   format_fields = []
   for field in fields:
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index d05fbc2..e79ae8d 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -103,7 +103,6 @@ def GenericOpCodes(opts, args):
 ToStdout("Loading...")
   for job_idx in range(opts.rep_job):
 for fname in args:
-  # pylint: disable=W0142
   op_data = simplejson.loads(utils.ReadFile(fname))
   op_list = [opcodes.OpCode.LoadOpCode(val) for val in op_data]
   op_list = op_list * opts.rep_op
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 52da28e..6280b83 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -101,7 +101,6 @@ def _ExpandMultiNames(mode, names, client=None):
   @raise errors.OpPrereqError: for invalid input parameters
 
   """
-  # pylint: disable=W0142
 
   if client is None:
 client = GetClient()
@@ -300,7 +299,7 @@ def BatchCreate(opts, args):
  (idx, utils.CommaJoin(unknown)),
  errors.ECODE_INVAL)
 
-op = opcodes.OpInstanceCreate(**inst) # pylint: disable=W0142
+op = opcodes.OpInstanceCreate(**inst)
 op.Validate(False)
 instances.append(op)
 
diff --git a/lib/client/gnt_network.py b/lib/client/gnt_network.py
index 09c1121..41348c4 100644
--- a/lib/client/gnt_network.py
+++ 

Re: [PATCH stable-2.15 14/37] Disable pylint warning about unrecognised --disable

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 10:36:04AM +, Brian Foley wrote:
> Allows us to disable warnings that are present in some versions of
> pylint but not others.
> 
> Signed-off-by: Brian Foley 
> ---
>  Makefile.am | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8b859c1..abaa325 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2686,7 +2686,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
> $(notdir $(built_python_sources
>  
>  # A space-separated list of pylint warnings to completely ignore:
>  # I0013 = disable warnings for ignoring whole files
> -LINT_DISABLE = I0013
> +# E0012 = disable bad option vals: some warnings are removed from newer 
> pylints,
> +# and older pylints don't understand newer pylint's options
> +LINT_DISABLE = I0013 E0012
>  # Additional pylint options
>  LINT_OPTS =
>  # The combined set of pylint options
> -- 
> 2.8.0.rc3.226.g39d4020
> 

I'm dropping this patch as we're not going to bother supporting pylint <1.6.4

[PATCH stable-2.15 13/36] Fix pylint >1.4 pycurl no-member warnings

['Brian Foley' via ganeti-devel]

Add pycurl to the whitelist of C extensions that can be loaded for the
purposes of attribute checking. This works around a security feature
introduced in pylint 1.4 to avoid arbitrary code injection.

Signed-off-by: Brian Foley 
---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0b30033..108cbba 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2693,6 +2693,9 @@ LINT_OPTS =
 LINT_OPTS_ALL = $(LINT_OPTS) \
   $(addprefix --disable=,$(LINT_DISABLE))
 
+# Whitelist loading pycurl C extension for attribute checking
+LINT_OPTS_ALL += --extension-pkg-whitelist=pycurl
+
 LINT_TARGETS = pylint pylint-qa pylint-test
 if HAS_PEP8
 LINT_TARGETS += pep8
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 02/36] Cleanup: Simplify boolean assignment

['Brian Foley' via ganeti-devel]

No functional change. Found by pylint's simplifiable-if-statement.

Signed-off-by: Brian Foley 
---
 lib/client/gnt_cluster.py | 5 +
 tools/cluster-merge   | 5 +
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 40792e2..d63310e 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -294,10 +294,7 @@ def InitCluster(opts, args):
 
   default_ialloc_params = opts.default_iallocator_params
 
-  if opts.enabled_user_shutdown:
-enabled_user_shutdown = True
-  else:
-enabled_user_shutdown = False
+  enabled_user_shutdown = bool(opts.enabled_user_shutdown)
 
   bootstrap.InitCluster(cluster_name=args[0],
 secondary_ip=opts.secondary_ip,
diff --git a/tools/cluster-merge b/tools/cluster-merge
index 926b705..c83e350 100755
--- a/tools/cluster-merge
+++ b/tools/cluster-merge
@@ -448,10 +448,7 @@ class Merger(object):
   check_params_strict.append("shared_file_storage_dir")
 check_params.extend(check_params_strict)
 
-if self.params == _PARAMS_STRICT:
-  params_strict = True
-else:
-  params_strict = False
+params_strict = (self.params == _PARAMS_STRICT)
 
 for param_name in check_params:
   my_param = getattr(my_cluster, param_name)
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 02:29:05PM +, Federico Pareschi wrote:
>LGTM but little nit.
>s/doensn't/doesn't/ in the commit message :)

Thanks. :)

Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 01:43:57PM +, Federico Pareschi wrote:
>  to avoid arbitrary code injection.
> 
>Is this safe? Should we be looking more into this or is it something
>that does not affect us?

The attack vector is someone writing their own pycurl.so with malicious code
in it, puting that on the python search path, and using that to perform
arbitrary actions as the user running pylint when pylint loads pycurl.

I think this is a legitimate concern for developers running pylint as
themselves and potentially accepting arbitary patches from the internet without
checking what's in them, but it shouldn't be much of a problem for a sandboxed
buildbot.

The only alternatives I can think of are to explicitly disable the warning for
any ganeti modules that use pycurl directly, or to explicitly add disables to
each use of pycurl.foo but I don't much like those.

Cheers,
Brian.

Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 02:17:32PM +0200, Yiannis Tsiouris wrote:
> > OK, that's fine by me. I don't have strong feelings about it one way or the
> > other, but I thought I'd suggest the alternative.
> 
> That was actually a very good suggestion! I don't have any strong objections
> against it (i.e. using "*" as a "special" parameter); We discussed this with
> Alex and we think that maybe the
> --clear-os-parameters(-private)/--remove-os-parameters(-private) options are
> "closer" to what the user might expect while the "*" approach might be
> non-intuisive as this isn't used anywhere else
> 
> In regards of using "*"-expansion to OS parameters, I mostly agree with Morg: 
> I
> don't think it deserves the effort and it also messes with how bash and other
> shells handle expansion.
> 
> Should I resend the whole patchset addressing all the other comments and leave
> this as-is for now?

Hi Yiannis,

that sounds good, and thanks for the work on this.

Cheers,
Brian.

Re: [PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 11:13:18AM +, Federico Pareschi wrote:
>  diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
>  index 40792e2..9ad82d8 100644
>  --- a/lib/client/gnt_cluster.py
>  +++ b/lib/client/gnt_cluster.py
>  @@ -294,11 +294,6 @@ def InitCluster(opts, args):
> default_ialloc_params = opts.default_iallocator_params
>  -  if opts.enabled_user_shutdown:
>  -enabled_user_shutdown = True
>  -  else:
>  -enabled_user_shutdown = False
>  -
> bootstrap.InitCluster(cluster_name=args[0],
>   secondary_ip=opts.secondary_ip,
>   vg_name=vg_name,
>  @@ -332,7 +327,7 @@ def InitCluster(opts, args):
>   install_image=install_image,
>   zeroing_image=zeroing_image,
>   compression_tools=compression_tools,
>  -enabled_user_shutdown=enabled_
>  user_shutdown,
>  +enabled_user_shutdown=opts.
>  enabled_user_shutdown,
>   )
> op = opcodes.OpClusterPostInit()
> SubmitOpCode(op, opts=opts)
> 
>Can't this cause some issues if opts.enabled_user_shutdown is None?
>Pre-patch this would set enabled_user_shutdown in the function as
>False,
>post-patch, it would pass None down the chain call.
>The code calls into bootstrap.py's InitCluster which defaults the value
>of
>enabled_user_shutdown to False. Passing None to it would completely
>change the dynamics of the function and would likely trigger some
>errors.

Good point. I'll cast this to bool instead.

Thanks,
Brian.

Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 12:01:14PM +0100, Iustin Pop wrote:
>Quick question: is there a reason to keep that compat, as opposed to
>switching the "blessed" (and required) pylint version to 1.6.4?
>(If this is addressed in the patch series, sorry, didn't read the
>patches)
>iustin

Hi Iustin,

no, there isn't any very good reason, I was just playing it safe, and kinda
assumed that make dist or the debian build process might run pylint, but it
appears tht neither do.

In that case, I'll move us to requiring 1.6.4, and remove a couple of the
compatibility workarounds.

Cheers,
Brian.

Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 01:31:15PM +0200, Alex Kiousis wrote:
> Hello (I work with Yiannis at grnet),
> the main reason for this patchset is that a command like
> clear-os-params is needed if you are reinstalling between 2 os
> providers with different parameters. Without a mechanism to remove a
> defined osparam the reinstall will fail when ganeti tries to validate
> the params against the new provider.
> 
> The remove-os-params was added when we figured that there should be a
> way to bring the Instance osparams to the desired state without
> respecifying everything.
> 
> Given the above, i believe that --clear-os-params is much more
> intuitive than --remove-os-params "\*". We can add both to the patch
> but unless there is considerable overhead for the extra option,  we'd
> like for clean-os-params to remain seperate.

OK, that's fine by me. I don't have strong feelings about it one way or the
other, but I thought I'd suggest the alternative.

Cheers,
Brian.

[PATCH stable-2.15 28/37] Disable pylint unsubscriptable-object warning

['Brian Foley' via ganeti-devel]

In these cases result is subscriptable.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py| 8 
 lib/cmdlib/instance_migration.py | 3 ++-
 lib/cmdlib/instance_storage.py   | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index 1b1abf8..d69ec50 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -472,8 +472,8 @@ class LUInstanceCreate(LogicalUnit):
  (self.op.iallocator, ial.info),
  ecode)
 
-(self.op.pnode_uuid, self.op.pnode) = \
-  ExpandNodeUuidAndName(self.cfg, None, ial.result[0])
+(self.op.pnode_uuid, self.op.pnode) = ExpandNodeUuidAndName(
+self.cfg, None, ial.result[0]) # pylint: disable=E1136
 self.LogInfo("Selected nodes for instance %s via iallocator %s: %s",
  self.op.instance_name, self.op.iallocator,
  utils.CommaJoin(ial.result))
@@ -481,8 +481,8 @@ class LUInstanceCreate(LogicalUnit):
 assert req.RequiredNodes() in (1, 2), "Wrong node count from iallocator"
 
 if req.RequiredNodes() == 2:
-  (self.op.snode_uuid, self.op.snode) = \
-ExpandNodeUuidAndName(self.cfg, None, ial.result[1])
+  (self.op.snode_uuid, self.op.snode) = ExpandNodeUuidAndName(
+  self.cfg, None, ial.result[1]) # pylint: disable=E1136
 
   def BuildHooksEnv(self):
 """Build hooks env.
diff --git a/lib/cmdlib/instance_migration.py b/lib/cmdlib/instance_migration.py
index ad6f9c3..b1321fa 100644
--- a/lib/cmdlib/instance_migration.py
+++ b/lib/cmdlib/instance_migration.py
@@ -499,7 +499,8 @@ class TLMigrateInstance(Tasklet):
  " iallocator '%s': %s" %
  (self.lu.op.iallocator, ial.info),
  errors.ECODE_NORES)
-self.target_node_uuid = self.cfg.GetNodeInfoByName(ial.result[0]).uuid
+self.target_node_uuid = self.cfg.GetNodeInfoByName(
+ial.result[0]).uuid # pylint: disable=E1136
 self.lu.LogInfo("Selected nodes for instance %s via iallocator %s: %s",
 self.instance_name, self.lu.op.iallocator,
 utils.CommaJoin(ial.result))
diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 4c1c964..0093edc 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -2246,7 +2246,7 @@ class TLReplaceDisks(Tasklet):
  " %s" % (iallocator_name, ial.info),
  errors.ECODE_NORES)
 
-remote_node_name = ial.result[0]
+remote_node_name = ial.result[0] # pylint: disable=E1136
 remote_node = lu.cfg.GetNodeInfoByName(remote_node_name)
 
 if remote_node is None:
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 31/37] Quell trailing newline

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 autotools/build-rpc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autotools/build-rpc b/autotools/build-rpc
index d3ccad6..1d59aae 100755
--- a/autotools/build-rpc
+++ b/autotools/build-rpc
@@ -212,7 +212,7 @@ def main():
 for (clsname, calls) in sorted(module.CALLS.items()):
   _WriteBaseClass(sw, clsname, calls.values())
 
-  print buf.getvalue()
+  print buf.getvalue().rstrip()
 
 
 if __name__ == "__main__":
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 35/37] Quell cell-var-from-loop warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 autotools/build-bash-completion | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 4a064ba..e0ad9b2 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -779,18 +779,18 @@ def ParseHaskellOptsArgs(script, output):
   cli_args = []
   for line in output.splitlines():
 v = line.split(None)
-exc = lambda msg: Exception("Invalid %s output from %s: %s" %
-(msg, script, v))
+exc = lambda msg, v: Exception("Invalid %s output from %s: %s" %
+   (msg, script, v))
 if len(v) < 2:
-  raise exc("help completion")
+  raise exc("help completion", v)
 if v[0].startswith("-"):
   if len(v) != 2:
-raise exc("option format")
+raise exc("option format", v)
   (opts, kind) = v
   cli_opts.append(HaskellOptToOptParse(opts, kind))
 else:
   if len(v) != 3:
-raise exc("argument format")
+raise exc("argument format", v)
   (kind, min_cnt, max_cnt) = v
   cli_args.append(HaskellArgToCliArg(kind, min_cnt, max_cnt))
   return (cli_opts, cli_args)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 23/37] Disable pylint unpacking-non-sequence warning

['Brian Foley' via ganeti-devel]

For these cases, self.ia_result actually will have a tuple assigned to
it, it's just hard for static analysis tools to see it.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index a8335dc..d20aa81 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -658,7 +658,7 @@ class LUInstanceMultiAlloc(NoHooksLU):
 
 """
 if self.op.iallocator:
-  (allocatable, failed_insts) = self.ia_result
+  (allocatable, failed_insts) = self.ia_result # pylint: disable=W0633
   allocatable_insts = map(compat.fst, allocatable)
 else:
   allocatable_insts = [op.instance_name for op in self.op.instances]
@@ -676,7 +676,7 @@ class LUInstanceMultiAlloc(NoHooksLU):
 jobs = []
 if self.op.iallocator:
   op2inst = dict((op.instance_name, op) for op in self.op.instances)
-  (allocatable, failed) = self.ia_result
+  (allocatable, failed) = self.ia_result # pylint: disable=W0633
 
   for (name, node_names) in allocatable:
 op = op2inst.pop(name)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 22/37] Disable pylint misplaced-comparison-constant warning

['Brian Foley' via ganeti-devel]

It's correct, and easier to read a < x < b in this case.

Signed-off-by: Brian Foley 
---
 lib/utils/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index bdd9761..d3a71d5 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -532,7 +532,7 @@ def SplitTime(value):
 
   """
   (seconds, microseconds) = divmod(int(value * 100), 100)
-
+  # pylint: disable=C0122
   assert 0 <= seconds, \
 "Seconds must be larger than or equal to 0, but are %s" % seconds
   assert 0 <= microseconds <= 99, \
@@ -550,7 +550,7 @@ def MergeTime(timetuple):
 
   """
   (seconds, microseconds) = timetuple
-
+  # pylint: disable=C0122
   assert 0 <= seconds, \
 "Seconds must be larger than or equal to 0, but are %s" % seconds
   assert 0 <= microseconds <= 99, \
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 33/37] Quell too-many-boolean-expressions

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 tools/move-instance | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/move-instance b/tools/move-instance
index 4ca6327..84b5972 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -943,9 +943,9 @@ def _CheckInstanceOptions(parser, options, instance_names):
   options.nics = cli.ParseNicOption(options.nics)
   else:
 # Moving more than one instance
-if (options.dest_instance_name or options.dest_primary_node or
-options.dest_secondary_node or options.hvparams or
-options.beparams or options.osparams or options.nics):
+if compat.any(options.dest_instance_name, options.dest_primary_node,
+  options.dest_secondary_node, options.hvparams,
+  options.beparams, options.osparams, options.nics):
   parser.error("The options --dest-instance-name, --dest-primary-node,"
" --dest-secondary-node, --hypervisor-parameters,"
" --backend-parameters, --os-parameters and --net can"
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 24/37] Disable incorrect pylint not-callable warning

['Brian Foley' via ganeti-devel]

REQ_REQUEST will be a parameter validating function from ht, but pylint
can't statically determine this.

Signed-off-by: Brian Foley 
---
 lib/masterd/iallocator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/masterd/iallocator.py b/lib/masterd/iallocator.py
index a94d880..6d6ed48 100644
--- a/lib/masterd/iallocator.py
+++ b/lib/masterd/iallocator.py
@@ -156,7 +156,7 @@ class IARequestBase(outils.ValidatedSlots):
 @raises ResultValidationError: If validation fails
 
 """
-if ia.success and not self.REQ_RESULT(result):
+if ia.success and not self.REQ_RESULT(result): # pylint: disable=E1102
   raise errors.ResultValidationError("iallocator returned invalid result,"
  " expected %s, got %s" %
  (self.REQ_RESULT, result))
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 29/37] Quell consider-using-enumerate warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/config/verify.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/config/verify.py b/lib/config/verify.py
index e53b384..2d8b99e 100644
--- a/lib/config/verify.py
+++ b/lib/config/verify.py
@@ -107,8 +107,8 @@ def VerifyIpolicy(owner, ipolicy, iscluster, callback):
 callback("%s has invalid instance policy: %s" % (owner, err))
   for key, value in ipolicy.items():
 if key == constants.ISPECS_MINMAX:
-  for k in range(len(value)):
-VerifyIspecs(owner, "ipolicy/%s[%s]" % (key, k), value[k], callback)
+  for i, val in enumerate(value):
+VerifyIspecs(owner, "ipolicy/%s[%s]" % (key, i), val, callback)
 elif key == constants.ISPECS_STD:
   VerifyType(owner, "ipolicy/" + key, value,
  constants.ISPECS_PARAMETER_TYPES, callback)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 30/37] Quell bad-whitespace warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/objects.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/objects.py b/lib/objects.py
index 6235947..13e5ee2 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -1254,7 +1254,7 @@ class Instance(TaggableObject):
 if _with_private:
   bo["osparams_private"] = self.osparams_private.Unprivate()
 
-for attr in "nics", :
+for attr in ("nics",):
   alist = bo.get(attr, None)
   if alist:
 nlist = outils.ContainerToDicts(alist)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 26/37] Disable unwanted pylint wrong-import-position warnings

['Brian Foley' via ganeti-devel]

For these, we want to import some modules in a particular order, so
disable the pylint warnings.

Signed-off-by: Brian Foley 
---
 autotools/check-imports | 3 ++-
 lib/build/sphinx_ext.py | 3 +++
 lib/ovf.py  | 3 ++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/autotools/check-imports b/autotools/check-imports
index c90ea70..27790cf 100755
--- a/autotools/check-imports
+++ b/autotools/check-imports
@@ -32,8 +32,9 @@
 
 """
 
-# pylint: disable=C0103
+# pylint: disable=C0103, C0413
 # C0103: Invalid name
+# C0413: Wrong import position
 
 import sys
 
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 37f1574..4ab7b29 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -32,6 +32,9 @@
 
 """
 
+# pylint: disable=C0413
+# C0413: Wrong import position
+
 import re
 from cStringIO import StringIO
 
diff --git a/lib/ovf.py b/lib/ovf.py
index 2662ca7..66dbf83 100644
--- a/lib/ovf.py
+++ b/lib/ovf.py
@@ -32,10 +32,11 @@
 
 """
 
-# pylint: disable=F0401, E1101
+# pylint: disable=F0401, E1101, C0413
 
 # F0401 because ElementTree is not default for python 2.4
 # E1101 makes no sense - pylint assumes that ElementTree object is a tuple
+# C0413 Wrong import position
 
 
 import ConfigParser
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 20/37] Disable pylint eval-used warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/build/sphinx_ext.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 6b30894..37f1574 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -288,7 +288,7 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   code = docutils.utils.unescape(text, restore_backslashes=True)
 
   try:
-result = eval(code, EVAL_NS)
+result = eval(code, EVAL_NS) # pylint: disable=W0123
   except Exception, err: # pylint: disable=W0703
 msg = inliner.reporter.error("Failed to evaluate %r: %s" % (code, err),
  line=lineno)
@@ -321,7 +321,7 @@ class PythonAssert(s_compat.Directive):
 code = "\n".join(self.content)
 
 try:
-  result = eval(code, EVAL_NS)
+  result = eval(code, EVAL_NS) # pylint: disable=W0123
 except Exception, err:
   raise self.error("Failed to evaluate %r: %s" % (code, err))
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

['Brian Foley' via ganeti-devel]

socket.timeout doensn't have string or errno attributes.

Signed-off-by: Brian Foley 
---
 lib/hypervisor/hv_kvm/monitor.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py
index 98c8a6f..f7ca814 100644
--- a/lib/hypervisor/hv_kvm/monitor.py
+++ b/lib/hypervisor/hv_kvm/monitor.py
@@ -385,7 +385,7 @@ class QmpConnection(MonitorSocket):
   self.sock.sendall(message_str)
 except socket.timeout, err:
   raise errors.HypervisorError("Timeout while sending a QMP message: "
-   "%s (%s)" % (err.string, err.errno))
+   "%s" % err)
 except socket.error, err:
   raise errors.HypervisorError("Unable to send data from KVM using the"
" QMP protocol: %s" % err)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 25/37] Disable pylint unused-wildcard-import warning

['Brian Foley' via ganeti-devel]

We're deliberately importing all the variables defined in cli_opt into
cli's namespace.

Signed-off-by: Brian Foley 
---
 lib/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cli.py b/lib/cli.py
index 3c3241d..8622738 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -56,7 +56,7 @@ from ganeti import pathutils
 from ganeti import serializer
 import ganeti.cli_opts
 # Import constants
-from ganeti.cli_opts import *  # pylint: disable=W0401
+from ganeti.cli_opts import *  # pylint: disable=W0401,W0614
 
 from ganeti.runtime import (GetClient)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 27/37] Disable pylint bare-except warning

['Brian Foley' via ganeti-devel]

This is actually dead code which wasn't removed until Ganeti 2.16

Signed-off-by: Brian Foley 
---
 lib/server/masterd.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/server/masterd.py b/lib/server/masterd.py
index be98f69..f8787d4 100644
--- a/lib/server/masterd.py
+++ b/lib/server/masterd.py
@@ -107,7 +107,7 @@ class ClientRequestWorker(workerpool.BaseWorker):
   logging.exception("Unexpected exception")
   success = False
   result = errors.EncodeException(err)
-except:
+except: # pylint: disable=W0702
   logging.exception("Unexpected exception")
   err = sys.exc_info()
   result = "Caught exception: %s" % str(err[1])
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 21/37] Disable incorect pylint simplify-if-statement warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index aec9d9f..1b1abf8 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -113,7 +113,7 @@ class LUInstanceCreate(LogicalUnit):
 for disk in self.op.disks:
   if self.op.disk_template != constants.DT_EXT:
 utils.ForceDictType(disk, constants.IDISK_PARAMS_TYPES)
-  if constants.IDISK_ADOPT in disk:
+  if constants.IDISK_ADOPT in disk: # pylint: disable=R0102
 has_adopt = True
   else:
 has_no_adopt = True
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 32/37] Remove pylint tests removed in pylint 2.0

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 pylintrc  | 2 --
 pylintrc-test | 2 --
 2 files changed, 4 deletions(-)

diff --git a/pylintrc b/pylintrc
index 47be2b8..342eeff 100644
--- a/pylintrc
+++ b/pylintrc
@@ -20,7 +20,6 @@ evaluation = 10.0 - ((float(5 * error + warning + refactor + 
convention) / state
 comment = yes
 
 [BASIC]
-required-attributes =
 # disabling docstring checks since we have way too many without (complex
 # inheritance hierarchies)
 #no-docstring-rgx = __.*__
@@ -53,7 +52,6 @@ dummy-variables-rgx = _
 additional-builtins =
 
 [CLASSES]
-ignore-iface-methods =
 defining-attr-methods = __init__,__new__,setUp
 valid-classmethod-first-arg = cls,mcs
 
diff --git a/pylintrc-test b/pylintrc-test
index d61735a..686c5ef 100644
--- a/pylintrc-test
+++ b/pylintrc-test
@@ -19,7 +19,6 @@ evaluation = 10.0 - ((float(5 * error + warning + refactor + 
convention) / state
 comment = yes
 
 [BASIC]
-required-attributes =
 # disabling docstring checks since we have way too many without (complex
 # inheritance hierarchies)
 #no-docstring-rgx = __.*__
@@ -51,7 +50,6 @@ dummy-variables-rgx = _
 additional-builtins =
 
 [CLASSES]
-ignore-iface-methods =
 defining-attr-methods = __init__,__new__,setUp
 
 [DESIGN]
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 34/37] Use default value lambda param to avoid cell-var-from-loop

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/tools/burnin.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/tools/burnin.py b/lib/tools/burnin.py
index 3054d37..a32e4db 100755
--- a/lib/tools/burnin.py
+++ b/lib/tools/burnin.py
@@ -677,8 +677,10 @@ class Burner(JobHandler):
 hypervisor=self.hypervisor,
 osparams=self.opts.osparams,
 )
-  remove_instance = lambda name: lambda: self.to_rem.append(name)
-  self.ExecOrQueue(instance, [op], post_process=remove_instance(instance))
+  # NB the i=instance default param is needed here so the lambda captures
+  # the variable. See https://docs.python.org/2/faq/programming.html#id11
+  rm_inst = lambda i=instance: self.to_rem.append(i) # pylint: 
disable=C0322
+  self.ExecOrQueue(instance, [op], post_process=rm_inst)
 
   @_DoBatch(False)
   def BurnModifyRuntimeMemory(self):
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 36/37] Quell the pylint wrong-import-order warnings

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/backend.py| 7 ---
 lib/cli.py| 4 ++--
 lib/cli_opts.py   | 5 +++--
 lib/client/gnt_cluster.py | 8 +---
 lib/client/gnt_debug.py   | 7 ---
 lib/client/gnt_instance.py| 3 ++-
 lib/cmdlib/backup.py  | 3 ++-
 lib/cmdlib/instance_create.py | 2 +-
 lib/http/__init__.py  | 5 +++--
 lib/http/auth.py  | 4 ++--
 lib/http/client.py| 4 +++-
 lib/masterd/iallocator.py | 6 +++---
 lib/objects.py| 3 +--
 lib/rapi/client.py| 7 ---
 lib/rapi/testutils.py | 6 --
 lib/rpc/node.py   | 9 +
 lib/storage/drbd_info.py  | 3 ++-
 lib/tools/common.py   | 4 +++-
 lib/utils/security.py | 5 +++--
 lib/utils/x509.py | 9 +
 20 files changed, 61 insertions(+), 43 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 3cd07d1..634f2e5 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -47,11 +47,12 @@
 
 
 import base64
+import contextlib
+import collections
 import errno
 import logging
 import os
 import os.path
-import pycurl
 import random
 import re
 import shutil
@@ -60,8 +61,8 @@ import stat
 import tempfile
 import time
 import zlib
-import contextlib
-import collections
+
+import pycurl
 
 from ganeti import errors
 from ganeti import http
diff --git a/lib/cli.py b/lib/cli.py
index 8622738..f01d8d9 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -39,7 +39,9 @@ import logging
 import errno
 import itertools
 import shlex
+
 from cStringIO import StringIO
+from optparse import (OptionParser, TitledHelpFormatter)
 
 from ganeti import utils
 from ganeti import errors
@@ -60,8 +62,6 @@ from ganeti.cli_opts import *  # pylint: disable=W0401,W0614
 
 from ganeti.runtime import (GetClient)
 
-from optparse import (OptionParser, TitledHelpFormatter)
-
 
 __all__ = [
   # Generic functions for CLI programs
diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index 6a7a33a..0b1bb0c 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -31,6 +31,9 @@
 """Module containing Ganeti's command line parsing options"""
 
 import re
+
+from optparse import (Option, OptionValueError)
+
 import simplejson
 
 from ganeti import utils
@@ -40,8 +43,6 @@ from ganeti import compat
 from ganeti import pathutils
 from ganeti import serializer
 
-from optparse import (Option, OptionValueError)
-
 
 __all__ = [
   "ABSOLUTE_OPT",
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 9ad82d8..924a3f3 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -35,12 +35,14 @@
 # W0614: Unused import %s from wildcard import (since we need cli)
 # C0103: Invalid name gnt-cluster
 
-from cStringIO import StringIO
+import itertools
 import os
 import time
-import OpenSSL
 import tempfile
-import itertools
+
+from cStringIO import StringIO
+
+import OpenSSL
 
 from ganeti.cli import *
 from ganeti import bootstrap
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index e79ae8d..2393691 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -34,10 +34,11 @@
 # W0614: Unused import %s from wildcard import (since we need cli)
 # C0103: Invalid name gnt-backup
 
-import simplejson
-import time
-import socket
 import logging
+import socket
+import time
+
+import simplejson
 
 from ganeti.cli import *
 from ganeti import cli
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 6280b83..1804c39 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -36,9 +36,10 @@
 
 import copy
 import itertools
-import simplejson
 import logging
 
+import simplejson
+
 from ganeti.cli import *
 from ganeti import opcodes
 from ganeti import constants
diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
index 84ceecf..88e0f0e 100644
--- a/lib/cmdlib/backup.py
+++ b/lib/cmdlib/backup.py
@@ -30,9 +30,10 @@
 
 """Logical units dealing with backup operations."""
 
-import OpenSSL
 import logging
 
+import OpenSSL
+
 from ganeti import compat
 from ganeti import constants
 from ganeti import errors
diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index d69ec50..462522c 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -29,10 +29,10 @@
 
 """Logical unit for creating a single instance."""
 
-import OpenSSL
 import logging
 import os
 
+import OpenSSL
 
 from ganeti import compat
 from ganeti import constants
diff --git a/lib/http/__init__.py b/lib/http/__init__.py
index 596dd3d..1b04c2b 100644
--- a/lib/http/__init__.py
+++ b/lib/http/__init__.py
@@ -31,15 +31,16 @@
 
 """
 
+import errno
 import logging
 import mimetools
-import OpenSSL
 import select
 import socket
-import errno
 
 from cStringIO import StringIO
 
+import OpenSSL
+
 from ganeti import constants
 from ganeti import utils
 
diff --git a/lib/http/auth.py b/lib/http/auth.py
index 

[PATCH stable-2.15 19/37] Disable pylint invalid-name warning

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/cli_opts.py | 2 +-
 lib/compat.py   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index ae58ede..6a7a33a 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -551,7 +551,7 @@ class CliOption(Option):
 
 
 # optparse.py sets make_option, so we do it for our own option class, too
-cli_option = CliOption
+cli_option = CliOption # pylint: disable=C0103
 
 
 _YORNO = "yes|no"
diff --git a/lib/compat.py b/lib/compat.py
index c1f9b83..7aac59e 100644
--- a/lib/compat.py
+++ b/lib/compat.py
@@ -59,12 +59,12 @@ try:
   from hashlib import md5 as md5_hash # pylint: disable=W0611,E0611,F0401
   from hashlib import sha1 as sha1_hash # pylint: disable=W0611,E0611,F0401
   # this additional version is needed for compatibility with the hmac module
-  sha1 = sha1_hash
+  sha1 = sha1_hash # pylint: disable=C0103
 except ImportError:
   from md5 import new as md5_hash # pylint: disable=W0611
   import sha
   sha1 = sha
-  sha1_hash = sha.new
+  sha1_hash = sha.new # pylint: disable=C0103
 
 
 def _all(seq):
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 16/37] Disable pylint warning about use of deprecated pragmas

['Brian Foley' via ganeti-devel]

Autogenerated RPC code like that in metad.py uses pylint: disable-all
to disable unimportant warnings. This option has been deprecated in
favour of the new skip-file option in pylint >= 0.27, but we want to
keep the old option for backward compatibility with earlier pylints.

Signed-off-by: Brian Foley 
---
 Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index d139d8b..3174c2c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,10 +2686,11 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
+# I0022 = disable warning on use of deprecated pragma (disable-all)
 # W0142 = disable star-args warning. pylint 1.6 removed this warn as a bad idea
 # E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
 # and older pylints don't understand newer pylint's options
-LINT_DISABLE = I0013 W0142 E0012
+LINT_DISABLE = I0013 I0022 W0142 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 18/37] Disable pylint import-self warning

['Brian Foley' via ganeti-devel]

In this case it's a deliberate test to make sure that the contents
of lib/ have been correctly installed in a dir called ganeti.

Signed-off-by: Brian Foley 
---
 lib/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/__init__.py b/lib/__init__.py
index 1bb4f1a..d754172 100644
--- a/lib/__init__.py
+++ b/lib/__init__.py
@@ -33,7 +33,7 @@
 """Ganeti python modules"""
 
 try:
-  from ganeti import ganeti
+  from ganeti import ganeti # pylint: disable=W0406
 except ImportError:
   pass
 else:
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 17/37] Disable some pylint unused-import warnings

['Brian Foley' via ganeti-devel]

Technically the imorts are unused in the module, but they're being
imported to make them available to users of the module.

Signed-off-by: Brian Foley 
---
 lib/compat.py | 2 +-
 lib/rapi/testutils.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/compat.py b/lib/compat.py
index e04d68d..c1f9b83 100644
--- a/lib/compat.py
+++ b/lib/compat.py
@@ -61,7 +61,7 @@ try:
   # this additional version is needed for compatibility with the hmac module
   sha1 = sha1_hash
 except ImportError:
-  from md5 import new as md5_hash
+  from md5 import new as md5_hash # pylint: disable=W0611
   import sha
   sha1 = sha
   sha1_hash = sha.new
diff --git a/lib/rapi/testutils.py b/lib/rapi/testutils.py
index 3c09db2..40989dd 100644
--- a/lib/rapi/testutils.py
+++ b/lib/rapi/testutils.py
@@ -49,8 +49,8 @@ import ganeti.rpc.client as rpccl
 from ganeti import rapi
 
 import ganeti.http.server # pylint: disable=W0611
-import ganeti.server.rapi
-import ganeti.rapi.client
+import ganeti.server.rapi # pylint: disable=W0611
+import ganeti.rapi.client # pylint: disable=W0611
 
 
 _URI_RE = re.compile(r"https://(?P.*):(?P\d+)(?P/.*)")
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 15/37] Remove pylint's star-args warning

['Brian Foley' via ganeti-devel]

pylint 1.6 removed this warning as it's perfectly legitimate Python.
Unconditionally switch off the warning when running pylint and
remove the pylint disable=W0142 pragmas.

Signed-off-by: Brian Foley 
---
 Makefile.am   | 3 ++-
 autotools/build-bash-completion   | 4 
 lib/build/sphinx_ext.py   | 3 +--
 lib/cli.py| 4 ++--
 lib/client/gnt_debug.py   | 1 -
 lib/client/gnt_instance.py| 3 +--
 lib/client/gnt_network.py | 1 -
 lib/cmdlib/cluster/verify.py  | 2 +-
 lib/cmdlib/instance_storage.py| 1 -
 lib/cmdlib/instance_utils.py  | 2 +-
 lib/cmdlib/network.py | 4 ++--
 lib/cmdlib/query.py   | 2 --
 lib/compat.py | 2 +-
 lib/config/__init__.py| 2 +-
 lib/errors.py | 1 -
 lib/objects.py| 2 +-
 lib/rapi/baserlib.py  | 2 +-
 lib/rapi/testutils.py | 2 +-
 lib/rpc/node.py   | 2 +-
 lib/server/noded.py   | 5 +
 lib/server/rapi.py| 2 +-
 lib/storage/container.py  | 2 +-
 lib/tools/burnin.py   | 4 ++--
 lib/utils/algo.py | 2 --
 lib/utils/retry.py| 3 ---
 lib/utils/text.py | 3 +--
 lib/workerpool.py | 2 +-
 test/py/cmdlib/testsupport/cmdlib_testcase.py | 2 --
 test/py/testutils/config_mock.py  | 2 +-
 tools/confd-client| 3 ---
 tools/ganeti-listrunner   | 2 +-
 31 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index abaa325..d139d8b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,9 +2686,10 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
+# W0142 = disable star-args warning. pylint 1.6 removed this warn as a bad idea
 # E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
 # and older pylints don't understand newer pylint's options
-LINT_DISABLE = I0013 E0012
+LINT_DISABLE = I0013 W0142 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 3e35ee1..4a064ba 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -703,8 +703,6 @@ def HaskellOptToOptParse(opts, kind):
   kept in sync
 
   """
-  # pylint: disable=W0142
-  # since we pass *opts in a number of places
   opts = opts.split(",")
   if kind == "none":
 return cli.cli_option(*opts, action="store_true")
@@ -761,8 +759,6 @@ def HaskellArgToCliArg(kind, min_cnt, max_cnt):
 max_cnt = None
   else:
 max_cnt = int(max_cnt)
-  # pylint: disable=W0142
-  # since we pass **kwargs
   kwargs = {"min": min_cnt, "max": max_cnt}
 
   if kind.startswith("choices=") or kind.startswith("suggest="):
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 2150288..6b30894 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -281,9 +281,8 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   The expression's result is included as a literal.
 
   """
-  # pylint: disable=W0102,W0613,W0142
+  # pylint: disable=W0102,W0613
   # W0102: Dangerous default value as argument
-  # W0142: Used * or ** magic
   # W0613: Unused argument
 
   code = docutils.utils.unescape(text, restore_backslashes=True)
diff --git a/lib/cli.py b/lib/cli.py
index e29bc02..3c3241d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1732,8 +1732,8 @@ def GenerateTable(headers, fields, separator, data,
   if unitfields is None:
 unitfields = []
 
-  numfields = utils.FieldSet(*numfields)   # pylint: disable=W0142
-  unitfields = utils.FieldSet(*unitfields) # pylint: disable=W0142
+  numfields = utils.FieldSet(*numfields)
+  unitfields = utils.FieldSet(*unitfields)
 
   format_fields = []
   for field in fields:
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index d05fbc2..e79ae8d 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -103,7 +103,6 @@ def GenericOpCodes(opts, args):
 ToStdout("Loading...")
   for job_idx in range(opts.rep_job):
 for fname in args:
-  # pylint: disable=W0142
   op_data = simplejson.loads(utils.ReadFile(fname))
   op_list = [opcodes.OpCode.LoadOpCode(val) for val in op_data]
   op_list = op_list * opts.rep_op
diff --git a/lib/client/gnt_instance.py 

[PATCH stable-2.15 12/37] Cleanup: Remove unused/duplicate module/fn import

['Brian Foley' via ganeti-devel]

No functional change. Found by pylint's reimported warning.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/cluster/__init__.py | 2 +-
 lib/cmdlib/instance_utils.py   | 2 +-
 lib/metad.py   | 4 ++--
 lib/server/rapi.py | 1 -
 tools/move-instance| 2 --
 5 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/cmdlib/cluster/__init__.py b/lib/cmdlib/cluster/__init__.py
index cfe5feb..06dcb84 100644
--- a/lib/cmdlib/cluster/__init__.py
+++ b/lib/cmdlib/cluster/__init__.py
@@ -67,7 +67,7 @@ from ganeti.cmdlib.common import ShareAll, RunPostHook, \
   CheckIpolicyVsDiskTemplates, CheckDiskAccessModeValidity, \
   CheckDiskAccessModeConsistency, GetClientCertDigest, \
   AddInstanceCommunicationNetworkOp, ConnectInstanceCommunicationNetworkOp, \
-  CheckImageValidity, CheckDiskAccessModeConsistency, EnsureKvmdOnNodes
+  CheckImageValidity, EnsureKvmdOnNodes
 
 import ganeti.masterd.instance
 
diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py
index 20cc3d4..3037d9e 100644
--- a/lib/cmdlib/instance_utils.py
+++ b/lib/cmdlib/instance_utils.py
@@ -44,7 +44,7 @@ from ganeti import pathutils
 from ganeti import utils
 from ganeti.cmdlib.common import AnnotateDiskParams, \
   ComputeIPolicyInstanceViolation, CheckDiskTemplateEnabled, \
-  CheckDiskTemplateEnabled, ComputeIPolicySpecViolation
+  ComputeIPolicySpecViolation
 
 
 #: Type description for changes as returned by L{ApplyContainerMods}'s
diff --git a/lib/metad.py b/lib/metad.py
index 48f543e..fbd24f1 100644
--- a/lib/metad.py
+++ b/lib/metad.py
@@ -40,7 +40,7 @@ from ganeti import constants
 from ganeti import errors
 import ganeti.rpc.client as cl
 from ganeti.rpc.transport import Transport
-from ganeti.rpc import errors
+from ganeti.rpc.errors import TimeoutError
 
 
 # If the metadata daemon is disabled, there is no stub generated for it.
@@ -70,7 +70,7 @@ if constants.ENABLE_METAD:
 try:
   self._InitTransport()
   return
-except errors.TimeoutError:
+except TimeoutError:
   logging.debug("Timout trying to connect to MetaD")
   if try_no == retries - 1:
 raise
diff --git a/lib/server/rapi.py b/lib/server/rapi.py
index 9782ada..0626b32 100644
--- a/lib/server/rapi.py
+++ b/lib/server/rapi.py
@@ -61,7 +61,6 @@ from ganeti.rapi import connector
 from ganeti.rapi import baserlib
 
 import ganeti.http.auth   # pylint: disable=W0611
-import ganeti.http.server
 
 
 class RemoteApiRequestContext(object):
diff --git a/tools/move-instance b/tools/move-instance
index 8913f62..4ca6327 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -51,8 +51,6 @@ from ganeti import compat
 from ganeti import rapi
 from ganeti import errors
 
-import ganeti.rapi.client # pylint: disable=W0611
-import ganeti.rapi.client_utils
 from ganeti.rapi.client import UsesRapiClient
 
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 14/37] Disable pylint warning about unrecognised --disable

['Brian Foley' via ganeti-devel]

Allows us to disable warnings that are present in some versions of
pylint but not others.

Signed-off-by: Brian Foley 
---
 Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 8b859c1..abaa325 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,7 +2686,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir 
$(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
-LINT_DISABLE = I0013
+# E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
+# and older pylints don't understand newer pylint's options
+LINT_DISABLE = I0013 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Brian Foley' via ganeti-devel]

Add pycurl to the whitelist of C extensions that can be loaded for the
purposes of attribute checking. This works around a security feature
introduced in pylint 1.4 to avoid arbitrary code injection.

Signed-off-by: Brian Foley 
---
 Makefile.am  | 4 
 configure.ac | 4 
 2 files changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0b30033..8b859c1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2692,6 +2692,10 @@ LINT_OPTS =
 # The combined set of pylint options
 LINT_OPTS_ALL = $(LINT_OPTS) \
   $(addprefix --disable=,$(LINT_DISABLE))
+if !PYLINT_LT_14
+# Whitelist loading pycurl C extension for attribute checking
+LINT_OPTS_ALL += --extension-pkg-whitelist=pycurl
+endif
 
 LINT_TARGETS = pylint pylint-qa pylint-test
 if HAS_PEP8
diff --git a/configure.ac b/configure.ac
index 9b5d06f..f57b98c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -576,6 +576,10 @@ then
   AC_MSG_WARN([pylint not found, checking code will not be possible])
 fi
 
+# Note: Character classes ([...]) need to be double quoted due to autoconf
+# using m4
+AM_CONDITIONAL([PYLINT_LT_14], [$PYLINT --version | grep -q '^pylint 
\(0\.[[0-9]]\|1\.[[0-3]]\.'])
+
 # Check for pep8
 AC_ARG_VAR(PEP8, [pep8 path])
 AC_PATH_PROG(PEP8, [pep8], [])
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 09/37] Cleanup: Iterate dict rather than key list

['Brian Foley' via ganeti-devel]

The later explicitly constructs a new list of the dictionary's keys, which
we don't need here. Quietens pylint's consider-iterating-dictionary warning.

Signed-off-by: Brian Foley 
---
 autotools/check-imports | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autotools/check-imports b/autotools/check-imports
index d82bd40..c90ea70 100755
--- a/autotools/check-imports
+++ b/autotools/check-imports
@@ -62,7 +62,7 @@ def main():
 
   for filename in args:
 # Reset global state
-for name in sys.modules.keys():
+for name in sys.modules:
   if name not in _STANDARD_MODULES:
 sys.modules.pop(name, None)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 10/37] Cleanup: Remove some unneeded pylint disables

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/cmdlib/cluster/verify.py | 1 -
 lib/cmdlib/test.py   | 1 -
 lib/server/noded.py  | 1 -
 3 files changed, 3 deletions(-)

diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py
index dc5bac7..fe9e55e 100644
--- a/lib/cmdlib/cluster/verify.py
+++ b/lib/cmdlib/cluster/verify.py
@@ -141,7 +141,6 @@ class _VerifyErrors(object):
 # Report messages via the feedback_fn
 # pylint: disable=E1101
 self._feedback_fn(constants.ELOG_MESSAGE_LIST, prefixed_list)
-# pylint: enable=E1101
 
 # do not mark the operation as failed for WARN cases only
 if log_type == self.ETYPE_ERROR:
diff --git a/lib/cmdlib/test.py b/lib/cmdlib/test.py
index 5ec7c92..ce83cad 100644
--- a/lib/cmdlib/test.py
+++ b/lib/cmdlib/test.py
@@ -173,7 +173,6 @@ class LUTestDelay(NoHooksLU):
   # Instance of '_socketobject' has no ... member
   conn.settimeout(time_to_go)
   conn.recv(1)
-  # pylint: enable=E1101
 except socket.timeout, _:
   # A second timeout can occur if no data is sent
   return False
diff --git a/lib/server/noded.py b/lib/server/noded.py
index 880f2e1..31588b2 100644
--- a/lib/server/noded.py
+++ b/lib/server/noded.py
@@ -1370,7 +1370,6 @@ def SSLVerifyPeer(conn, cert, errnum, errdepth, ok):
   else:
 logging.error("Invalid errdepth value: %s.", errdepth)
 return False
-  # pylint: enable=W0613
 
 
 def PrepNoded(options, _):
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 04/37] Cleanup: del is a statement not a function

['Brian Foley' via ganeti-devel]

Remove unnecessary parens to make pylint happy.

Signed-off-by: Brian Foley 
---
 lib/hypervisor/hv_kvm/monitor.py | 2 +-
 qa/qa_config.py  | 2 +-
 test/py/ganeti.hypervisor.hv_kvm_unittest.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py
index f2b7d02..98c8a6f 100644
--- a/lib/hypervisor/hv_kvm/monitor.py
+++ b/lib/hypervisor/hv_kvm/monitor.py
@@ -104,7 +104,7 @@ class QmpMessage(object):
 """Delete the specified element from the QmpMessage.
 
 """
-del(self.data[key])
+del self.data[key]
 
   @staticmethod
   def BuildFromJsonString(json_string):
diff --git a/qa/qa_config.py b/qa/qa_config.py
index f84daf8..6035e4f 100644
--- a/qa/qa_config.py
+++ b/qa/qa_config.py
@@ -473,7 +473,7 @@ class _QaConfig(object):
 """Deletes a value from the configuration.
 
 """
-del(self._data[key])
+del self._data[key]
 
   def __len__(self):
 """Return the number of configuration items.
diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py 
b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
index 4f0fbf6..09b74dc 100755
--- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
+++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
@@ -174,7 +174,7 @@ class TestQmpMessage(testutils.GanetiTestCase):
 message = hv_kvm.QmpMessage(test_data)
 
 oldLen = len(message)
-del(message[toDelete])
+del message[toDelete]
 newLen = len(message)
 self.assertEqual(oldLen - 1, newLen)
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 11/37] Cleanup: Fix unidiomatic-typecheck

['Brian Foley' via ganeti-devel]

Signed-off-by: Brian Foley 
---
 lib/cli.py  | 2 +-
 lib/ovf.py  | 2 +-
 lib/rpc_defs.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index c8d8d1d..e29bc02 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2796,7 +2796,7 @@ def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, 
ispecs_cpu_count,
   else:
 forced_type = TISPECS_CLUSTER_TYPES
   for specs in ispecs_transposed.values():
-assert type(specs) is dict
+assert isinstance(specs, dict)
 utils.ForceDictType(specs, forced_type)
 
   # then transpose
diff --git a/lib/ovf.py b/lib/ovf.py
index 4b65cb5..2662ca7 100644
--- a/lib/ovf.py
+++ b/lib/ovf.py
@@ -1278,7 +1278,7 @@ class OVFImporter(Converter):
   options
 
 """
-assert type(self.options.hypervisor) is tuple
+assert isinstance(self.options.hypervisor, tuple)
 assert len(self.options.hypervisor) == 2
 results = {}
 if self.options.hypervisor[0]:
diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py
index 74b74d1..22637c6 100644
--- a/lib/rpc_defs.py
+++ b/lib/rpc_defs.py
@@ -147,7 +147,7 @@ def _NodeInfoPreProc(node, args):
   assert len(args) == 2
   # The storage_units argument is either a dictionary with one value for each
   # node, or a fixed value to be used for all the nodes
-  if type(args[0]) is dict:
+  if isinstance(args[0], dict):
 return [args[0][node], args[1]]
   else:
 return args
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 06/37] Cleanup: Fix for/else with no break in AddAuthorizedKeys

['Brian Foley' via ganeti-devel]

After a refactoring in c178396, the only break statement in the for/else
loop in AddAuthorizedKeys was removed. This means the else block is
always executed, so remove the else: to quell a pylint warning.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/ssh.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/ssh.py b/lib/ssh.py
index 7d34f29..6e16674 100644
--- a/lib/ssh.py
+++ b/lib/ssh.py
@@ -170,13 +170,13 @@ def AddAuthorizedKeys(file_obj, keys):
in key_field_list
if split_key != line_key]
   nl = line.endswith("\n")
-else:
-  if not nl:
-f.write("\n")
-  for (key, _) in key_field_list:
-f.write(key.rstrip("\r\n"))
-f.write("\n")
-  f.flush()
+
+if not nl:
+  f.write("\n")
+for (key, _) in key_field_list:
+  f.write(key.rstrip("\r\n"))
+  f.write("\n")
+f.flush()
   finally:
 f.close()
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 07/37] Cleanup: StartInstance and RebootInstance return None

['Brian Foley' via ganeti-devel]

Quell assignment-from-none warning

Signed-off-by: Brian Foley 
---
 lib/backend.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 82aa493..3cd07d1 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -2988,9 +2988,8 @@ def InstanceReboot(instance, reboot_type, 
shutdown_timeout, reason):
   elif reboot_type == constants.INSTANCE_REBOOT_HARD:
 try:
   InstanceShutdown(instance, shutdown_timeout, reason, store_reason=False)
-  result = StartInstance(instance, False, reason, store_reason=False)
+  StartInstance(instance, False, reason, store_reason=False)
   _StoreInstReasonTrail(instance.name, reason)
-  return result
 except errors.HypervisorError, err:
   _Fail("Failed to hard reboot instance '%s': %s", instance.name, err)
   else:
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 08/37] Cleanup: Remove unused format key

['Brian Foley' via ganeti-devel]

No functional change, but quietens pylint unused-format-string-argument
warning.

Signed-off-by: Brian Foley 
---
 lib/objects.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/objects.py b/lib/objects.py
index 96e7092..29dbbbc 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -2061,8 +2061,7 @@ class Cluster(TaggableObject):
   - at public visibility:  {public}
   - at private visibility: {private}
   - at secret visibility:  {secret}
-  """.format(dupes=formatter(duplicate_keys),
- public=formatter(params_public & duplicate_keys),
+  """.format(public=formatter(params_public & duplicate_keys),
  private=formatter(params_private & duplicate_keys),
  secret=formatter(params_secret & duplicate_keys))
   raise errors.OpPrereqError(msg)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

['Brian Foley' via ganeti-devel]

No functional change. Found by pylint's simplifiable-if-statement.

Signed-off-by: Brian Foley 
---
 lib/client/gnt_cluster.py | 7 +--
 tools/cluster-merge   | 5 +
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 40792e2..9ad82d8 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -294,11 +294,6 @@ def InitCluster(opts, args):
 
   default_ialloc_params = opts.default_iallocator_params
 
-  if opts.enabled_user_shutdown:
-enabled_user_shutdown = True
-  else:
-enabled_user_shutdown = False
-
   bootstrap.InitCluster(cluster_name=args[0],
 secondary_ip=opts.secondary_ip,
 vg_name=vg_name,
@@ -332,7 +327,7 @@ def InitCluster(opts, args):
 install_image=install_image,
 zeroing_image=zeroing_image,
 compression_tools=compression_tools,
-enabled_user_shutdown=enabled_user_shutdown,
+enabled_user_shutdown=opts.enabled_user_shutdown,
 )
   op = opcodes.OpClusterPostInit()
   SubmitOpCode(op, opts=opts)
diff --git a/tools/cluster-merge b/tools/cluster-merge
index 926b705..c83e350 100755
--- a/tools/cluster-merge
+++ b/tools/cluster-merge
@@ -448,10 +448,7 @@ class Merger(object):
   check_params_strict.append("shared_file_storage_dir")
 check_params.extend(check_params_strict)
 
-if self.params == _PARAMS_STRICT:
-  params_strict = True
-else:
-  params_strict = False
+params_strict = (self.params == _PARAMS_STRICT)
 
 for param_name in check_params:
   my_param = getattr(my_cluster, param_name)
-- 
2.8.0.rc3.226.g39d4020

[PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

['Brian Foley' via ganeti-devel]

Ganeti's python code passes the pylint checks in the version of pylint
included with Debian Jessie. Unfortunately this is a really old pylint
(0.26 from 2012) and the latest stable pylint (1.6.4 from 2016) has a
lot of additional checks. Ganeti produces many many warnings and errors
when run against 1.6.4.

This patchset is a first cut at fixing these, while still maintaining
 compatibility with pylint 0.26.

Brian Foley (37):
  Cleanup: Remove some unnecessary if (...) parens
  Cleanup: Simplify boolean assignment
  Cleanup: Use FOO not in BAR instead of not FOO in BAR
  Cleanup: del is a statement not a function
  Cleanup: Replace map/filters with list comprehensions
  Cleanup: Fix for/else with no break in AddAuthorizedKeys
  Cleanup: StartInstance and RebootInstance return None
  Cleanup: Remove unused format key
  Cleanup: Iterate dict rather than key list
  Cleanup: Remove some unneeded pylint disables
  Cleanup: Fix unidiomatic-typecheck
  Cleanup: Remove unused/duplicate module/fn import
  Fix pylint >1.4 pycurl no-member warnings
  Disable pylint warning about unrecognised --disable
  Remove pylint's star-args warning
  Disable pylint warning about use of deprecated pragmas
  Disable some pylint unused-import warnings
  Disable pylint import-self warning
  Disable pylint invalid-name warning
  Disable pylint eval-used warning
  Disable incorect pylint simplify-if-statement warning
  Disable pylint misplaced-comparison-constant warning
  Disable pylint unpacking-non-sequence warning
  Disable incorrect pylint not-callable warning
  Disable pylint unused-wildcard-import warning
  Disable unwanted pylint wrong-import-position warnings
  Disable pylint bare-except warning
  Disable pylint unsubscriptable-object warning
  Quell consider-using-enumerate warning
  Quell bad-whitespace warning
  Quell trailing newline
  Remove pylint tests removed in pylint 2.0
  Quell too-many-boolean-expressions
  Use default value lambda param to avoid cell-var-from-loop
  Quell cell-var-from-loop warning
  Quell the pylint wrong-import-order warnings
  Quell pylint socket.timeout warning

 Makefile.am   | 10 -
 autotools/build-bash-completion   | 14 +
 autotools/build-rpc   |  2 +-
 autotools/check-imports   |  5 +++--
 configure.ac  |  4 
 lib/__init__.py   |  2 +-
 lib/backend.py| 10 -
 lib/build/sphinx_ext.py   | 10 +
 lib/cli.py| 30 +--
 lib/cli_opts.py   |  7 ---
 lib/client/gnt_cluster.py | 15 ++
 lib/client/gnt_debug.py   |  8 +++
 lib/client/gnt_instance.py|  6 +++---
 lib/client/gnt_network.py |  1 -
 lib/cmdlib/backup.py  |  3 ++-
 lib/cmdlib/cluster/__init__.py|  2 +-
 lib/cmdlib/cluster/verify.py  |  9 
 lib/cmdlib/instance.py|  6 +++---
 lib/cmdlib/instance_create.py | 12 +--
 lib/cmdlib/instance_migration.py  |  3 ++-
 lib/cmdlib/instance_storage.py|  5 ++---
 lib/cmdlib/instance_utils.py  |  4 ++--
 lib/cmdlib/network.py |  4 ++--
 lib/cmdlib/query.py   |  2 --
 lib/cmdlib/test.py|  1 -
 lib/compat.py |  8 +++
 lib/config/__init__.py| 13 ++--
 lib/config/verify.py  |  4 ++--
 lib/errors.py |  1 -
 lib/http/__init__.py  |  5 +++--
 lib/http/auth.py  |  4 ++--
 lib/http/client.py|  6 --
 lib/hypervisor/hv_kvm/__init__.py |  4 ++--
 lib/hypervisor/hv_kvm/monitor.py  |  4 ++--
 lib/hypervisor/hv_xen.py  |  4 ++--
 lib/jqueue/__init__.py|  2 +-
 lib/luxi.py   |  9 
 lib/masterd/iallocator.py |  8 +++
 lib/metad.py  |  4 ++--
 lib/objects.py| 10 -
 lib/ovf.py|  5 +++--
 lib/rapi/baserlib.py  |  2 +-
 lib/rapi/client.py|  7 ---
 lib/rapi/testutils.py | 12 ++-
 lib/rpc/node.py   | 16 +++---
 lib/rpc_defs.py   |  2 +-
 lib/server/masterd.py |  2 +-
 lib/server/noded.py   |  6 +-
 lib/server/rapi.py

[PATCH stable-2.15 05/37] Cleanup: Replace map/filters with list comprehensions

['Brian Foley' via ganeti-devel]

No functional change, but more pythonic and quells pylint's
 deprecated-lambda warning.

Signed-off-by: Brian Foley 
---
 lib/cli.py | 14 ++
 lib/cmdlib/cluster/verify.py   |  6 +++---
 lib/cmdlib/instance_storage.py |  2 +-
 lib/config/__init__.py | 11 +--
 lib/http/client.py |  2 +-
 lib/hypervisor/hv_xen.py   |  4 ++--
 lib/jqueue/__init__.py |  2 +-
 lib/luxi.py|  9 +
 lib/rpc/node.py|  5 ++---
 lib/storage/bdev.py| 11 +--
 lib/storage/drbd.py|  4 ++--
 lib/storage/filestorage.py |  6 +++---
 lib/tools/node_cleanup.py  |  5 ++---
 lib/utils/livelock.py  |  4 ++--
 lib/utils/text.py  |  2 +-
 15 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 2971c60..c8d8d1d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2358,10 +2358,9 @@ def GetNodesSshPorts(nodes, cl):
   @rtype: a list of tuples
 
   """
-  return map(lambda t: t[0],
- cl.QueryNodes(names=nodes,
-   fields=["ndp/ssh_port"],
-   use_locking=False))
+  return [t[0] for t in cl.QueryNodes(names=nodes,
+  fields=["ndp/ssh_port"],
+  use_locking=False)]
 
 
 def GetNodeUUIDs(nodes, cl):
@@ -2375,10 +2374,9 @@ def GetNodeUUIDs(nodes, cl):
   @rtype: a list of tuples
 
   """
-  return map(lambda t: t[0],
- cl.QueryNodes(names=nodes,
-   fields=["uuid"],
-   use_locking=False))
+  return [t[0] for t in cl.QueryNodes(names=nodes,
+  fields=["uuid"],
+  use_locking=False)]
 
 
 def _ToStream(stream, txt, *args):
diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py
index 77358dc..dc5bac7 100644
--- a/lib/cmdlib/cluster/verify.py
+++ b/lib/cmdlib/cluster/verify.py
@@ -685,7 +685,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
 # exclusive_storage wants all PVs to have the same size (approximately),
 # if the smallest and the biggest ones are okay, everything is fine.
 # pv_min is None iff pv_max is None
-vals = filter((lambda ni: ni.pv_min is not None), node_image.values())
+vals = [ni for ni in node_image.values() if ni.pv_min is not None]
 if not vals:
   return
 (pvmin, minnode_uuid) = min((ni.pv_min, ni.uuid) for ni in vals)
@@ -1972,8 +1972,8 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
 if self._exclusive_storage:
   node_verify_param[constants.NV_EXCLUSIVEPVS] = True
 
-node_group_uuids = dict(map(lambda n: (n.name, n.group),
-self.cfg.GetAllNodesInfo().values()))
+node_group_uuids = dict((n.name, n.group) for n in
+self.cfg.GetAllNodesInfo().values())
 groups_config = self.cfg.GetAllNodeGroupsInfoDict()
 
 # At this point, we have the in-memory data structures complete,
diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 92f5118..5508eb8 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -1347,7 +1347,7 @@ def ImageDisks(lu, instance, image, disks=None):
   if disks is None:
 disks = [(0, inst_disks[0])]
   else:
-disks = map(lambda idx: (idx, inst_disks[idx]), disks)
+disks = [(idx, inst_disks[idx]) for idx in disks]
 
   logging.info("Pausing synchronization of disks of instance '%s'",
instance.name)
diff --git a/lib/config/__init__.py b/lib/config/__init__.py
index 7011da2..954a09c 100644
--- a/lib/config/__init__.py
+++ b/lib/config/__init__.py
@@ -1253,8 +1253,7 @@ class ConfigWriter(object):
 if self._offline:
   raise errors.ProgrammerError("Can't call ComputeDRBDMap in offline mode")
 else:
-  return dict(map(lambda (k, v): (k, dict(v)),
-  self._wconfd.ComputeDRBDMap()))
+  return dict((k, dict(v)) for (k, v) in self._wconfd.ComputeDRBDMap())
 
   def AllocateDRBDMinor(self, node_uuids, disk_uuid):
 """Allocate a drbd minor.
@@ -1723,8 +1722,8 @@ class ConfigWriter(object):
 dictionaries.
 
 """
-return dict(map(lambda (uuid, ng): (uuid, ng.ToDict()),
-self._UnlockedGetAllNodeGroupsInfo().items()))
+return dict((uuid, ng.ToDict()) for (uuid, ng) in
+self._UnlockedGetAllNodeGroupsInfo().items())
 
   @ConfigSync(shared=1)
   def GetNodeGroupList(self):
@@ -2016,7 +2015,7 @@ class ConfigWriter(object):
 
 if expanded_name is not None:
   # there has to be exactly one instance with that name
-  inst = (filter(lambda n: n.name == expanded_name, all_insts)[0])
+  inst = [n for n in all_insts if n.name == expanded_name][0]
   return (inst.uuid, inst.name)
 else:
   return