[libvirt] [PATCH v5 7/9] domain_conf: Read and Write quorum config
Matthias Gatto
matthias.gatto at outscale.com
Thu Jun 4 08:10:54 UTC 2015
On Tue, May 12, 2015 at 5:04 PM, Peter Krempa <pkrempa at redhat.com> wrote:
> On Thu, Apr 23, 2015 at 14:41:19 +0200, Matthias Gatto wrote:
>> Add the capabiltty to libvirt to parse and format the quorum syntax
>> as described here:
>> http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html
>>
>> Signed-off-by: Matthias Gatto <matthias.gatto at outscale.com>
>> ---
>> src/conf/domain_conf.c | 164 +++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 119 insertions(+), 45 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index a3a6c13..ec93b6f 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -5952,20 +5952,56 @@ virDomainDiskSourceParse(xmlNodePtr node,
>> }
>>
>>
>> +static bool
>> +virDomainDiskThresholdParse(virStorageSourcePtr src,
>> + xmlNodePtr node)
>> +{
>> + char *threshold = virXMLPropString(node, "threshold");
>> + int ret;
>> +
>> + if (!threshold) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + "%s", _("missing threshold in quorum"));
>> + return false;
>> + }
>> + ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold);
>> + if (ret < 0 || src->threshold < 2) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unexpected threshold %s"),
>> + "threshold must be a decimal number superior to 2 "
>> + "and inferior to the number of children");
>> + VIR_FREE(threshold);
>> + return false;
>> + }
>> + VIR_FREE(threshold);
>> + return true;
>> +}
>> +
>> +
>> static int
>> virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
>> - virStorageSourcePtr src)
>> + virStorageSourcePtr src,
>> + xmlNodePtr node,
>> + size_t pos)
>> {
>> virStorageSourcePtr backingStore = NULL;
>> xmlNodePtr save_ctxt = ctxt->node;
>> - xmlNodePtr source;
>> + xmlNodePtr source = NULL;
>> char *type = NULL;
>> char *format = NULL;
>> int ret = -1;
>>
>> - if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
>> - ret = 0;
>> - goto cleanup;
>> + if (src->type == VIR_STORAGE_TYPE_QUORUM) {
>> + if (!node) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> + ctxt->node = node;
>> + } else {
>> + if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> }
>>
>> if (VIR_ALLOC(backingStore) < 0)
>> @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
>> goto cleanup;
>> }
>>
>> + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) {
>> + xmlNodePtr cur = NULL;
>> +
>> + if (!virDomainDiskThresholdParse(backingStore, node))
>> + goto cleanup;
>> +
>> + for (cur = node->children; cur != NULL; cur = cur->next) {
>> + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) {
>> + if ((virDomainDiskBackingStoreParse(ctxt,
>> + backingStore,
>> + cur,
>> + backingStore->nBackingStores) < 0)) {
>> + goto cleanup;
>> + }
>> + }
>> + }
>> + goto exit;
>> + }
>> +
>> if (!(source = virXPathNode("./source", ctxt))) {
>> virReportError(VIR_ERR_XML_ERROR, "%s",
>> _("missing disk backing store source"));
>> @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
>> }
>>
>> if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 ||
>> - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
>> + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0)
>> goto cleanup;
>>
>> - if (!virStorageSourceSetBackingStore(src, backingStore, 0))
>> + exit:
>> + if (!virStorageSourceSetBackingStore(src, backingStore, pos))
>> goto cleanup;
>> ret = 0;
>>
>> @@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
>> return ret;
>> }
>>
>> -
>> #define VENDOR_LEN 8
>> #define PRODUCT_LEN 16
>>
>> @@ -6518,6 +6573,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>> }
>> } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
>> /* boot is parsed as part of virDomainDeviceInfoParseXML */
>> + } else if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) {
>> + if (virDomainDiskBackingStoreParse(ctxt, def->src, cur,
>> + def->src->nBackingStores) < 0)
>> + goto error;
>> }
>> }
>> cur = cur->next;
>> @@ -6541,12 +6600,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>> def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
>> }
>>
>> + if (def->src->type == VIR_STORAGE_TYPE_QUORUM &&
>> + !virDomainDiskThresholdParse(def->src, node))
>> + goto error;
>> +
>> + snapshot = virXMLPropString(node, "snapshot");
>> +
>> /* Only CDROM and Floppy devices are allowed missing source path
>> * to indicate no media present. LUN is for raw access CD-ROMs
>> * that are not attached to a physical device presently */
>> if (virStorageSourceIsEmpty(def->src) &&
>> (def->device == VIR_DOMAIN_DISK_DEVICE_DISK ||
>> - (flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) {
>> + (flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) &&
>> + def->src->type != VIR_STORAGE_TYPE_QUORUM) {
>> virReportError(VIR_ERR_NO_SOURCE,
>> target ? "%s" : NULL, target);
>> goto error;
>> @@ -6892,9 +6958,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>> if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
>> && virDomainDiskDefAssignAddress(xmlopt, def) < 0)
>> goto error;
>> -
>> - if (virDomainDiskBackingStoreParse(ctxt, def->src) < 0)
>> - goto error;
>> }
>>
>> cleanup:
>> @@ -17717,12 +17780,14 @@ virDomainDiskSourceFormat(virBufferPtr buf,
>>
>> static int
>> virDomainDiskBackingStoreFormat(virBufferPtr buf,
>> - virStorageSourcePtr backingStore,
>> - const char *backingStoreRaw,
>> + virStorageSourcePtr src,
>> unsigned int idx)
>> {
>> + const char *backingStoreRaw = src->backingStoreRaw;
>> + virStorageSourcePtr backingStore = virStorageSourceGetBackingStore(src, 0);
>> const char *type;
>> const char *format;
>> + size_t i = 0;
>>
>> if (!backingStore) {
>> if (!backingStoreRaw)
>> @@ -17730,37 +17795,43 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
>> return 0;
>> }
>>
>> - if (!backingStore->type ||
>> - !(type = virStorageTypeToString(backingStore->type))) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("unexpected disk backing store type %d"),
>> - backingStore->type);
>> - return -1;
>> - }
>> + do {
>> + backingStore = virStorageSourceGetBackingStore(src, i);
>> + if (!backingStore->type ||
>> + !(type = virStorageTypeToString(backingStore->type))) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("unexpected disk backing store type %d"),
>> + backingStore->type);
>> + return -1;
>> + }
>>
>> - if (backingStore->format <= 0 ||
>> - !(format = virStorageFileFormatTypeToString(backingStore->format))) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("unexpected disk backing store format %d"),
>> - backingStore->format);
>> - return -1;
>> - }
>> + if (backingStore->format <= 0 ||
>> + !(format = virStorageFileFormatTypeToString(backingStore->format))) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("unexpected disk backing store format %d"),
>> + backingStore->format);
>> + return -1;
>> + }
>>
>> - virBufferAsprintf(buf, "<backingStore type='%s' index='%u'>\n",
>> - type, idx);
>> - virBufferAdjustIndent(buf, 2);
>> + virBufferAsprintf(buf, "<backingStore type='%s' index='%u'",
>> + type, idx);
>
> So now this change is wrong. Every single backing store element of the
> driver will have the same index, so you will not be able to reference
> them uniquely later. The indexes were designed in such way that they can
> be used to address backing chain elements (as node names in qemu) so you
> cannot make them pointless.
>
> Note that there's a lot of functions that use the information especially
> in the block job code that will need to be fixed.
>
> Additionally you'll need to fix the block job code in such way that it
> will work correctly with the quorums.
>
> Also note that a lot of places in the code don't properly implement
> transformations of the backing chain after block operations finish and
> resort to re-detection of the backing chain. That obviously won't work
> with quorums since they are not exposed in the backing chain (afaik).
>
> This means that basically for a proper implementation of this you'll
> need either to fix all code that alters the backing chains so that it
> works properly and then implement quorums on top of that or block
> basically every operation that transforms the backing chain so that the
> internal state does not get corrupted. Additionally this will probably
> also require modifying the storage handling code in libvirt in such way
> that it will handle the backing store information internally and not try
> to reload the state from the metadata in the image.
>
> Peter
>
Ok, I'm working on fixing the backing chains.
More information about the libvir-list
mailing list