[virt-manager PATCH 0/3] Create pool: show only available types

[Cole Robinson]

On 11/24/20 9:21 AM, Pino Toscano wrote:
> Hi,
> 
> this series adds a minimal StoragePoolCapabilities handling using the
> virConnect.getStoragePoolCapabilities libvirt API; this is used to
> filter the available pool types in the "Create pool" dialog, so it does
> not offer anymore pool types that cannot be created (as unsupported by
> the libvirt connection).
> 
> Pino Toscano (3):
>   support: check for virConnect.getStoragePoolCapabilities
>   virtinst: add a basic StoragePoolCapabilities
>   createpool: show only available pool types
> 
>  tests/data/capabilities/poolcaps-fs.xml | 207 ++++++++++++++++++++++++
>  tests/test_capabilities.py              |  25 +++
>  virtManager/createpool.py               |   7 +-
>  virtinst/__init__.py                    |   1 +
>  virtinst/storagepoolcapabilities.py     |  61 +++++++
>  virtinst/support.py                     |   2 +
>  6 files changed, 302 insertions(+), 1 deletion(-)
>  create mode 100644 tests/data/capabilities/poolcaps-fs.xml
>  create mode 100644 virtinst/storagepoolcapabilities.py
> 

The code looks fine but I am conflicted about this. I'm not sure it's
worth adding code to read and process storage capabilities XML at all.
I'd rather see the storage dialogs become smaller, not more
featureful/smarter at the cost of increased maintenance and potential
for future feature creep. For example sheepdog and mpath can be dropped
entirely IMO. rbd pool creation should probably be dropped because the
UI is never going to be comprehensive enough to handle the common case
which requires specifying an auth secret. Same may apply to gluster but
I'd need to double check.

As implemented I'm a little iffy on the UI too. Just hiding options
without giving the user a hint can cause confusion, like why is FOO
available as a pool option for one connection but not the other. There's
ways to fix it but at the cost of more code with goes back to my point
above.

Can you explain your motivation a bit: Has this caused you issues
before? Or is there something more useful in the storage XML that you
want to add support for afterwards?

Thanks,
Cole