[virt-tools-list] [virt-manager PATCH 3/3] Rework disk target assignment

[Cole Robinson]

On 02/12/2014 12:18 PM, Martin Kletzander wrote:
> Using previous patches, this one makes the disk target generation a
> bit more intelligent, so adding multiple disks with various
> controllers is not a problem anymore.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1036716
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  virtManager/addhardware.py | 48 +++++++++++++++++++++++++++++-----------------
>  virtinst/devicedisk.py     |  3 ++-
>  2 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> index 121c75b..ecb5fc8 100644
> --- a/virtManager/addhardware.py
> +++ b/virtManager/addhardware.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2006-2007, 2013, 2014 Red Hat, Inc.
> +# Copyright (C) 2006-2007, 2012-2014 Red Hat, Inc.
>  # Copyright (C) 2006 Hugh O. Brock <hbrock at redhat.com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -20,6 +20,7 @@
> 
>  import logging
>  import traceback
> +import collections
> 
>  # pylint: disable=E0611
>  from gi.repository import Gtk
> @@ -1412,34 +1413,45 @@ class vmmAddHardware(vmmGObjectUI):
>              self._dev.validate()
>          return ret
> 
> -    def _set_disk_controller(self, disk, controller_model):
> +    def _set_disk_controller(self, disk, controller_model, used_disks):
>          # Add a SCSI controller with model virtio-scsi if needed
>          disk.vmm_controller = None
>          if controller_model != "virtio-scsi":
> -            return
> +            return None
> 
> +        # Get SCSI controllers
>          controllers = self.vm.get_controller_devices()
>          ctrls_scsi = [x for x in controllers if
>                  (x.type == VirtualController.TYPE_SCSI)]
> -        if len(ctrls_scsi) > 0:
> -            index_new = max([x.index for x in ctrls_scsi]) + 1
> -        else:
> -            index_new = 0
> 
> +        # Create possible new controller
>          controller = VirtualController(self.conn.get_backend())
>          controller.type = "scsi"
>          controller.model = controller_model
> -        disk.vmm_controller = controller
> -        for d in controllers:
> -            if controller.type == d.type:
> -                controller.index = index_new
> -            if controller_model == d.model:
> -                disk.vmm_controller = None
> -                controller = d
> +
> +        # And set its index
> +        controller.index = 0
> +        if ctrls_scsi:
> +            controller.index = max([x.index for x in ctrls_scsi]) + 1
> +
> +        # Take only virtio-scsi ones
> +        ctrls_scsi = [x for x in ctrls_scsi
> +                      if x.model == controller_model]
> +
> +        # Save occupied places per controller
> +        occupied = collections.defaultdict(int)
> +        for d in used_disks:
> +            if d.bus == disk.bus:
> +                num = virtinst.VirtualDisk.target_to_num(d.target)
> +                occupied[num / 7] += 1
> +        for c in ctrls_scsi:
> +            if occupied[c.index] < 7:
> +                controller = c
>                  break
> +        else:
> +            disk.vmm_controller = controller
> 
> -        disk.address.type = disk.address.ADDRESS_TYPE_DRIVE
> -        disk.address.controller = controller.index
> +        return controller.index
> 
>      def validate_page_storage(self):
>          bus = self.get_config_disk_bus()
> @@ -1471,9 +1483,9 @@ class vmmAddHardware(vmmGObjectUI):
>                  for d in disks:
>                      used.append(d.target)
> 
> -                disk.generate_target(used)
> +            prefer_ctrl = self._set_disk_controller(disk, controller_model, disks)
> +            disk.generate_target(used, prefer_ctrl)
> 
> -            self._set_disk_controller(disk, controller_model)
>          except Exception, e:
>              return self.err.val_err(_("Storage parameter error."), e)
> 
> diff --git a/virtinst/devicedisk.py b/virtinst/devicedisk.py
> index 935add0..28c55ba 100644
> --- a/virtinst/devicedisk.py
> +++ b/virtinst/devicedisk.py
> @@ -480,7 +480,8 @@ class VirtualDisk(VirtualDevice):
>          Convert disk /dev number (like hda, hdb, hdaa, etc.) to an index
>          """
>          num = 0
> -        if tgt[0] == 'x'
> +        if tgt[0] == 'x':
> +            # This case is here for 'xvda'
>              tgt = tgt[1:]
>          for i, c in enumerate(reversed(tgt[2:])):
>              num += (ord(c) - ord('a') + 1) * (26 ** i)
> 

This bit should be squashed into patch #1. ACK series with that change

- Cole