Don't pass -1 to unsigned long long in swig bindings (bsc#1221222)

[shundhammer]

Bugzilla

https://bugzilla.suse.com/show_bug.cgi?id=1221222

Problem

Build failure of yast-storage-ng with the latest SWIG bindings.

Cause

The unit tests use DiskSize::unlimited in the YAML files for many test scenarios for LVs, and that results in a value -1 which clashes with the libstorage function prototype that expects unsigned long long for create_lvm_lv().

Obviously SWIG decides at runtime (!) which C++ function to call, and now the checks appear to be stricter: A -1 worked well until this version; it had obviously converted it to the bit pattern of the expected unsigned type, in this case resulting in 16 EiB - 1. Now that doesn't work anymore.

Fix

If DiskSize::unlimited (-1) is detected in the FakeDeviceFactory for that function, use a very large unsigned long long value instead (4 EiB).

[coveralls]

coverage: 97.81%. remained the same when pulling 480b72f36e9bce07fcd4c6f63871ef9c482554a9 on huha-fix-swig into b3e7d0d6ab334f7f6484646a615fe1ad5e66744d on master.

[shundhammer]

I thought about that, but that might be a more sweeping change with more side effects.

Actually, one benefit of Ruby is that there are no such limitations anymore in the language itself; they exist in interfaces such as the bindings to C++. Internally, we can make a clear distinction between unlimited and a very large value like used here.

In this concrete function call, the former unlimited value only worked (as Arvin wrote in the bug) because it's all just fake device graphs; in the real world this would fail miserably, as this would be vastly overcommitting in every thinkable real-world storage device. Outside of unit tests, this is not an option.

I had thought that libstorage-ng simply uses the maximum available value, but in reality that is not the case; it really uses that huge value.

[mvidner]

I had thought that libstorage-ng simply uses the maximum available value, but in reality that is not the case; it really uses that huge value.

Ah, then please add a comment,

# libstorage-ng uses 4EiB too, in foo.c

[shundhammer]

I think that is the new example that Arvin just wrote for this bug. Is that relevant here?

[mvidner]

I think that is the new example that Arvin just wrote for this bug. Is that relevant here?

Which one? This one uses 1GiB. https://github.com/openSUSE/libstorage-ng/blob/d9593b6570edf2dac9e2fc719da66397b8399ac7/bindings/ruby/examples/lvm.rb#L16

[shundhammer]

I don't know; he wrote about it on Slack. Maybe it's not checked in yet; maybe it was just a quick and dirty test to see if our theory with -1 and unsigned long long was correct.

One way or the other, this was meant as a pragmatic fix for the build failure, and since the failure only occurred in our unit tests which create a lot of fake device graphs with sometimes very theoretical values (there is no such thing as unlimited in the real storage world), I wanted to keep the changes localized.

Do we want a pragmatic solution, or do we really need endless discussions about theoretical cases that can never happen in real life anyway?

[aschnell]

I don't know; he wrote about it on Slack. Maybe it's not checked in yet; maybe it was just a quick and dirty test to see if our theory with -1 and unsigned long long was correct.

Yes, it was just a quick check and not checked into git.

[shundhammer]

Superseded by PR #1375