Hi Dan.

On Fri, Jan 11, 2013 at 12:42 AM, Dan Kenigsberg <dan...@redhat.com> wrote:

> Hi Andrey,
>
> I just have seen your http://www.dreyou.org/ovirt/vdsm.patch for
> ovirt-3.2. I would like to thank you for your packaging and engineering
> work. I'd like to incorporate as much of it into vdsm's upstream so that
> it runs properly on any EL6 out-of-the-box.
>
> Could you help me understand some of your changes?
>
>
Glad to help you.



> I've posted http://gerrit.ovirt.org/#/c/10893/ which is taken from a
> former patchset of yours - is it intentionally missing form the 3.2
> version of your patchset?
>
> > From adf7dc96767783ab81993504267c3cfd65b4c1bb Mon Sep 17 00:00:00 2001
> > From: Andrey Gordeev <dre...@gmail.com>
> > Date: Fri, 7 Dec 2012 13:12:19 +0400
> > Subject: [PATCH 1/2] CentOS 6.2 changes
> >
> > ---
> >  build-aux/pkg-version |    6 +++++-
> >  vds_bootstrap/setup   |    2 +-
> >  vdsm.spec.in          |   30 ++++++++++++++++++++++++++----
> >  vdsm/configNetwork.py |    5 +++++
> >  vdsm/storage/misc.py  |    2 +-
> >  vdsm/vdsmd.init.in    |    3 ++-
> >  6 files changed, 40 insertions(+), 8 deletions(-)
> >
> > diff --git a/build-aux/pkg-version b/build-aux/pkg-version
> > index 346ad23..9c410ff 100755
> > --- a/build-aux/pkg-version
> > +++ b/build-aux/pkg-version
> > @@ -32,7 +32,11 @@ if test "x$1" = "x--full"; then
> >  elif test "x$1" = "x--version"; then
> >      echo $PKG_VERSION | awk "$AWK_VERSION" | tr -cd '[:alnum:].'
> >  elif test "x$1" = "x--release"; then
> > -    echo $PKG_VERSION | awk "$AWK_RELEASE" | tr -cd '[:alnum:].'
> > +    if [ $LOCAL_BUILDID ]; then
> > +       echo $PKG_VERSION | awk "$AWK_RELEASE" | sed
> "s/git.*$/{$LOCAL_BUILDID}/" | tr -cd '[:alnum:].'
> > +    else
> > +       echo $PKG_VERSION | awk "$AWK_RELEASE" | tr -cd '[:alnum:].'
> > +    fi
> >  else
> >      echo "usage: $0 [--full|--version|--release]"
> >      exit 1
> > diff --git a/vds_bootstrap/setup b/vds_bootstrap/setup
> > index 01306bd..177e089 100755
> > --- a/vds_bootstrap/setup
> > +++ b/vds_bootstrap/setup
> > @@ -29,7 +29,7 @@ import re
> >  import tempfile
> >  from time import strftime
> >
> > -SUPPORTED_PLATFORMS = [ "RedHatEnterpriseServer", "Fedora" ]
> > +SUPPORTED_PLATFORMS = [ "RedHatEnterpriseServer", "Fedora", "CentOS",
> "Scientific", "Oracle" ]
>
> I see no reason not to take this upstream
>
> >  HYPERVISOR_PLATFORMS = [ "RedHatEnterpriseVirtualizationHypervisor",
> "RedHatEnterpriseHypervisor", "oVirtNodeHypervisor" ]
> >  HYPERVISOR_RELEASE_FILE = '/etc/rhev-hypervisor-release'
> >  REDHAT_RELEASE_FILE = '/etc/redhat-release'
> > diff --git a/vdsm.spec.in b/vdsm.spec.in
> > index 465d548..a9de23d 100644
> > --- a/vdsm.spec.in
> > +++ b/vdsm.spec.in
> > @@ -102,8 +102,10 @@ Requires: iscsi-initiator-utils >= 6.2.0.872-15
> >  Requires: device-mapper-multipath >= 0.4.9-52
> >  Requires: e2fsprogs >= 1.41.12-11
> >  Requires: kernel >= 2.6.32-279.9.1
> > -Requires: sanlock >= 2.3-4, sanlock-python
> > -Requires: initscripts >= 9.03.31-2.el6_3.1
> > +#Requires: sanlock >= 2.3-4, sanlock-python
> > +#Requires: initscripts >= 9.03.31-2.el6_3.1
>
> These requirements are for real bugs. Better include them into your EL6
> flavor
> or build yourself.
>
>
Agreed with sanlock, I can build newest version from rhev rpms repo,
but initscripts packages has the different names with the same version in
different distributions i.e. 9.03.31-2.el6_3.1 - rhel
, 9.03.31-2.el6.centos.1 - centos



> > +Requires: sanlock >= 2.3-1, sanlock-python
> > +Requires: initscripts >= 9.03.31-2
> >  Requires: mom >= 0.3.0
> >  Requires: selinux-policy-targeted >= 3.7.19-80
> >  Requires: lvm2 >= 2.02.95-10.el6_3.2
> > @@ -431,6 +433,8 @@ install -Dm 0644 vdsm/limits.conf \
> >  # Install the SysV init scripts
> >  install -Dm 0755 vdsm/vdsmd.init %{buildroot}%{_initrddir}/vdsmd
> >  install -Dm 0755 vdsm_reg/vdsm-reg.init
> %{buildroot}%{_initrddir}/vdsm-reg
> > +install -Dm 0644 vdsm/vdsm.conf.sample \
> > +                 %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
>
> Federico, do you recall why don't we ship vdsm.conf on el?
>
> >
> >  # This is not commonplace, but we want /var/log/core to be a
> world-writable
> >  # dropbox for core dumps
> > @@ -475,6 +479,15 @@ export LC_ALL=C
> >  /usr/sbin/usermod -a -G %{qemu_group},%{vdsm_group} %{snlk_user}
> >
> >  %post
> > +if [ -f /etc/sysconfig/modules/softdog.modules ]
> > +then
> > +    cp /etc/sysconfig/modules/softdog.modules
> /etc/vdsm/softdog.modules.prevdsm
> > +fi
> > +echo -e '#!/bin/sh\nmodprobe softdog\nexit 0' >
> /etc/sysconfig/modules/softdog.modules
> > +chmod +x /etc/sysconfig/modules/softdog.modules
> > +/sbin/chkconfig wdmd on
> > +/sbin/chkconfig sanlock on
> > +
>
> Andrey/Federico: why is this hack required?
>
>
Really don't know, but in one of my installation on Intel Blade System, i
got error:

*"VM testVm is down. Exit message: internal error Failed to open socket to
sanlock daemon: No such file or directory.* "

I.e. softdog module not loaded, then I add this hack.



> >  %{_bindir}/vdsm-tool sebool-config
> >  # set the vdsm "secret" password for libvirt
> >  %{_bindir}/vdsm-tool set-saslpasswd
> > @@ -538,6 +551,15 @@ exit 0
> >  %endif
> >
> >  %postun
> > +if [ -f /etc/sysconfig/modules/softdog.modules ]
> > +then
> > +    cp /etc/sysconfig/modules/softdog.modules
> /etc/vdsm/softdog.modules.prevdsm
> > +fi
> > +echo -e '#!/bin/sh\nmodprobe softdog\nexit 0' >
> /etc/sysconfig/modules/softdog.modules
> > +chmod +x /etc/sysconfig/modules/softdog.modules
> > +/sbin/chkconfig wdmd on
> > +/sbin/chkconfig sanlock on
> > +
> >  %if 0%{?rhel}
> >  if [ "$1" -ge 1 ]; then
> >      /sbin/service vdsmd condrestart > /dev/null 2>&1
> > @@ -762,9 +784,9 @@ exit 0
> >  %files python
> >  %defattr(-, root, root, -)
> >  %{_bindir}/vdsm-tool
> > -%if !0%{?rhel}
> > +#%if !0%{?rhel}
> >  %config(noreplace) %{_sysconfdir}/%{vdsm_name}/vdsm.conf
> > -%endif
> > +#%endif
> >  %{python_sitearch}/%{vdsm_name}/__init__.py*
> >  %{python_sitearch}/%{vdsm_name}/config.py*
> >  %{python_sitearch}/%{vdsm_name}/constants.py*
> > diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
> > index 78fd3af..7bf6af7 100755
> > --- a/vdsm/configNetwork.py
> > +++ b/vdsm/configNetwork.py
> > @@ -890,6 +890,11 @@ def addNetwork(network, vlan=None, bonding=None,
> nics=None, ipaddr=None,
> >      _netinfo = netinfo.NetInfo()
> >      bridged = utils.tobool(bridged)
> >
> > +    # Hack here, netmask may be not defined, if this happen,
> > +    # set netmask to 255.255.255.0
> > +    if not netmask:
> > +        netmask = "255.255.255.0"
> > +
>
> When is this needed? Could it be related to PREFIX instead of NETMASK
> issue of http://gerrit.ovirt.org/9322 ?
>

This may be needed if you manually writing yours ifcfg files and not set
NETMASK string, in this case default netmask is 255.255.255.0, and this
check must be changed to:

if ipaddr and not netmask:
         netmask = "255.255.255.0"

to correct work with none or dhcp protocol.


>
> >      if mtu:
> >          mtu = int(mtu)
> >
> > diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
> > index 17d38ee..ada3196 100644
> > --- a/vdsm/storage/misc.py
> > +++ b/vdsm/storage/misc.py
> > @@ -748,7 +748,7 @@ class RollbackContext(object):
> >
> >          # re-raise the earliest exception
> >          if firstException is not None:
> > -            raise firstException, None, traceback
> > +            raise firstException(None, traceback)
>
> I do not believe that it is correct - is it only to satisfy old pep8
> tool not recognizing the peculiar "raise" syntax that is used here?
> Could you ship
> http://danken.fedorapeople.org/python-pep8-1.3.3-3.el6.noarch.rpm
> instead?
>
>
Yes, I hope.



> >
> >      def defer(self, func, *args, **kwargs):
> >          self._finally.append((func, args, kwargs))
> > diff --git a/vdsm/vdsmd.init.in b/vdsm/vdsmd.init.in
> > index 7c709a6..0fd206e 100755
> > --- a/vdsm/vdsmd.init.in
> > +++ b/vdsm/vdsmd.init.in
> > @@ -136,7 +136,8 @@ shutdown_conflicting_srv() {
> >  }
> >
> >  libvirt_should_use_upstart() {
> > -    [[ -x /sbin/initctl ]]
> > +#    [[ -x /sbin/initctl ]]
> > +    [[ -x /sbin/foo ]]
>
> Why is that? Don't you want to use upstart's libvirtd autostart after
> crash?
>
>
Because libvirtd service ships as systemv scripts in centos (and in rhel
6.3), and its not restarted when you restart vdsmd service.


> >  }
> >
> >  start_needed_srv() {
> > --
> > 1.7.1
> >
> >
> > From 6f7ea3241047f06a196c90549960460c174362fd Mon Sep 17 00:00:00 2001
> > From: Andrey Gordeev <dre...@gmail.com>
> > Date: Tue, 11 Dec 2012 11:47:33 +0400
> > Subject: [PATCH 2/2] Reset SecureXMLRPCServer.py due to M2Crypto errors
>
> This worries me quite a bit. Which M2Crypto errors are you referring to?
> We may need to revert http://gerrit.ovirt.org/8123 - but it has
> performance improvement that is a shame to miss.
>

Sorry about this string about M2Crypto errors, this was happen because
I misunderstood one message in one of internet forums :-).


Original problem was in strange timeout error when vm trying to migrate
from one host to another. I'm consulted with Juan Hernandez, but I'm can't
fully resolve this problem. I found the solution, but I'm not sure in it. I
override the "close" method in SSLSocket clas (SecureXMLRPCServer.py), in
this case time out error was gone. Here is this method:

    def close(self):
        import socket
        self.connection.shutdown(socket.SHUT_RDWR)
        self.connection.close()



> >
> > ---
> >  vdsm/SecureXMLRPCServer.py |   92
> +++++++-------------------------------------
> >  1 files changed, 14 insertions(+), 78 deletions(-)
> >
> > diff --git a/vdsm/SecureXMLRPCServer.py b/vdsm/SecureXMLRPCServer.py
> > index 2de1cf7..9396a28 100644
> > --- a/vdsm/SecureXMLRPCServer.py
> > +++ b/vdsm/SecureXMLRPCServer.py
> > @@ -34,86 +34,10 @@ import ssl
> >  import httplib
> >  import socket
> >  import SocketServer
> > -import logging
> > -
> > -from M2Crypto import SSL, X509
> >
> >  SecureXMLRPCRequestHandler =
> SimpleXMLRPCServer.SimpleXMLRPCRequestHandler
> >
> >
> > -class SSLSocket(object):
> > -    """SSL decorator for sockets.
> > -
> > -    This class wraps a socket returned by the accept method of a
> > -    server socket providing the SSL socket methods that are missing in
> > -    the connection class. The rest of the methods are just delegated.
> > -    """
> > -
> > -    def __init__(self, connection):
> > -        # Save the reference to the connection so that we can delegate
> > -        # calls to it later:
> > -        self.connection = connection
> > -
> > -    def gettimeout(self):
> > -        return self.connection.socket.gettimeout()
> > -
> > -    def __getattr__(self, name):
> > -        # This is how we delegate all the rest of the methods to the
> > -        # underlying SSL connection:
> > -        return getattr(self.connection, name)
> > -
> > -
> > -class SSLServerSocket(SSLSocket):
> > -    """SSL decorator for server sockets.
> > -
> > -    This class wraps a normal socket so that when the accept method is
> > -    called the accepted socket is also decorated.
> > -    """
> > -
> > -    def __init__(self, raw, certfile=None, keyfile=None, ca_certs=None,
> > -                 session_id="vdsm", protocol="sslv23"):
> > -        # Create the SSL context:
> > -        self.context = SSL.Context(protocol)
> > -        self.context.set_session_id_ctx(session_id)
> > -
> > -        # Load the server certificate and key files:
> > -        if certfile and keyfile:
> > -            self.context.load_cert_chain(certfile, keyfile)
> > -
> > -        def verify(context, certificate, error, depth, result):
> > -            # The validation of the client certificate has already been
> > -            # performed by the OpenSSL library and the handhake already
> aborted
> > -            # if it fails as we use the verify_fail_if_no_peer_cert
> mode. We
> > -            # are not doing any additional validation, so we just need
> to log
> > -            # it and return the same result.
> > -            if not result:
> > -                certificate = X509.X509(certificate)
> > -                logging.error(
> > -                    "invalid client certificate with subject \"%s\"",
> > -                    certificate.get_subject())
> > -            return result
> > -
> > -        # Load the certificates of the CAs used to verify client
> > -        # connections:
> > -        if ca_certs:
> > -            self.context.load_verify_locations(ca_certs)
> > -            self.context.set_verify(
> > -                mode=SSL.verify_peer | SSL.verify_fail_if_no_peer_cert,
> > -                depth=10,
> > -                callback=verify)
> > -
> > -        # Create the SSL connection:
> > -        self.connection = SSL.Connection(self.context, sock=raw)
> > -
> > -    def accept(self):
> > -        # The SSL connection already returns a SSL prepared socket, but
> it
> > -        # misses some of the methods that the XML PRC server uses, so
> we need
> > -        # to wrap it as well:
> > -        client, address = self.connection.accept()
> > -        client = SSLSocket(client)
> > -        return client, address
> > -
> > -
> >  class SecureXMLRPCServer(SimpleXMLRPCServer.SimpleXMLRPCServer):
> >      def __init__(self, addr,
> >
> requestHandler=SimpleXMLRPCServer.SimpleXMLRPCRequestHandler,
> > @@ -129,15 +53,27 @@ class
> SecureXMLRPCServer(SimpleXMLRPCServer.SimpleXMLRPCServer):
> >                   requestHandler,
> >                   logRequests, allow_none, encoding,
> >                   bind_and_activate=False)
> > -        self.socket = SSLServerSocket(raw=self.socket,
> certfile=certfile,
> > -                                      keyfile=keyfile,
> ca_certs=ca_certs)
> > +        self.socket = ssl.wrap_socket(self.socket,
> > +                 keyfile=keyfile, certfile=certfile,
> > +                 ca_certs=ca_certs, server_side=True,
> > +                 cert_reqs=ssl.CERT_REQUIRED,
> > +                 do_handshake_on_connect=False)
> >          if timeout is not None:
> >              self.socket.settimeout = timeout
> >          if bind_and_activate:
> >              self.server_bind()
> >              self.server_activate()
> >
> > +    def finish_request(self, request, client_address):
> > +        request.do_handshake()
> > +
> > +        return SimpleXMLRPCServer.SimpleXMLRPCServer.finish_request(
> > +                                                             self,
> > +                                                             request,
> > +
> client_address)
> > +
> >      def handle_error(self, request, client_address):
> > +        import logging
> >          logging.error('client %s', client_address, exc_info=True)
> >
> >
>

Andrey Gordeev aka dreyou
_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to