LGTM, thanks On Thu, Oct 8, 2015 at 5:40 AM, Dimitris Aragiorgis < [email protected]> wrote:
> Change hotplug-related methods (i.e., HotAddNic() and HotAddDisk()) > to use slot 'hvinfo' instead of slot 'pci'. > > Change HasPCIDevice() to HasDevice(). This will parse the output of > the 'query-block' and 'query-pci' QMP commands and try to match the > given device ID. > > Make _VerifyHotplugCommand() depend only on the device ID and not > on the whole device object. > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > lib/hypervisor/hv_kvm/__init__.py | 14 +++---- > lib/hypervisor/hv_kvm/monitor.py | 75 > ++++++++++++++++++++++++++++++------- > 2 files changed, 67 insertions(+), 22 deletions(-) > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > index 67f2845..d81ced4 100644 > --- a/lib/hypervisor/hv_kvm/__init__.py > +++ b/lib/hypervisor/hv_kvm/__init__.py > @@ -283,8 +283,7 @@ def _GetExistingDeviceInfo(dev_type, device, runtime): > @type runtime: tuple (cmd, nics, hvparams, disks) > @param runtime: the runtime data to search for the device > @raise errors.HotplugError: in case the requested device does not > - exist (e.g. device has been added without --hotplug option) or > - device info has not pci slot (e.g. old devices in the cluster) > + exist (e.g. device has been added without --hotplug option) > > """ > index = _DEVICE_RUNTIME_INDEX[dev_type] > @@ -2041,18 +2040,17 @@ class KVMHypervisor(hv_base.BaseHypervisor): > return bus_slots > > @_with_qmp > - def _VerifyHotplugCommand(self, _instance, > - device, kvm_devid, should_exist): > + def _VerifyHotplugCommand(self, _instance, kvm_devid, should_exist): > """Checks if a previous hotplug command has succeeded. > > Depending on the should_exist value, verifies that an entry > identified by > - the PCI slot and device ID is present or not. > + device ID is present or not. > > @raise errors.HypervisorError: if result is not the expected one > > """ > for i in range(5): > - found = self.qmp.HasPCIDevice(device, kvm_devid) > + found = self.qmp.HasDevice(kvm_devid) > logging.info("Verifying hotplug command (retry %s): %s", i, found) > if found and should_exist: > break > @@ -2097,7 +2095,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > self.qmp.HotAddNic(device, kvm_devid, tapfds, vhostfds, features) > utils.WriteFile(self._InstanceNICFile(instance.name, seq), > data=tap) > > - self._VerifyHotplugCommand(instance, device, kvm_devid, True) > + self._VerifyHotplugCommand(instance, kvm_devid, True) > # update relevant entries in runtime file > index = _DEVICE_RUNTIME_INDEX[dev_type] > entry = _RUNTIME_ENTRY[dev_type](device, extra) > @@ -2124,7 +2122,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > elif dev_type == constants.HOTPLUG_TARGET_NIC: > self.qmp.HotDelNic(kvm_devid) > utils.RemoveFile(self._InstanceNICFile(instance.name, seq)) > - self._VerifyHotplugCommand(instance, kvm_device, kvm_devid, False) > + self._VerifyHotplugCommand(instance, kvm_devid, False) > index = _DEVICE_RUNTIME_INDEX[dev_type] > runtime[index].remove(entry) > self._SaveKVMRuntime(instance, runtime) > diff --git a/lib/hypervisor/hv_kvm/monitor.py > b/lib/hypervisor/hv_kvm/monitor.py > index f2b7d02..1d68849 100644 > --- a/lib/hypervisor/hv_kvm/monitor.py > +++ b/lib/hypervisor/hv_kvm/monitor.py > @@ -256,6 +256,11 @@ class QmpConnection(MonitorSocket): > _QUERY_COMMANDS = "query-commands" > _MESSAGE_END_TOKEN = "\r\n" > _QEMU_PCI_SLOTS = 32 # The number of PCI slots QEMU exposes by default > + # List of valid attributes for the device_add QMP command. > + # Extra attributes found in device's hvinfo will be ignored. > + _DEVICE_ATTRIBUTES = [ > + "driver", "id", "bus", "addr", "channel", "scsi-id", "lun" > + ] > > def __init__(self, monitor_filename): > super(QmpConnection, self).__init__(monitor_filename) > @@ -459,6 +464,15 @@ class QmpConnection(MonitorSocket): > > return response[self._RETURN_KEY] > > + def _filter_hvinfo(self, hvinfo): > + """Filter non valid keys of the device's hvinfo (if any).""" > + ret = {} > + for k in self._DEVICE_ATTRIBUTES: > + if k in hvinfo: > + ret[k] = hvinfo[k] > + > + return ret > + > @_ensure_connection > def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, > features=None): > """Hot-add a NIC > @@ -501,13 +515,12 @@ class QmpConnection(MonitorSocket): > self.Execute("netdev_add", arguments) > > arguments = { > - "driver": "virtio-net-pci", > - "id": devid, > - "bus": "pci.0", > - "addr": hex(nic.pci), > "netdev": devid, > "mac": nic.mac, > } > + # Note that hvinfo that _GenerateDeviceHVInfo() creates > + # sould include *only* the driver, id, bus, and addr keys > + arguments.update(self._filter_hvinfo(nic.hvinfo)) > if enable_mq: > arguments.update({ > "mq": "on", > @@ -559,12 +572,12 @@ class QmpConnection(MonitorSocket): > self._RemoveFdset(fdset) > > arguments = { > - "driver": "virtio-blk-pci", > - "id": devid, > - "bus": "pci.0", > - "addr": hex(disk.pci), > "drive": devid, > } > + # Note that hvinfo that _GenerateDeviceHVInfo() creates > + # sould include *only* the driver, id, bus, and > + # addr or channel, scsi-id, and lun keys > + arguments.update(self._filter_hvinfo(disk.hvinfo)) > self.Execute("device_add", arguments) > > @_ensure_connection > @@ -589,17 +602,51 @@ class QmpConnection(MonitorSocket): > devices = bus["devices"] > return devices > > + def _HasPCIDevice(self, devid): > + """Check if a specific device ID exists on the PCI bus. > + > + """ > + for d in self._GetPCIDevices(): > + if d["qdev_id"] == devid: > + return True > + > + return False > + > + def _GetBlockDevices(self): > + """Get the block devices of a running instance. > + > + The query-block QMP command returns a list of dictionaries > + including information for each virtual disk. For example: > + > + [{"device": "disk-049f140d", "inserted": {"file": ..., "image": ...}}] > + > + @rtype: list of dicts > + @return: Info about the virtual disks of the instance. > + > + """ > + self._check_connection() > + devices = self.Execute("query-block") > + return devices > + > + def _HasBlockDevice(self, devid): > + """Check if a specific device ID exists among block devices. > + > + """ > + for d in self._GetBlockDevices(): > + if d["device"] == devid: > + return True > + > + return False > + > @_ensure_connection > - def HasPCIDevice(self, device, devid): > + def HasDevice(self, devid): > """Check if a specific device exists or not on a running instance. > > - It will match the PCI slot of the device and the id currently > - obtained by _GenerateDeviceKVMId(). > + It first checks the PCI devices and then the block devices. > > """ > - for d in self._GetPCIDevices(): > - if d["qdev_id"] == devid and d["slot"] == device.pci: > - return True > + if (self._HasPCIDevice(devid) or self._HasBlockDevice(devid)): > + return True > > return False > > -- > 1.7.10.4 > > Hrvoje Ribicic Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.
