Re: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities

On Wed, Jan 30, 2019 at 03:55:05PM +0000, Daniel P. Berrangé wrote: > On Wed, Jan 30, 2019 at 10:03:43AM -0500, John Ferlan wrote: > > > > > > On 1/30/19 3:31 AM, Pavel Hrdina wrote: > > > On Tue, Jan 15, 2019 at 08:15:47PM -0500, John Ferlan wrote: > > >> https://bugzilla.redhat.com/show_bug.cgi?id=1581670 > > >> > > >> Introduce the bare bones functions to processing capability > > >> data for the storage driver. Currently just looking to store > > >> and format the storage pool types in output, such as: > > >> > > >> <pool> > > >> <type>dir</pool> > > >> > > >> <pool> > > >> <type>fs</pool> > > >> </pool> > > >> > > >> ... > > >> > > >> <pool> > > >> <type>iscsi-direct</pool> > > >> </pool> > > > > > > This looks weird, if you look into output of domcapabilities we use > > > different formatting to list type values, so how about this: > > > > > > <pool> > > > <enum name='type'> > > > <value>dir</value> > > > <value>fs</value> > > > ... > > > </enum> > > > </pool> > > > > > > The name of the enum could be 'backend' as well. > > > > > > Pavel > > > > > > > This format is fine by me... Keeping enum is fine as well since it > > follows other examples > > > > Do you have any opinions on whether listing the API's supported each > > pool is a worthwhile effort in any form? Building on the above, the > > output could be API by API: > > > > <pool> > > <enum name='type'> > > <value>dir</value> > > <value>fs</value> > > ... > > </enum> > > <pool_api name='virConnectFindStoragePoolSources'> > > <value>fs</value> > > <value>gluster</value> > > ... > > </pool_api> > > ... N pool_api's > > <vol_api name='virStorageVolUpload'> > > <value>disk</value> > > <value>fs</value> > > ... > > </vol_api> > > ... N vol_api's > > I really don't think we should go down that route. Whether a specific > API works with a specific feature is really something that is practically > only determined at the time the API is invoked, as whether it works or > not may depend on the full set of arguments you pass to the API. > > IOW, at most I would list which pool driver backends are present > in the capabilities. I wonder if there's a case to be made for not putting this in the global capabilities at all, and instead creating a virStoragePoolGetCapabilities() API that works in the same way as virDomainGetCapabilities (ie just accepts a pool type as an API parameter. If it returns an error you can assume the pool type is not supported. The XML doc can then be used to report on various enums the storage XML format may or may not support, such as disk file formats for file backends.