Merge lp:~danilo/maas/virsh-storage-units into lp:maas/2.2
Status: | Rejected |
---|---|
Rejected by: | MAAS Lander |
Proposed branch: | lp:~danilo/maas/virsh-storage-units |
Merge into: | lp:maas/2.2 |
Diff against target: |
141 lines (+63/-12) 2 files modified
src/provisioningserver/drivers/pod/tests/test_virsh.py (+23/-0) src/provisioningserver/drivers/pod/virsh.py (+40/-12) |
To merge this branch: | bzr merge lp:~danilo/maas/virsh-storage-units |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Needs Fixing | ||
Review via email: mp+325913@code.launchpad.net |
Commit message
Consider size units for storage when evaluating virsh pod storage.
Description of the change
This changes .get_key_value() on the Virsh driver to not strip the unit, and introduces a "_unitless" variant that still does that. This highlights a few potential future problems as well (memory size in KiB, CPU speed in Mhz, if those units change, we'll have similar problems).
I wonder why did we not end up using libvirt instead which would provide us stable units, but I imagine they might not be py3 bindings available or something along those lines.
Testing instructions:
I have not tested this yet (I only have one system where I have >1TiB LVM volume), so it seems easier to test this with a small storage partition instead (eg. MiB to ensure that still works), but generally you need to:
- set up a libvirt on a system and add a storage pool of >1TiB
- ensure "virsh pool-info" lists the storage pool details in TiB
- add a pod to MAAS and look at available storage: it should still be correct
Unmerged revisions
- 6066. By Данило Шеган
-
Fix test data so it adds up.
- 6065. By Данило Шеган
-
Interpret units for virsh calls.
This looks fine, but I'm not sure why we aren't using "virsh pool-dumpxml", which would report the units consistently in the first place. For example, I see the following:
... >61966548992< /capacity> >35070156800< /allocation> >26896392192< /available>
<capacity unit='bytes'
<allocation unit='bytes'
<available unit='bytes'
...
I'm going to approve anyway, since it seems like that work would be well beyond the scope of this branch.