[libvirt] [PATCH 1/5] conf: add startupPolicy attribute for harddisk

[Osier Yang]

On 2013年03月18日 17:10, Guannan Ren wrote:
> Add startupPolicy attribute policy for harddisk with type "file",
> "block" and "dir". The "network" type disk is still not supported.
> ---
>   docs/formatdomain.html.in |  9 ++++++---
>   src/conf/domain_conf.c    | 19 +++++++++++++------
>   2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 8a3c3b7..a32bdc3 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1450,8 +1450,8 @@
>           For a "file" 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:
> +        This is done by the<code>startupPolicy</code>  attribute
> +        (<span class="since">Since 0.9.7</span>), accepting these values:
>           <table class="top_table">
>             <tr>
>               <td>  mandatory</td>
> @@ -1467,7 +1467,10 @@
>               <td>  drop if missing at any start attempt</td>
>             </tr>
>           </table>
> -<span class="since">Since 0.9.7</span>
> +<span class="since">Since 1.0.4</span>, the<code>startupPolicy</code>  extends

s/extends/is extended/,

> +        to support hard disks besides cdrom and floppy. However, the disk of "network"
> +        type is still not reached. For the guest which is using per-device<code>boot</code>
> +        element, the boot devices will be reordered after dropping its bootable disks.

The paragraph starts with "For a 'file' disk type". But your patch
extends the "startupPolicy" for all disk types except network. Which
means mismatching here.

>           </dd>
>         <dt><code>mirror</code></dt>
>         <dd>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3278e9c..177faaa 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4044,7 +4044,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>                   switch (def->type) {
>                   case VIR_DOMAIN_DISK_TYPE_FILE:
>                       source = virXMLPropString(cur, "file");
> -                    startupPolicy = virXMLPropString(cur, "startupPolicy");
>                       break;
>                   case VIR_DOMAIN_DISK_TYPE_BLOCK:
>                       source = virXMLPropString(cur, "dev");
> @@ -4137,6 +4136,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>                       goto error;
>                   }
>
> +                startupPolicy = virXMLPropString(cur, "startupPolicy");
> +
>                   /* People sometimes pass a bogus '' source path
>                      when they mean to omit the source element
>                      completely (e.g. CDROM without media). This is
> @@ -4674,11 +4675,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>               goto error;
>           }
>
> -        if (def->device != VIR_DOMAIN_DISK_DEVICE_CDROM&&
> -            def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
> +        if (def->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
>               virReportError(VIR_ERR_INVALID_ARG,
> -                           _("Setting disk %s is allowed only for "
> -                             "cdrom or floppy"),
> +                           _("Setting disk %s is not allowed for "
> +                             "disk of network type"),
>                              startupPolicy);
>               goto error;

So you should change the schema, to allow the startupPolicy for
disk of types except network.

>           }
> @@ -12838,6 +12838,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
>           case VIR_DOMAIN_DISK_TYPE_BLOCK:
>               virBufferEscapeString(buf, "<source dev='%s'",
>                                     def->src);
> +            if (def->startupPolicy)
> +                virBufferEscapeString(buf, " startupPolicy='%s'",
> +                                      startupPolicy);
>               if (def->nseclabels) {
>                   virBufferAddLit(buf, ">\n");
>                   virBufferAdjustIndent(buf, 8);
> @@ -12850,8 +12853,12 @@ virDomainDiskDefFormat(virBufferPtr buf,
>               }
>               break;
>           case VIR_DOMAIN_DISK_TYPE_DIR:
> -            virBufferEscapeString(buf, "<source dir='%s'/>\n",
> +            virBufferEscapeString(buf, "<source dir='%s'",
>                                     def->src);
> +            if (def->startupPolicy)
> +                virBufferEscapeString(buf, " startupPolicy='%s'",
> +                                      startupPolicy);
> +            virBufferAddLit(buf, "/>\n");
>               break;
>           case VIR_DOMAIN_DISK_TYPE_NETWORK:
>               virBufferAsprintf(buf, "<source protocol='%s'",