Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Jenkins CI has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 15: Propagate review hook: Continuous Integration value inherited from patch 14 -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 14: * #1321018::Update tracker: OK * #59725::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 15: * #1321018::Update tracker: OK * #59725::Update tracker: OK * Set MODIFIED::bug 1321018#1321018::IGNORE, skipping for branch 'master' -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has submitted this change and it was merged. Change subject: Live Merge: Remove volume run link after live merge .. Live Merge: Remove volume run link after live merge When deleting a volume while the VM is running, volume teardown doesn't remove the volume run symbolic link: /run/vdsm/storage/sdUUID/volUUID. In patch Iec3b6a (Live Merge: teardown volume on HSM after live merge) we added volume teardown logic that, for block storage it deactivated the volume. In this patch we extend volume teardown logic to unlink volume run link. Note that this change isn't required for file storage as no symbolic links are created. Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Bug-Url: https://bugzilla.redhat.com/1321018 Signed-off-by: Ala Hino Reviewed-on: https://gerrit.ovirt.org/59725 Reviewed-by: Nir Soffer Reviewed-by: Adam Litke Continuous-Integration: Nir Soffer --- M vdsm/storage/blockSD.py 1 file changed, 15 insertions(+), 0 deletions(-) Approvals: Adam Litke: Looks good to me, approved Nir Soffer: Looks good to me, but someone else must approve; Passed CI tests Ala Hino: Verified -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 14: Continuous-Integration+1 -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Adam Litke has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 13: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 13: Code-Review+1 Waiting for more reviews. -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 13: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 12: Verified+1 -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 12: * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/59725/10/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 821: """
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
> Which comment? I thought I addressed all comments
The comment about this log line.
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: raise
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Ala Hino has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 10:
(2 comments)
https://gerrit.ovirt.org/#/c/59725/10/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 821: """
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
> ./vdsm/storage/blockSD.py:825: undefined name 'volRunLink'
Which comment? I thought I addressed all comments
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: raise
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
Line 826: os.unlink(volRunLink)
> Same error here.
Done
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: raise
Line 830: self.log.debug("Volume run link %r does not exist",
volRunlink)
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 11: * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 10: Code-Review-1
(2 comments)
https://gerrit.ovirt.org/#/c/59725/10/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 821: """
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
./vdsm/storage/blockSD.py:825: undefined name 'volRunLink'
This is the result of using mixedCase names in Python. In Java this works since
the compiler detect this errors, in Python using this style is a recipe for
failure.
To avoid this, lets use only lowercase_with_underscores names for all local
variables.
Unfortunately, for method names and arguments we should match the module style.
Also, you ignored one of my comment about the log above in previous version,
please address it.
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: raise
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
Line 826: os.unlink(volRunLink)
Same error here.
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: raise
Line 830: self.log.debug("Volume run link %r does not exist",
volRunlink)
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Ala Hino has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 9:
(2 comments)
https://gerrit.ovirt.org/#/c/59725/9/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
PS9, Line 829: self.log.error("Failed to unlink volume run link: %r",
:volRunlink)
> I'd just raise without logging
Done
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: self.log.error("Failed to unlink volume run link: %r",
Line 830:volRunlink)
> I commented about this in
Done
Line 831: raise
Line 832: self.log.debug("Volume run link %r does not exist",
volRunlink)
Line 833:
Line 834:
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 10: * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Nir Soffer has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 9:
(2 comments)
https://gerrit.ovirt.org/#/c/59725/8/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 821: """
Line 822: volRunlink = os.path.join(constants.P_VDSM_STORAGE,
Line 823: self.sdUUID, imgUUID, volUUID)
Line 824: try:
Line 825: self.log.info("Unlinking volme runtime link: %r",
volRunLink)
Read again the comments in version 6 (your replied Done, but ignored the
comments).
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: self.log.error("Failed to unlink volume run link: %r",
https://gerrit.ovirt.org/#/c/59725/9/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 826: os.unlink(volRunLink)
Line 827: except OSError as e:
Line 828: if e.error != errno.ENOENT:
Line 829: self.log.error("Failed to unlink volume run link: %r",
Line 830:volRunlink)
> I'd just raise without logging
I commented about this in
https://gerrit.ovirt.org/#/c/59725/6/vdsm/storage/blockSD.py
Seems like leftover from older version.
Line 831: raise
Line 832: self.log.debug("Volume run link %r does not exist",
volRunlink)
Line 833:
Line 834:
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Allon Mureinik has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 9: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/59725/9/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
PS9, Line 829: self.log.error("Failed to unlink volume run link: %r",
:volRunlink)
I'd just raise without logging
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 9: * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
Ala Hino has posted comments on this change.
Change subject: Live Merge: Remove volume run link after live merge
..
Patch Set 6:
(9 comments)
https://gerrit.ovirt.org/#/c/59725/6//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2016-09-22 16:41:56 +0300
Line 4: Commit: Ala Hino
Line 5: CommitDate: 2016-09-22 17:01:55 +0300
Line 6:
Line 7: Live Merge: Unlink volume runtime dir after live merge
> Remove volume run link after live merge?
Done
Line 8:
Line 9: Unlink volume runtime dir after live megre.
Line 10:
Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Line 5: CommitDate: 2016-09-22 17:01:55 +0300
Line 6:
Line 7: Live Merge: Unlink volume runtime dir after live merge
Line 8:
Line 9: Unlink volume runtime dir after live megre.
> Please explain the context like you did in the previous patch.
Done
Line 10:
Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Line 12: Bug-Url: https://bugzilla.redhat.com/1321018
Line 5: CommitDate: 2016-09-22 17:01:55 +0300
Line 6:
Line 7: Live Merge: Unlink volume runtime dir after live merge
Line 8:
Line 9: Unlink volume runtime dir after live megre.
> Please also explain here why this is not needed in file storage.
Done
Line 10:
Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Line 12: Bug-Url: https://bugzilla.redhat.com/1321018
https://gerrit.ovirt.org/#/c/59725/6/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 810: offset = ((slot + blockVolume.RESERVED_LEASES) *
self.logBlkSize *
Line 811: sd.LEASE_BLOCKS)
Line 812: return clusterlock.Lease(volUUID, self.getLeasesFilePath(),
offset)
Line 813:
Line 814: def teardownVolume(self, imgUUID, volUUID):
> This change belongs to the previous patch.
Done
Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID])
Line 816: self.removeVolumeRunDir(imgUUID, volUUID)
Line 817:
Line 818: def removeVolumeRunDir(self, imgUUID, volUUID):
Line 814: def teardownVolume(self, imgUUID, volUUID):
Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID])
Line 816: self.removeVolumeRunDir(imgUUID, volUUID)
Line 817:
Line 818: def removeVolumeRunDir(self, imgUUID, volUUID):
> removeVolumeRunLink
Done
Line 819: """
Line 820: Remove /run/vdsm/storage/sdUUID/imgUUID/volUUID
Line 821: """
Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID)
Line 820: Remove /run/vdsm/storage/sdUUID/imgUUID/volUUID
Line 821: """
Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID)
Line 823: imgRunDir = os.path.join(sdRunDir, imgUUID)
Line 824: volRunDir = os.path.join(imgRunDir, volUUID)
> volSymlink, this is not a directory
Done
Line 825: try:
Line 826: self.log.info("Unlinking volme runtime dir: %s",
volRunDir)
Line 827: os.unlink(volRunDir)
Line 828: except OSError as e:
Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID)
Line 823: imgRunDir = os.path.join(sdRunDir, imgUUID)
Line 824: volRunDir = os.path.join(imgRunDir, volUUID)
Line 825: try:
Line 826: self.log.info("Unlinking volme runtime dir: %s",
volRunDir)
> The log cannot raise OSError, it should always be out of the try block.
Done
Line 827: os.unlink(volRunDir)
Line 828: except OSError as e:
Line 829: if e.error == errno.ENOENT:
Line 830: self.log.debug("Link doesn't exist")
Line 830: self.log.debug("Link doesn't exist")
Line 831: else:
Line 832: self.log.error("Failed to unlink vol runtime dir: %s",
Line 833:volRunDir)
Line 834: raise
> Logging and raising is bad practice. If you cannot handle the error, raise,
Done
Line 835:
Line 836:
Line 837: class BlockStorageDomain(sd.StorageDomain):
Line 838: manifestClass = BlockStorageDomainManifest
https://gerrit.ovirt.org/#/c/59725/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4781: self.success = True
Line 4782: self.vm.log.info("Synchronization completed (job %s)",
Line 4783: self.job['jobID'])
Line 4784: # teardown the merged volume
Line 4785: self.teardownVolume(self.drive.imageID,
self.job['topVolume'])
> All these changes belong to the previous patch.
Done
Line 4786:
Line 4787: def isSuccessful(self):
Line 4788: """
Line 4789: Returns True if this phase completed successfully.
--
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer
Change in vdsm[master]: Live Merge: Remove volume run link after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge .. Patch Set 8: * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1321018::OK, public bug * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59725 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
