Re: [PATCH v5 7/9] domain_conf: Read and Write quorum config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]