7:15 p.m.
Although I suppose this could have been an RFC - I just went with
a v1. I would think at least the first 4 patches are non controversial.
Beyond that it depends on what is "expected" as output for the
capabilities output for the storage driver.
John Ferlan (7):
conf: Extract host XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatHostXML to take virCapsHostPtr
conf: Extract guest XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatGuestXML to take virCapsGuestPtr
conf: Introduce storage pool functions into capabilities
storage: Process storage pool capabilities
storage: Add storage backend pool/vol API's to capability output
src/conf/capabilities.c | 449 ++++++++++++++++++++++------------
src/conf/capabilities.h | 19 ++
src/conf/virstorageobj.h | 4 +
src/libvirt_private.syms | 1 +
src/storage/storage_backend.c | 65 +++++
src/storage/storage_backend.h | 3 +
src/storage/storage_driver.c | 17 ++
7 files changed, 400 insertions(+), 158 deletions(-)
--
2.20.1
7:15 p.m.
New subject: [libvirt] [PATCH 1/7] conf: Extract host XML formatting from virCapabilitiesFormatXML
Let's extract out the <host> code into it's own method/helper.
NB: One minor change between the two is usage of "buf" instead
of "&buf" in the new code since we pass the address of &buf to
the helper.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 135 ++++++++++++++++++++++------------------
1 file changed, 76 insertions(+), 59 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 8e9bba0dbe..33c0c37fbf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1056,130 +1056,147 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf,
return 0;
}
-/**
- * virCapabilitiesFormatXML:
- * @caps: capabilities to format
- *
- * Convert the capabilities object into an XML representation
- *
- * Returns the XML document as a string
- */
-char *
-virCapabilitiesFormatXML(virCapsPtr caps)
+
+static int
+virCapabilitiesFormatHostXML(virCapsPtr caps,
+ virBufferPtr buf)
{
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- size_t i, j, k;
+ size_t i, j;
char host_uuid[VIR_UUID_STRING_BUFLEN];
- virBufferAddLit(&buf, "<capabilities>\n\n");
- virBufferAdjustIndent(&buf, 2);
- virBufferAddLit(&buf, "<host>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<host>\n");
+ virBufferAdjustIndent(buf, 2);
if (virUUIDIsValid(caps->host.host_uuid)) {
virUUIDFormat(caps->host.host_uuid, host_uuid);
- virBufferAsprintf(&buf, "<uuid>%s</uuid>\n",
host_uuid);
+ virBufferAsprintf(buf, "<uuid>%s</uuid>\n", host_uuid);
}
- virBufferAddLit(&buf, "<cpu>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<cpu>\n");
+ virBufferAdjustIndent(buf, 2);
if (caps->host.arch)
- virBufferAsprintf(&buf, "<arch>%s</arch>\n",
+ virBufferAsprintf(buf, "<arch>%s</arch>\n",
virArchToString(caps->host.arch));
if (caps->host.nfeatures) {
- virBufferAddLit(&buf, "<features>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<features>\n");
+ virBufferAdjustIndent(buf, 2);
for (i = 0; i < caps->host.nfeatures; i++) {
- virBufferAsprintf(&buf, "<%s/>\n",
+ virBufferAsprintf(buf, "<%s/>\n",
caps->host.features[i]);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</features>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</features>\n");
}
- virCPUDefFormatBuf(&buf, caps->host.cpu);
+ virCPUDefFormatBuf(buf, caps->host.cpu);
for (i = 0; i < caps->host.nPagesSize; i++) {
- virBufferAsprintf(&buf, "<pages unit='KiB'
size='%u'/>\n",
+ virBufferAsprintf(buf, "<pages unit='KiB'
size='%u'/>\n",
caps->host.pagesSize[i]);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</cpu>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</cpu>\n");
/* The PM query was successful. */
if (caps->host.powerMgmt) {
/* The host supports some PM features. */
unsigned int pm = caps->host.powerMgmt;
- virBufferAddLit(&buf, "<power_management>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<power_management>\n");
+ virBufferAdjustIndent(buf, 2);
while (pm) {
int bit = ffs(pm) - 1;
- virBufferAsprintf(&buf, "<%s/>\n",
+ virBufferAsprintf(buf, "<%s/>\n",
virCapsHostPMTargetTypeToString(bit));
pm &= ~(1U << bit);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</power_management>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</power_management>\n");
} else {
/* The host does not support any PM feature. */
- virBufferAddLit(&buf, "<power_management/>\n");
+ virBufferAddLit(buf, "<power_management/>\n");
}
- virBufferAsprintf(&buf, "<iommu support='%s'/>\n",
+ virBufferAsprintf(buf, "<iommu support='%s'/>\n",
caps->host.iommu ? "yes" : "no");
if (caps->host.offlineMigrate) {
- virBufferAddLit(&buf, "<migration_features>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<migration_features>\n");
+ virBufferAdjustIndent(buf, 2);
if (caps->host.liveMigrate)
- virBufferAddLit(&buf, "<live/>\n");
+ virBufferAddLit(buf, "<live/>\n");
if (caps->host.nmigrateTrans) {
- virBufferAddLit(&buf, "<uri_transports>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<uri_transports>\n");
+ virBufferAdjustIndent(buf, 2);
for (i = 0; i < caps->host.nmigrateTrans; i++) {
- virBufferAsprintf(&buf,
"<uri_transport>%s</uri_transport>\n",
+ virBufferAsprintf(buf,
"<uri_transport>%s</uri_transport>\n",
caps->host.migrateTrans[i]);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</uri_transports>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</uri_transports>\n");
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</migration_features>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</migration_features>\n");
}
if (caps->host.netprefix)
- virBufferAsprintf(&buf, "<netprefix>%s</netprefix>\n",
+ virBufferAsprintf(buf, "<netprefix>%s</netprefix>\n",
caps->host.netprefix);
if (caps->host.nnumaCell &&
- virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
+ virCapabilitiesFormatNUMATopology(buf, caps->host.nnumaCell,
caps->host.numaCell) < 0)
goto error;
- if (virCapabilitiesFormatCaches(&buf, &caps->host.cache) < 0)
+ if (virCapabilitiesFormatCaches(buf, &caps->host.cache) < 0)
goto error;
- if (virCapabilitiesFormatMemoryBandwidth(&buf, &caps->host.memBW) < 0)
+ if (virCapabilitiesFormatMemoryBandwidth(buf, &caps->host.memBW) < 0)
goto error;
for (i = 0; i < caps->host.nsecModels; i++) {
- virBufferAddLit(&buf, "<secmodel>\n");
- virBufferAdjustIndent(&buf, 2);
- virBufferAsprintf(&buf, "<model>%s</model>\n",
+ virBufferAddLit(buf, "<secmodel>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<model>%s</model>\n",
caps->host.secModels[i].model);
- virBufferAsprintf(&buf, "<doi>%s</doi>\n",
+ virBufferAsprintf(buf, "<doi>%s</doi>\n",
caps->host.secModels[i].doi);
for (j = 0; j < caps->host.secModels[i].nlabels; j++) {
- virBufferAsprintf(&buf, "<baselabel
type='%s'>%s</baselabel>\n",
+ virBufferAsprintf(buf, "<baselabel
type='%s'>%s</baselabel>\n",
caps->host.secModels[i].labels[j].type,
caps->host.secModels[i].labels[j].label);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</secmodel>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</secmodel>\n");
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</host>\n\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</host>\n\n");
+
+ return 0;
+
+ error:
+ return -1;
+}
+
+
+/**
+ * virCapabilitiesFormatXML:
+ * @caps: capabilities to format
+ *
+ * Convert the capabilities object into an XML representation
+ *
+ * Returns the XML document as a string
+ */
+char *
+virCapabilitiesFormatXML(virCapsPtr caps)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ size_t i, j, k;
+ virBufferAddLit(&buf, "<capabilities>\n\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ if (virCapabilitiesFormatHostXML(caps, &buf) < 0)
+ goto error;
for (i = 0; i < caps->nguests; i++) {
virBufferAddLit(&buf, "<guest>\n");
--
2.20.1
7:15 p.m.
New subject: [libvirt] [PATCH 2/7] conf: Alter virCapabilitiesFormatHostXML to take virCapsHostPtr
Rather than deref off of "caps->host.", let's pass
"&caps->host"
and make the helper use "host->" instead.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 66 ++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 33c0c37fbf..863cd06a8d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1058,7 +1058,7 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf,
static int
-virCapabilitiesFormatHostXML(virCapsPtr caps,
+virCapabilitiesFormatHostXML(virCapsHostPtr host,
virBufferPtr buf)
{
size_t i, j;
@@ -1066,40 +1066,40 @@ virCapabilitiesFormatHostXML(virCapsPtr caps,
virBufferAddLit(buf, "<host>\n");
virBufferAdjustIndent(buf, 2);
- if (virUUIDIsValid(caps->host.host_uuid)) {
- virUUIDFormat(caps->host.host_uuid, host_uuid);
+ if (virUUIDIsValid(host->host_uuid)) {
+ virUUIDFormat(host->host_uuid, host_uuid);
virBufferAsprintf(buf, "<uuid>%s</uuid>\n", host_uuid);
}
virBufferAddLit(buf, "<cpu>\n");
virBufferAdjustIndent(buf, 2);
- if (caps->host.arch)
+ if (host->arch)
virBufferAsprintf(buf, "<arch>%s</arch>\n",
- virArchToString(caps->host.arch));
- if (caps->host.nfeatures) {
+ virArchToString(host->arch));
+ if (host->nfeatures) {
virBufferAddLit(buf, "<features>\n");
virBufferAdjustIndent(buf, 2);
- for (i = 0; i < caps->host.nfeatures; i++) {
+ for (i = 0; i < host->nfeatures; i++) {
virBufferAsprintf(buf, "<%s/>\n",
- caps->host.features[i]);
+ host->features[i]);
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</features>\n");
}
- virCPUDefFormatBuf(buf, caps->host.cpu);
+ virCPUDefFormatBuf(buf, host->cpu);
- for (i = 0; i < caps->host.nPagesSize; i++) {
+ for (i = 0; i < host->nPagesSize; i++) {
virBufferAsprintf(buf, "<pages unit='KiB'
size='%u'/>\n",
- caps->host.pagesSize[i]);
+ host->pagesSize[i]);
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</cpu>\n");
/* The PM query was successful. */
- if (caps->host.powerMgmt) {
+ if (host->powerMgmt) {
/* The host supports some PM features. */
- unsigned int pm = caps->host.powerMgmt;
+ unsigned int pm = host->powerMgmt;
virBufferAddLit(buf, "<power_management>\n");
virBufferAdjustIndent(buf, 2);
while (pm) {
@@ -1116,19 +1116,19 @@ virCapabilitiesFormatHostXML(virCapsPtr caps,
}
virBufferAsprintf(buf, "<iommu support='%s'/>\n",
- caps->host.iommu ? "yes" : "no");
+ host->iommu ? "yes" : "no");
- if (caps->host.offlineMigrate) {
+ if (host->offlineMigrate) {
virBufferAddLit(buf, "<migration_features>\n");
virBufferAdjustIndent(buf, 2);
- if (caps->host.liveMigrate)
+ if (host->liveMigrate)
virBufferAddLit(buf, "<live/>\n");
- if (caps->host.nmigrateTrans) {
+ if (host->nmigrateTrans) {
virBufferAddLit(buf, "<uri_transports>\n");
virBufferAdjustIndent(buf, 2);
- for (i = 0; i < caps->host.nmigrateTrans; i++) {
+ for (i = 0; i < host->nmigrateTrans; i++) {
virBufferAsprintf(buf,
"<uri_transport>%s</uri_transport>\n",
- caps->host.migrateTrans[i]);
+ host->migrateTrans[i]);
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</uri_transports>\n");
@@ -1137,32 +1137,32 @@ virCapabilitiesFormatHostXML(virCapsPtr caps,
virBufferAddLit(buf, "</migration_features>\n");
}
- if (caps->host.netprefix)
+ if (host->netprefix)
virBufferAsprintf(buf, "<netprefix>%s</netprefix>\n",
- caps->host.netprefix);
+ host->netprefix);
- if (caps->host.nnumaCell &&
- virCapabilitiesFormatNUMATopology(buf, caps->host.nnumaCell,
- caps->host.numaCell) < 0)
+ if (host->nnumaCell &&
+ virCapabilitiesFormatNUMATopology(buf, host->nnumaCell,
+ host->numaCell) < 0)
goto error;
- if (virCapabilitiesFormatCaches(buf, &caps->host.cache) < 0)
+ if (virCapabilitiesFormatCaches(buf, &host->cache) < 0)
goto error;
- if (virCapabilitiesFormatMemoryBandwidth(buf, &caps->host.memBW) < 0)
+ if (virCapabilitiesFormatMemoryBandwidth(buf, &host->memBW) < 0)
goto error;
- for (i = 0; i < caps->host.nsecModels; i++) {
+ for (i = 0; i < host->nsecModels; i++) {
virBufferAddLit(buf, "<secmodel>\n");
virBufferAdjustIndent(buf, 2);
virBufferAsprintf(buf, "<model>%s</model>\n",
- caps->host.secModels[i].model);
+ host->secModels[i].model);
virBufferAsprintf(buf, "<doi>%s</doi>\n",
- caps->host.secModels[i].doi);
- for (j = 0; j < caps->host.secModels[i].nlabels; j++) {
+ host->secModels[i].doi);
+ for (j = 0; j < host->secModels[i].nlabels; j++) {
virBufferAsprintf(buf, "<baselabel
type='%s'>%s</baselabel>\n",
- caps->host.secModels[i].labels[j].type,
- caps->host.secModels[i].labels[j].label);
+ host->secModels[i].labels[j].type,
+ host->secModels[i].labels[j].label);
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</secmodel>\n");
@@ -1195,7 +1195,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&buf, "<capabilities>\n\n");
virBufferAdjustIndent(&buf, 2);
- if (virCapabilitiesFormatHostXML(caps, &buf) < 0)
+ if (virCapabilitiesFormatHostXML(&caps->host, &buf) < 0)
goto error;
for (i = 0; i < caps->nguests; i++) {
--
2.20.1
7:15 p.m.
New subject: [libvirt] [PATCH 3/7] conf: Extract guest XML formatting from virCapabilitiesFormatXML
Let's extract out the <guest> code into it's own method/helper.
NB: One minor change between the two is usage of "buf" instead
of "&buf" in the new code since we pass the address of &buf to
the helper.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 112 ++++++++++++++++++++++------------------
1 file changed, 61 insertions(+), 51 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 863cd06a8d..c320c0f72d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1178,91 +1178,77 @@ virCapabilitiesFormatHostXML(virCapsHostPtr host,
}
-/**
- * virCapabilitiesFormatXML:
- * @caps: capabilities to format
- *
- * Convert the capabilities object into an XML representation
- *
- * Returns the XML document as a string
- */
-char *
-virCapabilitiesFormatXML(virCapsPtr caps)
+static void
+virCapabilitiesFormatGuestXML(virCapsPtr caps,
+ virBufferPtr buf)
{
- virBuffer buf = VIR_BUFFER_INITIALIZER;
size_t i, j, k;
- virBufferAddLit(&buf, "<capabilities>\n\n");
- virBufferAdjustIndent(&buf, 2);
-
- if (virCapabilitiesFormatHostXML(&caps->host, &buf) < 0)
- goto error;
-
for (i = 0; i < caps->nguests; i++) {
- virBufferAddLit(&buf, "<guest>\n");
- virBufferAdjustIndent(&buf, 2);
- virBufferAsprintf(&buf, "<os_type>%s</os_type>\n",
+ virBufferAddLit(buf, "<guest>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<os_type>%s</os_type>\n",
virDomainOSTypeToString(caps->guests[i]->ostype));
if (caps->guests[i]->arch.id)
- virBufferAsprintf(&buf, "<arch name='%s'>\n",
+ virBufferAsprintf(buf, "<arch name='%s'>\n",
virArchToString(caps->guests[i]->arch.id));
- virBufferAdjustIndent(&buf, 2);
- virBufferAsprintf(&buf, "<wordsize>%d</wordsize>\n",
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<wordsize>%d</wordsize>\n",
caps->guests[i]->arch.wordsize);
if (caps->guests[i]->arch.defaultInfo.emulator)
- virBufferAsprintf(&buf,
"<emulator>%s</emulator>\n",
+ virBufferAsprintf(buf, "<emulator>%s</emulator>\n",
caps->guests[i]->arch.defaultInfo.emulator);
if (caps->guests[i]->arch.defaultInfo.loader)
- virBufferAsprintf(&buf, "<loader>%s</loader>\n",
+ virBufferAsprintf(buf, "<loader>%s</loader>\n",
caps->guests[i]->arch.defaultInfo.loader);
for (j = 0; j < caps->guests[i]->arch.defaultInfo.nmachines; j++) {
virCapsGuestMachinePtr machine =
caps->guests[i]->arch.defaultInfo.machines[j];
- virBufferAddLit(&buf, "<machine");
+ virBufferAddLit(buf, "<machine");
if (machine->canonical)
- virBufferAsprintf(&buf, " canonical='%s'",
machine->canonical);
+ virBufferAsprintf(buf, " canonical='%s'",
machine->canonical);
if (machine->maxCpus > 0)
- virBufferAsprintf(&buf, " maxCpus='%d'",
machine->maxCpus);
- virBufferAsprintf(&buf, ">%s</machine>\n",
machine->name);
+ virBufferAsprintf(buf, " maxCpus='%d'",
machine->maxCpus);
+ virBufferAsprintf(buf, ">%s</machine>\n",
machine->name);
}
for (j = 0; j < caps->guests[i]->arch.ndomains; j++) {
- virBufferAsprintf(&buf, "<domain type='%s'",
+ virBufferAsprintf(buf, "<domain type='%s'",
virDomainVirtTypeToString(caps->guests[i]->arch.domains[j]->type));
if (!caps->guests[i]->arch.domains[j]->info.emulator &&
!caps->guests[i]->arch.domains[j]->info.loader &&
!caps->guests[i]->arch.domains[j]->info.nmachines) {
- virBufferAddLit(&buf, "/>\n");
+ virBufferAddLit(buf, "/>\n");
continue;
}
- virBufferAddLit(&buf, ">\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 2);
if (caps->guests[i]->arch.domains[j]->info.emulator)
- virBufferAsprintf(&buf,
"<emulator>%s</emulator>\n",
+ virBufferAsprintf(buf,
"<emulator>%s</emulator>\n",
caps->guests[i]->arch.domains[j]->info.emulator);
if (caps->guests[i]->arch.domains[j]->info.loader)
- virBufferAsprintf(&buf,
"<loader>%s</loader>\n",
+ virBufferAsprintf(buf, "<loader>%s</loader>\n",
caps->guests[i]->arch.domains[j]->info.loader);
for (k = 0; k < caps->guests[i]->arch.domains[j]->info.nmachines;
k++) {
virCapsGuestMachinePtr machine =
caps->guests[i]->arch.domains[j]->info.machines[k];
- virBufferAddLit(&buf, "<machine");
+ virBufferAddLit(buf, "<machine");
if (machine->canonical)
- virBufferAsprintf(&buf, " canonical='%s'",
machine->canonical);
+ virBufferAsprintf(buf, " canonical='%s'",
machine->canonical);
if (machine->maxCpus > 0)
- virBufferAsprintf(&buf, " maxCpus='%d'",
machine->maxCpus);
- virBufferAsprintf(&buf, ">%s</machine>\n",
machine->name);
+ virBufferAsprintf(buf, " maxCpus='%d'",
machine->maxCpus);
+ virBufferAsprintf(buf, ">%s</machine>\n",
machine->name);
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</domain>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</domain>\n");
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</arch>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</arch>\n");
if (caps->guests[i]->nfeatures) {
- virBufferAddLit(&buf, "<features>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<features>\n");
+ virBufferAdjustIndent(buf, 2);
for (j = 0; j < caps->guests[i]->nfeatures; j++) {
if (STREQ(caps->guests[i]->features[j]->name, "pae")
||
@@ -1270,22 +1256,46 @@ virCapabilitiesFormatXML(virCapsPtr caps)
STREQ(caps->guests[i]->features[j]->name,
"ia64_be") ||
STREQ(caps->guests[i]->features[j]->name,
"cpuselection") ||
STREQ(caps->guests[i]->features[j]->name,
"deviceboot")) {
- virBufferAsprintf(&buf, "<%s/>\n",
+ virBufferAsprintf(buf, "<%s/>\n",
caps->guests[i]->features[j]->name);
} else {
- virBufferAsprintf(&buf, "<%s default='%s'
toggle='%s'/>\n",
+ virBufferAsprintf(buf, "<%s default='%s'
toggle='%s'/>\n",
caps->guests[i]->features[j]->name,
caps->guests[i]->features[j]->defaultOn ?
"on" : "off",
caps->guests[i]->features[j]->toggle ?
"yes" : "no");
}
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</features>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</features>\n");
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</guest>\n\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</guest>\n\n");
}
+}
+
+
+/**
+ * virCapabilitiesFormatXML:
+ * @caps: capabilities to format
+ *
+ * Convert the capabilities object into an XML representation
+ *
+ * Returns the XML document as a string
+ */
+char *
+virCapabilitiesFormatXML(virCapsPtr caps)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAddLit(&buf, "<capabilities>\n\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ if (virCapabilitiesFormatHostXML(&caps->host, &buf) < 0)
+ goto error;
+
+ virCapabilitiesFormatGuestXML(caps, &buf);
+
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</capabilities>\n");
--
2.20.1
7:15 p.m.
New subject: [libvirt] [PATCH 4/7] conf: Alter virCapabilitiesFormatGuestXML to take virCapsGuestPtr
Rather than deref off of "caps->guests", let's pass
"caps->guests" and
caps->nguests to have the helper use "guests[i]->" instead.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 71 +++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 35 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index c320c0f72d..1269a4c739 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1179,31 +1179,32 @@ virCapabilitiesFormatHostXML(virCapsHostPtr host,
static void
-virCapabilitiesFormatGuestXML(virCapsPtr caps,
+virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
+ size_t nguests,
virBufferPtr buf)
{
size_t i, j, k;
- for (i = 0; i < caps->nguests; i++) {
+ for (i = 0; i < nguests; i++) {
virBufferAddLit(buf, "<guest>\n");
virBufferAdjustIndent(buf, 2);
virBufferAsprintf(buf, "<os_type>%s</os_type>\n",
- virDomainOSTypeToString(caps->guests[i]->ostype));
- if (caps->guests[i]->arch.id)
+ virDomainOSTypeToString(guests[i]->ostype));
+ if (guests[i]->arch.id)
virBufferAsprintf(buf, "<arch name='%s'>\n",
- virArchToString(caps->guests[i]->arch.id));
+ virArchToString(guests[i]->arch.id));
virBufferAdjustIndent(buf, 2);
virBufferAsprintf(buf, "<wordsize>%d</wordsize>\n",
- caps->guests[i]->arch.wordsize);
- if (caps->guests[i]->arch.defaultInfo.emulator)
+ guests[i]->arch.wordsize);
+ if (guests[i]->arch.defaultInfo.emulator)
virBufferAsprintf(buf, "<emulator>%s</emulator>\n",
- caps->guests[i]->arch.defaultInfo.emulator);
- if (caps->guests[i]->arch.defaultInfo.loader)
+ guests[i]->arch.defaultInfo.emulator);
+ if (guests[i]->arch.defaultInfo.loader)
virBufferAsprintf(buf, "<loader>%s</loader>\n",
- caps->guests[i]->arch.defaultInfo.loader);
+ guests[i]->arch.defaultInfo.loader);
- for (j = 0; j < caps->guests[i]->arch.defaultInfo.nmachines; j++) {
- virCapsGuestMachinePtr machine =
caps->guests[i]->arch.defaultInfo.machines[j];
+ for (j = 0; j < guests[i]->arch.defaultInfo.nmachines; j++) {
+ virCapsGuestMachinePtr machine = guests[i]->arch.defaultInfo.machines[j];
virBufferAddLit(buf, "<machine");
if (machine->canonical)
virBufferAsprintf(buf, " canonical='%s'",
machine->canonical);
@@ -1212,26 +1213,26 @@ virCapabilitiesFormatGuestXML(virCapsPtr caps,
virBufferAsprintf(buf, ">%s</machine>\n",
machine->name);
}
- for (j = 0; j < caps->guests[i]->arch.ndomains; j++) {
+ for (j = 0; j < guests[i]->arch.ndomains; j++) {
virBufferAsprintf(buf, "<domain type='%s'",
-
virDomainVirtTypeToString(caps->guests[i]->arch.domains[j]->type));
- if (!caps->guests[i]->arch.domains[j]->info.emulator &&
- !caps->guests[i]->arch.domains[j]->info.loader &&
- !caps->guests[i]->arch.domains[j]->info.nmachines) {
+ virDomainVirtTypeToString(guests[i]->arch.domains[j]->type));
+ if (!guests[i]->arch.domains[j]->info.emulator &&
+ !guests[i]->arch.domains[j]->info.loader &&
+ !guests[i]->arch.domains[j]->info.nmachines) {
virBufferAddLit(buf, "/>\n");
continue;
}
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
- if (caps->guests[i]->arch.domains[j]->info.emulator)
+ if (guests[i]->arch.domains[j]->info.emulator)
virBufferAsprintf(buf,
"<emulator>%s</emulator>\n",
-
caps->guests[i]->arch.domains[j]->info.emulator);
- if (caps->guests[i]->arch.domains[j]->info.loader)
+ guests[i]->arch.domains[j]->info.emulator);
+ if (guests[i]->arch.domains[j]->info.loader)
virBufferAsprintf(buf, "<loader>%s</loader>\n",
-
caps->guests[i]->arch.domains[j]->info.loader);
+ guests[i]->arch.domains[j]->info.loader);
- for (k = 0; k < caps->guests[i]->arch.domains[j]->info.nmachines;
k++) {
- virCapsGuestMachinePtr machine =
caps->guests[i]->arch.domains[j]->info.machines[k];
+ for (k = 0; k < guests[i]->arch.domains[j]->info.nmachines; k++) {
+ virCapsGuestMachinePtr machine =
guests[i]->arch.domains[j]->info.machines[k];
virBufferAddLit(buf, "<machine");
if (machine->canonical)
virBufferAsprintf(buf, " canonical='%s'",
machine->canonical);
@@ -1246,23 +1247,23 @@ virCapabilitiesFormatGuestXML(virCapsPtr caps,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</arch>\n");
- if (caps->guests[i]->nfeatures) {
+ if (guests[i]->nfeatures) {
virBufferAddLit(buf, "<features>\n");
virBufferAdjustIndent(buf, 2);
- for (j = 0; j < caps->guests[i]->nfeatures; j++) {
- if (STREQ(caps->guests[i]->features[j]->name, "pae")
||
- STREQ(caps->guests[i]->features[j]->name,
"nonpae") ||
- STREQ(caps->guests[i]->features[j]->name,
"ia64_be") ||
- STREQ(caps->guests[i]->features[j]->name,
"cpuselection") ||
- STREQ(caps->guests[i]->features[j]->name,
"deviceboot")) {
+ for (j = 0; j < guests[i]->nfeatures; j++) {
+ if (STREQ(guests[i]->features[j]->name, "pae") ||
+ STREQ(guests[i]->features[j]->name, "nonpae") ||
+ STREQ(guests[i]->features[j]->name, "ia64_be") ||
+ STREQ(guests[i]->features[j]->name, "cpuselection")
||
+ STREQ(guests[i]->features[j]->name, "deviceboot")) {
virBufferAsprintf(buf, "<%s/>\n",
- caps->guests[i]->features[j]->name);
+ guests[i]->features[j]->name);
} else {
virBufferAsprintf(buf, "<%s default='%s'
toggle='%s'/>\n",
- caps->guests[i]->features[j]->name,
- caps->guests[i]->features[j]->defaultOn ?
"on" : "off",
- caps->guests[i]->features[j]->toggle ?
"yes" : "no");
+ guests[i]->features[j]->name,
+ guests[i]->features[j]->defaultOn ?
"on" : "off",
+ guests[i]->features[j]->toggle ?
"yes" : "no");
}
}
@@ -1294,7 +1295,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
if (virCapabilitiesFormatHostXML(&caps->host, &buf) < 0)
goto error;
- virCapabilitiesFormatGuestXML(caps, &buf);
+ virCapabilitiesFormatGuestXML(caps->guests, caps->nguests, &buf);
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</capabilities>\n");
--
2.20.1
7:15 p.m.
New subject: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities
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>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 60 ++++++++++++++++++++++++++++++++++++++++
src/conf/capabilities.h | 15 ++++++++++
src/libvirt_private.syms | 1 +
3 files changed, 76 insertions(+)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1269a4c739..c60743a38d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -28,6 +28,7 @@
#include "cpu_conf.h"
#include "domain_conf.h"
#include "physmem.h"
+#include "storage_conf.h"
#include "viralloc.h"
#include "virarch.h"
#include "virbuffer.h"
@@ -180,6 +181,17 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
VIR_FREE(guest);
}
+
+static void
+virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
+{
+ if (!pool)
+ return;
+
+ VIR_FREE(pool);
+}
+
+
void
virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
{
@@ -221,6 +233,10 @@ virCapsDispose(void *object)
virCapsPtr caps = object;
size_t i;
+ for (i = 0; i < caps->npools; i++)
+ virCapabilitiesFreeStoragePool(caps->pools[i]);
+ VIR_FREE(caps->pools);
+
for (i = 0; i < caps->nguests; i++)
virCapabilitiesFreeGuest(caps->guests[i]);
VIR_FREE(caps->guests);
@@ -792,6 +808,30 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
emulator, machinetype);
}
+
+int
+virCapabilitiesAddStoragePool(virCapsPtr caps,
+ int poolType)
+{
+ virCapsStoragePoolPtr pool;
+
+ if (VIR_ALLOC(pool) < 0)
+ goto error;
+
+ pool->type = poolType;
+
+ if (VIR_RESIZE_N(caps->pools, caps->npools_max, caps->npools, 1) < 0)
+ goto error;
+ caps->pools[caps->npools++] = pool;
+
+ return 0;
+
+ error:
+ virCapabilitiesFreeStoragePool(pool);
+ return -1;
+}
+
+
static int
virCapabilitiesFormatNUMATopology(virBufferPtr buf,
size_t ncells,
@@ -1276,6 +1316,24 @@ virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
}
+static void
+virCapabilitiesFormatStoragePoolXML(virCapsStoragePoolPtr *pools,
+ size_t npools,
+ virBufferPtr buf)
+{
+ size_t i;
+
+ for (i = 0; i < npools; i++) {
+ virBufferAddLit(buf, "<pool>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<type>%s</pool>\n",
+ virStoragePoolTypeToString(pools[i]->type));
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</pool>\n\n");
+ }
+}
+
+
/**
* virCapabilitiesFormatXML:
* @caps: capabilities to format
@@ -1297,6 +1355,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virCapabilitiesFormatGuestXML(caps->guests, caps->nguests, &buf);
+ virCapabilitiesFormatStoragePoolXML(caps->pools, caps->npools, &buf);
+
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</capabilities>\n");
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 31c2a07a9b..cca1a20949 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -211,6 +211,13 @@ struct _virCapsHost {
bool iommu;
};
+typedef struct _virCapsStoragePool virCapsStoragePool;
+typedef virCapsStoragePool *virCapsStoragePoolPtr;
+struct _virCapsStoragePool {
+ int type;
+};
+
+
typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
xmlXPathContextPtr, void **);
typedef void (*virDomainDefNamespaceFree)(void *);
@@ -235,6 +242,10 @@ struct _virCaps {
size_t nguests;
size_t nguests_max;
virCapsGuestPtr *guests;
+
+ size_t npools;
+ size_t npools_max;
+ virCapsStoragePoolPtr *pools;
};
typedef struct _virCapsDomainData virCapsDomainData;
@@ -318,6 +329,10 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
bool defaultOn,
bool toggle);
+int
+virCapabilitiesAddStoragePool(virCapsPtr caps,
+ int poolType);
+
int
virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
const char *type,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3d6306809..9aaa8830e4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -49,6 +49,7 @@ virCapabilitiesAddGuestFeature;
virCapabilitiesAddHostFeature;
virCapabilitiesAddHostMigrateTransport;
virCapabilitiesAddHostNUMACell;
+virCapabilitiesAddStoragePool;
virCapabilitiesAllocMachines;
virCapabilitiesClearHostNUMACellCPUTopology;
virCapabilitiesDomainDataLookup;
--
2.20.1
2:31 a.m.
New subject: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities
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
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 60 ++++++++++++++++++++++++++++++++++++++++
src/conf/capabilities.h | 15 ++++++++++
src/libvirt_private.syms | 1 +
3 files changed, 76 insertions(+)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1269a4c739..c60743a38d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -28,6 +28,7 @@
#include "cpu_conf.h"
#include "domain_conf.h"
#include "physmem.h"
+#include "storage_conf.h"
#include "viralloc.h"
#include "virarch.h"
#include "virbuffer.h"
@@ -180,6 +181,17 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
VIR_FREE(guest);
}
+
+static void
+virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
+{
+ if (!pool)
+ return;
+
+ VIR_FREE(pool);
+}
+
+
void
virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
{
@@ -221,6 +233,10 @@ virCapsDispose(void *object)
virCapsPtr caps = object;
size_t i;
+ for (i = 0; i < caps->npools; i++)
+ virCapabilitiesFreeStoragePool(caps->pools[i]);
+ VIR_FREE(caps->pools);
+
for (i = 0; i < caps->nguests; i++)
virCapabilitiesFreeGuest(caps->guests[i]);
VIR_FREE(caps->guests);
@@ -792,6 +808,30 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
emulator, machinetype);
}
+
+int
+virCapabilitiesAddStoragePool(virCapsPtr caps,
+ int poolType)
+{
+ virCapsStoragePoolPtr pool;
+
+ if (VIR_ALLOC(pool) < 0)
+ goto error;
+
+ pool->type = poolType;
+
+ if (VIR_RESIZE_N(caps->pools, caps->npools_max, caps->npools, 1) < 0)
+ goto error;
+ caps->pools[caps->npools++] = pool;
+
+ return 0;
+
+ error:
+ virCapabilitiesFreeStoragePool(pool);
+ return -1;
+}
+
+
static int
virCapabilitiesFormatNUMATopology(virBufferPtr buf,
size_t ncells,
@@ -1276,6 +1316,24 @@ virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
}
+static void
+virCapabilitiesFormatStoragePoolXML(virCapsStoragePoolPtr *pools,
+ size_t npools,
+ virBufferPtr buf)
+{
+ size_t i;
+
+ for (i = 0; i < npools; i++) {
+ virBufferAddLit(buf, "<pool>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<type>%s</pool>\n",
+ virStoragePoolTypeToString(pools[i]->type));
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</pool>\n\n");
+ }
+}
+
+
/**
* virCapabilitiesFormatXML:
* @caps: capabilities to format
@@ -1297,6 +1355,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virCapabilitiesFormatGuestXML(caps->guests, caps->nguests, &buf);
+ virCapabilitiesFormatStoragePoolXML(caps->pools, caps->npools, &buf);
+
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</capabilities>\n");
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 31c2a07a9b..cca1a20949 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -211,6 +211,13 @@ struct _virCapsHost {
bool iommu;
};
+typedef struct _virCapsStoragePool virCapsStoragePool;
+typedef virCapsStoragePool *virCapsStoragePoolPtr;
+struct _virCapsStoragePool {
+ int type;
+};
+
+
typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
xmlXPathContextPtr, void **);
typedef void (*virDomainDefNamespaceFree)(void *);
@@ -235,6 +242,10 @@ struct _virCaps {
size_t nguests;
size_t nguests_max;
virCapsGuestPtr *guests;
+
+ size_t npools;
+ size_t npools_max;
+ virCapsStoragePoolPtr *pools;
};
typedef struct _virCapsDomainData virCapsDomainData;
@@ -318,6 +329,10 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
bool defaultOn,
bool toggle);
+int
+virCapabilitiesAddStoragePool(virCapsPtr caps,
+ int poolType);
+
int
virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
const char *type,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3d6306809..9aaa8830e4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -49,6 +49,7 @@ virCapabilitiesAddGuestFeature;
virCapabilitiesAddHostFeature;
virCapabilitiesAddHostMigrateTransport;
virCapabilitiesAddHostNUMACell;
+virCapabilitiesAddStoragePool;
virCapabilitiesAllocMachines;
virCapabilitiesClearHostNUMACellCPUTopology;
virCapabilitiesDomainDataLookup;
--
2.20.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:03 a.m.
New subject: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities
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
</pool>
Tks -
John
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/capabilities.c | 60 ++++++++++++++++++++++++++++++++++++++++
> src/conf/capabilities.h | 15 ++++++++++
> src/libvirt_private.syms | 1 +
> 3 files changed, 76 insertions(+)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 1269a4c739..c60743a38d 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -28,6 +28,7 @@
> #include "cpu_conf.h"
> #include "domain_conf.h"
> #include "physmem.h"
> +#include "storage_conf.h"
> #include "viralloc.h"
> #include "virarch.h"
> #include "virbuffer.h"
> @@ -180,6 +181,17 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
> VIR_FREE(guest);
> }
>
> +
> +static void
> +virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
> +{
> + if (!pool)
> + return;
> +
> + VIR_FREE(pool);
> +}
> +
> +
> void
> virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
> {
> @@ -221,6 +233,10 @@ virCapsDispose(void *object)
> virCapsPtr caps = object;
> size_t i;
>
> + for (i = 0; i < caps->npools; i++)
> + virCapabilitiesFreeStoragePool(caps->pools[i]);
> + VIR_FREE(caps->pools);
> +
> for (i = 0; i < caps->nguests; i++)
> virCapabilitiesFreeGuest(caps->guests[i]);
> VIR_FREE(caps->guests);
> @@ -792,6 +808,30 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> emulator, machinetype);
> }
>
> +
> +int
> +virCapabilitiesAddStoragePool(virCapsPtr caps,
> + int poolType)
> +{
> + virCapsStoragePoolPtr pool;
> +
> + if (VIR_ALLOC(pool) < 0)
> + goto error;
> +
> + pool->type = poolType;
> +
> + if (VIR_RESIZE_N(caps->pools, caps->npools_max, caps->npools, 1) <
0)
> + goto error;
> + caps->pools[caps->npools++] = pool;
> +
> + return 0;
> +
> + error:
> + virCapabilitiesFreeStoragePool(pool);
> + return -1;
> +}
> +
> +
> static int
> virCapabilitiesFormatNUMATopology(virBufferPtr buf,
> size_t ncells,
> @@ -1276,6 +1316,24 @@ virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
> }
>
>
> +static void
> +virCapabilitiesFormatStoragePoolXML(virCapsStoragePoolPtr *pools,
> + size_t npools,
> + virBufferPtr buf)
> +{
> + size_t i;
> +
> + for (i = 0; i < npools; i++) {
> + virBufferAddLit(buf, "<pool>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<type>%s</pool>\n",
> + virStoragePoolTypeToString(pools[i]->type));
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</pool>\n\n");
> + }
> +}
> +
> +
> /**
> * virCapabilitiesFormatXML:
> * @caps: capabilities to format
> @@ -1297,6 +1355,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>
> virCapabilitiesFormatGuestXML(caps->guests, caps->nguests, &buf);
>
> + virCapabilitiesFormatStoragePoolXML(caps->pools, caps->npools, &buf);
> +
> virBufferAdjustIndent(&buf, -2);
> virBufferAddLit(&buf, "</capabilities>\n");
>
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index 31c2a07a9b..cca1a20949 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -211,6 +211,13 @@ struct _virCapsHost {
> bool iommu;
> };
>
> +typedef struct _virCapsStoragePool virCapsStoragePool;
> +typedef virCapsStoragePool *virCapsStoragePoolPtr;
> +struct _virCapsStoragePool {
> + int type;
> +};
> +
> +
> typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
> xmlXPathContextPtr, void **);
> typedef void (*virDomainDefNamespaceFree)(void *);
> @@ -235,6 +242,10 @@ struct _virCaps {
> size_t nguests;
> size_t nguests_max;
> virCapsGuestPtr *guests;
> +
> + size_t npools;
> + size_t npools_max;
> + virCapsStoragePoolPtr *pools;
> };
>
> typedef struct _virCapsDomainData virCapsDomainData;
> @@ -318,6 +329,10 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
> bool defaultOn,
> bool toggle);
>
> +int
> +virCapabilitiesAddStoragePool(virCapsPtr caps,
> + int poolType);
> +
> int
> virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
> const char *type,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c3d6306809..9aaa8830e4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -49,6 +49,7 @@ virCapabilitiesAddGuestFeature;
> virCapabilitiesAddHostFeature;
> virCapabilitiesAddHostMigrateTransport;
> virCapabilitiesAddHostNUMACell;
> +virCapabilitiesAddStoragePool;
> virCapabilitiesAllocMachines;
> virCapabilitiesClearHostNUMACellCPUTopology;
> virCapabilitiesDomainDataLookup;
> --
> 2.20.1
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
9:55 a.m.
New subject: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities
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.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:04 a.m.
New subject: [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.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:24 a.m.
New subject: [libvirt] [PATCH 5/7] conf: Introduce storage pool functions into capabilities
On Wed, Jan 30, 2019 at 04:04:07PM +0000, Daniel P. Berrangé wrote:
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.
This might be better than putting storage capabilities into the host
capabilities, however, I would not make the API in a way that you would
have to call it for every single storage pool type. IMHO management
application would rather call one single API which would return single
XML with all storage capabilities including a list of supported storage
pools and their features.
Pavel
2:17 a.m.
New subject: [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.
Agreed, if we start listing supported storage APIs someone will
eventually ask us to list also APIs for hypervisor drivers. It's
overkill and we should not do that.
Pavel
7:15 p.m.
New subject: [libvirt] [PATCH 6/7] storage: Process storage pool capabilities
https://bugzilla.redhat.com/show_bug.cgi?id=1581670
During storage driver backend initialization, let's save
which backends are available in the storage pool capabilities.
In order to format those, we need add a connectGetCapabilities
processor to the storageHypervisorDriver. This allows a storage
connection, such as "storage:///system" to find the API and
format the results.
NB: This is not available from other hypervisor drivers such as
QEMU type connections.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.h | 4 ++++
src/storage/storage_backend.c | 16 ++++++++++++++++
src/storage/storage_backend.h | 3 +++
src/storage/storage_driver.c | 17 +++++++++++++++++
4 files changed, 40 insertions(+)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 1106aa71bd..34adc26df3 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -24,6 +24,8 @@
# include "storage_conf.h"
+# include "capabilities.h"
+
typedef struct _virStoragePoolObj virStoragePoolObj;
typedef virStoragePoolObj *virStoragePoolObjPtr;
@@ -45,6 +47,8 @@ struct _virStorageDriverState {
/* Immutable pointer, self-locking APIs */
virObjectEventStatePtr storageEventState;
+
+ virCapsPtr caps;
};
typedef bool
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a54c338cf0..0ccc616db4 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -187,3 +187,19 @@ virStorageBackendForType(int type)
type, NULLSTR(virStoragePoolTypeToString(type)));
return NULL;
}
+
+
+virCapsPtr
+virStorageBackendGetCapabilities(void)
+{
+ virCapsPtr caps;
+ size_t i;
+
+ if (!(caps = virCapabilitiesNew(virArchFromHost(), false, false)))
+ return NULL;
+
+ for (i = 0; i < virStorageBackendsCount; i++)
+ virCapabilitiesAddStoragePool(caps, virStorageBackends[i]->type);
+
+ return caps;
+}
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 2b178494ae..c670c66287 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -126,4 +126,7 @@ int virStorageBackendDriversRegister(bool allmodules);
int virStorageBackendRegister(virStorageBackendPtr backend);
+virCapsPtr
+virStorageBackendGetCapabilities(void);
+
#endif /* LIBVIRT_STORAGE_BACKEND_H */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4a13e90481..04beff6b07 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -300,6 +300,9 @@ storageStateInitialize(bool privileged,
driver->storageEventState = virObjectEventStateNew();
+ if (!(driver->caps = virStorageBackendGetCapabilities()))
+ goto error;
+
storageDriverUnlock();
ret = 0;
@@ -368,6 +371,7 @@ storageStateCleanup(void)
storageDriverLock();
+ virObjectUnref(driver->caps);
virObjectUnref(driver->storageEventState);
/* free inactive pools */
@@ -577,6 +581,18 @@ storageConnectListStoragePools(virConnectPtr conn,
names, maxnames);
}
+
+static char *
+storageConnectGetCapabilities(virConnectPtr conn)
+{
+
+ if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
+ return NULL;
+
+ return virCapabilitiesFormatXML(driver->caps);
+}
+
+
static int
storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
{
@@ -2850,6 +2866,7 @@ static virHypervisorDriver storageHypervisorDriver = {
.connectIsEncrypted = storageConnectIsEncrypted, /* 4.1.0 */
.connectIsSecure = storageConnectIsSecure, /* 4.1.0 */
.connectIsAlive = storageConnectIsAlive, /* 4.1.0 */
+ .connectGetCapabilities = storageConnectGetCapabilities, /* 5.1.0 */
};
static virConnectDriver storageConnectDriver = {
--
2.20.1
4:18 a.m.
New subject: [libvirt] [PATCH 6/7] storage: Process storage pool capabilities
On 1/16/19 2:15 AM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1581670
During storage driver backend initialization, let's save
which backends are available in the storage pool capabilities.
In order to format those, we need add a connectGetCapabilities
processor to the storageHypervisorDriver. This allows a storage
connection, such as "storage:///system" to find the API and
format the results.
NB: This is not available from other hypervisor drivers such as
QEMU type connections.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.h | 4 ++++
src/storage/storage_backend.c | 16 ++++++++++++++++
src/storage/storage_backend.h | 3 +++
src/storage/storage_driver.c | 17 +++++++++++++++++
4 files changed, 40 insertions(+)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 1106aa71bd..34adc26df3 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -24,6 +24,8 @@
# include "storage_conf.h"
+# include "capabilities.h"
+
typedef struct _virStoragePoolObj virStoragePoolObj;
typedef virStoragePoolObj *virStoragePoolObjPtr;
@@ -45,6 +47,8 @@ struct _virStorageDriverState {
/* Immutable pointer, self-locking APIs */
virObjectEventStatePtr storageEventState;
+
+ virCapsPtr caps;
Would be nice to put a comment here:
/* Immutable pointer, read only after initialized */
};
typedef bool
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a54c338cf0..0ccc616db4 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -187,3 +187,19 @@ virStorageBackendForType(int type)
type, NULLSTR(virStoragePoolTypeToString(type)));
return NULL;
}
+
+
+virCapsPtr
+virStorageBackendGetCapabilities(void)
+{
+ virCapsPtr caps;
+ size_t i;
+
+ if (!(caps = virCapabilitiesNew(virArchFromHost(), false, false)))
+ return NULL;
+
The fact that you need to pass virArchFromHost() and false, false means,
that we need to abstract our virCaps even more. I mean, what has the
host architecture has to do with storage capabilities? Moreover, this is
how the beginning of storage caps look like:
# virsh -c storage:///system capabilities
<capabilities>
<host>
<cpu>
<arch>x86_64</arch>
</cpu>
<power_management/>
<iommu support='no'/>
</host>
+ for (i = 0; i < virStorageBackendsCount; i++)
+ virCapabilitiesAddStoragePool(caps, virStorageBackends[i]->type);
+
+ return caps;
+}
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 2b178494ae..c670c66287 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -126,4 +126,7 @@ int virStorageBackendDriversRegister(bool allmodules);
int virStorageBackendRegister(virStorageBackendPtr backend);
+virCapsPtr
+virStorageBackendGetCapabilities(void);
+
#endif /* LIBVIRT_STORAGE_BACKEND_H */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4a13e90481..04beff6b07 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -300,6 +300,9 @@ storageStateInitialize(bool privileged,
driver->storageEventState = virObjectEventStateNew();
+ if (!(driver->caps = virStorageBackendGetCapabilities()))
+ goto error;
+
Initially, I suspected that we want to refresh the caps on every 'virsh
capabilities' (just like we're doing in say qemu driver). But this is
different because we wouldn't load a storage driver out of the blue
(where as admin might install new qemu binaries once we're running).
Makes sense.
Michal
3:09 p.m.
New subject: [libvirt] [PATCH 6/7] storage: Process storage pool capabilities
On 1/29/19 5:18 AM, Michal Privoznik wrote:
On 1/16/19 2:15 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1581670
>
> During storage driver backend initialization, let's save
> which backends are available in the storage pool capabilities.
>
> In order to format those, we need add a connectGetCapabilities
> processor to the storageHypervisorDriver. This allows a storage
> connection, such as "storage:///system" to find the API and
> format the results.
>
> NB: This is not available from other hypervisor drivers such as
> QEMU type connections.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/virstorageobj.h | 4 ++++
> src/storage/storage_backend.c | 16 ++++++++++++++++
> src/storage/storage_backend.h | 3 +++
> src/storage/storage_driver.c | 17 +++++++++++++++++
> 4 files changed, 40 insertions(+)
>
> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
> index 1106aa71bd..34adc26df3 100644
> --- a/src/conf/virstorageobj.h
> +++ b/src/conf/virstorageobj.h
> @@ -24,6 +24,8 @@
> # include "storage_conf.h"
> +# include "capabilities.h"
> +
> typedef struct _virStoragePoolObj virStoragePoolObj;
> typedef virStoragePoolObj *virStoragePoolObjPtr;
> @@ -45,6 +47,8 @@ struct _virStorageDriverState {
> /* Immutable pointer, self-locking APIs */
> virObjectEventStatePtr storageEventState;
> +
> + virCapsPtr caps;
Would be nice to put a comment here:
/* Immutable pointer, read only after initialized */
Sure.
> };
> typedef bool
> diff --git a/src/storage/storage_backend.c
> b/src/storage/storage_backend.c
> index a54c338cf0..0ccc616db4 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -187,3 +187,19 @@ virStorageBackendForType(int type)
> type, NULLSTR(virStoragePoolTypeToString(type)));
> return NULL;
> }
> +
> +
> +virCapsPtr
> +virStorageBackendGetCapabilities(void)
> +{
> + virCapsPtr caps;
> + size_t i;
> +
> + if (!(caps = virCapabilitiesNew(virArchFromHost(), false, false)))
> + return NULL;
> +
The fact that you need to pass virArchFromHost() and false, false means,
that we need to abstract our virCaps even more. I mean, what has the
host architecture has to do with storage capabilities? Moreover, this is
how the beginning of storage caps look like:
# virsh -c storage:///system capabilities
<capabilities>
<host>
<cpu>
<arch>x86_64</arch>
</cpu>
<power_management/>
<iommu support='no'/>
</host>
Yes, I found it odd; however, from your summary comment "This one is
fine on its own, but will need a preceeding patch to abstract virCaps
even more."
How much abstraction do you really^^infinity want? Usage of ->caps.host
is more prevalent than I think it's worth to try and abstract it away
such that it would only be formatted if virCapsHostPtr != NULL. There's
a lot of code that wants to poke around and perhaps know more about
caps.host than should... Certainly more than I'd prefer altering.
Or could it be "simple enough" as modifying virCapabilitiesFormatHostXML
to return early:
/* Without the presence of some data means we have nothing
* minimally to format, so just return. */
if (!virUUIDIsValid(host->host_uuid) &&
!host->arch && !host->powerMgmt && !host->iommu)
return 0;
Initially I tried !host->cpu, but that tripped up all the
tests/qemucaps2xmloutdata/caps_*.xml files.
That also means the above call would pass VIR_ARCH_NONE.
> + for (i = 0; i < virStorageBackendsCount; i++)
> + virCapabilitiesAddStoragePool(caps,
> virStorageBackends[i]->type);
> +
> + return caps;
> +}
> diff --git a/src/storage/storage_backend.h
> b/src/storage/storage_backend.h
> index 2b178494ae..c670c66287 100644
> --- a/src/storage/storage_backend.h
> +++ b/src/storage/storage_backend.h
> @@ -126,4 +126,7 @@ int virStorageBackendDriversRegister(bool
> allmodules);
> int virStorageBackendRegister(virStorageBackendPtr backend);
> +virCapsPtr
> +virStorageBackendGetCapabilities(void);
> +
> #endif /* LIBVIRT_STORAGE_BACKEND_H */
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 4a13e90481..04beff6b07 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -300,6 +300,9 @@ storageStateInitialize(bool privileged,
> driver->storageEventState = virObjectEventStateNew();
> + if (!(driver->caps = virStorageBackendGetCapabilities()))
> + goto error;
> +
Initially, I suspected that we want to refresh the caps on every 'virsh
capabilities' (just like we're doing in say qemu driver). But this is
different because we wouldn't load a storage driver out of the blue
(where as admin might install new qemu binaries once we're running).
Makes sense.
I can add a comment here has well...
Tks -
John
7:15 p.m.
New subject: [libvirt] [PATCH 7/7] storage: Add storage backend pool/vol API's to capability output
https://bugzilla.redhat.com/show_bug.cgi?id=1581670
Add the Storage Pool and Volume API's defined for each generated
capability output, such as:
<pool>
<type>dir</pool>
<poolapis>
<buildPool/>
<refreshPool/>
<deletePool/>
</poolapis>
<volapis>
<buildVol/>
<buildVolFrom/>
<createVol/>
<refreshVol/>
<deleteVol/>
<resizeVol/>
<uploadVol/>
<downloadVol/>
<wipeVol/>
</volapis>
</pool>
...
<pool>
<type>iscsi</pool>
<poolapis>
<findPoolSources/>
<startPool/>
<refreshPool/>
<stopPool/>
</poolapis>
<volapis>
<uploadVol/>
<downloadVol/>
<wipeVol/>
</volapis>
</pool>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/capabilities.c | 49 ++++++++++++++++++++++++++++++--
src/conf/capabilities.h | 6 +++-
src/storage/storage_backend.c | 53 +++++++++++++++++++++++++++++++++--
3 files changed, 103 insertions(+), 5 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index c60743a38d..f7e9873f64 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -188,6 +188,8 @@ virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
if (!pool)
return;
+ VIR_FREE(pool->poolapis);
+ VIR_FREE(pool->volapis);
VIR_FREE(pool);
}
@@ -811,7 +813,9 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
int
virCapabilitiesAddStoragePool(virCapsPtr caps,
- int poolType)
+ int poolType,
+ const char *poolstr,
+ const char *volstr)
{
virCapsStoragePoolPtr pool;
@@ -823,7 +827,10 @@ virCapabilitiesAddStoragePool(virCapsPtr caps,
if (VIR_RESIZE_N(caps->pools, caps->npools_max, caps->npools, 1) < 0)
goto error;
caps->pools[caps->npools++] = pool;
-
+ if (VIR_STRDUP(pool->poolapis, poolstr) < 0)
+ goto error;
+ if (VIR_STRDUP(pool->volapis, volstr) < 0)
+ goto error;
return 0;
error:
@@ -1322,15 +1329,53 @@ virCapabilitiesFormatStoragePoolXML(virCapsStoragePoolPtr *pools,
virBufferPtr buf)
{
size_t i;
+ char **poolapis = NULL;
+ size_t npoolapis = 0;
+ char **volapis = NULL;
+ size_t nvolapis = 0;
for (i = 0; i < npools; i++) {
+ size_t j;
+
+ if (!(poolapis = virStringSplitCount(pools[i]->poolapis, ",",
+ 0, &npoolapis)) ||
+ !(volapis = virStringSplitCount(pools[i]->volapis, ",",
+ 0, &nvolapis)))
+ goto cleanup;
+
virBufferAddLit(buf, "<pool>\n");
virBufferAdjustIndent(buf, 2);
virBufferAsprintf(buf, "<type>%s</pool>\n",
virStoragePoolTypeToString(pools[i]->type));
+
+ virBufferAddLit(buf, "<poolapis>\n");
+ virBufferAdjustIndent(buf, 2);
+ for (j = 0; j < npoolapis; j++)
+ virBufferAsprintf(buf, "<%s/>\n", poolapis[j]);
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</poolapis>\n");
+
+ virBufferAddLit(buf, "<volapis>\n");
+ virBufferAdjustIndent(buf, 2);
+ for (j = 0; j < nvolapis; j++)
+ virBufferAsprintf(buf, "<%s/>\n", volapis[j]);
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</volapis>\n");
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</pool>\n\n");
+
+ virStringListFree(poolapis);
+ poolapis = NULL;
+ npoolapis = 0;
+ virStringListFree(volapis);
+ volapis = NULL;
+ nvolapis = 0;
}
+
+ cleanup:
+ virStringListFree(poolapis);
+ virStringListFree(volapis);
}
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index cca1a20949..525f19f195 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -215,6 +215,8 @@ typedef struct _virCapsStoragePool virCapsStoragePool;
typedef virCapsStoragePool *virCapsStoragePoolPtr;
struct _virCapsStoragePool {
int type;
+ char *poolapis;
+ char *volapis;
};
@@ -331,7 +333,9 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
int
virCapabilitiesAddStoragePool(virCapsPtr caps,
- int poolType);
+ int poolType,
+ const char *poolstr,
+ const char *volstr);
int
virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 0ccc616db4..95873c151f 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -193,13 +193,62 @@ virCapsPtr
virStorageBackendGetCapabilities(void)
{
virCapsPtr caps;
+ virBuffer poolbuf = VIR_BUFFER_INITIALIZER;
+ virBuffer volbuf = VIR_BUFFER_INITIALIZER;
size_t i;
if (!(caps = virCapabilitiesNew(virArchFromHost(), false, false)))
return NULL;
- for (i = 0; i < virStorageBackendsCount; i++)
- virCapabilitiesAddStoragePool(caps, virStorageBackends[i]->type);
+#define BUF_POOL_BACKEND(field) \
+ if (backend->field) \
+ virBufferAsprintf(&poolbuf, "%s,", #field);
+
+#define BUF_VOL_BACKEND(field) \
+ if (backend->field) \
+ virBufferAsprintf(&volbuf, "%s,", #field);
+
+ for (i = 0; i < virStorageBackendsCount; i++) {
+ char *poolstr = NULL;
+ char *volstr = NULL;
+ virStorageBackendPtr backend = virStorageBackends[i];
+
+ /* NB: checkPool is an internal only mechanism each pool has */
+ BUF_POOL_BACKEND(findPoolSources);
+ BUF_POOL_BACKEND(startPool);
+ BUF_POOL_BACKEND(buildPool);
+ BUF_POOL_BACKEND(refreshPool);
+ BUF_POOL_BACKEND(stopPool);
+ BUF_POOL_BACKEND(deletePool);
+ virBufferTrim(&poolbuf, ",", -1);
+
+ BUF_VOL_BACKEND(buildVol);
+ BUF_VOL_BACKEND(buildVolFrom);
+ BUF_VOL_BACKEND(createVol);
+ BUF_VOL_BACKEND(refreshVol);
+ BUF_VOL_BACKEND(deleteVol);
+ BUF_VOL_BACKEND(resizeVol);
+ BUF_VOL_BACKEND(uploadVol);
+ BUF_VOL_BACKEND(downloadVol);
+ BUF_VOL_BACKEND(wipeVol);
+ virBufferTrim(&volbuf, ",", -1);
+
+ if (virBufferCheckError(&poolbuf) < 0 ||
+ virBufferCheckError(&volbuf) < 0)
+ goto error;
+
+ poolstr = virBufferContentAndReset(&poolbuf);
+ volstr = virBufferContentAndReset(&volbuf);
+ virCapabilitiesAddStoragePool(caps, backend->type, poolstr, volstr);
+ VIR_FREE(poolstr);
+ VIR_FREE(volstr);
+ }
return caps;
+
+ error:
+ virBufferFreeAndReset(&poolbuf);
+ virBufferFreeAndReset(&volbuf);
+ virObjectUnref(caps);
+ return NULL;
}
--
2.20.1
4:18 a.m.
New subject: [libvirt] [PATCH 7/7] storage: Add storage backend pool/vol API's to capability output
On 1/16/19 2:15 AM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1581670
Add the Storage Pool and Volume API's defined for each generated
capability output, such as:
<pool>
<type>dir</pool>
<poolapis>
<buildPool/>
<refreshPool/>
<deletePool/>
</poolapis>
<volapis>
<buildVol/>
<buildVolFrom/>
<createVol/>
<refreshVol/>
<deleteVol/>
<resizeVol/>
<uploadVol/>
<downloadVol/>
<wipeVol/>
</volapis>
</pool>
...
<pool>
<type>iscsi</pool>
<poolapis>
<findPoolSources/>
<startPool/>
<refreshPool/>
<stopPool/>
</poolapis>
<volapis>
<uploadVol/>
<downloadVol/>
<wipeVol/>
</volapis>
</pool>
Frankly, I don't like this. Firstly, our users don't see 'uploadVol'
they see 'virStorageVolUpload'. Secondly, I am not sure if we want to be
introspectable this much. I mean, our users can just call the API and
deal with VIR_ERR_NO_SUPPORT. This looks like an overkill to me.
Michal
3:13 p.m.
New subject: [libvirt] [PATCH 7/7] storage: Add storage backend pool/vol API's to capability output
On 1/29/19 5:18 AM, Michal Privoznik wrote:
On 1/16/19 2:15 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1581670
>
> Add the Storage Pool and Volume API's defined for each generated
> capability output, such as:
>
> <pool>
> <type>dir</pool>
> <poolapis>
> <buildPool/>
> <refreshPool/>
> <deletePool/>
> </poolapis>
> <volapis>
> <buildVol/>
> <buildVolFrom/>
> <createVol/>
> <refreshVol/>
> <deleteVol/>
> <resizeVol/>
> <uploadVol/>
> <downloadVol/>
> <wipeVol/>
> </volapis>
> </pool>
>
> ...
>
> <pool>
> <type>iscsi</pool>
> <poolapis>
> <findPoolSources/>
> <startPool/>
> <refreshPool/>
> <stopPool/>
> </poolapis>
> <volapis>
> <uploadVol/>
> <downloadVol/>
> <wipeVol/>
> </volapis>
> </pool>
Frankly, I don't like this. Firstly, our users don't see 'uploadVol'
they see 'virStorageVolUpload'. Secondly, I am not sure if we want to be
introspectable this much. I mean, our users can just call the API and
deal with VIR_ERR_NO_SUPPORT. This looks like an overkill to me.
That's fine - hence the reason for separation. I guess I was thinking
that developers may like to know what API's are supported by each pool -
kind of a "features" list of sorts. Using the backend naming scheme
is/was kind of a cheat. I'm not against being more creative - ideas are
welcome ;-).
I think most important is perhaps knowing what volume operations a pool
supports or at least the upload/download, wipe, and resize.
Without any kind of extra data, the output is kind of boring:
<capabilities>
<pool>
<type>dir</pool>
</pool>
<pool>
<type>fs</pool>
</pool>
<pool>
<type>netfs</pool>
</pool>
....
<pool>
<type>zfs</pool>
</pool>
</capabilities>
John
5:57 a.m.
ping?
Tks,
John
On 1/15/19 8:15 PM, John Ferlan wrote:
Although I suppose this could have been an RFC - I just went with
a v1. I would think at least the first 4 patches are non controversial.
Beyond that it depends on what is "expected" as output for the
capabilities output for the storage driver.
John Ferlan (7):
conf: Extract host XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatHostXML to take virCapsHostPtr
conf: Extract guest XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatGuestXML to take virCapsGuestPtr
conf: Introduce storage pool functions into capabilities
storage: Process storage pool capabilities
storage: Add storage backend pool/vol API's to capability output
src/conf/capabilities.c | 449 ++++++++++++++++++++++------------
src/conf/capabilities.h | 19 ++
src/conf/virstorageobj.h | 4 +
src/libvirt_private.syms | 1 +
src/storage/storage_backend.c | 65 +++++
src/storage/storage_backend.h | 3 +
src/storage/storage_driver.c | 17 ++
7 files changed, 400 insertions(+), 158 deletions(-)
4:18 a.m.
On 1/16/19 2:15 AM, John Ferlan wrote:
Although I suppose this could have been an RFC - I just went with
a v1. I would think at least the first 4 patches are non controversial.
Beyond that it depends on what is "expected" as output for the
capabilities output for the storage driver.
John Ferlan (7):
conf: Extract host XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatHostXML to take virCapsHostPtr
conf: Extract guest XML formatting from virCapabilitiesFormatXML
conf: Alter virCapabilitiesFormatGuestXML to take virCapsGuestPtr
ACK to these four ^^
conf: Introduce storage pool functions into capabilities
This one is fine on its own, but will need a preceeding patch to
abstract virCaps even more.
storage: Process storage pool capabilities
This one will be okay if you fix those small issues I've raised.
storage: Add storage backend pool/vol API's to capability
output
src/conf/capabilities.c | 449 ++++++++++++++++++++++------------
src/conf/capabilities.h | 19 ++
src/conf/virstorageobj.h | 4 +
src/libvirt_private.syms | 1 +
src/storage/storage_backend.c | 65 +++++
src/storage/storage_backend.h | 3 +
src/storage/storage_driver.c | 17 ++
7 files changed, 400 insertions(+), 158 deletions(-)
Micha
2122
days inactive
2137
days old
19 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
John Ferlan -
Michal Privoznik -
Pavel Hrdina