On Tue, May 12, 2015 at 5:04 PM, Peter Krempa <pkrempa@xxxxxxxxxx> 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@xxxxxxxxxxxx> >> --- >> 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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list