Re: [libvirt] [PATCH] introducing <source> <name> (for logical storage pools)
David Lively <dlively(a)virtualiron.com> wrote:
I've attached a (very) small incremental patch (i.e., to be
applied
after the one you've already merged) that addresses a couple things I
noticed missing:
(a) documents the new <source> <name> element in formatstorage.html.in
(b) adds --source-name to the (optional) args for virsh pool-define-as
I've also attached a new version of the full patch containing this
change, in case that's easier.
Hi Dave,
That looks fine.
The only nit I saw was in the context:
diff --git a/src/storage_conf.c b/src/storage_conf.c
...
{
+ ret->name = virXPathString(conn, "string(/pool/name)", ctxt);
+ if (ret->name == NULL &&
+ options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_NAME)
+ ret->name = virXPathString(conn, "string(/pool/source/name)",
ctxt);
+ if (ret->name == NULL) {
virStorageReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("missing name element"));
There, it'd be clearer to diagnose with something like
"missing pool source device name", since there are a few
other "name" elements.
Not a show-stopper at all, but it'd be great if you could add a unit
test or two, so this new code gets some regular coverage.