From f246e5d310812dea8e430d918b8932924d319630 Mon Sep 17 00:00:00 2001 From: Zheng Li Date: Wed, 2 Oct 2013 10:59:35 +0000 Subject: [PATCH 01/10] CP-6167: The xenbuilder chroot env has no /dev/tty It seems that the xenbuilder chroot env doesn't have /dev/tty. So in the build log, we saw "tee: /dev/tty: No such device or address" This is strangely different from the local chroot testing env I created using standard xenbuilder command "make sm-chroot" when doing test. This quick fix avoids using the "tee /dev/tty" trick and do it in a plain/verbose way. Signed-off-by: Zheng Li Reviewed-by: Vineeth Thampi Raveendran --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 68b252fd3..8a79f2eec 100755 --- a/Makefile +++ b/Makefile @@ -85,8 +85,8 @@ precommit: build CHANGED=$$(git status --porcelain $(SM_PY_FILES) | awk '{print $$2}'); \ for i in $$CHANGED; do \ echo Checking $${i} ...; \ - RESULT=$$(PYTHONPATH=./snapwatchd:./drivers:$$PAYTHONPATH pylint --rcfile=tests/pylintrc $${i} | tee /dev/tty); \ - [ -z "$$RESULT" ] || QUIT=1; \ + RESULT=$$(PYTHONPATH=./snapwatchd:./drivers:$$PAYTHONPATH pylint --rcfile=tests/pylintrc $${i}); \ + [ -z "$$RESULT" ] || { echo "$$RESULT"; QUIT=1; }; \ done; \ if [ $$QUIT -ne 0 ]; then \ exit 1; \ @@ -99,8 +99,8 @@ precheck: build @ QUIT=0; \ for i in $(SM_PY_FILES); do \ echo Checking $${i} ...; \ - RESULT=$$(PYTHONPATH=./snapwatchd:./drivers:$$PAYTHONPATH pylint --rcfile=tests/pylintrc $${i} | tee /dev/tty); \ - [ -z "$$RESULT" ] || QUIT=1; \ + RESULT=$$(PYTHONPATH=./snapwatchd:./drivers:$$PAYTHONPATH pylint --rcfile=tests/pylintrc $${i}); \ + [ -z "$$RESULT" ] || { echo "$$RESULT"; QUIT=1; }; \ done; \ if [ $$QUIT -ne 0 ]; then \ exit 1; \ From 7829f18c0c168ddd62e0e9e0bebbdd43ce882c8c Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Wed, 25 Sep 2013 10:42:37 +0100 Subject: [PATCH 02/10] SCTX-1536: Don't trust the footer Don't use the primary VHD footer for file-based VHD VDIs that are attached in R/W mode because it has been erased. Use the back-up one instead. Signed-off-by: Thanos Makatos Reviewed-by: Keith Petley GitHub: closes #35 on xapi-project/sm --- drivers/FileSR.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/FileSR.py b/drivers/FileSR.py index af0333b47..8812486f4 100755 --- a/drivers/FileSR.py +++ b/drivers/FileSR.py @@ -425,7 +425,14 @@ def load(self, vdi_uuid): return try: - diskinfo = util.ioretry(lambda: self._query_info(self.path)) + # If the VDI is activated in R/W mode, the VHD footer won't be + # valid, use the back-up one instead. + vdi_ref = self.sr.srcmd.params['vdi_ref'] + sm_config = self.session.xenapi.VDI.get_sm_config(vdi_ref) + use_bkp_footer = util.attached_as(sm_config) == 'RW' + diskinfo = util.ioretry(lambda: self._query_info(self.path, + use_bkp_footer)) + if diskinfo.has_key('parent'): self.parent = diskinfo['parent'] self.sm_config_override = {'vhd-parent':self.parent} @@ -841,9 +848,12 @@ def _query_p_uuid(self, path): ls = parent.split('/') return ls[len(ls) - 1].replace(vhdutil.FILE_EXTN_VHD, '') - def _query_info(self, path): + def _query_info(self, path, use_bkp_footer=False): diskinfo = {} - cmd = [SR.TAPDISK_UTIL, "query", vhdutil.VDI_TYPE_VHD, "-vpf", path] + qopts = '-vpf' + if use_bkp_footer: + qopts += 'b' + cmd = [SR.TAPDISK_UTIL, "query", vhdutil.VDI_TYPE_VHD, qopts, path] txt = util.pread(cmd).split('\n') diskinfo['size'] = txt[0] lst = [txt[1].split('/')[-1].replace(vhdutil.FILE_EXTN_VHD, "")] From 5bdfca0f30b7db7cf0663eac8643e569a6ad8b2c Mon Sep 17 00:00:00 2001 From: Germano Percossi Date: Mon, 30 Sep 2013 16:53:05 +0100 Subject: [PATCH 03/10] [CP-6333][CP-6334]: rm dm-multipath-dm-event, dm-multipath-disable-udev-rules This patch is the counterpart of 2 patches removed in dm-multipath (formerly dm-multipath in guest-packages.hg): - dm-multipath-disable-udev-rules - dm-multipath-dm-event This patch basically does the following: - ship and override multipath udev rules from sm RPM - add rules to invoke mpathcount from udev Note: for udev to catch the relevant events, the kernel must have DM_UEVENT enabled Signed-off-by: Germano Percossi Reviewed-by: Vineeth Thampi Raveendran GitHub: closes #40 on xapi-project/sm --- Makefile | 7 +++++++ drivers/mpathcount.py | 9 ++++++--- mk/sm.spec.in | 1 + multipath/40-multipath.rules | 12 ++++++++++++ 4 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 multipath/40-multipath.rules diff --git a/Makefile b/Makefile index 8a79f2eec..e04afc111 100755 --- a/Makefile +++ b/Makefile @@ -56,6 +56,8 @@ SM_LIBS += lcache SM_LIBS += resetvdis SM_LIBS += B_util +UDEV_RULES = 40-multipath + CRON_JOBS += ringwatch SM_XML := XE_SR_ERRORCODES @@ -67,6 +69,7 @@ MASTER_SCRIPT_DEST := /etc/xensource/master.d/ PLUGIN_SCRIPT_DEST := /etc/xapi.d/plugins/ LIBEXEC := /opt/xensource/libexec/ CRON_DEST := /etc/cron.d/ +UDEV_RULES_DIR := /etc/udev/rules.d/ SM_STAGING := $(DESTDIR) SM_STAMP := $(MY_OBJ_DIR)/.staging_stamp @@ -112,6 +115,7 @@ install: precheck mkdir -p $(SM_STAGING) $(call mkdir_clean,$(SM_STAGING)) mkdir -p $(SM_STAGING)$(SM_DEST) + mkdir -p $(SM_STAGING)$(UDEV_RULES_DIR) mkdir -p $(SM_STAGING)$(DEBUG_DEST) mkdir -p $(SM_STAGING)$(BIN_DEST) mkdir -p $(SM_STAGING)$(MASTER_SCRIPT_DEST) @@ -120,6 +124,9 @@ install: precheck for i in $(SM_PY_FILES); do \ install -m 755 $$i $(SM_STAGING)$(SM_DEST); \ done + for i in $(UDEV_RULES); do \ + install -m 644 multipath/$$i.rules \ + $(SM_STAGING)$(UDEV_RULES_DIR); done for i in $(SM_XML); do \ install -m 755 drivers/$$i.xml \ $(SM_STAGING)$(SM_DEST); done diff --git a/drivers/mpathcount.py b/drivers/mpathcount.py index cf8364da6..305526fbf 100755 --- a/drivers/mpathcount.py +++ b/drivers/mpathcount.py @@ -104,9 +104,12 @@ def match_dmpLUN(s): def match_pathup(s): s = re.sub('\]',' ',re.sub('\[','',s)).split() - dm_status = s[-1] - path_status = s[-2] - for val in [dm_status, path_status]: + # The new multipath has a different output. Fixed it + dm_status = s[-2] + path_status = s[-3] + # path_status is more reliable, at least for failures initiated or spotted by multipath + # To be tested for failures during high I/O when dm should spot errors first + for val in [path_status]: if val in ['faulty','shaky','failed']: return False return True diff --git a/mk/sm.spec.in b/mk/sm.spec.in index 0de737d1e..2bf219a47 100755 --- a/mk/sm.spec.in +++ b/mk/sm.spec.in @@ -229,6 +229,7 @@ rm -rf $RPM_BUILD_ROOT /opt/xensource/sm/xs_errors.pyc /opt/xensource/sm/xs_errors.pyo /sbin/mpathutil +%config /etc/udev/rules.d/40-multipath.rules %changelog diff --git a/multipath/40-multipath.rules b/multipath/40-multipath.rules new file mode 100644 index 000000000..24377197d --- /dev/null +++ b/multipath/40-multipath.rules @@ -0,0 +1,12 @@ +SUBSYSTEM!="block", GOTO="end_mpath" +RUN+="socket:/org/kernel/dm/multipath_event" +KERNEL!="dm-*", GOTO="end_mpath" +ACTION=="add", PROGRAM=="/bin/bash -c '/sbin/dmsetup info -c -o name --noheadings -j %M -m %m | /bin/grep VG_XenStorage'", OPTIONS+="ignore_device" +ACTION=="add", PROGRAM=="/bin/bash -c '/sbin/dmsetup info -c -o name --noheadings -j %M -m %m | /bin/grep XSLocalEXT'", OPTIONS+="ignore_device" +KERNEL=="dm-*", ACTION=="add", PROGRAM=="/sbin/dmsetup info -c -o name --noheadings -j %M -m %m", RESULT=="?*", SYMLINK+="disk/by-scsid/%c/mapper" +ACTION=="change", PROGRAM!="/sbin/dmsetup info -c --noheadings -j %M -m %m", GOTO="end_mpath" +PROGRAM!="/sbin/dmsetup info -c -o uuid,name --separator ' ' --noheadings -j %M -m %m", GOTO="end_mpath" +RESULT!="mpath-*", GOTO="end_mpath" +# ENV is necessary otherwise the child process of mpathcount cannot find multipathd executable +ACTION=="change", ENV{PATH}="/sbin:/bin:/usr/sbin:/usr/bin", RUN+="/opt/xensource/sm/mpathcount.py %c{2}" +LABEL="end_mpath" From b1864c1fca99a3c03f524c3eab289222b0f8b45b Mon Sep 17 00:00:00 2001 From: Germano Percossi Date: Tue, 15 Oct 2013 13:35:34 +0100 Subject: [PATCH 04/10] CA-111813: race condition setting "iscsi_sessions" in other_conf This changeset basically reinstate previous code, backing out "changeset: 1555:ac091775c8ea" in the Mercurial repository. To be safe, we remove the key before adding, just in case. Signed-off-by: Germano Percossi Reviewed-by: Vineeth Thampi Raveendran GitHub: closes #41 on xapi-project/sm --- drivers/ISCSISR.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/ISCSISR.py b/drivers/ISCSISR.py index 5aea88ad0..36a082d81 100755 --- a/drivers/ISCSISR.py +++ b/drivers/ISCSISR.py @@ -360,9 +360,12 @@ def attach(self, sr_uuid): try: pbdref = util.find_my_pbd(self.session, self.host_ref, self.sr_ref) if pbdref <> None: - other_conf = self.session.xenapi.PBD.get_other_config(pbdref) - other_conf['iscsi_sessions'] = str(sessions) - self.session.xenapi.PBD.set_other_config(pbdref, other_conf) + # Just to be safe in case of garbage left during crashes + # we remove the key and add it + self.session.xenapi.PBD.remove_from_other_config( + pbdref, "iscsi_sessions") + self.session.xenapi.PBD.add_to_other_config( + pbdref, "iscsi_sessions", str(sessions)) except: pass From 34452aaddca8ad6225f1bf8f606ad6c61ff70b94 Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Fri, 27 Sep 2013 13:04:40 +0100 Subject: [PATCH 05/10] CP-5050: Notify XAPI about critical coalesce errors The rate XAPI is notified is throttled to avoid spamming (currently defaults to one message per hour, per SR). This rate can be overridden by setting the "coalesce-error-rate" key in an SR's other-config. Signed-off-by: Thanos Makatos Suggested-by: Andrei Lifchits Reviewed-by: Germano Percossi GitHub: closes #38 on xapi-project/sm --- drivers/cleanup.py | 79 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/drivers/cleanup.py b/drivers/cleanup.py index 2ff117cc6..231ed3a92 100755 --- a/drivers/cleanup.py +++ b/drivers/cleanup.py @@ -29,6 +29,7 @@ import traceback import base64 import zlib +import errno import XenAPI import util @@ -61,6 +62,12 @@ LOCK_TYPE_RUNNING = "running" lockRunning = None +# Default coalesce error rate limit, in messages per minute. A zero value +# disables throttling, and a negative value disables error reporting. +DEFAULT_COALESCE_ERR_RATE = 1.0/60 + +COALESCE_LAST_ERR_TAG = 'last-coalesce-error' +COALESCE_ERR_RATE_TAG = 'coalesce-error-rate' class AbortException(util.SMException): pass @@ -687,10 +694,80 @@ def _runTapdiskDiff(self): Util.doexec(cmd, 0) return True + def _reportCoalesceError(vdi, ce): + """Reports a coalesce error to XenCenter. + + vdi: the VDI object on which the coalesce error occured + ce: the CommandException that was raised""" + + msg_name = os.strerror(ce.code) + if ce.code == errno.ENOSPC: + # TODO We could add more information here, e.g. exactly how much + # space is required for the particular coalesce, as well as actions + # to be taken by the user and consequences of not taking these + # actions. + msg_body = 'Run out of space while coalescing.' + elif ce.code == errno.EIO: + msg_body = 'I/O error while coalescing.' + else: + msg_body = '' + util.SMlog('Coalesce failed on SR %s: %s (%s)' + % (vdi.sr.uuid, msg_name, msg_body)) + + # Create a XenCenter message, but don't spam. + xapi = vdi.sr.xapi.session.xenapi + sr_ref = xapi.SR.get_by_uuid(vdi.sr.uuid) + oth_cfg = xapi.SR.get_other_config(sr_ref) + if COALESCE_ERR_RATE_TAG in oth_cfg: + coalesce_err_rate = float(oth_cfg[COALESCE_ERR_RATE_TAG]) + else: + coalesce_err_rate = DEFAULT_COALESCE_ERR_RATE + + xcmsg = False + if coalesce_err_rate == 0: + xcmsg = True + elif coalesce_err_rate > 0: + now = datetime.datetime.now() + sm_cfg = xapi.SR.get_sm_config(sr_ref) + if COALESCE_LAST_ERR_TAG in sm_cfg: + # seconds per message (minimum distance in time between two + # messages in seconds) + spm = datetime.timedelta(seconds=(1.0/coalesce_err_rate)*60) + last = datetime.datetime.fromtimestamp( + float(sm_cfg[COALESCE_LAST_ERR_TAG])) + if now - last >= spm: + xapi.SR.remove_from_sm_config(sr_ref, + COALESCE_LAST_ERR_TAG) + xcmsg = True + else: + xcmsg = True + if xcmsg: + xapi.SR.add_to_sm_config(sr_ref, COALESCE_LAST_ERR_TAG, + str(now.strftime('%s'))) + if xcmsg: + xapi.message.create(msg_name, "3", "SR", vdi.sr.uuid, msg_body) + _reportCoalesceError = staticmethod(_reportCoalesceError) + + def _doCoalesceVHD(vdi): + try: + vhdutil.coalesce(vdi.path) + except util.CommandException, ce: + # We use try/except for the following piece of code because it runs + # in a separate process context and errors will not be caught and + # reported by anyone. + try: + VDI._reportCoalesceError(vdi, ce) + except Exception, e: + util.SMlog('failed to create XenCenter message: %s' % e) + raise ce + except: + raise + _doCoalesceVHD = staticmethod(_doCoalesceVHD) + def _coalesceVHD(self, timeOut): Util.log(" Running VHD coalesce on %s" % self) abortTest = lambda:IPCFlag(self.sr.uuid).test(FLAG_TYPE_ABORT) - Util.runAbortable(lambda: vhdutil.coalesce(self.path), None, + Util.runAbortable(lambda: VDI._doCoalesceVHD(self), None, self.sr.uuid, abortTest, VDI.POLL_INTERVAL, timeOut) util.fistpoint.activate("LVHDRT_coalescing_VHD_data",self.sr.uuid) From 083722c17dcc8a29371ae2a4fd00c6a87ce3275d Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Wed, 9 Oct 2013 16:55:56 +0100 Subject: [PATCH 06/10] CA-106886: Don't update vhd-blocks during SR.scan Don't bother updating the vhd-blocks in the XAPI database as this is owned by the GC and there's no synchronisation for this. Signed-off-by: Thanos Makatos Reviewed-by: Vineeth Thampi Raveendran GitHub: closes #37 on xapi-project/sm --- drivers/VDI.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/VDI.py b/drivers/VDI.py index 8f53369d6..74c739f6a 100755 --- a/drivers/VDI.py +++ b/drivers/VDI.py @@ -320,9 +320,11 @@ def _override_sm_config(self, sm_config): del sm_config[key] def _db_update_sm_config(self, ref, sm_config): + import cleanup current_sm_config = self.sr.session.xenapi.VDI.get_sm_config(ref) for key, val in sm_config.iteritems(): - if key.startswith("host_") or key == "paused": + if key.startswith("host_") or \ + key in ["paused", cleanup.VDI.DB_VHD_BLOCKS]: continue if sm_config.get(key) != current_sm_config.get(key): util.SMlog("_db_update_sm_config: %s sm-config:%s %s->%s" % \ @@ -331,7 +333,9 @@ def _db_update_sm_config(self, ref, sm_config): self.sr.session.xenapi.VDI.add_to_sm_config(ref, key, val) for key in current_sm_config.keys(): - if key.startswith("host_") or key == "paused" or key in self.sm_config_keep: + if key.startswith("host_") or \ + key in ["paused", cleanup.VDI.DB_VHD_BLOCKS] or \ + key in self.sm_config_keep: continue if not sm_config.get(key): util.SMlog("_db_update_sm_config: %s del sm-config:%s" % \ From c5876945944c777b6034493d75b03808d130bfc5 Mon Sep 17 00:00:00 2001 From: Germano Percossi Date: Thu, 3 Oct 2013 23:53:24 +0100 Subject: [PATCH 07/10] CP-6335: Removal of dm-multipath-init patch This functionality has stopped being a patch to upstream multipath and it is shipped in its own startup script provided by sm RPM Signed-off-by: Germano Percossi Reviewed-by: Vineeth Thampi Raveendran GitHub: closes #42 on xapi-project/sm --- Makefile | 5 +++ mk/sm.spec.in | 5 +++ multipath/sm-multipath | 86 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100755 multipath/sm-multipath diff --git a/Makefile b/Makefile index e04afc111..a2bd4c271 100755 --- a/Makefile +++ b/Makefile @@ -57,6 +57,7 @@ SM_LIBS += resetvdis SM_LIBS += B_util UDEV_RULES = 40-multipath +MPATH_DAEMON = sm-multipath CRON_JOBS += ringwatch @@ -70,6 +71,7 @@ PLUGIN_SCRIPT_DEST := /etc/xapi.d/plugins/ LIBEXEC := /opt/xensource/libexec/ CRON_DEST := /etc/cron.d/ UDEV_RULES_DIR := /etc/udev/rules.d/ +INIT_DIR := /etc/rc.d/init.d/ SM_STAGING := $(DESTDIR) SM_STAMP := $(MY_OBJ_DIR)/.staging_stamp @@ -116,6 +118,7 @@ install: precheck $(call mkdir_clean,$(SM_STAGING)) mkdir -p $(SM_STAGING)$(SM_DEST) mkdir -p $(SM_STAGING)$(UDEV_RULES_DIR) + mkdir -p $(SM_STAGING)$(INIT_DIR) mkdir -p $(SM_STAGING)$(DEBUG_DEST) mkdir -p $(SM_STAGING)$(BIN_DEST) mkdir -p $(SM_STAGING)$(MASTER_SCRIPT_DEST) @@ -124,6 +127,8 @@ install: precheck for i in $(SM_PY_FILES); do \ install -m 755 $$i $(SM_STAGING)$(SM_DEST); \ done + install -m 755 multipath/$(MPATH_DAEMON) \ + $(SM_STAGING)/$(INIT_DIR) for i in $(UDEV_RULES); do \ install -m 644 multipath/$$i.rules \ $(SM_STAGING)$(UDEV_RULES_DIR); done diff --git a/mk/sm.spec.in b/mk/sm.spec.in index 2bf219a47..445f5b444 100755 --- a/mk/sm.spec.in +++ b/mk/sm.spec.in @@ -28,6 +28,10 @@ rm -rf $RPM_BUILD_ROOT %post [ ! -x /sbin/chkconfig ] || chkconfig --add mpathroot +[ ! -x /sbin/chkconfig ] || chkconfig --add sm-multipath + +%preun +[ ! -x /sbin/chkconfig ] || chkconfig --del sm-multipath %files %defattr(-,root,root,-) @@ -229,6 +233,7 @@ rm -rf $RPM_BUILD_ROOT /opt/xensource/sm/xs_errors.pyc /opt/xensource/sm/xs_errors.pyo /sbin/mpathutil +/etc/rc.d/init.d/sm-multipath %config /etc/udev/rules.d/40-multipath.rules diff --git a/multipath/sm-multipath b/multipath/sm-multipath new file mode 100755 index 000000000..bb96d5de2 --- /dev/null +++ b/multipath/sm-multipath @@ -0,0 +1,86 @@ +#!/bin/bash +# +# sm-multipath Support function for multipath in SM +# +# chkconfig: 2345 16 77 +# description: Create proper symlinks in /dev/ if root is multipathed + +### BEGIN INIT INFO +# Provides: sm-multipath +# Required-Start: +# Required-Stop: +# Default-Start: +# Default-Stop: +# Short-Description: Support function for multipath in SM +# Description: Create proper symlinks in /dev/ if +# root is multipathed +### END INIT INFO + +DAEMON=/sbin/multipathd +prog=sm-multipath +initdir=/etc/rc.d/init.d + +. $initdir/functions + +RETVAL=0 + +# +# See how we were called. +# + +start() { + test -x $DAEMON || exit 5 + echo -n $"Multipath check for root device: " + $DAEMON -k"show top" &>/dev/null + RETVAL=$? + [ $RETVAL -eq 0 ] || failure || exit 0 + success + + # Create an mpInuse symlink for the root device if that is multipath. + DEVICE_MAPPER_MAJOR=$(sed -ne 's/^\([0-9]\+\) device-mapper$/\1/p' /proc/devices) + DEVICE_MAPPER_MAJOR_HEX=$(printf "%x" ${DEVICE_MAPPER_MAJOR}) + ROOT_PART_MAJOR=$(stat --format=%t /dev/root) + if [ "$ROOT_PART_MAJOR" == "$DEVICE_MAPPER_MAJOR_HEX" ] ; then + ROOT_PART_MINOR=$(stat --format=%T /dev/root) + ROOT_PART_SLAVE=$(/bin/ls /sys/block/dm-$ROOT_PART_MINOR/slaves) + ROOT_DISK_MINOR=${ROOT_PART_SLAVE#dm-} + MPATH_NODES="$(dmsetup ls --target multipath --exec ls)" + for n in $MPATH_NODES ; do + NODE_MINOR="$(stat --format=%T $n)" + if [ "$ROOT_DISK_MINOR" = "$NODE_MINOR" ] ; then + mkdir -p /dev/disk/mpInuse + ln -sf $n /dev/disk/mpInuse + fi + done + fi + + echo +} + +stop() { + echo -n $"Stopping $prog daemon: " + success + echo +} + +restart() { + stop + start +} + +case "$1" in +start) + start + ;; +stop) + stop + ;; +restart) + restart + ;; +*) + echo $"Usage: $0 {start|stop|status|restart|condrestart|reload}" + RETVAL=2 +esac + +exit $RETVAL From 7c21ec7d2e2b5e00c099d489fb784fc024c585e7 Mon Sep 17 00:00:00 2001 From: Zheng Li Date: Fri, 18 Oct 2013 10:55:09 +0000 Subject: [PATCH 08/10] CA-118762: Bring self.ty back My previous pylint change 96e56bf7 replaced self.ty as it was not defined anywhere in the VDI object. But this breaks some horrible hack in the underlying layer, which was probably implementated that way for backward "compatability" reason by itself. Signed-off-by: Zheng Li Reviewed-by: Vineeth Thampi Raveendran --- drivers/LVHDSR.py | 2 +- drivers/VDI.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/LVHDSR.py b/drivers/LVHDSR.py index f0d9fdb5b..d11e33710 100755 --- a/drivers/LVHDSR.py +++ b/drivers/LVHDSR.py @@ -1277,7 +1277,7 @@ def create(self, sr_uuid, vdi_uuid, size): IS_A_SNAPSHOT_TAG: 0, SNAPSHOT_OF_TAG: '', SNAPSHOT_TIME_TAG: '', - TYPE_TAG: self.type, + TYPE_TAG: self.ty, VDI_TYPE_TAG: self.vdi_type, READ_ONLY_TAG: int(self.read_only), MANAGED_TAG: int(self.managed), diff --git a/drivers/VDI.py b/drivers/VDI.py index 74c739f6a..98349d8fb 100755 --- a/drivers/VDI.py +++ b/drivers/VDI.py @@ -103,7 +103,7 @@ def __init__(self, sr, uuid): self.sm_config_override = {} self.sm_config_keep = [] self.path = None - self.type = None + self.ty = "user" self.load(uuid) From 7018eaa16d55cba6c66a1dba2877da2f1b8c1a4c Mon Sep 17 00:00:00 2001 From: Zheng Li Date: Fri, 18 Oct 2013 11:34:32 +0000 Subject: [PATCH 09/10] CA-118781: Initialize parent class first Another incompatability issue introduced by my pylint serie. On the other hand, the root cause was some bad code manifested due by this change. In some subclass of VDI.VDI, we set some fields before initializting the VDI.VDI parent class, so that these fields might be overwritten during the parent class initialization. As a common rule of thumb, we should generally do parent class initialization first and then do overwritten and extention in the following steps in the children initializor. And any information essential for parent class initialization should be defined as a parameter of the parent __init__ method. E.g. uuid is a predefined parameter of the initialization method in the parent class (VDI.VDI), but many children classes still don't use this parameter, and instead do uuid assignment in the children classes instead and then call the parent initializor. This patchset doesn't correct these issues for the moment, but we should pay attention to this, otherwise we might get bitten in the future. Signed-off-by: Zheng Li Reviewed-by: Vineeth Thampi Raveendran GitHub: closes #43 on xapi-project/sm --- drivers/SHMSR.py | 2 +- drivers/udevSR.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/SHMSR.py b/drivers/SHMSR.py index 42bf1b794..bc645720b 100644 --- a/drivers/SHMSR.py +++ b/drivers/SHMSR.py @@ -109,8 +109,8 @@ def load(self, vdi_uuid): def __init__(self, mysr, uuid, filename): self.uuid = uuid - self.path = os.path.join(mysr.dconf['location'], filename) VDI.VDI.__init__(self, mysr, None) + self.path = os.path.join(mysr.dconf['location'], filename) self.label = filename self.location = filename self.vdi_type = 'file' diff --git a/drivers/udevSR.py b/drivers/udevSR.py index ae22db533..024cfed11 100755 --- a/drivers/udevSR.py +++ b/drivers/udevSR.py @@ -114,8 +114,8 @@ def read_whole_file(filename): class udevVDI(VDI.VDI): def __init__(self, sr, location): - self.location = location VDI.VDI.__init__(self, sr, None) + self.location = location def load(self, location): self.path = self.location From 101d74929c0c6b4acf08334a64dbdd44f3a824c3 Mon Sep 17 00:00:00 2001 From: chandrikas Date: Tue, 22 Oct 2013 12:22:10 +0100 Subject: [PATCH 10/10] CP-5817: Productise removal of lvm2-environment-device.patch Part of the change for rolling back lvm2-environment-device.patch. Removed code that exports special environment variable to direct LVM to look for an abbreviated device cache. The second part of the change (actual removal of lvm patch) is under guest-packages. Signed-off-by: Chandrikas Srinivasan Reviewed-by: Zheng Li --- drivers/SR.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/SR.py b/drivers/SR.py index f2c073c83..9fef10d2a 100755 --- a/drivers/SR.py +++ b/drivers/SR.py @@ -107,12 +107,6 @@ def __init__(self, srcmd, sr_uuid): if 'sr_ref' in self.srcmd.params: self.sr_ref = self.srcmd.params['sr_ref'] - if 'device_config' in self.srcmd.params: - if self.srcmd.params['device_config'].has_key('SCSIid'): - dev_path = '/dev/disk/by-scsid/'+self.srcmd.params['device_config']['SCSIid'] - os.environ['LVM_DEVICE'] = dev_path - util.SMlog('Setting LVM_DEVICE to %s' % dev_path) - except Exception, e: raise e raise xs_errors.XenError('SRBadXML')