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

Re: [libvirt] [PATCH 02/10] Support leases in guest XML and lock manager



On Thu, May 19, 2011 at 07:24:17AM -0400, Daniel P. Berrange wrote:
> A lock manager may operate in various modes. The direct mode of
> operation is to obtain locks based on the resources associated
> with devices in the XML. The indirect mode is where the app
> creating the domain provides explicit leases for each resource
> that needs to be locked. This XML extension allows for listing
> resources in the XML
> 
>   <devices>
>      ...
>      <lease>
>        <lockspace>somearea</lockspace>
>        <key>thequickbrownfoxjumpsoverthelazydog</key>
>        <target path='/some/lease/path' offset='23432'/>
>      </lease>
>      ...
>   </devices>
> 
> The 'lockspace' is a unique identifier for the lockspace which
> the lease is associated
> 
> The 'key' is a unique identifier for the resource associated
> with the lease.
> 
> The 'target' is the file on disk where the leases are held.
> 
> * docs/schemas/domain.rng: Add lease schema
> * src/conf/domain_conf.c, src/conf/domain_conf.h: parsing and
>   formatting for leases
> * tests/qemuxml2argvdata/qemuxml2argv-lease.args,
>   tests/qemuxml2argvdata/qemuxml2argv-lease.xml,
>   tests/qemuxml2xmltest.c: Test XML handling for leases
> ---
>  docs/formatdomain.html.in                      |   39 +++++++
>  docs/schemas/domain.rng                        |   24 ++++
>  src/conf/domain_conf.c                         |  134 ++++++++++++++++++++++++
>  src/conf/domain_conf.h                         |   14 +++
>  tests/qemuxml2argvdata/qemuxml2argv-lease.args |    4 +
>  tests/qemuxml2argvdata/qemuxml2argv-lease.xml  |   36 +++++++
>  tests/qemuxml2xmltest.c                        |    1 +
>  7 files changed, 252 insertions(+), 0 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-lease.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-lease.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index facdaf2..d59779d 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1071,6 +1071,45 @@
>        sub-element.
>      </p>
>  
> +    <h4><a name="elementsLease">Device leases</a></h4>
> +
> +    <p>
> +      When using a lock manager, it may be desirable to record device leases
> +      against a VM. The lock manager will ensure the VM won't start unless
> +      the leases can be acquired.
> +    </p>
> +
> +<pre>
> +  ...
> +  &lt;devices&gt;
> +    ...
> +    &lt;lease&gt;
> +      &lt;lockspace&gt;somearea&lt;/lockspace&gt;
> +      &lt;key&gt;somekey&lt;/key&gt;
> +      &lt;target path='/some/lease/path' offset='1024'/&gt;
> +    &lt;/lease&gt;
> +    ...
> +  &lt;/devices&gt;
> +  ...</pre>
> +
> +    <dl>
> +      <dt>lockspace</dt>
> +      <dd>This is an arbitrary string, identifying the lockspace
> +        within which the key is held. Lock managers may impose
> +        extra restrictions on the format, or length of the lockspace
> +        name.</dd>
> +      <dt>key</dt>
> +      <dd>This is an arbitrary string, uniquely identifying the
> +        lease to be acquired. Lock managers may impose extra
> +        restrictions on the format, or length of the key.
> +      </dd>
> +      <dt>target</dt>
> +      <dd>This is the fully qualified path of the file associated
> +        with the lockspace. The offset specifies where the lease
> +        is stored within the file. If the lock manager does not
> +        require a offset, just pass 0.
> +      </dd>
> +    </dl>
>  
>      <h4><a name="elementsUSB">USB and PCI devices</a></h4>
>  
> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
> index b252547..43c811f 100644
> --- a/docs/schemas/domain.rng
> +++ b/docs/schemas/domain.rng
> @@ -586,6 +586,29 @@
>        <ref name="address"/>
>      </optional>
>    </define>
> +
> +  <define name="lease">
> +    <element name="lease">
> +      <interleave>
> +        <element name="lockspace">
> +          <text/>
> +        </element>
> +        <element name="key">
> +          <text/>
> +        </element>
> +        <element name="target">
> +          <attribute name="path">
> +            <text/>

  This should use <ref name="absFilePath"/> instead of <text/> for added
checking

> +          </attribute>
> +          <optional>
> +            <attribute name="offset">
> +              <ref name="unsignedInt"/>
> +            </attribute>
> +          </optional>
> +        </element>
> +      </interleave>
> +    </element>
> +  </define>
>    <!--
>        A disk description can be either of type file or block
>        The name of the attribute on the source element depends on the type
> @@ -1940,6 +1963,7 @@
>            <choice>
>              <ref name="disk"/>
>              <ref name="controller"/>
> +            <ref name="lease"/>
>              <ref name="filesystem"/>
>              <ref name="interface"/>
>              <ref name="input"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 420d104..b6f7740 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -102,6 +102,7 @@ VIR_ENUM_IMPL(virDomainLifecycleCrash, VIR_DOMAIN_LIFECYCLE_CRASH_LAST,
>  
>  VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
>                "disk",
> +              "lease",
>                "filesystem",
>                "interface",
>                "input",
> @@ -638,6 +639,18 @@ void virDomainInputDefFree(virDomainInputDefPtr def)
>      VIR_FREE(def);
>  }
>  
> +static void virDomainLeaseDefFree(virDomainLeaseDefPtr def)
> +{
> +    if (!def)
> +        return;
> +
> +    VIR_FREE(def->lockspace);
> +    VIR_FREE(def->key);
> +    VIR_FREE(def->path);
> +
> +    VIR_FREE(def);
> +}
> +
>  void virDomainDiskDefFree(virDomainDiskDefPtr def)
>  {
>      unsigned int i;
> @@ -900,6 +913,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
>      case VIR_DOMAIN_DEVICE_DISK:
>          virDomainDiskDefFree(def->data.disk);
>          break;
> +    case VIR_DOMAIN_DEVICE_LEASE:
> +        virDomainLeaseDefFree(def->data.lease);
> +        break;
>      case VIR_DOMAIN_DEVICE_NET:
>          virDomainNetDefFree(def->data.net);
>          break;
> @@ -974,6 +990,10 @@ void virDomainDefFree(virDomainDefPtr def)
>      if (!def)
>          return;
>  
> +    for (i = 0 ; i < def->nleases ; i++)
> +        virDomainLeaseDefFree(def->leases[i]);
> +    VIR_FREE(def->leases);
> +
>      for (i = 0 ; i < def->ngraphics ; i++)
>          virDomainGraphicsDefFree(def->graphics[i]);
>      VIR_FREE(def->graphics);
> @@ -1880,6 +1900,79 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
>      return 0;
>  }
>  
> +/* Parse the XML definition for a lease
> + */
> +static virDomainLeaseDefPtr
> +virDomainLeaseDefParseXML(xmlNodePtr node)
> +{
> +    virDomainLeaseDefPtr def;
> +    xmlNodePtr cur;
> +    char *lockspace = NULL;
> +    char *key = NULL;
> +    char *path = NULL;
> +    char *offset = NULL;
> +
> +    if (VIR_ALLOC(def) < 0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    cur = node->children;
> +    while (cur != NULL) {
> +        if (cur->type == XML_ELEMENT_NODE) {
> +            if ((key == NULL) &&
> +                (xmlStrEqual(cur->name, BAD_CAST "key"))) {
> +                key = (char *)xmlNodeGetContent(cur);
> +            } else if ((lockspace == NULL) &&
> +                (xmlStrEqual(cur->name, BAD_CAST "lockspace"))) {
> +                lockspace = (char *)xmlNodeGetContent(cur);
> +            } else if ((path == NULL) &&
> +                       (xmlStrEqual(cur->name, BAD_CAST "target"))) {
> +                path = virXMLPropString(cur, "path");
> +                offset = virXMLPropString(cur, "offset");
> +            }
> +        }
> +        cur = cur->next;
> +    }
> +
> +    if (!key) {
> +        virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> +                             _("Missing 'key' element for lease"));
> +        goto error;
> +    }
> +    if (!path) {
> +        virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> +                             _("Missing 'target' element for lease"));
> +        goto error;
> +    }
> +
> +    if (offset &&
> +        virStrToLong_ull(offset, NULL, 10, &def->offset) < 0) {
> +        virDomainReportError(VIR_ERR_XML_ERROR,
> +                             _("Malformed lease target offset %s"), offset);
> +        goto error;
> +    }
> +
> +    def->key = key;
> +    def->lockspace = lockspace;
> +    def->path = path;
> +    path = key = lockspace = NULL;
> +
> +cleanup:
> +    VIR_FREE(lockspace);
> +    VIR_FREE(key);
> +    VIR_FREE(path);
> +    VIR_FREE(offset);
> +
> +    return def;
> +
> + error:
> +    virDomainLeaseDefFree(def);
> +    def = NULL;
> +    goto cleanup;
> +}
> +
> +
>  /* Parse the XML definition for a disk
>   * @param node XML nodeset to parse for disk definition
>   */
> @@ -4966,6 +5059,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
>          if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node,
>                                                          NULL, flags)))
>              goto error;
> +    } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
> +        dev->type = VIR_DOMAIN_DEVICE_LEASE;
> +        if (!(dev->data.lease = virDomainLeaseDefParseXML(node)))
> +            goto error;
>      } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) {
>          dev->type = VIR_DOMAIN_DEVICE_FS;
>          if (!(dev->data.fs = virDomainFSDefParseXML(node, flags)))
> @@ -5838,6 +5935,23 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>      }
>      VIR_FREE(nodes);
>  
> +    /* analysis of the resource leases */
> +    if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) {
> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                             "%s", _("cannot extract device leases"));
> +        goto error;
> +    }
> +    if (n && VIR_ALLOC_N(def->leases, n) < 0)
> +        goto no_memory;
> +    for (i = 0 ; i < n ; i++) {
> +        virDomainLeaseDefPtr lease = virDomainLeaseDefParseXML(nodes[i]);
> +        if (!lease)
> +            goto error;
> +
> +        def->leases[def->nleases++] = lease;
> +    }
> +    VIR_FREE(nodes);
> +
>      /* analysis of the filesystems */
>      if ((n = virXPathNodeSet("./devices/filesystem", ctxt, &nodes)) < 0) {
>          goto error;
> @@ -7012,6 +7126,22 @@ virDomainLifecycleDefFormat(virBufferPtr buf,
>  
>  
>  static int
> +virDomainLeaseDefFormat(virBufferPtr buf,
> +                        virDomainLeaseDefPtr def)
> +{
> +    virBufferAddLit(buf, "    <lease>\n");
> +    virBufferEscapeString(buf, "      <lockspace>%s</lockspace>\n", def->lockspace);
> +    virBufferEscapeString(buf, "      <key>%s</key>\n", def->key);
> +    virBufferEscapeString(buf, "      <target path='%s'", def->path);
> +    if (def->offset)
> +        virBufferAsprintf(buf, " offset='%llu'", def->offset);
> +    virBufferAddLit(buf, "/>\n");
> +    virBufferAddLit(buf, "    </lease>\n");
> +
> +    return 0;
> +}
> +
> +static int
>  virDomainDiskDefFormat(virBufferPtr buf,
>                         virDomainDiskDefPtr def,
>                         int flags)
> @@ -8415,6 +8545,10 @@ char *virDomainDefFormat(virDomainDefPtr def,
>          if (virDomainControllerDefFormat(&buf, def->controllers[n], flags) < 0)
>              goto cleanup;
>  
> +    for (n = 0 ; n < def->nleases ; n++)
> +        if (virDomainLeaseDefFormat(&buf, def->leases[n]) < 0)
> +            goto cleanup;
> +
>      for (n = 0 ; n < def->nfss ; n++)
>          if (virDomainFSDefFormat(&buf, def->fss[n], flags) < 0)
>              goto cleanup;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8560076..b0771aa 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -123,6 +123,15 @@ struct _virDomainDeviceInfo {
>      } addr;
>  };
>  
> +typedef struct _virDomainLeaseDef virDomainLeaseDef;
> +typedef virDomainLeaseDef *virDomainLeaseDefPtr;
> +struct _virDomainLeaseDef {
> +    char *lockspace;
> +    char *key;
> +    char *path;
> +    unsigned long long offset;
> +};
> +
>  
>  /* Two types of disk backends */
>  enum virDomainDiskType {
> @@ -816,6 +825,7 @@ enum virDomainSmbiosMode {
>  /* Flags for the 'type' field in next struct */
>  enum virDomainDeviceType {
>      VIR_DOMAIN_DEVICE_DISK,
> +    VIR_DOMAIN_DEVICE_LEASE,
>      VIR_DOMAIN_DEVICE_FS,
>      VIR_DOMAIN_DEVICE_NET,
>      VIR_DOMAIN_DEVICE_INPUT,
> @@ -836,6 +846,7 @@ struct _virDomainDeviceDef {
>      union {
>          virDomainDiskDefPtr disk;
>          virDomainControllerDefPtr controller;
> +        virDomainLeaseDefPtr lease;
>          virDomainFSDefPtr fs;
>          virDomainNetDefPtr net;
>          virDomainInputDefPtr input;
> @@ -1171,6 +1182,9 @@ struct _virDomainDef {
>      int nchannels;
>      virDomainChrDefPtr *channels;
>  
> +    int nleases;
> +    virDomainLeaseDefPtr *leases;
> +
>      /* Only 1 */
>      virDomainChrDefPtr console;
>      virSecurityLabelDef seclabel;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-lease.args b/tests/qemuxml2argvdata/qemuxml2argv-lease.args
> new file mode 100644
> index 0000000..63f9bef
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-lease.args
> @@ -0,0 +1,4 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \
> +-M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
> +-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -cdrom /root/boot.iso -net none \
> +-serial none -parallel none -usb

  Hum is it really useful to add the test case yet while the driver code
isn't applied, but minor, I assume the test get expanded with the
driver addition later on

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-lease.xml b/tests/qemuxml2argvdata/qemuxml2argv-lease.xml
> new file mode 100644
> index 0000000..7efe1ef
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-lease.xml
> @@ -0,0 +1,36 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory>219200</memory>
> +  <currentMemory>219200</currentMemory>
> +  <vcpu>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' unit='0'/>
> +    </disk>
> +    <disk type='file' device='cdrom'>
> +      <source file='/root/boot.iso'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' unit='0'/>
> +    </disk>
> +    <controller type='ide' index='0'/>
> +    <lease>
> +      <lockspace>somearea</lockspace>
> +      <key>thequickbrownfoxjumpedoverthelazydog</key>
> +      <target path='/some/lease/path' offset='1024'/>
> +    </lease>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 5bfbcab..e74c337 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -179,6 +179,7 @@ mymain(void)
>      DO_TEST("cputune");
>  
>      DO_TEST("smp");
> +    DO_TEST("lease");
>  
>      /* These tests generate different XML */
>      DO_TEST_DIFFERENT("balloon-device-auto");

  ACK, with the small improvement to the RNG

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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