hscan should save corrected fixdata

[ksanislo]

When this PR was merged back in 2017, hscan was updated to look at the new data and present it to stdout, however the change was never applied to the saved copy (LOCAL.data) of the information.

When this incorrect data is reloaded through hbal, under some conditions the invalid memory information can cause it to silently stop considering otherwise viable instance moves.

Correcting hscan to actually save the fixed data for external use, instead of saving the unmodified copy keeps things working as expected.

[rbott]

@apoikos @iustin do you have any insights on this? From my rather limited understanding of Haskell, it looks good.

[iustin]

Hmm, I'm not sure this is the right thing. The basic question is whether a saved data file will have "corrected" or "uncorrected" data baked in. The code as it is before this commit, seems to keep "uncorrected" data and hbal should correct it on the fly as well, via the ExtLoader.loadExternalData, no? Is that not happening?

The patch would change things so that saved data is "corrected", but then we would need to know to prevent all the downstream uses from again applying the correction.

[ksanislo]

@iustin The problem is that hbal won't correct data when loading it from a dump.

Please keep in mind that I'm not particularly knowledgeable in Haskell, and it has been 6 months since I looked into this, but in my limited understanding it seemed that the data gets 'fixed' only when it is missing. By saving the original uncorrected data, when that dump gets reloaded in the future, it now has what seems to be valid data in those fields. Since the saved LOCAL.data file doesn't specify if nodes are Xen vs KVM or even something else, there seems to be no way to determine that a specific datapoint would even need changed on reloading. In this case, trusting the saved file content happens automatically, so ensuring the corrected data is what gets written into the file seems like the proper choice.

[iustin]

Thanks for the expanded explanation. You're right, I misunderstood the original commit; it makes more sense to always have "correct" data in the file.

I will have to look again at the current/code since I thought it does attempt to correct the data somehow - it's all, I think, going through the same pipeline.

At least for this commit, the cleanup can be expanded - the cdata argument is not used, and thus should be removed from the argument list.