Re: [libvirt] [PATCH v6 08/13] domain_conf: Read and Write quorum config

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 As explain Peter Krempa in the V5, we need a different index for each child to manipulate individually each child of a quorum, this tack is done in this patch. Sadly this versions is a little buggy: if one day we allow a quorum child to have a backing store, we would be unable to made a difference between a quorum child and a backing store, worst than that, if we have a quorum, with a quorum as a child, we have no way to use the index for quorum child manipulation.

For now, this serie of patch forbid all actions which need to use indexes with quorum.

Therefore even if the index manipulation is buggy, this should not be a problem because the buggy code should never be call.

Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/domain_conf.c | 178 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 126 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce8edef..363ef5d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6607,20 +6607,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 (virStorageSourceIsRAID(src)) { + 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) @@ -6652,17 +6688,36 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } - if (!(source = virXPathNode("./source", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing disk backing store source")); - goto cleanup; - } + if (virStorageSourceIsRAID(backingStore)) { + xmlNodePtr cur = NULL; - if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) - goto cleanup; + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; - if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + 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; + } + } + } + } else { + + if (!(source = virXPathNode("./source", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing disk backing store source")); + goto cleanup; + } + + if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) + goto cleanup; + } + + if (!virStorageSourceSetBackingStore(src, backingStore, pos)) goto cleanup; ret = 0;

@@ -7177,6 +7232,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; @@ -7204,12 +7263,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)) && + !(virStorageSourceIsRAID(def->src))) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); goto error; @@ -7552,10 +7618,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } - if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { - if (virDomainDiskBackingStoreParse(ctxt, def->src) < 0) - goto error; - } cleanup: VIR_FREE(bus); @@ -18863,12 +18925,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) @@ -18876,37 +18940,44 @@ 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); + if (backingStore->threshold) + virBufferAsprintf(buf, " threshold='%lu'", backingStore->threshold); + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<format type='%s'/>\n", format); - /* We currently don't output seclabels for backing chain element */ - if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 || - virDomainDiskBackingStoreFormat(buf, - virStorageSourceGetBackingStore(backingStore, 0), - backingStore->backingStoreRaw, - idx + 1) < 0) - return -1; + virBufferAsprintf(buf, "<format type='%s'/>\n", format); + /* We currently don't output seclabels for backing chain element */ + if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 || + virDomainDiskBackingStoreFormat(buf, + backingStore, + idx + 1) < 0) + return -1; - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backingStore>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</backingStore>\n"); + ++i; + ++idx; + } while (i < src->nBackingStores);

return 0; }