* Hrvoje Ribicic <[email protected]> [2015-09-01 12:44:41 +0200]: > LGTM with nits. > > On Tue, Aug 18, 2015 at 12:07 PM, Dimitris Aragiorgis < > [email protected]> wrote: > > > Use the new interface and hvinfo during hotplug actions. This means > > that _GenerateKVMDeviceID(), _GetBusSlots() and > > _GenerateDeviceHVInfo() will be used to obtain the proper device ID > > and bus position. > > > > Add an extra check in _VerifyHotplugSupport() that allows hotplug > > only for paravirtual devices (virtio-blk-pci, virtio-net-pci) and > > SCSI devices (scsi-cd, scsi-hd, scsi-block, scsi-generic) that > > use the latest device model in QEMU (via the -device option). > > > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > > --- > > lib/hypervisor/hv_kvm/__init__.py | 51 > > ++++++++++++++++++++++++++++++------- > > 1 file changed, 42 insertions(+), 9 deletions(-) > > > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > > b/lib/hypervisor/hv_kvm/__init__.py > > index 3bec290..5d345c1 100644 > > --- a/lib/hypervisor/hv_kvm/__init__.py > > +++ b/lib/hypervisor/hv_kvm/__init__.py > > @@ -135,6 +135,17 @@ _DEVICE_BUS = { > > lambda ht: _SCSI_BUS if ht in _SCSI_DEVICES else _PCI_BUS > > } > > > > +_HOTPLUGABLE_DEVICE_TYPES = { > > > > Nit: Wouldn't s/HOTPLUGABLE/HOTPLUGGABLE/ be better, as in the real word > pluggable? >
HOTPLUGGABLE it is then. > > > + constants.HOTPLUG_TARGET_NIC: [constants.HT_NIC_PARAVIRTUAL], > > + constants.HOTPLUG_TARGET_DISK: [ > > + constants.HT_DISK_PARAVIRTUAL, > > + constants.HT_DISK_SCSI_BLOCK, > > + constants.HT_DISK_SCSI_GENERIC, > > + constants.HT_DISK_SCSI_HD, > > + constants.HT_DISK_SCSI_CD, > > + ] > > + } > > + > > _PCI_BUS = "pci.0" > > _SCSI_BUS = "scsi.0" > > > > @@ -1942,9 +1953,21 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > def VerifyHotplugSupport(self, instance, action, dev_type): > > """Verifies that hotplug is supported. > > > > - @raise errors.HypervisorError: in one of the previous cases > > + Hotplug is not supported if: > > + > > + - the instance is not running > > + - the device type is not hotplug-able > > + - the QMP version does not support the correponding commands > > > > Nit: s/correponding/corresponding/ > > > > + > > + @raise errors.HypervisorError: if one of the above applies > > > > """ > > + runtime = self._LoadKVMRuntime(instance) > > + device_type = _DEVICE_TYPE[dev_type](runtime[2]) > > + if device_type not in _HOTPLUGABLE_DEVICE_TYPES[dev_type]: > > + msg = "Hotplug is not supported for device type %s" % device_type > > + raise errors.HypervisorError(msg) > > + > > if dev_type == constants.HOTPLUG_TARGET_DISK: > > if action == constants.HOTPLUG_ACTION_ADD: > > self.qmp.CheckDiskHotAddSupport() > > @@ -2052,16 +2075,27 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > def HotAddDevice(self, instance, dev_type, device, extra, seq): > > """ Helper method to hot-add a new device > > > > - It gets free pci slot generates the device name and invokes the > > + It generates the device ID, and hvinfo and invokes the > > > > Nit: ... the device ID and hvinfo, and invokes ... > > > > device specific method. > > > > Nit: device-specific > > > > > > """ > > - # in case of hot-mod this is given > > - if device.pci is None: > > - device.pci = self.qmp.GetFreePCISlot() > > kvm_devid = _GenerateDeviceKVMId(dev_type, device) > > runtime = self._LoadKVMRuntime(instance) > > + up_hvp = runtime[2] > > + device_type = _DEVICE_TYPE[dev_type](up_hvp) > > + bus_state = self._GetBusSlots(runtime) > > + # in case of hot-mod this is given > > + if not device.hvinfo: > > + device.hvinfo = _GenerateDeviceHVInfo(dev_type, kvm_devid, > > + device_type, bus_state) > > if dev_type == constants.HOTPLUG_TARGET_DISK: > > + # Add a drive via HMP > > + # This must be done indirectly due to the fact that we pass the > > drive's > > + # file descriptor via QMP first, then we add the corresponding > > drive that > > + # refers to this fd. Note that if the QMP connection terminates > > before > > + # a drive which keeps a reference to the fd passed via the add-fd > > QMP > > + # command has been created, then the fd gets closed and cannot be > > used > > + # later (e.g., via an drive_add HMP command). > > > > Ah, this probably belongs to the earlier patch. If you move this comment > into it, it should resolve my complaint. > Cool. > > > uri = _GetDriveURI(device, extra[0], extra[1]) > > > > def drive_add_fn(filename): > > @@ -2074,7 +2108,6 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > kvmpath = instance.hvparams[constants.HV_KVM_PATH] > > kvmhelp = self._GetKVMOutput(kvmpath, self._KVMOPT_HELP) > > devlist = self._GetKVMOutput(kvmpath, self._KVMOPT_DEVICELIST) > > - up_hvp = runtime[2] > > features, _, _ = self._GetNetworkDeviceFeatures(up_hvp, devlist, > > kvmhelp) > > (tap, tapfds, vhostfds) = OpenTap(features=features) > > self._ConfigureNIC(instance, seq, device, tap) > > @@ -2113,7 +2146,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > runtime[index].remove(entry) > > self._SaveKVMRuntime(instance, runtime) > > > > - return kvm_device.pci > > + return kvm_device.hvinfo > > > > def HotModDevice(self, instance, dev_type, device, _, seq): > > """ Helper method for hot-mod device > > @@ -2123,8 +2156,8 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > > > """ > > if dev_type == constants.HOTPLUG_TARGET_NIC: > > - # putting it back in the same pci slot > > - device.pci = self.HotDelDevice(instance, dev_type, device, _, seq) > > + # putting it back in the same bus and slot > > + device.hvinfo = self.HotDelDevice(instance, dev_type, device, _, > > seq) > > self.HotAddDevice(instance, dev_type, device, _, seq) > > > > @classmethod > > -- > > 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
