[NEW PATCH] BZ#733373, BZ#677149 - Assert clenupSpmLinks success. (via gerrit-bot)

New patch submitted by Eduardo Warszawski (ewarszaw(a)redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/864

commit 17827d6522cc7812d9cecbda04baeb1cf1e826e3
Author: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Date:   Sun Aug 28 17:49:32 2011 +0300

    BZ#733373, BZ#677149 - Assert clenupSpmLinks success.
    
    Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367

diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
index ae55e44..7ab969b 100644
--- a/vdsm/storage/spm.py
+++ b/vdsm/storage/spm.py
@@ -222,13 +222,10 @@ class SPM:
         """
         Cleanup All SPM related links.
         """
-        vms = glob.glob(os.path.join(cls.storage_repository, constants.UUID_GLOB_PATTERN,
sd.VMS_DIR))
-        tasks = glob.glob(os.path.join(cls.storage_repository,
constants.UUID_GLOB_PATTERN, sd.TASKS_DIR))
+        vms = os.path.join(cls.storage_repository, constants.UUID_GLOB_PATTERN,
sd.VMS_DIR)
+        tasks = os.path.join(cls.storage_repository, constants.UUID_GLOB_PATTERN,
sd.TASKS_DIR)
         cls.log.debug("cleaning links; %s %s", vms, tasks)
-        for d in vms:
-            os.unlink(d)
-        for d in tasks:
-            os.unlink(d)
+        misc.execCmd(['rm', "-rf", vms, tasks])
 
 
     def __overrideMethods(self):

Change in vdsm[master]: BZ#733373, BZ#677149 - Assert cleanupSpmLinks success.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#733373, BZ#677149 - Assert cleanupSpmLinks success.
......................................................................


Patch Set 2: Do not submit

It's wrong

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


BZ#733373 - Remove the pool level vsm and tasks links.

As verified by the previous version of this patch, the pool level
links to the tasks and vms are superflous, dangerous and can
prevent spm restart.
Since they are unused, best if they are not created at all.
cleanupSpmLinks() removed so it can't fail.
_createSpmLinks() removed too.

Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
---
M vdsm/storage/spm.py
1 file changed, 2 insertions(+), 43 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved


--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 6: Verified; Looks good to me, approved

finally Verified by Haim

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 5: Verified

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 5: Fails

still fails on can't persist task for some domains upon createVolume operation.

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 4: Fails

NACK - there is a problem with 'master' dir which now, get overwritten with
vdsm:qemu group.
guess that Edu's fix reviles an old bug.

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

Saggi Mizrahi has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 4: Looks good to me, but someone else must approve

You assume that no one ever accessed the links. Possible, but I need some preinteg
verification on this.

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373 - Remove the pool level vsm and tasks links.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#733373 - Remove the pool level vsm and tasks links.
......................................................................


Patch Set 4: Looks good to me, but someone else must approve

Should be carefully verified in different flows (including vdsm restart during async task
running).

Note: be aware that 'vms' and 'tasks' links under poll no more available

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373, BZ#677149 - Remove cleanupSpmLinks() so it can't ...

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#733373, BZ#677149 - Remove cleanupSpmLinks() so it can't fail.
......................................................................


Patch Set 3: Looks good to me, but someone else must approve

Should be carefully verified in different flows (including vdsm restart during async task
running). 

Note: be aware that 'vms' and 'tasks' links under poll no more available

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#733373, BZ#677149 - Assert cleanupSpmLinks success.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#733373, BZ#677149 - Assert cleanupSpmLinks success.
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/spm.py
Line 228:         misc.execCmd(['rm', "-rf", vms, tasks])
why do you trust `rm` more than ourselves? Is there a race here? wouldn't it stay?
use EXT_RM if you have a good excuse.

--
To view, visit http://gerrit.usersys.redhat.com/864
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id16baf5c23a7b71f25030c0443710f9412252367
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;