[libvirt] [PATCH 4/8] Support startupPolicy for 'volume' disk

[John Ferlan]

On 04/04/2013 03:37 PM, Osier Yang wrote:
> "startupPolicy" is only valid for file type storage volume, otherwise
> it fails on starting the domain.
> ---
>  docs/formatdomain.html.in                                | 7 ++++---
>  docs/schemas/domaincommon.rng                            | 3 +++
>  src/conf/domain_conf.c                                   | 9 +++++++--
>  src/qemu/qemu_command.c                                  | 7 +++++++
>  tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml | 2 +-
>  5 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index bdc815f..ce185a9 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1467,11 +1467,12 @@
>          0.7.5; <code>type='network'</code> since
>          0.8.7; <code>protocol='iscsi'</code> since 1.0.4;
>          <code>type='volume'</code> since 1.0.5;</span><br/>
> -        For a "file" disk type which represents a cdrom or floppy
> +        For a "file" or "volume" disk type which represents a cdrom or floppy
>          (the <code>device</code> attribute), it is possible to define
>          policy what to do with the disk if the source file is not accessible.
> -        This is done by the <code>startupPolicy</code> attribute, accepting
> -        these values:
> +        (NB, <code>startupPolicy</code> is not valid for "volume" disk unless
> +         the specified storage volume is of "file" type). This is done by the
> +        <code>startupPolicy</code> attribute, accepting these values:
>          <table class="top_table">
>            <tr>
>              <td> mandatory </td>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 46cccc4..4e7e712 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1103,6 +1103,9 @@
>                  <attribute name="volume">
>                    <ref name="volName"/>
>                  </attribute>
> +                <optional>
> +                  <ref name="startupPolicy"/>
> +                </optional>
>                </element>
>              </optional>
>              <ref name="diskspec"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8538d5f..c1d2cbb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4180,6 +4180,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>                  case VIR_DOMAIN_DISK_TYPE_VOLUME:
>                      if (virDomainDiskSourcePoolDefParse(cur, def) < 0)
>                          goto error;
> +                    startupPolicy = virXMLPropString(cur, "startupPolicy");

Is there no way later on in this module to validate that the
startupPolicy for a DISK_TYPE_VOLUME is for a file only?  Seems a shame
to wait for building the qemu command to tell someone the bad news.


John
>                      break;
>                  default:
>                      virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -12890,9 +12891,13 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
>              /* Parsing guarantees the def->srcpool->volume cannot be NULL
>               * if def->srcpool->pool is not NULL.
>               */
> -            if (def->srcpool->pool)
> -                virBufferAsprintf(buf, "      <source pool='%s' volume='%s'/>\n",
> +            if (def->srcpool)
> +                virBufferAsprintf(buf, "      <source pool='%s' volume='%s'",
>                                    def->srcpool->pool, def->srcpool->volume);
> +            if (def->startupPolicy)
> +                virBufferEscapeString(buf, " startupPolicy='%s'/>\n", startupPolicy);
> +            else
> +                virBufferAddLit(buf, "/>\n");
>              break;
>          default:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 03c7195..cdfe801 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2703,6 +2703,13 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
>      if (virStorageVolGetInfo(vol, &info) < 0)
>          goto cleanup;
>  
> +    if (def->startupPolicy &&
> +        info.type != VIR_STORAGE_VOL_FILE) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("'startupPolicy' is only valid for 'file' type volume"));
> +        goto cleanup;
> +    }
> +
>      switch (info.type) {
>      case VIR_STORAGE_VOL_FILE:
>      case VIR_STORAGE_VOL_BLOCK:
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
> index 876eebe..a218e78 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
> @@ -15,7 +15,7 @@
>    <devices>
>      <emulator>/usr/bin/qemu</emulator>
>      <disk type='volume' device='cdrom'>
> -      <source pool='blk-pool0' volume='blk-pool0-vol0'/>
> +      <source pool='blk-pool0' volume='blk-pool0-vol0' startupPolicy='optional'/>
>        <target dev='hda' bus='ide'/>
>        <readonly/>
>        <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>