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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org