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

[Daniel P. Berrange]

On Mon, Jan 30, 2012 at 06:08:35PM +0900, 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.
>      
> 
> Signed-off-by: Taku Izumi <izumi.taku at jp.fujitsu.com>
> ---
>  docs/formatdomain.html.in     |    7 +++++--
>  docs/schemas/domaincommon.rng |    8 ++++++++
>  src/conf/domain_conf.c        |   36 ++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |    3 +++
>  4 files changed, 52 insertions(+), 2 deletions(-)
> 
> Index: libvirt/docs/schemas/domaincommon.rng
> ===================================================================
> --- libvirt.orig/docs/schemas/domaincommon.rng
> +++ libvirt/docs/schemas/domaincommon.rng
> @@ -806,6 +806,14 @@
>          </attribute>
>        </optional>
>        <optional>
> +        <attribute name="rawio">
> +          <choice>
> +            <value>yes</value>
> +            <value>no</value>
> +          </choice>
> +        </attribute>
> +      </optional>
> +      <optional>
>          <ref name="snapshot"/>
>        </optional>
>        <choice>
> Index: libvirt/src/conf/domain_conf.c
> ===================================================================
> --- libvirt.orig/src/conf/domain_conf.c
> +++ libvirt/src/conf/domain_conf.c
> @@ -30,6 +30,7 @@
>  #include <dirent.h>
>  #include <sys/time.h>
>  #include <strings.h>
> +#include <linux/capability.h>

Remove this include.

>  
>  #include "virterror_internal.h"
>  #include "datatypes.h"
> @@ -2751,6 +2752,7 @@ virDomainDiskDefParseXML(virCapsPtr caps
>      char *type = NULL;
>      char *device = NULL;
>      char *snapshot = NULL;
> +    char *rawio = NULL;
>      char *driverName = NULL;
>      char *driverType = NULL;
>      char *source = NULL;
> @@ -2795,6 +2797,8 @@ virDomainDiskDefParseXML(virCapsPtr caps
>  
>      snapshot = virXMLPropString(node, "snapshot");
>  
> +    rawio = virXMLPropString(node, "rawio");
> +
>      cur = node->children;
>      while (cur != NULL) {
>          if (cur->type == XML_ELEMENT_NODE) {
> @@ -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;
> +        }
> +    }
> +
>      if (bus) {
>          if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) {
>              virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -7517,6 +7541,13 @@ static virDomainDefPtr virDomainDefParse
>          if (!disk)
>              goto error;
>  
> +        /* cap_sys_rawio check */
> +        if (disk->rawio == 1 &&
> +            (def->process_caps & (1ULL << CAP_SYS_RAWIO)) == 0) {
> +            def->process_caps |= (1ULL << CAP_SYS_RAWIO);
> +            VIR_WARN("domain %s will be granted CAP_SYS_RAWIO", def->name);
> +        }
> +

Don't do this here. 'process_caps' is an implementation detail for
the QEMU driver. We don't need to store any field for that, since
the QEMU driver can figure it out from the 'rawio' field when it
comes to start the domain.

>          virDomainDiskInsertPreAlloced(def, disk);
>      }
>      VIR_FREE(nodes);
> @@ -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'");
> +    }
>      if (def->snapshot &&
>          !(def->snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO && def->readonly))
>          virBufferAsprintf(buf, " snapshot='%s'",
> Index: libvirt/src/conf/domain_conf.h
> ===================================================================
> --- libvirt.orig/src/conf/domain_conf.h
> +++ libvirt/src/conf/domain_conf.h
> @@ -401,6 +401,7 @@ struct _virDomainDiskDef {
>      unsigned int transient : 1;
>      virDomainDeviceInfo info;
>      virStorageEncryptionPtr encryption;
> +    int rawio; /* unspecified:-1 no:0 yes:1 */
>  };
>  
>  
> @@ -1464,6 +1465,8 @@ struct _virDomainDef {
>      char *emulator;
>      int features;
>  
> +    unsigned long long process_caps;
> +

Remove this field.

>      virDomainClockDef clock;
>  
>      int ngraphics;


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|