* Apollon Oikonomopoulos <[email protected]> [2015-10-02 13:07:37 +0300]:

> On 19:52 Thu 01 Oct     , Dimitris Aragiorgis wrote:
> > Replace pci slot with hvinfo in Disk and NIC config Objects. This
> > will contain all the necessary info to construct a -device option.
> > Specifically, for PCI devices it will have driver, id, bus, and addr
> > fields. SCSI devices will have driver, id, bus, channel, scsi-id,
> > and lun fields.
> > 
> > Change the way we generate the device ID so that it gets derived
> > only from the type and the UUID of the device. We ensure seamless
> > migration by upgrading the existing runtime files in
> > _UpgradeSerializedRuntime(). All devices found with a 'pci' slot will
> > get an 'hvinfo' dict with the old id, bus 'pci.0', and addr based on
> > the old value of 'pci'.
> > 
> > Change the way we put devices into buses. Guessing the default
> > reserved PCI slots of an instance has proven to be error-prone, since
> > they may change from version to version, and also because the PCI bus
> > state may be modified with a specific option passed by the user with the
> > kvm_extra hvpamam. Therefore, we decide to start adding PCI devices from
> > the 9th slot and forward. The SCSI bus does not need to have default
> 
>       ^ This is in contrast with the design doc (and the following code 
>         AFAICT)
> 

True. We have decided to go with 12. I will change it everywhere
including the commit messages.

> > reservations.
> > 
> > Let QEMU decide the PCI slot of spice, balloon and SCSI controller
> > devices. Finally, QEMU adds by default an LSI controller in case a
> > SCSI disk is attached to the instance. Here, we set it explicitly,
> > and name the created bus 'scsi.0' (as QEMU does by default).
> > 
> > Until now, only SCSI emulated disks were supported. Allow
> > scsi-generic, scsi-block, scsi-hd, and scsi-cd for the disk_type
> > hvparams. The first two do SCSI pass-through and thus require a
> > physical SCSI device on the host. The third is the equivalent of
> > if=scsi, which means that the virtual disk of the instance will be an
> > emulated SCSI device.
> > 
> > Signed-off-by: Dimitris Aragiorgis <[email protected]>
> > ---
> >  doc/design-scsi-kvm.rst                      |    4 +-
> >  lib/hypervisor/hv_kvm/__init__.py            |  304 
> > ++++++++++++++++++++------
> >  lib/objects.py                               |    3 +-
> >  src/Ganeti/Constants.hs                      |   25 ++-
> >  src/Ganeti/Types.hs                          |    4 +-
> >  test/data/kvm_runtime.json                   |   21 +-
> >  test/py/ganeti.hypervisor.hv_kvm_unittest.py |   53 ++++-
> >  7 files changed, 332 insertions(+), 82 deletions(-)
> > 
> > diff --git a/doc/design-scsi-kvm.rst b/doc/design-scsi-kvm.rst
> > index 30cc7de..42968db 100644
> > --- a/doc/design-scsi-kvm.rst
> > +++ b/doc/design-scsi-kvm.rst
> > @@ -219,8 +219,8 @@ disk_type to `scsi-hd`. The QEMU command line will 
> > include:
> >  
> >    ::
> >  
> > -    -drive 
> > file=/var/run/ganeti/instance-disks/test:0,if=none,format=raw,id=hotdisk-12345
> > -    -device 
> > scsi-hd,id=hotdisk-12345,drive=hotdisk-12345,bus=scsi.0,channel=0,scsi-id=0,lun=0
> > +    -drive 
> > file=/var/run/ganeti/instance-disks/test:0,if=none,format=raw,id=disk-12345
> > +    -device 
> > scsi-hd,id=disk-12345,drive=disk-12345,bus=scsi.0,channel=0,scsi-id=0,lun=0
> >  
> >  
> >  User interface
> > diff --git a/lib/hypervisor/hv_kvm/__init__.py 
> > b/lib/hypervisor/hv_kvm/__init__.py
> > index 340ddb1..cf56943 100644
> > --- a/lib/hypervisor/hv_kvm/__init__.py
> > +++ b/lib/hypervisor/hv_kvm/__init__.py
> > @@ -109,6 +109,32 @@ _RUNTIME_ENTRY = {
> >    constants.HOTPLUG_TARGET_DISK: lambda d, e: (d, e[0], e[1])
> >    }
> >  
> > +_DEVICE_TYPE = {
> > +  constants.HOTPLUG_TARGET_NIC: lambda hvp: hvp[constants.HV_NIC_TYPE],
> > +  constants.HOTPLUG_TARGET_DISK: lambda hvp: hvp[constants.HV_DISK_TYPE],
> > +  }
> > +
> > +_DEVICE_DRIVER = {
> > +  constants.HOTPLUG_TARGET_NIC:
> > +    lambda ht: "virtio-net-pci" if ht == constants.HT_NIC_PARAVIRTUAL else 
> > ht,
> > +  constants.HOTPLUG_TARGET_DISK:
> > +    lambda ht: "virtio-blk-pci" if ht == constants.HT_DISK_PARAVIRTUAL 
> > else ht,
> > +  }
> > +
> > +
> > +# NICs and paravirtual disks
> > +# show up as devices on the PCI bus (one slot per device).
> > +# SCSI disks will be placed on the SCSI bus.
> > +_DEVICE_BUS = {
> > +  constants.HOTPLUG_TARGET_NIC:
> > +    lambda _: _PCI_BUS,
> > +  constants.HOTPLUG_TARGET_DISK:
> > +    lambda ht: _SCSI_BUS if ht in constants.HT_SCSI_DEVICE_TYPES else 
> > _PCI_BUS
> > +  }
> > +
> > +_PCI_BUS = "pci.0"
> > +_SCSI_BUS = "scsi.0"
> > +
> >  _MIGRATION_CAPS_DELIM = ":"
> >  
> >  
> > @@ -155,23 +181,91 @@ def _GetDriveURI(disk, link, uri):
> >  def _GenerateDeviceKVMId(dev_type, dev):
> >    """Helper function to generate a unique device name used by KVM
> >  
> > -  QEMU monitor commands use names to identify devices. Here we use their 
> > pci
> > -  slot and a part of their UUID to name them. dev.pci might be None for old
> > -  devices in the cluster.
> > +  QEMU monitor commands use names to identify devices. Since the UUID
> > +  is too long for a device ID (37 chars vs. 30), we choose to use
> > +  only the part until the first '-' with a disk/nic prefix.
> >  
> > -  @type dev_type: sting
> > -  @param dev_type: device type of param dev
> > +  @type dev_type: string
> > +  @param dev_type: device type of param dev (HOTPLUG_TARGET_DISK|NIC)
> >    @type dev: L{objects.Disk} or L{objects.NIC}
> >    @param dev: the device object for which we generate a kvm name
> > -  @raise errors.HotplugError: in case a device has no pci slot (old 
> > devices)
> >  
> >    """
> > +  return "%s-%s" % (dev_type.lower(), dev.uuid.split("-")[0])
> > +
> > +
> > +def _GenerateDeviceHVInfoStr(hvinfo):
> > +  """Construct the -device option string for hvinfo dict
> > +
> > +  PV disk: virtio-blk-pci,id=disk-1234,bus=pci.0,addr=0x9
> > +  PV NIC:  virtio-net-pci,id=nic-1234,bus=pci.0,addr=0x9
> > +  SG disk: scsi-generic,id=disk-1234,bus=scsi.0,channel=0,scsi-id=1,lun=0
> > +
> > +  @type hvinfo: dict
> > +  @param hvinfo: dictionary created by _GenerateDeviceHVInfo()
> > +
> > +  @rtype: string
> > +  @return: The constructed string to be passed along with a -device option
> > +
> > +  """
> > +
> > +  # work on a copy
> > +  d = dict(hvinfo)
> > +  hvinfo_str = d.pop("driver")
> > +  for k, v in d.items():
> > +    hvinfo_str += ",%s=%s" % (k, v)
> > +
> > +  return hvinfo_str
> > +
> > +
> > +def _GenerateDeviceHVInfo(dev_type, kvm_devid, hv_dev_type, bus_slots):
> > +  """Helper function to generate hvinfo of a device (disk, NIC)
> > +
> > +  hvinfo will hold all necessary info for generating the -device QEMU 
> > option.
> > +  We have two main buses: a PCI bus and a SCSI bus (created by a SCSI
> > +  controller on the PCI bus).
> > +
> > +  In case of PCI devices we add them on a free PCI slot (addr) on the 
> > first PCI
> > +  bus (pci.0), and in case of SCSI devices we decide to put each disk on a
> > +  different SCSI target (scsi-id) on the first SCSI bus (scsi.0).
> > +
> > +  @type dev_type: string
> > +  @param dev_type: either HOTPLUG_TARGET_DISK or HOTPLUG_TARGET_NIC
> > +  @type kvm_devid: string
> > +  @param kvm_devid: the id of the device
> > +  @type hv_dev_type: string
> > +  @param hv_dev_type: either disk_type or nic_type hvparam
> > +  @type bus_slots: dict
> > +  @param bus_slots: the current slots of the first PCI and SCSI buses
> > +
> > +  @rtype: dict
> > +  @return: dict including all necessary info (driver, id, bus and bus 
> > location)
> > +           for generating a -device QEMU option for either a disk or a NIC
> > +
> > +  """
> > +  driver = _DEVICE_DRIVER[dev_type](hv_dev_type)
> > +  bus = _DEVICE_BUS[dev_type](hv_dev_type)
> > +  slots = bus_slots[bus]
> > +  slot = utils.GetFreeSlot(slots, reserve=True)
> > +
> > +  hvinfo = {
> > +    "driver": driver,
> > +    "id": kvm_devid,
> > +    "bus": bus,
> > +    }
> >  
> > -  if not dev.pci:
> > -    raise errors.HotplugError("Hotplug is not supported for %s with UUID 
> > %s" %
> > -                              (dev_type, dev.uuid))
> > +  if bus == _PCI_BUS:
> > +    hvinfo.update({
> > +      "addr": hex(slot),
> > +      })
> > +  elif bus == _SCSI_BUS:
> > +    hvinfo.update({
> > +      "channel": 0,
> > +      "scsi-id": slot,
> > +      "lun": 0,
> > +      })
> >  
> > -  return "%s-%s-pci-%d" % (dev_type.lower(), dev.uuid.split("-")[0], 
> > dev.pci)
> > +  return hvinfo
> >  
> >  
> >  def _GetExistingDeviceInfo(dev_type, device, runtime):
> > @@ -220,10 +314,38 @@ def _UpgradeSerializedRuntime(serialized_runtime):
> >    else:
> >      serialized_disks = []
> >  
> > +  def update_hvinfo(dev, dev_type):
> > +    """ Remove deprecated pci slot and substitute it with hvinfo """
> > +    if "hvinfo" not in dev:
> > +      dev["hvinfo"] = {}
> > +      uuid = dev["uuid"]
> > +      # Ganeti used to save the PCI slot of paravirtual devices
> > +      # (virtio-blk-pci, virtio-net-pci) in runtime files during
> > +      # _GenerateKVMRuntime() and HotAddDevice().
> > +      # In this case we had a -device QEMU option in the command line with 
> > id,
> > +      # drive|netdev, bus, and addr params. All other devices did not have 
> > an
> > +      # id nor placed explicitly on a bus.
> > +      # hot- prefix is removed in 2.16. Here we add it explicitly to
> > +      # handle old instances in the cluster properly.
> > +      if "pci" in dev:
> > +        # This is practically the old _GenerateDeviceKVMId()
> > +        dev["hvinfo"]["id"] = "hot%s-%s-%s-%s" % (dev_type.lower(),
> > +                                                  uuid.split("-")[0],
> > +                                                  "pci",
> > +                                                  dev["pci"])
> > +        dev["hvinfo"]["addr"] = hex(dev["pci"])
> > +        dev["hvinfo"]["bus"] = _PCI_BUS
> > +        del dev["pci"]
> 
>             ^ You will want to preserve this for downgrades (and 
>               possibly store it for new devices as well)
> 

No need to keep this value around. It can be generated
deterministically, thus the downgrade code should restore it
explicitly.

> > +
> >    for nic in serialized_nics:
> >      # Add a dummy uuid slot if an pre-2.8 NIC is found
> >      if "uuid" not in nic:
> >        nic["uuid"] = utils.NewUUID()
> > +    update_hvinfo(nic, constants.HOTPLUG_TARGET_NIC)
> > +
> > +  for disk_entry in serialized_disks:
> > +    # We have a (Disk, link, uri) tuple
> > +    update_hvinfo(disk_entry[0], constants.HOTPLUG_TARGET_DISK)
> >  
> >    return kvm_cmd, serialized_nics, hvparams, serialized_disks
> >  
> > @@ -405,8 +527,9 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >    _NETDEV_RE = re.compile(r"^-netdev\s", re.M)
> >    _DISPLAY_RE = re.compile(r"^-display\s", re.M)
> >    _MACHINE_RE = re.compile(r"^-machine\s", re.M)
> > -  _VIRTIO_NET_RE = re.compile(r"^name \"%s\"" % _VIRTIO_NET_PCI, re.M)
> > -  _VIRTIO_BLK_RE = re.compile(r"^name \"%s\"" % _VIRTIO_BLK_PCI, re.M)
> > +  _DEVICE_DRIVER_SUPPORTED = \
> > +    staticmethod(lambda drv, devlist:
> > +                 re.compile(r"^name \"%s\"" % drv, re.M).search(devlist))
> >    # match  -drive.*boot=on|off on different lines, but in between accept 
> > only
> >    # dashes not preceeded by a new line (which would mean another option
> >    # different than -drive is starting)
> > @@ -418,8 +541,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >    _INFO_VERSION_CMD = "info version"
> >  
> >    # Slot 0 for Host bridge, Slot 1 for ISA bridge, Slot 2 for VGA 
> > controller
> > -  _DEFAULT_PCI_RESERVATIONS = "11100000000000000000000000000000"
> > -  _SOUNDHW_WITH_PCI_SLOT = ["ac97", "es1370", "hda"]
> > +  # and the rest up to slot 16 will be used by QEMU implicitly.
> > +  # Ganeti will add disks and NICs from the 17th slot onwards.
> > +  _DEFAULT_PCI_RESERVATIONS = "11111111111111110000000000000000"
> > +  # The SCSI bus is created on demand or automatically and is empty.
> > +  # For simplicity we decide to use a different target (scsi-id)
> > +  # for each SCSI disk.
> > +  _DEFAULT_SCSI_RESERVATIONS = "0000000000000000"
> 
> Different SCSI controllers have different max device counts (7 for lsi, 
> 64 for megasas, 255 for virtio-block-pci) and obviously we are capped by 
> constants.MAX_DISKS.
> 

As you say, we are already capped by constants.MAX_DISKS.
Provided the user has set the scsi_controller_type appropriately, this
value of _DEFAULT_SCSI_RESERVATIONS imposes no further limit.

> >  
> >    ANCILLARY_FILES = [
> >      _KVM_NETWORK_SCRIPT,
> > @@ -898,19 +1026,23 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >      needs_boot_flag = self._BOOT_RE.search(kvmhelp)
> >  
> >      dev_opts = []
> > -    device_driver = None
> >      disk_type = up_hvp[constants.HV_DISK_TYPE]
> > +    # paravirtual implies either '-device virtio-blk-pci... -drive 
> > if=none...'
> > +    # for new QEMU versions or '-drive if=virtio' for old QEMU versions
> >      if disk_type == constants.HT_DISK_PARAVIRTUAL:
> > -      if_val = ",if=%s" % self._VIRTIO
> > -      try:
> > -        if self._VIRTIO_BLK_RE.search(devlist):
> > -          if_val = ",if=none"
> > -          # will be passed in -device option as driver
> > -          device_driver = self._VIRTIO_BLK_PCI
> > -      except errors.HypervisorError, _:
> > -        pass
> > +      driver = self._VIRTIO_BLK_PCI
> > +      iface = self._VIRTIO
> > +    else:
> > +      driver = iface = disk_type
> > +
> > +    # Check if a specific driver is supported by QEMU device model.
> > +    if self._DEVICE_DRIVER_SUPPORTED(driver, devlist):
> > +      if_val = ",if=none" # for the -drive option
> > +      device_driver = driver # for the -device option
> >      else:
> > -      if_val = ",if=%s" % disk_type
> > +      if_val = ",if=%s" % iface # for the -drive option
> > +      device_driver = None # without -device option
> > +
> >      # AIO mode
> >      aio_mode = up_hvp[constants.HV_KVM_DISK_AIO]
> >      if aio_mode == constants.HT_KVM_AIO_NATIVE:
> > @@ -947,16 +1079,16 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >        drive_val = "file=%s,format=raw%s%s%s%s" % \
> >                    (drive_uri, if_val, boot_val, cache_val, aio_val)
> >  
> > -      if device_driver:
> > -        # kvm_disks are the 4th entry of runtime file that did not exist in
> > -        # the past. That means that cfdev should always have pci slot and
> > -        # _GenerateDeviceKVMId() will not raise a exception.
> > -        kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_DISK, 
> > cfdev)
> > -        drive_val += (",id=%s" % kvm_devid)
> > -        drive_val += (",bus=0,unit=%d" % cfdev.pci)
> > -        dev_val = ("%s,drive=%s,id=%s" %
> > -                   (device_driver, kvm_devid, kvm_devid))
> > -        dev_val += ",bus=pci.0,addr=%s" % hex(cfdev.pci)
> > +      # virtio-blk-pci case
> > +      if device_driver is not None:
> > +        # hvinfo will exist for paravirtual devices either due to
> > +        # _UpgradeSerializedRuntime() for old instances or due to
> > +        # _GenerateKVMRuntime() for new instances.
> > +        kvm_devid = cfdev.hvinfo["id"]
> > +        drive_val += ",id=%s" % kvm_devid
> > +        # Add driver, id, bus, and addr or channel, scsi-id, lun if any.
> > +        dev_val = _GenerateDeviceHVInfoStr(cfdev.hvinfo)
> > +        dev_val += ",drive=%s" % kvm_devid
> >          dev_opts.extend(["-device", dev_val])
> >  
> >        dev_opts.extend(["-drive", drive_val])
> > @@ -1062,26 +1194,20 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >  
> >      kvm_cmd.extend(["-pidfile", pidfile])
> >  
> > -    pci_reservations = bitarray(self._DEFAULT_PCI_RESERVATIONS)
> > +    bus_slots = self._GetBusSlots()
> >  
> >      # As requested by music lovers
> >      if hvp[constants.HV_SOUNDHW]:
> >        soundhw = hvp[constants.HV_SOUNDHW]
> > -      # For some reason only few sound devices require a PCI slot
> > -      # while the Audio controller *must* be in slot 3.
> > -      # That's why we bridge this option early in command line
> > -      if soundhw in self._SOUNDHW_WITH_PCI_SLOT:
> > -        _ = utils.GetFreeSlot(pci_reservations, reserve=True)
> >        kvm_cmd.extend(["-soundhw", soundhw])
> >  
> >      if hvp[constants.HV_DISK_TYPE] == constants.HT_DISK_SCSI:
> > -      # The SCSI controller requires another PCI slot.
> > -      _ = utils.GetFreeSlot(pci_reservations, reserve=True)
> > +      # In case a SCSI disk is given, QEMU adds
> > +      # a SCSI contorller (LSI Logic / Symbios Logic 53c895a)
> > +      # implicitly.
> > +      pass
> >  
> > -    # Add id to ballon and place to the first available slot (3 or 4)
> > -    addr = utils.GetFreeSlot(pci_reservations, reserve=True)
> > -    pci_info = ",bus=pci.0,addr=%s" % hex(addr)
> > -    kvm_cmd.extend(["-balloon", "virtio,id=balloon%s" % pci_info])
> > +    kvm_cmd.extend(["-balloon", "virtio"])
> >      kvm_cmd.extend(["-daemonize"])
> >      if not instance.hvparams[constants.HV_ACPI]:
> >        kvm_cmd.extend(["-no-acpi"])
> > @@ -1317,9 +1443,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >        else:
> >          # Enable the spice agent communication channel between the host 
> > and the
> >          # agent.
> > -        addr = utils.GetFreeSlot(pci_reservations, reserve=True)
> > -        pci_info = ",bus=pci.0,addr=%s" % hex(addr)
> > -        kvm_cmd.extend(["-device", "virtio-serial-pci,id=spice%s" % 
> > pci_info])
> > +        kvm_cmd.extend(["-device", "virtio-serial-pci,id=spice"])
> >          kvm_cmd.extend([
> >            "-device",
> >            "virtserialport,chardev=spicechannel0,name=com.redhat.spice.0",
> > @@ -1366,14 +1490,21 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >      if hvp[constants.HV_KVM_EXTRA]:
> >        kvm_cmd.extend(hvp[constants.HV_KVM_EXTRA].split(" "))
> >  
> > +    def _generate_kvm_device(dev_type, dev):
> > +      """Helper for generating a kvm device out of a Ganeti device."""
> > +      kvm_devid = _GenerateDeviceKVMId(dev_type, dev)
> > +      hv_dev_type = _DEVICE_TYPE[dev_type](hvp)
> > +      dev.hvinfo = _GenerateDeviceHVInfo(dev_type, kvm_devid,
> > +                                         hv_dev_type, bus_slots)
> > +
> >      kvm_disks = []
> >      for disk, link_name, uri in block_devices:
> > -      disk.pci = utils.GetFreeSlot(pci_reservations, disk.pci, True)
> > +      _generate_kvm_device(constants.HOTPLUG_TARGET_DISK, disk)
> >        kvm_disks.append((disk, link_name, uri))
> >  
> >      kvm_nics = []
> >      for nic in instance.nics:
> > -      nic.pci = utils.GetFreeSlot(pci_reservations, nic.pci, True)
> > +      _generate_kvm_device(constants.HOTPLUG_TARGET_NIC, nic)
> >        kvm_nics.append(nic)
> >  
> >      hvparams = hvp
> > @@ -1496,15 +1627,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >        }
> >      update_features = {}
> >      if nic_type == constants.HT_NIC_PARAVIRTUAL:
> > -      nic_model = self._VIRTIO
> > -      try:
> > -        if self._VIRTIO_NET_RE.search(devlist):
> > -          nic_model = self._VIRTIO_NET_PCI
> > -          update_features["vnet_hdr"] = up_hvp[constants.HV_VNET_HDR]
> > -      except errors.HypervisorError, _:
> > +      if self._DEVICE_DRIVER_SUPPORTED(self._VIRTIO_NET_PCI, devlist):
> > +        nic_model = self._VIRTIO_NET_PCI
> > +        update_features["vnet_hdr"] = up_hvp[constants.HV_VNET_HDR]
> > +      else:
> >          # Older versions of kvm don't support DEVICE_LIST, but they don't
> >          # have new virtio syntax either.
> > -        pass
> > +        nic_model = self._VIRTIO
> >  
> >        if up_hvp[constants.HV_VHOST_NET]:
> >          # Check for vhost_net support.
> > @@ -1620,16 +1749,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >            vhostfd = ""
> >  
> >          if kvm_supports_netdev:
> > -          nic_val = "%s,mac=%s" % (nic_model, nic.mac)
> > -          try:
> > -            # kvm_nics already exist in old runtime files and thus there 
> > might
> > -            # be some entries without pci slot (therefore try: except:)
> > -            kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_NIC, 
> > nic)
> > -            netdev = kvm_devid
> > -            nic_val += (",id=%s,bus=pci.0,addr=%s" % (kvm_devid, 
> > hex(nic.pci)))
> > -          except errors.HotplugError:
> > +          # Non paravirtual NICs hvinfo is empty
> > +          if "id" in nic.hvinfo:
> > +            nic_val = _GenerateDeviceHVInfoStr(nic.hvinfo)
> > +            netdev = nic.hvinfo["id"]
> > +          else:
> > +            nic_val = "%s" % nic_model
> >              netdev = "netdev%d" % nic_seq
> > -          nic_val += (",netdev=%s%s" % (netdev, nic_extra))
> > +          nic_val += (",netdev=%s,mac=%s%s" % (netdev, nic.mac, nic_extra))
> >            tap_val = ("type=tap,id=%s,%s%s%s" %
> >                       (netdev, tapfd, vhostfd, tap_extra))
> >            kvm_cmd.extend(["-netdev", tap_val, "-device", nic_val])
> > @@ -1861,6 +1988,49 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >      if (int(v_major), int(v_min)) < (1, 7):
> >        raise errors.HotplugError("Hotplug not supported for qemu versions < 
> > 1.7")
> >  
> > +  def _GetBusSlots(self, runtime=None):
> > +    """Helper function to get the slots of PCI and SCSI QEMU buses.
> > +
> > +    This will return the status of the first PCI and SCSI buses. By default
> > +    QEMU boots with one PCI bus (pci.0) and occupies the first 3 PCI 
> > slots. If
> > +    a SCSI disk is found then a SCSI controller is added on the PCI bus 
> > and a
> > +    SCSI bus (scsi.0) is created.
> > +
> > +    During hotplug we could query QEMU via info qtree HMP command but 
> > parsing
> > +    the result is too complicated. Instead we use the info stored in 
> > runtime
> > +    files. We parse NIC and disk entries and based on their hvinfo we 
> > reserve
> > +    the corresponding slots.
> > +
> > +    The runtime argument is a tuple as returned by _LoadKVMRuntime(). 
> > Obtain
> > +    disks and NICs from it. In case a runtime file is not available (see
> > +    _GenerateKVMRuntime()) we return the bus slots that QEMU boots with by
> > +    default.
> > +
> > +    """
> > +    # This is by default and returned during _GenerateKVMRuntime()
> > +    bus_slots = {
> > +      _PCI_BUS: bitarray(self._DEFAULT_PCI_RESERVATIONS),
> > +      _SCSI_BUS: bitarray(self._DEFAULT_SCSI_RESERVATIONS),
> > +      }
> > +
> > +    # This is during hot-add
> > +    if runtime:
> > +      _, nics, _, disks = runtime
> > +      disks = [d for d, _, _ in disks]
> > +      for d in disks + nics:
> > +        if not d.hvinfo or "bus" not in d.hvinfo:
> > +          continue
> > +        bus = d.hvinfo["bus"]
> > +        slots = bus_slots[bus]
> > +        if bus == _PCI_BUS:
> > +          slot = d.hvinfo["addr"]
> > +          slots[int(slot, 16)] = True
> > +        elif bus == _SCSI_BUS:
> > +          slot = d.hvinfo["scsi-id"]
> > +          slots[slot] = True
> > +
> > +    return bus_slots
> > +
> >    @_with_qmp
> >    def _VerifyHotplugCommand(self, _instance,
> >                              device, kvm_devid, should_exist):
> > diff --git a/lib/objects.py b/lib/objects.py
> > index 0ce3c0f..8b5a926 100644
> > --- a/lib/objects.py
> > +++ b/lib/objects.py
> > @@ -522,7 +522,7 @@ class ConfigData(ConfigObject):
> >  class NIC(ConfigObject):
> >    """Config object representing a network card."""
> >    __slots__ = ["name", "mac", "ip", "network",
> > -               "nicparams", "netinfo", "pci"] + _UUID
> > +               "nicparams", "netinfo", "pci", "hvinfo"] + _UUID
> >  
> >    @classmethod
> >    def CheckParameterSyntax(cls, nicparams):
> > @@ -564,6 +564,7 @@ class Disk(ConfigObject):
> >      "params",
> >      "spindles",
> >      "pci",
> > +    "hvinfo",
> >      "serial_no",
> >      # dynamic_params is special. It depends on the node this instance
> >      # is sent to, and should not be persisted.
> > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> > index da5233f..7b8ece5 100644
> > --- a/src/Ganeti/Constants.hs
> > +++ b/src/Ganeti/Constants.hs
> > @@ -2727,6 +2727,25 @@ htDiskScsi = "scsi"
> >  htDiskSd :: String
> >  htDiskSd = "sd"
> >  
> > +htDiskScsiGeneric :: String
> > +htDiskScsiGeneric = "scsi-generic"
> > +
> > +htDiskScsiBlock :: String
> > +htDiskScsiBlock = "scsi-block"
> > +
> > +htDiskScsiCd :: String
> > +htDiskScsiCd = "scsi-cd"
> > +
> > +htDiskScsiHd :: String
> > +htDiskScsiHd = "scsi-hd"
> > +
> > +htScsiDeviceTypes :: FrozenSet String
> > +htScsiDeviceTypes =
> > +  ConstantUtils.mkSet [htDiskScsiGeneric,
> > +                       htDiskScsiBlock,
> > +                       htDiskScsiCd,
> > +                       htDiskScsiHd]
> > +
> >  htHvmValidDiskTypes :: FrozenSet String
> >  htHvmValidDiskTypes = ConstantUtils.mkSet [htDiskIoemu, htDiskParavirtual]
> >  
> > @@ -2737,7 +2756,11 @@ htKvmValidDiskTypes =
> >                         htDiskParavirtual,
> >                         htDiskPflash,
> >                         htDiskScsi,
> > -                       htDiskSd]
> > +                       htDiskSd,
> > +                       htDiskScsiGeneric,
> > +                       htDiskScsiBlock,
> > +                       htDiskScsiHd,
> > +                       htDiskScsiCd]
> >  
> >  htCacheDefault :: String
> >  htCacheDefault = "default"
> > diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs
> > index 52c30f1..f311827 100644
> > --- a/src/Ganeti/Types.hs
> > +++ b/src/Ganeti/Types.hs
> > @@ -947,8 +947,8 @@ $(THH.makeJSONInstance ''HotplugAction)
> >  -- | Hotplug Device Target.
> >  
> >  $(THH.declareLADT ''String "HotplugTarget"
> > -  [ ("HTDisk", "hotdisk")
> > -  , ("HTNic",  "hotnic")
> > +  [ ("HTDisk", "disk")
> > +  , ("HTNic",  "nic")
> >    ])
> >  $(THH.makeJSONInstance ''HotplugTarget)
> >  
> > diff --git a/test/data/kvm_runtime.json b/test/data/kvm_runtime.json
> > index d9fb8c1..ee5394b 100644
> > --- a/test/data/kvm_runtime.json
> > +++ b/test/data/kvm_runtime.json
> > @@ -31,7 +31,12 @@
> >          "link": "br0",
> >          "mode": "bridged"
> >        },
> > -      "pci": 6,
> > +      "hvinfo": {
> > +        "driver": "virtio-net-pci",
> > +        "id": "nic-003fc157",
> > +        "bus": "pci.0",
> > +        "addr": "0x8"
> > +      },
> >        "uuid": "003fc157-66a8-4e6d-8b7e-ec4f69751396"
> >      }
> >    ],
> > @@ -108,7 +113,12 @@
> >          "params": {
> >            "stripes": 1
> >          },
> > -        "pci": 4,
> > +        "hvinfo": {
> > +          "driver": "virtio-blk-pci",
> > +          "id": "disk-7c079136",
> > +          "bus": "pci.0",
> > +          "addr": "0x9"
> > +        },
> >          "size": 1024,
> >          "uuid": "7c079136-2573-4112-82d0-0d3d2aa90d8f"
> >        },
> > @@ -128,7 +138,12 @@
> >          "params": {
> >            "stripes": 1
> >          },
> > -        "pci": 5,
> > +        "hvinfo": {
> > +          "driver": "virtio-blk-pci",
> > +          "id": "disk-9f5c5bd4",
> > +          "bus": "pci.0",
> > +          "addr": "0xa"
> > +        },
> >          "size": 512,
> >          "uuid": "9f5c5bd4-6f60-480b-acdc-9bb1a4b7df79"
> >        },
> > diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py 
> > b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> > index 4f0fbf6..bec3177 100755
> > --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> > +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> > @@ -462,11 +462,52 @@ class TestGenerateDeviceKVMId(unittest.TestCase):
> >      device = objects.NIC()
> >      target = constants.HOTPLUG_TARGET_NIC
> >      fn = hv_kvm._GenerateDeviceKVMId
> > -    self.assertRaises(errors.HotplugError, fn, target, device)
> > -
> > -    device.pci = 5
> >      device.uuid = "003fc157-66a8-4e6d-8b7e-ec4f69751396"
> > -    self.assertTrue(re.match("hotnic-003fc157-pci-5", fn(target, device)))
> > +    self.assertTrue(re.match("nic-003fc157", fn(target, device)))
> > +
> > +
> > +class TestGenerateDeviceHVInfo(testutils.GanetiTestCase):
> > +  def testPCI(self):
> > +    self.MockOut(mock.patch('ganeti.utils.EnsureDirs'))
> > +    hypervisor = hv_kvm.KVMHypervisor()
> > +    dev_type = constants.HOTPLUG_TARGET_NIC
> > +    kvm_devid = "nic-9f5c5bd4"
> > +    hv_dev_type = constants.HT_NIC_PARAVIRTUAL
> > +    bus_slots = hypervisor._GetBusSlots()
> > +    hvinfo = hv_kvm._GenerateDeviceHVInfo(dev_type,
> > +                                          kvm_devid,
> > +                                          hv_dev_type,
> > +                                          bus_slots)
> > +    expected_hvinfo = {
> > +      "driver": "virtio-net-pci",
> > +      "id": kvm_devid,
> > +      "bus": "pci.0",
> > +      "addr": hex(16),
> > +      }
> > +
> > +    self.assertTrue(hvinfo == expected_hvinfo)
> > +
> > +  def testSCSI(self):
> > +    self.MockOut(mock.patch('ganeti.utils.EnsureDirs'))
> > +    hypervisor = hv_kvm.KVMHypervisor()
> > +    dev_type = constants.HOTPLUG_TARGET_DISK
> > +    kvm_devid = "disk-9f5c5bd4"
> > +    hv_dev_type = constants.HT_DISK_SCSI_BLOCK
> > +    bus_slots = hypervisor._GetBusSlots()
> > +    hvinfo = hv_kvm._GenerateDeviceHVInfo(dev_type,
> > +                                          kvm_devid,
> > +                                          hv_dev_type,
> > +                                          bus_slots)
> > +    expected_hvinfo = {
> > +      "driver": "scsi-block",
> > +      "id": kvm_devid,
> > +      "bus": "scsi.0",
> > +      "channel": 0,
> > +      "scsi-id": 0,
> > +      "lun": 0,
> > +      }
> > +
> > +    self.assertTrue(hvinfo == expected_hvinfo)
> >  
> >  
> >  class TestGetRuntimeInfo(unittest.TestCase):
> > @@ -490,7 +531,7 @@ class TestGetRuntimeInfo(unittest.TestCase):
> >  
> >      device.uuid = "003fc157-66a8-4e6d-8b7e-ec4f69751396"
> >      devinfo = hv_kvm._GetExistingDeviceInfo(target, device, runtime)
> > -    self.assertTrue(devinfo.pci==6)
> > +    self.assertTrue(devinfo.hvinfo["addr"] == "0x8")
> >  
> >    def testDisk(self):
> >      device = objects.Disk()
> > @@ -501,7 +542,7 @@ class TestGetRuntimeInfo(unittest.TestCase):
> >  
> >      device.uuid = "9f5c5bd4-6f60-480b-acdc-9bb1a4b7df79"
> >      (devinfo, _, __) = hv_kvm._GetExistingDeviceInfo(target, device, 
> > runtime)
> > -    self.assertTrue(devinfo.pci==5)
> > +    self.assertTrue(devinfo.hvinfo["addr"] == "0xa")
> >  
> >  
> >  class PostfixMatcher(object):
> > -- 
> > 1.7.10.4
> > 

Attachment: signature.asc
Description: Digital signature

Reply via email to