[libvirt] [PATCH 1/4] conf: add rawio attribute to disk element of domain XML

[Eric Blake]

On 01/30/2012 02:08 AM, Taku Izumi wrote:
> 
>  This patch adds a new attribute "rawio" to the "disk" element of domain XML.
>  Valid values of "rawio" attribute are "yes" and "no".
>  rawio='yes' indicates the disk is desirous of CAP_SYS_RAWIO.
> 
>  If you specify the following XML:
> 
>    <disk type='block' device='lun' rawio='yes'>
>      ...
>    </disk>
> 
>  the domain will be granted CAP_SYS_RAWIO.
>  (of course, the domain have to be executed with root privilege)
> 
>  NOTE:
>    - "rawio" attribute is only valid when device='lun'
>    - At the moment, any other disks you won't use rawio can use rawio.
>      

Trailing space in your commit message.

In addition to Dan's comments,

> @@ -3103,6 +3107,26 @@ virDomainDiskDefParseXML(virCapsPtr caps
>          def->snapshot = VIR_DOMAIN_DISK_SNAPSHOT_NO;
>      }
>  
> +    def->rawio = -1; /* unspecified */
> +    if (rawio) {
> +        if (def->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
> +            if (STREQ(rawio, "yes")) {
> +                def->rawio = 1;
> +            } else if (STREQ(rawio, "no")) {
> +                def->rawio = 0;
> +            } else {
> +                virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                     _("unknown disk rawio setting '%s'"),
> +                                     rawio);
> +                goto error;
> +            }
> +        } else {
> +            virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                _("rawio can be used only with device='lun'"));
> +            goto error;
> +        }

This rejects any use of rawio, even rawio='no', for non-lun,

> @@ -9930,6 +9961,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
>      virBufferAsprintf(buf,
>                        "    <disk type='%s' device='%s'",
>                        type, device);
> +    if (def->rawio == 1) {
> +        virBufferAddLit(buf, " rawio='yes'");
> +    } else if (def->rawio == 0) {
> +        virBufferAddLit(buf, " rawio='no'");
> +    }

but since rawio defaults to 0, this generates rawio='no' for all disks,
making it impossible to restart libvirtd if you use non-lun.  You need
to conditionalize the output.

> Index: libvirt/docs/formatdomain.html.in
> ===================================================================
> --- libvirt.orig/docs/formatdomain.html.in
> +++ libvirt/docs/formatdomain.html.in
> @@ -1096,8 +1096,11 @@
>          - also note that device='lun' will only be recognized for
>          actual raw devices, never for individual partitions or LVM
>          partitions (in those cases, the kernel will reject the generic
> -        SCSI commands, making it identical to device='disk').  The
> -        optional <code>snapshot</code> attribute indicates the default
> +        SCSI commands, making it identical to device='disk').
> +        The optional <code>rawio</code> attribute indicates that the disk
> +        is desirous of rawio capability. This attribute is only valid when
> +        device is "lun".

I was about to say that you were missing a <span class="since">
notation; but since device='lun' was also introduced in 0.9.10, and
rawio only makes sense with lun, you don't need an extra tag.

Even though you got acks later in the series, I couldn't quickly repair
the items that Dan pointed out about patch 1, so I'll wait for the next
revision of this series.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120130/52b2dce5/attachment-0001.sig>