On 03/27/2017 05:19 PM, Jiri Denemark wrote:
On Mon, Mar 27, 2017 at 13:42:22 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1398087
Clean up the virsh man page description for --pool-create-as in order to better describe how the various arguments are used when creating (or defining) a logical pool.
Also modify the storage pool XML parsing algorithm to check for the mismatched "name" and "source-name".
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 8 ++++++++ tools/virsh.pod | 7 +++++++ 2 files changed, 15 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 585ca71..5213503 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -760,6 +760,14 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (VIR_STRDUP(ret->source.name, ret->name) < 0) goto error; } + if (ret->type == VIR_STORAGE_POOL_LOGICAL && + STRNEQ(ret->name, ret->source.name)) { + virReportError(VIR_ERR_XML_ERROR, + _("for a logical pool, the pool name='%s' " + "must match the pool source name='%s'"), + ret->name, ret->source.name); + goto error;
Wrong indentation...
+ }
but why exactly is this forbidden now? I should be able to create a pool with a (libvirt's) name which differs from the (system's) name of the volume group, shouldn't I? And apparently it used to work while it is not working now after this patch as failing virt-manager builds on ci.centos.org suggest.
If 'source.name' isn't supplied it defaults to ret->name. The ret->name is supposed to be the Volume Group name (it's the "unique" to the host name). Although I suppose it could be something different, but if only the name is required in order to define/create the pool, then how does one "ensure" that the name provided is the volume group name. It's kind of a conundrum... Still I certainly suppose someone could do something different, so I suppose this part could be reverted. FWIW: The 'source.name' is used is by the logical backend for various vg* and lv* commands. So if it's not the Volume Group name, then commands fail (as seen in the bz). I really think it's just "smart sense" to make them be the same to avoid "confusion". John