Change in vdsm[master]: Enable VM migration to old vdsm's.

Eduardo has uploaded a new change for review.

Change subject: Enable VM migration to old vdsm's.
......................................................................

Enable VM migration to old vdsm's.

Old vdsm's can't run VM from /var/run/vdsm/storage and rely on
/rhev/data-center hierarchy.

Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1011608
Signed-off-by: Eduardo <ewarszaw(a)redhat.com&gt;
---
M vdsm/storage/blockSD.py
M vdsm/storage/fileSD.py
M vdsm/storage/hsm.py
3 files changed, 22 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/19825/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index d27f331..b014253 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -1029,6 +1029,19 @@
         vols, rems = self.getAllVolumesImages()
         return rems
 
+    def linkBCImage(self, imgPath, imgUUID):
+        dst = os.path.join(self.mountpoint, self.sdUUID, sd.DOMAIN_IMAGES,
+                           imgUUID)
+        try:
+            os.symlink(imgPath, dst)
+        except OSError as e:
+            if e.errno == errno.EEXIST:
+                self.log.debug("img run vol already exists: %s", dstVol)
+            else:
+                self.log.error("Failed to create img run vol: %s", dstVol)
+                raise
+        return dst
+
     def createImageLinks(self, srcImgPath, imgUUID, volUUIDs):
         """
         qcow chain is build by reading each qcow header and reading the path
diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index 8e788a5..8da30bb 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -418,6 +418,10 @@
         return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent']))
                     for k, v in volumes.iteritems())
 
+    def linkBCImage(self, imgPath, imgUUID):
+        return os.path.join(self.mountpoint, self.sdUUID, sd.DOMAIN_IMAGES,
+                              imgUUID)
+
     def createImageLinks(self, srcImgPath, imgUUID):
         """
         qcow chain is build by reading each qcow header and reading the path
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 4f5a8a7..061182d 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3230,8 +3230,12 @@
         # Filter volumes related to this image
         imgVolumes = sd.getVolsOfImage(allVols, imgUUID).keys()
         imgPath = dom.activateVolumes(imgUUID, imgVolumes)
+        if spUUID and spUUID != sd.BLANK_UUID:
+            runImgPath = dom.linkBCImage(imgPath, imgUUID)
+        else:
+            runImgPath = imgPath
 
-        leafPath = os.path.join(imgPath, leafUUID)
+        leafPath = os.path.join(runImgPath, leafUUID)
         for volUUID in imgVolumes:
             path = os.path.join(dom.domaindir, sd.DOMAIN_IMAGES, imgUUID,
                                 volUUID)


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 5:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4698/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4774/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3889/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 5: Verified+1 Code-Review+2

Getting this patch in is more important than dropping the redundant pre-raise log line.

Thanks, Eduardo and Ayal!

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 4:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4697/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4773/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3888/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Ayal Baron has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("path to img directory already exists:
%s", dst)
s/img/image/
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create path to img dir:
%s", dst)
Line 1042:                 raise
Line 1043:         return dst


Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("path to img directory already exists:
%s", dst)
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create path to img dir:
%s", dst)
ditto
Line 1042:                 raise
Line 1043:         return dst
Line 1044: 
Line 1045:     def createImageLinks(self, srcImgPath, imgUUID, volUUIDs):


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2: Verified+1

I've tested migration between pre-"one shot prepare" vdsm to a version with
this patch applied. both directions worked after fixing http://gerrit.ovirt.org/#/c/19859/
.

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Eduardo has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(1 comment)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
Stale link to where? There are not too many dst that can be there.
Link to the image are created by other functions too and the net result is the same,
linking the block image.

This method is only called from vm.py.

Running again the same VM, activating the dame SD/image, will result in the same link.

In the case you described, you are talking about removing something that we don't know
what is, assuming that is engine trying to run a VM with an image that is not (but was) in
this SD.
This is a serious engine bug, and it should be solved in the LSM.
Having the same image UUID in two SDs, only engine can determine which is the correct
one.
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Yeela Kaplan has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(2 comments)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
Not readable.

Elaborate on what is img run vol.
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


....................................................
File vdsm/storage/hsm.py
Line 3229:         allVols = dom.getAllVolumes()
Line 3230:         # Filter volumes related to this image
Line 3231:         imgVolumes = sd.getVolsOfImage(allVols, imgUUID).keys()
Line 3232:         imgPath = dom.activateVolumes(imgUUID, imgVolumes)
Line 3233:         if spUUID and spUUID != sd.BLANK_UUID:
Why would you have a scenario where spUUID is None???
Line 3234:             runImgPath = dom.linkBCImage(imgPath, imgUUID)
Line 3235:         else:
Line 3236:             runImgPath = imgPath
Line 3237: 


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Ayal Baron has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(1 comment)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
is this the correct behaviour? shouldn't we overwrite it in this case? (in case the
link is wrong)
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

This is much much better than what I have envisaged.

....................................................
File vdsm/storage/blockSD.py
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
I hate these excessive logs. Just loves them...
Line 1042:                 raise
Line 1043:         return dst
Line 1044: 
Line 1045:     def createImageLinks(self, srcImgPath, imgUUID, volUUIDs):


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 1: Code-Review-1 Verified-1

Build Failed 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4668/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4744/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3859/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4670/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4746/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3861/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4695/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4771/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3886/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Enable VM migration to old vdsm's.

Old vdsm's can't run VM from /var/run/vdsm/storage and rely on
/rhev/data-center hierarchy.

Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1011608
Signed-off-by: Eduardo <ewarszaw(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/19825
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
Tested-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm/storage/blockSD.py
M vdsm/storage/fileSD.py
M vdsm/storage/hsm.py
3 files changed, 24 insertions(+), 1 deletion(-)

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



-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(1 comment)

Ayal, maybe I misunderstood the condition that worries you. If so, please re-explain it. I
am a bit anxious to unbreak migration in ovirt-3.3 by promptly taking this patch.

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
I tend to agree with Eduardo: the link source and destination contains the very same
sdUUID. If someone called prepareImage() with sdUUID, we are bound to return a path to
that domain, even though a newer image exists on another domain. In such a case, the bug
exists higher in the stack, with whomever asked to prepare the image on the stale storage
domain.
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(1 comment)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
how about "path to image directory already exists: %s"
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Ayal Baron has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
What?
I have a stale link there to the wrong image dir (from a previous run VM which wasn't
cleaned because host crashed or whatever) and since then user ran liveStorageMigration and
moved the disk to a new sd.
I think this is a bug.
Also, this is not runVM context, we're in blockSD
Please fix
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Eduardo has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 2:

(2 comments)

....................................................
File vdsm/storage/blockSD.py
Line 1035:         try:
Line 1036:             os.symlink(imgPath, dst)
Line 1037:         except OSError as e:
Line 1038:             if e.errno == errno.EEXIST:
Line 1039:                 self.log.debug("img run dir already exists: %s",
dst)
Yes. If the image exists and is not the "True" link, running a VM is not   where
we need to fix it.

In such case we need to remove the cause of the spurious link.

Anyway, it will contain the leaf or not, and in the last case the VM run will fail.
Line 1040:             else:
Line 1041:                 self.log.error("Failed to create img run dir: %s",
dst)
Line 1042:                 raise
Line 1043:         return dst


....................................................
File vdsm/storage/hsm.py
Line 3229:         allVols = dom.getAllVolumes()
Line 3230:         # Filter volumes related to this image
Line 3231:         imgVolumes = sd.getVolsOfImage(allVols, imgUUID).keys()
Line 3232:         imgPath = dom.activateVolumes(imgUUID, imgVolumes)
Line 3233:         if spUUID and spUUID != sd.BLANK_UUID:
Ugly.

It was required as the interface diferentiator for backwards compatibility.

For running VMs without pool in new hosts engine will not sent the pool ID parameter at
all.

Personally I dislike it.
Line 3234:             runImgPath = dom.linkBCImage(imgPath, imgUUID)
Line 3235:         else:
Line 3236:             runImgPath = imgPath
Line 3237: 


-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Eduardo has posted comments on this change.

Change subject: Enable VM migration to old vdsm's.
......................................................................


Patch Set 1:

vmId: b889a032-0a78-4591-bbc4-53157dd6d1be
<name>the_vm</name>

was started in compatibility mode (valid spUUID) with:

<source
dev="/rhev/data-center/mnt/blockSD/d0dccb15-bc4e-4815-889a-c116028f30b0/images/3a611e65-6261-4680-9a98-0fa8a4664ecc/775147a3-e652-4d8b-9e06-ef1a964a9cc5"/>

This should be OK for migrate to old vdsm's.
[root@aqua-vds5 ~]# virsh -r list
 Id    Name                           State
----------------------------------------------------
 1     the_vm                         running

-- 
To view, visit http://gerrit.ovirt.org/19825
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No