* Hrvoje Ribicic <[email protected]> [2015-08-31 19:14:20 +0200]: > On Tue, Aug 18, 2015 at 12:06 PM, Dimitris Aragiorgis < > [email protected]> 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 > > 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]> > > --- > > lib/hypervisor/hv_kvm/__init__.py | 291 > > ++++++++++++++++++++------ > > lib/objects.py | 3 +- > > src/Ganeti/Constants.hs | 18 +- > > test/data/kvm_runtime.json | 21 +- > > test/py/ganeti.hypervisor.hv_kvm_unittest.py | 9 +- > > 5 files changed, 265 insertions(+), 77 deletions(-) > > > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > > b/lib/hypervisor/hv_kvm/__init__.py > > index 340ddb1..7d5b0b2 100644 > > --- a/lib/hypervisor/hv_kvm/__init__.py > > +++ b/lib/hypervisor/hv_kvm/__init__.py > > @@ -109,6 +109,35 @@ _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], > > + } > > > > Out of curiosity: why not use a function? > You are effectively allowing someone to partially apply the function you > have defined, which does not seem like it has a use case here and could be > a source of bugs. > By using a function, you might lose a bit of readability, but get more > support from tools (lint, python not allowing you to partially apply this > by mistake). > > That said, I do not care that much, this is readable and it matches the > surrounding code, so this comment is non-binding. > >
So if you do not care that much... :)
> > +
> > +_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,
> > + }
> > +
> >
>
> Please add a comment explaining the special case.
>
OK.
>
> > +_SCSI_DEVICES = frozenset([
> > + constants.HT_DISK_SCSI_BLOCK,
> > + constants.HT_DISK_SCSI_GENERIC,
> > + constants.HT_DISK_SCSI_HD,
> > + constants.HT_DISK_SCSI_CD,
> > + ])
> >
>
> You can generate a constant set like this in Constants.hs as well. While I
> realize that it's very nice to have this definition near its place of use,
> we pretty much have a convention of doing it in Constants.hs which is
> probably worth following.
>
OK.
>
> > +
> > +_DEVICE_BUS = {
> > + constants.HOTPLUG_TARGET_NIC:
> > + lambda _: _PCI_BUS,
> > + constants.HOTPLUG_TARGET_DISK:
> > + lambda ht: _SCSI_BUS if ht in _SCSI_DEVICES else _PCI_BUS
> > + }
> > +
> > +_PCI_BUS = "pci.0"
> > +_SCSI_BUS = "scsi.0"
> > +
> > _MIGRATION_CAPS_DELIM = ":"
> >
> >
> > @@ -155,23 +184,81 @@ 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 hotdisk/hotnic 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)
> >
>
> Couldn't this be a good time to get rid of the hot- prefix for disk/NIC
> ids? As we appear to apply it to all devices equally, it does not seem
> meaningful.
>
Let's do it then! We are going to keep it only in
_UpgradeSerializedRuntime() when we substitute pci with hvinfo.
>
> > @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=hotdisk-1234,bus=pci.0,addr=0x9
> > + PV NIC: virtio-net-pci,id=hotnic-1234,bus=pci.0,addr=0x9
> > + SG disk:
> > scsi-generic,id=hotdisk-1234,bus=scsi.0,channel=0,scsi-id=1,lun=0
> > +
> >
>
> Some epydoc would be useful here for the hvinfo and the return value.
>
ACK.
>
> > + """
> > +
> > + # 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)
> > +
> > + @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
> > +
> >
>
> The epytags are supposed to come under the text of the explanation
> according to the style guide.
>
True. I'll fix it.
>
> > + hvinfo will held 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).
> >
>
> Since you reference addr, could you add some information about the return
> value in the docstring?
>
ACK.
>
> > +
> > + """
> > + 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 +307,36 @@ 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.
> > + if "pci" in dev:
> > + # This is practically the old _GenerateDeviceKVMId()
> > + dev["hvinfo"]["id"] = "%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"]
> >
>
> Should you acknowledge the comment in the design doc, please avoid deleting
> the pci field here, and add it in _GenerateKvmRuntime.
>
See my comment in the design doc, arguing that it is not necessary.
Instead we should extend _UpgradeSerializedRuntime() to handle cluster
downgrades. Will include it in v2.
>
> > +
> > 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 +518,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 +532,11 @@ 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"]
> > + _DEFAULT_PCI_RESERVATIONS = "1111111100000000"
> > + # 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"
> >
> > ANCILLARY_FILES = [
> > _KVM_NETWORK_SCRIPT,
> > @@ -898,19 +1015,22 @@ 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 or -drive
> > if=virtio
> >
>
> Do you mean both...and?
>
Well I mean that paravirtual implies either '-device virtio-blk-pci...
-drive if=none...' for new QEMU versions or '-drive if=virtio' for old
QEMU versions. I will rephrase it.
>
> > 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 +1067,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)
> >
> > + # virtio-blk-pci case
> > if device_driver:
> >
>
> Can we have a check for null here as the underlying type is string?
>
I guess you mean "if device_driver is not None:".
>
> > - # 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)
> > + # 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 +1182,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)
> > + # automatically. Here, we add it explicitly with the default id.
> > + kvm_cmd.extend(["-device", "lsi,id=scsi"])
> >
> > - # 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 +1431,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",
> > @@ -1368,12 +1480,20 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >
> > kvm_disks = []
> > for disk, link_name, uri in block_devices:
> > - disk.pci = utils.GetFreeSlot(pci_reservations, disk.pci, True)
> > + dev_type = constants.HOTPLUG_TARGET_DISK
> > + kvm_devid = _GenerateDeviceKVMId(dev_type, disk)
> > + hv_dev_type = _DEVICE_TYPE[dev_type](hvp)
> > + disk.hvinfo = _GenerateDeviceHVInfo(dev_type, kvm_devid,
> > + hv_dev_type, bus_slots)
> > kvm_disks.append((disk, link_name, uri))
> >
> > kvm_nics = []
> > for nic in instance.nics:
> > - nic.pci = utils.GetFreeSlot(pci_reservations, nic.pci, True)
> > + dev_type = constants.HOTPLUG_TARGET_NIC
> > + kvm_devid = _GenerateDeviceKVMId(dev_type, nic)
> > + hv_dev_type = _DEVICE_TYPE[dev_type](hvp)
> > + nic.hvinfo = _GenerateDeviceHVInfo(dev_type, kvm_devid,
> > + hv_dev_type, bus_slots)
> > kvm_nics.append(nic)
> >
> >
> A bit too much code duplication - make this into a function?
>
True. Will do.
>
> > hvparams = hvp
> > @@ -1496,15 +1616,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 +1738,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 +1977,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 correspoding slots.
> >
>
> Nit: s/correspoding/corresponding/
>
>
> > +
> > + The runtime argument is a tuple as returned by _LoadKVMRuntime().
> > Obtain
> > + disks and NICs from it. In case a runtime file is not availabe (see
> >
>
> Nit: s/availabe/available/
>
>
> > + _GenerateKVMRuntime()) we return the bus slots that a QEMU boots with
> > by
> > + default.
> >
>
> Nit: Surplus a?
>
>
> > +
> > + """
> > + # 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..1c9e16c 100644
> > --- a/src/Ganeti/Constants.hs
> > +++ b/src/Ganeti/Constants.hs
> > @@ -2727,6 +2727,18 @@ 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"
> > +
> > htHvmValidDiskTypes :: FrozenSet String
> > htHvmValidDiskTypes = ConstantUtils.mkSet [htDiskIoemu, htDiskParavirtual]
> >
> > @@ -2737,7 +2749,11 @@ htKvmValidDiskTypes =
> > htDiskParavirtual,
> > htDiskPflash,
> > htDiskScsi,
> > - htDiskSd]
> > + htDiskSd,
> > + htDiskScsiGeneric,
> > + htDiskScsiBlock,
> > + htDiskScsiHd,
> > + htDiskScsiCd]
> >
> > htCacheDefault :: String
> > htCacheDefault = "default"
> > diff --git a/test/data/kvm_runtime.json b/test/data/kvm_runtime.json
> > index d9fb8c1..ccdfb09 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": "hotnic-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": "hotdisk-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": "hotdisk-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..77bcac8 100755
> > --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> > +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> > @@ -462,11 +462,8 @@ 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("hotnic-003fc157", fn(target, device)))
> >
> >
> > class TestGetRuntimeInfo(unittest.TestCase):
> > @@ -490,7 +487,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 +498,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
> >
> >
> Hrvoje Ribicic
> Ganeti Engineering
> Google Germany GmbH
> Dienerstr. 12, 80331, München
>
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
signature.asc
Description: Digital signature
