Hello, I've fixed those lint errors, updated patch follows.
Thanks, Luka Dana ponedjeljak, 14. travnja 2014. 10:44:20 UTC+2, korisnik Jose A. Lopes napisao je: > > Hi, > > Thanks a lot for the patch. > I have found a few lint errors. > Can you please have a look? > > ganeti/hypervisor/hv_kvm/__init__.py:1566:3: E303 too many blank lines (2) > ganeti/hypervisor/hv_kvm/__init__.py:1623:7: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1624:7: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1629:35: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1630:35: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1951:7: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1952:7: E128 continuation line > under-indented for visual indent > ganeti/hypervisor/hv_kvm/__init__.py:1514: [W0613, > KVMHypervisor._GetNetworkDeviceFeatures] Unused argument 'instance' > > Cheers, > Jose > > On Apr 11 14:45, Luka Blaskovic wrote: > > This patch adds support for multiqueue virtio-net[1] which sets a number > of > > queues (file descriptors) from virtio_net_queues parameter for tap > device > > to parallelize packets sending or receiving. Tap devices will be created > with > > MULTI_QUEUE (IFF_MULTI_QUEUE) support. > > > > KVM paravirtual nics (virtio-net) are only one which supports this > feature. > > Number of queues are limited by kernel tap implementation (currently to > 8). > > Instances must manually set number of queues, on Linux using: > > > > ethtool -L ethX combined $queues > > > > Network device options logic is moved to separate method > > _GetNetworkDeviceFeatures which is now properly reused in hotplugging > part. > > This also fixes unreported bug when vhost_net parameter is set to true, > > hotplugged network device will be created without "vhost=on" parameter. > > > > [1] http://www.linux-kvm.org/page/Multiqueue > > > > Signed-off-by: Luka Blaskovic <[email protected] <javascript:>> > > --- > > lib/hypervisor/hv_base.py | 6 ++ > > lib/hypervisor/hv_kvm/__init__.py | 121 > +++++++++++++++++++++++++------------ > > lib/hypervisor/hv_kvm/netdev.py | 75 +++++++++++++++++------ > > man/gnt-instance.rst | 19 ++++++ > > src/Ganeti/Constants.hs | 5 ++ > > 5 files changed, 170 insertions(+), 56 deletions(-) > > > > diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py > > index 70a4a50..1f43e5c 100644 > > --- a/lib/hypervisor/hv_base.py > > +++ b/lib/hypervisor/hv_base.py > > @@ -108,6 +108,10 @@ _MULTI_CPU_MASK_CHECK = (_IsMultiCpuMaskWellFormed, > > _NET_PORT_CHECK = (lambda x: 0 < x < 65535, "invalid port number", > > None, None) > > > > +# Check if number of queues is in safe range > > +_VIRTIO_NET_QUEUES_CHECK = (lambda x: 0 < x < 9, "invalid number of > queues", > > + None, None) > > + > > # Check that an integer is non negative > > _NONNEGATIVE_INT_CHECK = (lambda x: x >= 0, "cannot be negative", None, > None) > > > > @@ -120,6 +124,8 @@ REQ_DIR_CHECK = (True, ) + _DIR_CHECK > > OPT_DIR_CHECK = (False, ) + _DIR_CHECK > > REQ_NET_PORT_CHECK = (True, ) + _NET_PORT_CHECK > > OPT_NET_PORT_CHECK = (False, ) + _NET_PORT_CHECK > > +REQ_VIRTIO_NET_QUEUES_CHECK = (True, ) + _VIRTIO_NET_QUEUES_CHECK > > +OPT_VIRTIO_NET_QUEUES_CHECK = (False, ) + _VIRTIO_NET_QUEUES_CHECK > > REQ_CPU_MASK_CHECK = (True, ) + _CPU_MASK_CHECK > > OPT_CPU_MASK_CHECK = (False, ) + _CPU_MASK_CHECK > > REQ_MULTI_CPU_MASK_CHECK = (True, ) + _MULTI_CPU_MASK_CHECK > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > > index 4a80b2d..6134b05 100644 > > --- a/lib/hypervisor/hv_kvm/__init__.py > > +++ b/lib/hypervisor/hv_kvm/__init__.py > > @@ -337,6 +337,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > constants.HV_KVM_FLAG: > > hv_base.ParamInSet(False, constants.HT_KVM_FLAG_VALUES), > > constants.HV_VHOST_NET: hv_base.NO_CHECK, > > + constants.HV_VIRTIO_NET_QUEUES: > hv_base.OPT_VIRTIO_NET_QUEUES_CHECK, > > constants.HV_KVM_USE_CHROOT: hv_base.NO_CHECK, > > constants.HV_KVM_USER_SHUTDOWN: hv_base.NO_CHECK, > > constants.HV_MEM_PATH: hv_base.OPT_DIR_CHECK, > > @@ -383,6 +384,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > _QMP_RE = re.compile(r"^-qmp\s", re.M) > > _SPICE_RE = re.compile(r"^-spice\s", re.M) > > _VHOST_RE = re.compile(r"^-net\s.*,vhost=on|off", re.M) > > + _VIRTIO_NET_QUEUES_RE = re.compile(r"^-net\s.*,fds=x:y:...:z", re.M) > > _ENABLE_KVM_RE = re.compile(r"^-enable-kvm\s", re.M) > > _DISABLE_KVM_RE = re.compile(r"^-disable-kvm\s", re.M) > > _NETDEV_RE = re.compile(r"^-netdev\s", re.M) > > @@ -1509,6 +1511,56 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > > > return "gnt.com.%d" % idx > > > > + def _GetNetworkDeviceFeatures(self, instance, up_hvp, devlist, > kvmhelp): > > + """Get network device options to properly enable supported > features. > > + > > + Return tuple of supported and enabled tap features with nic_model. > > + This function is called before opening a new tap device. > > + > > + @return: (nic_model, vnet_hdr, virtio_net_queues, tap_extra, > nic_extra) > > + @rtype: tuple > > + > > + """ > > + virtio_net_queues = 1 > > + nic_extra = "" > > + nic_type = up_hvp[constants.HV_NIC_TYPE] > > + tap_extra = "" > > + vnet_hdr = False > > + 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 > > + vnet_hdr = up_hvp[constants.HV_VNET_HDR] > > + except errors.HypervisorError, _: > > + # Older versions of kvm don't support DEVICE_LIST, but they > don't > > + # have new virtio syntax either. > > + pass > > + > > + if up_hvp[constants.HV_VHOST_NET]: > > + # Check for vhost_net support. > > + if self._VHOST_RE.search(kvmhelp): > > + tap_extra = ",vhost=on" > > + else: > > + raise errors.HypervisorError("vhost_net is configured" > > + " but it is not available") > > + if up_hvp[constants.HV_VIRTIO_NET_QUEUES] > 1: > > + # Check for multiqueue virtio-net support. > > + if self._VIRTIO_NET_QUEUES_RE.search(kvmhelp): > > + virtio_net_queues = up_hvp[constants.HV_VIRTIO_NET_QUEUES] > > + # As advised at > > http://www.linux-kvm.org/page/Multiqueueformula > > + # for calculating vector size is: vectors=2*N+1 where N is > the > > + # number of queues (HV_VIRTIO_NET_QUEUES). > > + nic_extra = ",mq=on,vectors=%d" % (2 * virtio_net_queues + > 1) > > + else: > > + raise errors.HypervisorError("virtio_net_queues is > configured" > > + " but it is not available") > > + else: > > + nic_model = nic_type > > + > > + return (nic_model, vnet_hdr, virtio_net_queues, tap_extra, > nic_extra) > > + > > + > > # too many local variables > > # pylint: disable=R0914 > > def _ExecuteKVMRuntime(self, instance, kvm_runtime, kvmhelp, > incoming=None): > > @@ -1567,37 +1619,19 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > if not kvm_nics: > > kvm_cmd.extend(["-net", "none"]) > > else: > > - vnet_hdr = False > > - tap_extra = "" > > - nic_type = up_hvp[constants.HV_NIC_TYPE] > > - 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 > > - vnet_hdr = up_hvp[constants.HV_VNET_HDR] > > - except errors.HypervisorError, _: > > - # Older versions of kvm don't support DEVICE_LIST, but they > don't > > - # have new virtio syntax either. > > - pass > > - > > - if up_hvp[constants.HV_VHOST_NET]: > > - # check for vhost_net support > > - if self._VHOST_RE.search(kvmhelp): > > - tap_extra = ",vhost=on" > > - else: > > - raise errors.HypervisorError("vhost_net is configured" > > - " but it is not available") > > - else: > > - nic_model = nic_type > > - > > + (nic_model, vnet_hdr, > > + virtio_net_queues, tap_extra, > > + nic_extra) = self._GetNetworkDeviceFeatures(instance, up_hvp, > > + devlist, kvmhelp) > > kvm_supports_netdev = self._NETDEV_RE.search(kvmhelp) > > - > > for nic_seq, nic in enumerate(kvm_nics): > > - tapname, tapfd = OpenTap(vnet_hdr=vnet_hdr, > > - name=self._GenerateTapName(nic)) > > - tapfds.append(tapfd) > > + tapname, nic_tapfds = OpenTap(vnet_hdr=vnet_hdr, > > + virtio_net_queues=virtio_net_queues, > > + name=self._GenerateTapName(nic)) > > + tapfds.extend(nic_tapfds) > > taps.append(tapname) > > + tapfd = "%s%s" % ("fds=" if len(nic_tapfds) > 1 else "fd=", > > + ":".join(str(fd) for fd in nic_tapfds)) > > if kvm_supports_netdev: > > nic_val = "%s,mac=%s" % (nic_model, nic.mac) > > try: > > @@ -1608,14 +1642,14 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > nic_val += (",id=%s,bus=pci.0,addr=%s" % (kvm_devid, > hex(nic.pci))) > > except errors.HotplugError: > > netdev = "netdev%d" % nic_seq > > - nic_val += (",netdev=%s" % netdev) > > - tap_val = ("type=tap,id=%s,fd=%d%s" % > > + nic_val += (",netdev=%s%s" % (netdev, nic_extra)) > > + tap_val = ("type=tap,id=%s,%s%s" % > > (netdev, tapfd, tap_extra)) > > kvm_cmd.extend(["-netdev", tap_val, "-device", nic_val]) > > else: > > nic_val = "nic,vlan=%s,macaddr=%s,model=%s" % (nic_seq, > > nic.mac, > nic_model) > > - tap_val = "tap,vlan=%s,fd=%d" % (nic_seq, tapfd) > > + tap_val = "tap,vlan=%s,%s" % (nic_seq, tapfd) > > kvm_cmd.extend(["-net", tap_val, "-net", nic_val]) > > > > if incoming: > > @@ -1909,12 +1943,24 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > cmds += ["device_add > virtio-blk-pci,bus=pci.0,addr=%s,drive=%s,id=%s" % > > (hex(device.pci), kvm_devid, kvm_devid)] > > elif dev_type == constants.HOTPLUG_TARGET_NIC: > > - (tap, fd) = OpenTap() > > + 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] > > + (_, vnet_hdr, > > + virtio_net_queues, tap_extra, > > + nic_extra) = self._GetNetworkDeviceFeatures(instance, up_hvp, > > + devlist, kvmhelp) > > + (tap, fds) = OpenTap(vnet_hdr=vnet_hdr, > > + virtio_net_queues=virtio_net_queues) > > + # netdev_add don't support "fds=" when multiple fds are > > + # requested, generate separate "fd=" string for every fd > > + tapfd = ",".join(["fd=%s" % fd for fd in fds]) > > self._ConfigureNIC(instance, seq, device, tap) > > - self._PassTapFd(instance, fd, device) > > - cmds = ["netdev_add tap,id=%s,fd=%s" % (kvm_devid, kvm_devid)] > > - args = "virtio-net-pci,bus=pci.0,addr=%s,mac=%s,netdev=%s,id=%s" > % \ > > - (hex(device.pci), device.mac, kvm_devid, kvm_devid) > > + self._PassTapFd(instance, fds, device) > > + cmds = ["netdev_add tap,id=%s,%s%s" % (kvm_devid, tapfd, > tap_extra)] > > + args = > "virtio-net-pci,bus=pci.0,addr=%s,mac=%s,netdev=%s,id=%s%s" % \ > > + (hex(device.pci), device.mac, kvm_devid, kvm_devid, > nic_extra) > > cmds += ["device_add %s" % args] > > utils.WriteFile(self._InstanceNICFile(instance.name, seq), > data=tap) > > > > @@ -1964,7 +2010,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > device.pci = self.HotDelDevice(instance, dev_type, device, _, > seq) > > self.HotAddDevice(instance, dev_type, device, _, seq) > > > > - def _PassTapFd(self, instance, fd, nic): > > + def _PassTapFd(self, instance, fds, nic): > > """Pass file descriptor to kvm process via monitor socket using > SCM_RIGHTS > > > > """ > > @@ -1972,7 +2018,6 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > # squash common parts between monitor and qmp > > kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_NIC, nic) > > command = "getfd %s\n" % kvm_devid > > - fds = [fd] > > logging.info("%s", fds) > > try: > > monsock = MonitorSocket(self._InstanceMonitor(instance.name)) > > diff --git a/lib/hypervisor/hv_kvm/netdev.py > b/lib/hypervisor/hv_kvm/netdev.py > > index dbbd6ce..8c48dee 100644 > > --- a/lib/hypervisor/hv_kvm/netdev.py > > +++ b/lib/hypervisor/hv_kvm/netdev.py > > @@ -40,6 +40,7 @@ TUNGETFEATURES = 0x800454cf > > IFF_TAP = 0x0002 > > IFF_NO_PI = 0x1000 > > IFF_VNET_HDR = 0x4000 > > +IFF_MULTI_QUEUE = 0x0100 > > > > > > def _GetTunFeatures(fd, _ioctl=fcntl.ioctl): > > @@ -90,42 +91,80 @@ def _ProbeTapVnetHdr(fd, > _features_fn=_GetTunFeatures): > > return result > > > > > > -def OpenTap(vnet_hdr=True, name=""): > > +def _ProbeTapMqVirtioNet(fd, _features_fn=_GetTunFeatures): > > + """Check whether to enable the IFF_MULTI_QUEUE flag. > > + > > + This flag was introduced in Linux kernel 3.8. > > + > > + @type fd: int > > + @param fd: the file descriptor of /dev/net/tun > > + > > + """ > > + flags = _features_fn(fd) > > + > > + if flags is None: > > + # Not supported > > + return False > > + > > + result = bool(flags & IFF_MULTI_QUEUE) > > + > > + if not result: > > + logging.warning("Kernel does not support IFF_MULTI_QUEUE, not > enabling") > > + > > + return result > > + > > + > > +def OpenTap(vnet_hdr=True, virtio_net_queues=1, name=""): > > """Open a new tap device and return its file descriptor. > > > > This is intended to be used by a qemu-type hypervisor together with > the -net > > - tap,fd=<fd> command line parameter. > > + tap,fd=<fd> or -net tap,fds=x:y:...:z command line parameter. > > > > @type vnet_hdr: boolean > > @param vnet_hdr: Enable the VNET Header > > > > + @type virtio_net_queues: int > > + @param virtio_net_queues: Set number of tap queues but not more than > 8, > > + queues only work with virtio-net device; > > + disabled by default (one queue). > > + > > @type name: string > > @param name: name for the TAP interface being created; if an empty > > string is passed, the OS will generate a unique name > > > > - @return: (ifname, tapfd) > > + @return: (ifname, [tapfds]) > > @rtype: tuple > > > > """ > > - try: > > - tapfd = os.open("/dev/net/tun", os.O_RDWR) > > - except EnvironmentError: > > - raise errors.HypervisorError("Failed to open /dev/net/tun") > > + tapfds = [] > > > > - flags = IFF_TAP | IFF_NO_PI > > + for _ in range(virtio_net_queues): > > + try: > > + tapfd = os.open("/dev/net/tun", os.O_RDWR) > > + except EnvironmentError: > > + raise errors.HypervisorError("Failed to open /dev/net/tun") > > > > - if vnet_hdr and _ProbeTapVnetHdr(tapfd): > > - flags |= IFF_VNET_HDR > > + flags = IFF_TAP | IFF_NO_PI > > > > - # The struct ifreq ioctl request (see netdevice(7)) > > - ifr = struct.pack("16sh", name, flags) > > + if vnet_hdr and _ProbeTapVnetHdr(tapfd): > > + flags |= IFF_VNET_HDR > > > > - try: > > - res = fcntl.ioctl(tapfd, TUNSETIFF, ifr) > > - except EnvironmentError, err: > > - raise errors.HypervisorError("Failed to allocate a new TAP device: > %s" % > > - err) > > + # Check if it's ok to enable IFF_MULTI_QUEUE > > + if virtio_net_queues > 1 and _ProbeTapMqVirtioNet(tapfd): > > + flags |= IFF_MULTI_QUEUE > > + > > + # The struct ifreq ioctl request (see netdevice(7)) > > + ifr = struct.pack("16sh", name, flags) > > + > > + try: > > + res = fcntl.ioctl(tapfd, TUNSETIFF, ifr) > > + except EnvironmentError, err: > > + raise errors.HypervisorError("Failed to allocate a new TAP > device: %s" % > > + err) > > + > > + tapfds.append(tapfd) > > > > # Get the interface name from the ioctl > > ifname = struct.unpack("16sh", res)[0].strip("\x00") > > - return (ifname, tapfd) > > + > > + return (ifname, tapfds) > > diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst > > index 3d667e6..89fa56b 100644 > > --- a/man/gnt-instance.rst > > +++ b/man/gnt-instance.rst > > @@ -832,6 +832,25 @@ vnet\_hdr > > > > It is set to ``true`` by default. > > > > +virtio\_net\_queues > > + Valid for the KVM hypervisor. > > + > > + Set a number of queues (file descriptors) for tap device to > > + parallelize packets sending or receiving. Tap devices will be > > + created with MULTI_QUEUE (IFF_MULTI_QUEUE) support. This only > > + works with KVM paravirtual nics (virtio-net) and the maximum > > + number of queues is limited to ``8``. Tehnically this is an > > + extension of ``vnet_hdr`` which must be enabled for multiqueue > > + support. > > + > > + If set to ``1`` queue, it effectively disables multiqueue support > > + on the tap and virio-net devices. > > + > > + For instances it is necessary to manually set number of queues (on > > + Linux using: ``ethtool -L ethX combined $queues``). > > + > > + It is set to ``1`` by default. > > + > > The ``-O (--os-parameters)`` option allows customisation of the OS > > parameters. The actual parameter names and values depends on the OS > > being used, but the syntax is the same key=value. For example, setting > > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs > > index f9c3ef1..54f365c 100644 > > --- a/src/Ganeti/Constants.hs > > +++ b/src/Ganeti/Constants.hs > > @@ -1664,6 +1664,9 @@ hvVga = "vga" > > hvVhostNet :: String > > hvVhostNet = "vhost_net" > > > > +hvVirtioNetQueues :: String > > +hvVirtioNetQueues = "virtio_net_queues" > > + > > hvVifScript :: String > > hvVifScript = "vif_script" > > > > @@ -1780,6 +1783,7 @@ hvsParameterTypes = Map.fromList > > , (hvUseLocaltime, VTypeBool) > > , (hvVga, VTypeString) > > , (hvVhostNet, VTypeBool) > > + , (hvVirtioNetQueues, VTypeInt) > > , (hvVifScript, VTypeString) > > , (hvVifType, VTypeString) > > , (hvViridian, VTypeBool) > > @@ -3766,6 +3770,7 @@ hvcDefaults = > > , (hvSecurityDomain, PyValueEx "") > > , (hvKvmFlag, PyValueEx "") > > , (hvVhostNet, PyValueEx False) > > + , (hvVirtioNetQueues, PyValueEx (1 :: Int)) > > , (hvKvmUseChroot, PyValueEx False) > > , (hvKvmUserShutdown, PyValueEx False) > > , (hvMemPath, PyValueEx "") > > -- > > 1.7.10.4 > > > > -- > Jose Antonio Lopes > Ganeti Engineering > Google Germany GmbH > Dienerstr. 12, 80331, München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores > Steuernummer: 48/725/00206 > Umsatzsteueridentifikationsnummer: DE813741370 >
