Eduardo has posted comments on this change.
Change subject: Use shared lock for setStorageDomainDescription command
......................................................................
Patch Set 3: No score
Ayal Baron 8:59 AM
Patch Set 3:
^ This review is temporary in order to avoid the merge of this patch until some questions
will be ellucidated.
This does not mean it should have a -2
We agree. Only for opening the debate. I'm sorry.
^ ^ By
https://bugzilla.redhat.com/show_bug.cgi?id=819316#c2 this is and old issue.
(Already known in 2009!)
That doesn't mean it shouldn't be fixed
We agree.
^ ^ The actual state is that this operation can be issued to a host which is not the SPM.
In the block case this can lead to DL.
Which in itself is a good enough reason to fix it
We agree.
^ ^ If such cases are not exist, we can deduce that the command was not issued very often
and/or that engine sent this command to the SPM only.
The fact that engine doesn't call it as an HSM verb doesn't mean we should leave
it as such.
We agree.
^ ^ The actual patch makes the command to fail if it was issued to a non SPM host, which
is absolutely right, but this can lead to removal the engine protection, if it exists. In
such case new engines sending this command to wrong hosts running the old vdsm
(unprotected) code and producing DL.
This doesn't even make sense and we're wasting time here on not merging a patch
that you agree is fixing the problem.
^ The SD description (name in the MD) is retrieved by getStorageDomainInfo command, unused
by vdsm, and never passed as an indentifier, compared, etc.
The purpose of it is to be able to correlate what this domain is from vdsm side without
needing to know what the UUID of the domain means.
^ ^ In case that the engine issued the getStorageDomain command I'm dobious if this
field content is used in any form. ^ ^ The same info is already in the engine DB. ^ ^ Can
this verb to be replaced by a NO OP in the vdsm side? When setStorageDomainDescription is
issued by the engine?
No.
--
To view, visit
http://gerrit.ovirt.org/17198
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idca7b7b6642c222d88a6cd2a94d4033c0c3ef70b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server