Fix missing informations on geo replication tab of GlusterFS volumes. #340
Conversation
There was a problem hiding this comment.
Thank you for the fix! In order not to break hosts running with older GlusterFS versions, it should either support both the old and new formats or a dependency on the new GlusterFS version should be added to the .spec.in file (which would probably break el8 environments, so supporting both the formats seems to be preferable).
|
OK, i'll update the code so that both version are supported. |
|
I updated the code to make sure that both (old and new) format of XML output are supported. |
There was a problem hiding this comment.
Looks good to me, just one suggestion for a possible improvement inside the code.
A technical note: Please squash the two commits -- we don't squash commits when merging a PR, commits are kept separate because they may implement different pieces. For this reason, we use force (replacement) commits rather than amendment commits when modifying a PR during the review process.
yasalos
force-pushed
the
fix-gluster-10-geo-replication-info
branch
from
966dd38 to
bb32139
Compare
yasalos
force-pushed
the
fix-gluster-10-geo-replication-info
branch
from
bb32139 to
b7f7950
Compare
There was a problem hiding this comment.
Looks good to me, after having addressed the comments from @mz-pdm.
A couple of minor comments from my side.
yasalos
force-pushed
the
fix-gluster-10-geo-replication-info
branch
3 times, most recently
from
604884b to
46ba996
Compare
|
Some more errors from the linter (see the CI failures -- too long lines). |
Since the upgrade to oVirt 4.5 and GlusterFS 10, information about snapshots and geo replication are no more visible on the engine Webui. After digging VDSM logs, I found that it's caused by the change of some XML elements name introduced in a new Gluster release (gluster/glusterfs#1568) and by the addition of the timezone in output of status command. This commit fix the parsing process of the output returned by gluster command, so that data sent to ovirt engine are as expected. Bug-URL: https://bugzilla.redhat.com/show_bug.cgi?id=2104278
yasalos
force-pushed
the
fix-gluster-10-geo-replication-info
branch
from
46ba996 to
d332442
Compare
Fixed |
|
Great, CI and I are happy now. Let's see whether OST is happy too, as well as Albert about the formatting :-). |
|
/ost |
|
/ost |
|
Although not reported here for whatever reason, OST actually ran and succeeded. |
|
Congratulations to completing your first contribution! |
It was not just my first contribution to oVirt, but to any project. I want to thank, both Albert and you for leading the way and helping me during this process. Now, I have to go and read the new contributors guidelines section in development.md file ;-) |
Since the upgrade to oVirt 4.5 and GlusterFS 10, information about snapshots and geo replication are no more visible on the engine Webui. After digging VDSM logs, I found that it's caused by the change of some XML elements name introduced in a new Gluster release (gluster/glusterfs#1568) and by the addition of the timezone in output of status command.
This commit fix the parsing process of the output returned by gluster command, so that data sent to ovirt engine are as expected.
Bug-URL: https://bugzilla.redhat.com/show_bug.cgi?id=2104278