On Wed, Mar 06, 2019 at 12:21:02PM -0500, John Ferlan wrote:
[...]
+ + <p>The following section decribes subelements of the + <code>poolOptions</code> and <code>volOptions</code> subelements </p>: + + <dl> + <dt><code>defaultFormat</code></dt> + <dd>For the <code>poolOptions</code>, the <code>type</code> attribute + describes the default format name used for the pool source. For the + <code>volOptions</code>, the <code>type</code> attribute describes + the default volume name used for each volume. + </dd> + <dl> + <dt><code>enum</code></dt> + <dd>Each enum uses a name from the list below with any number of + <code>value</code> value subelements describing the valid values. + <dl> + <dt><code>sourceFormatType</code></dt> + <dd>Lists all the possible <code>poolOptions</code> source + pool format types. + </dd> + <dt><code>requiredSourceElements</code></dt> + <dd>Lists all the required <code>poolOptions</code> source + subelements required for a valid source pool element. + </dd>
I know that this is now pushed and I just noticed that in the relevant BZ where you posted the output of storage capabilities.
Why do we export <requiredSourceElements> in storage capabilities? It doesn't make any sense to have it there. Management applications using libvirt have to have some knowledge of libvirt and they have to know what elements are required for each storage pool type in order to create some sensible UI. In addition this is something that will most likely never change and will not depend on what packages are installed or how libvirt/qemu were compiled.
Because it was data that perhaps someone would find useful when formulating XML for a storage pool. Each pool has different "required" elements that are hidden in the bowels of storage_conf and I figured it could be useful to have. Creating/defining a pool of a type that doesn't have a required element would cause a failure.
IMHO we should drop this element from storage capabilities unless there was some motivation to include this information.
IDC either way and am fine with dropping that element. The patches themselves were posted since 2/12, pinged on twice, sorry if you missed the details before I ended up pushing them. We have plenty of time before the 5.2.0 release to make a decision at least!
NP, my fault that I didn't notice that sooner, I just skimmed the patches telling myself that I'll review it later since I was working on virt-manager patches. Anyway, I'll send a patch to remove it from capabilities where we can decide what whether to keep it there or not. Pavel