Eduardo has posted comments on this change. Change subject: Use shared lock for setStorageDomainDescription command ......................................................................
Patch Set 3: Do not submit This review is temporary in order to avoid the merge of this patch until some questions will be ellucidated. By https://bugzilla.redhat.com/show_bug.cgi?id=819316#c2 this is and old issue. (Already known in 2009!) 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. 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 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. The SD description (name in the MD) is retrieved by getStorageDomainInfo command, unused by vdsm, and never passed as an indentifier, compared, etc. 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? -- 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 <sgot...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Eduardo <ewars...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
