[libvirt] [PATCH 3/7] conf: Add/modify storage formatting functions
John Ferlan
jferlan at redhat.com
Wed Mar 25 16:21:29 UTC 2015
On 03/24/2015 06:06 AM, Erik Skultety wrote:
> This patch introduces virStoragePoolSaveStatus to properly format the
> status XML. It also adds virStoragePoolDefFormatBuf function, which
> alike in the network driver, formats the whole storage pool definition into
> a buffer that might have been previously modified by
> virStoragePoolSaveStatus to insert <poolstatus> element. The original
> virStoragePoolDefFormat function had to be modified accordingly to use
> virBuffer.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
> ---
> src/conf/storage_conf.c | 139 ++++++++++++++++++++++++++++++++++-------------
> src/conf/storage_conf.h | 6 +-
> src/libvirt_private.syms | 1 +
> 3 files changed, 107 insertions(+), 39 deletions(-)
>
In a bit of bikeshedding - this patch does a couple of things and could
be split into a couple of patches... Ironically you bundled things
together here, but separated them for the stateDir changes (patches 2,
4, & 7).
first one just creates DefFormatBuf and has DefFormat call it
second one creates the virStoragePoolSaveXML, has the config code use it
third one creates virStoragePoolSaveStatus which use the new API's
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index b070448..5d984f3 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf,
> }
>
>
> -char *
> -virStoragePoolDefFormat(virStoragePoolDefPtr def)
> +int
static int - only this function cares; otherwise we have to ensure the
'buf' and 'def' arguments aren't NULL in the .h file... This way we
control having non NULL values being presented.
> +virStoragePoolDefFormatBuf(virBufferPtr buf,
> + virStoragePoolDefPtr def)
> {
> virStoragePoolOptionsPtr options;
> - virBuffer buf = VIR_BUFFER_INITIALIZER;
> - const char *type;
> char uuid[VIR_UUID_STRING_BUFLEN];
> + const char *type;
>
> options = virStoragePoolOptionsForPoolType(def->type);
> if (options == NULL)
> - return NULL;
> + goto error;
>
> type = virStoragePoolTypeToString(def->type);
> if (!type) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("unexpected pool type"));
> - goto cleanup;
> + goto error;
> }
> - virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
> - virBufferAdjustIndent(&buf, 2);
> - virBufferEscapeString(&buf, "<name>%s</name>\n", def->name);
> + virBufferAsprintf(buf, "<pool type='%s'>\n", type);
> + virBufferAdjustIndent(buf, 2);
> + virBufferEscapeString(buf, "<name>%s</name>\n", def->name);
>
> virUUIDFormat(def->uuid, uuid);
> - virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuid);
> + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid);
>
> - virBufferAsprintf(&buf, "<capacity unit='bytes'>%llu</capacity>\n",
> + virBufferAsprintf(buf, "<capacity unit='bytes'>%llu</capacity>\n",
> def->capacity);
> - virBufferAsprintf(&buf, "<allocation unit='bytes'>%llu</allocation>\n",
> + virBufferAsprintf(buf, "<allocation unit='bytes'>%llu</allocation>\n",
> def->allocation);
> - virBufferAsprintf(&buf, "<available unit='bytes'>%llu</available>\n",
> + virBufferAsprintf(buf, "<available unit='bytes'>%llu</available>\n",
> def->available);
>
> - if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0)
> - goto cleanup;
> + if (virStoragePoolSourceFormat(buf, options, &def->source) < 0)
> + goto error;
>
> /* RBD, Sheepdog, and Gluster devices are not local block devs nor
> * files, so they don't have a target */
> if (def->type != VIR_STORAGE_POOL_RBD &&
> def->type != VIR_STORAGE_POOL_SHEEPDOG &&
> def->type != VIR_STORAGE_POOL_GLUSTER) {
> - virBufferAddLit(&buf, "<target>\n");
> - virBufferAdjustIndent(&buf, 2);
> + virBufferAddLit(buf, "<target>\n");
> + virBufferAdjustIndent(buf, 2);
>
> - virBufferEscapeString(&buf, "<path>%s</path>\n", def->target.path);
> + virBufferEscapeString(buf, "<path>%s</path>\n", def->target.path);
>
> - virBufferAddLit(&buf, "<permissions>\n");
> - virBufferAdjustIndent(&buf, 2);
> - virBufferAsprintf(&buf, "<mode>0%o</mode>\n",
> + virBufferAddLit(buf, "<permissions>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<mode>0%o</mode>\n",
> def->target.perms.mode);
> - virBufferAsprintf(&buf, "<owner>%d</owner>\n",
> + virBufferAsprintf(buf, "<owner>%d</owner>\n",
> (int) def->target.perms.uid);
> - virBufferAsprintf(&buf, "<group>%d</group>\n",
> + virBufferAsprintf(buf, "<group>%d</group>\n",
> (int) def->target.perms.gid);
> - virBufferEscapeString(&buf, "<label>%s</label>\n",
> + virBufferEscapeString(buf, "<label>%s</label>\n",
> def->target.perms.label);
>
> - virBufferAdjustIndent(&buf, -2);
> - virBufferAddLit(&buf, "</permissions>\n");
> - virBufferAdjustIndent(&buf, -2);
> - virBufferAddLit(&buf, "</target>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</permissions>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</target>\n");
> }
> - virBufferAdjustIndent(&buf, -2);
> - virBufferAddLit(&buf, "</pool>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</pool>\n");
> +
> + return 0;
> +
> + error:
> + return -1;
> +}
> +
> +char *
> +virStoragePoolDefFormat(virStoragePoolDefPtr def)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (virStoragePoolDefFormatBuf(&buf, def) < 0)
> + goto error;
>
> if (virBufferCheckError(&buf) < 0)
> - goto cleanup;
> + goto error;
>
> return virBufferContentAndReset(&buf);
>
> - cleanup:
> + error:
> virBufferFreeAndReset(&buf);
> return NULL;
> }
> @@ -1899,11 +1913,60 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
> return ret;
> }
>
> +
> +static int virStoragePoolSaveXML(const char *configFile,
> + virStoragePoolDefPtr def,
> + const char *xml)
static int
virStoragePoolSaveXML(const char *path,
(since it won't be the "configFile" only...)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + int ret = -1;
> +
> + virUUIDFormat(def->uuid, uuidstr);
> + ret = virXMLSaveFile(configFile,
s/configFile/path
> + virXMLPickShellSafeComment(def->name, uuidstr),
> + "pool-edit", xml);
> +
> + return ret;
> +}
> +
> +
> +int virStoragePoolSaveStatus(const char *stateFile,
> + virStoragePoolDefPtr def)
int
virStoragePoolSaveStatus(...
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> + int ret = -1;
> + char *xml;
> +
> + virBufferAddLit(&buf, "<poolstatus>\n");
poolstate ?
> + virBufferAdjustIndent(&buf, 2);
> +
> + if (virStoragePoolDefFormatBuf(&buf, def) < 0)
> + goto error;
> +
> + virBufferAdjustIndent(&buf, -2);
> + virBufferAddLit(&buf, "</poolstatus>\n");
poolstate ?
> +
> + if (virBufferCheckError(&buf) < 0)
> + goto error;
> +
> + if (!(xml = virBufferContentAndReset(&buf)))
> + goto error;
> +
> + if (virStoragePoolSaveXML(stateFile, def, xml))
> + goto error;
> +
> + ret = 0;
> +
> + error:
> + VIR_FREE(xml);
> + return ret;
> +}
> +
> +
> int
> virStoragePoolSaveConfig(const char *configFile,
> virStoragePoolDefPtr def)
> {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> char *xml;
> int ret = -1;
>
> @@ -1913,12 +1976,12 @@ virStoragePoolSaveConfig(const char *configFile,
> return -1;
> }
>
> - virUUIDFormat(def->uuid, uuidstr);
> - ret = virXMLSaveFile(configFile,
> - virXMLPickShellSafeComment(def->name, uuidstr),
> - "pool-edit", xml);
> - VIR_FREE(xml);
> + if (virStoragePoolSaveXML(configFile, def, xml))
> + goto cleanup;
>
> + ret = 0;
> + cleanup:
> + VIR_FREE(xml);
> return ret;
> }
>
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index 8ccc947..99b2f4a 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -345,6 +345,8 @@ virStoragePoolDefPtr virStoragePoolDefParseFile(const char *filename);
> virStoragePoolDefPtr virStoragePoolDefParseNode(xmlDocPtr xml,
> xmlNodePtr root);
> char *virStoragePoolDefFormat(virStoragePoolDefPtr def);
> +int virStoragePoolDefFormatBuf(virBufferPtr buf,
> + virStoragePoolDefPtr def);
Unnecessary if static int
Obvious if this gets split into 3 patches, then there's a few changes to
the following stuff changes too
John
>
> typedef enum {
> /* do not require volume capacity at all */
> @@ -372,7 +374,9 @@ virStoragePoolObjPtr
> virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def);
>
> -int virStoragePoolSaveConfig(const char *configDir,
> +int virStoragePoolSaveStatus(const char *stateFile,
> + virStoragePoolDefPtr def);
> +int virStoragePoolSaveConfig(const char *configFile,
> virStoragePoolDefPtr def);
> int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
> virStoragePoolObjPtr pool,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 33222f0..689a08f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -811,6 +811,7 @@ virStoragePoolObjRemove;
> virStoragePoolObjSaveDef;
> virStoragePoolObjUnlock;
> virStoragePoolSaveConfig;
> +virStoragePoolSaveStatus;
> virStoragePoolSourceAdapterTypeFromString;
> virStoragePoolSourceAdapterTypeToString;
> virStoragePoolSourceClear;
>
More information about the libvir-list
mailing list