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

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



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 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

Attachment: signature.asc
Description: Digital signature


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