[libvirt] [PATCH 0/3] Add "rawio" for hostdev
The "rawio" setting for a <device.../> <disk.../> lun isn't duplicated in the scsi_host hostdev device for a lun. Rather than "requiring" some disk in the disk list to add the rawio, add the "rawio" property to the scsi_host hostdev lun. John Ferlan (3): hostdev: Add "rawio" attribute to _virDomainHostdevSubsysSCSI qemu: Process the hostdev "rawio" setting domain_conf: Process the "rawio" for a hostdev device docs/formatdomain.html.in | 12 ++++++-- docs/schemas/domaincommon.rng | 18 +++++++---- src/conf/domain_conf.c | 31 +++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c | 20 ++++++++++++- .../qemuxml2argv-hostdev-scsi-rawio.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml -- 1.9.3
Add the 'rawio' attribute to match _virDomainDiskDef Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1107fa8..b1d13ef 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -439,6 +439,8 @@ typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; struct _virDomainHostdevSubsysSCSI { int protocol; /* enum virDomainHostdevSCSIProtocolType */ int sgio; /* enum virDomainDeviceSGIO */ + bool rawio_specified; + int rawio; /* no = 0, yes = 1 */ union { virDomainHostdevSubsysSCSIHost host; virDomainHostdevSubsysSCSIiSCSI iscsi; -- 1.9.3
On 10.09.2014 01:40, John Ferlan wrote:
Add the 'rawio' attribute to match _virDomainDiskDef
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1107fa8..b1d13ef 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -439,6 +439,8 @@ typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; struct _virDomainHostdevSubsysSCSI { int protocol; /* enum virDomainHostdevSCSIProtocolType */ int sgio; /* enum virDomainDeviceSGIO */ + bool rawio_specified; + int rawio; /* no = 0, yes = 1 */
Instead of introducing these two values, would it be possible just to use virTristateBool which already contains the tristate values? Oh, and virDomainDiskDef should be fixed too.
union { virDomainHostdevSubsysSCSIHost host; virDomainHostdevSubsysSCSIiSCSI iscsi;
Michal
Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev lun device. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c | 20 +++++++++++++++++++- 4 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 306ff10..166fadb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, for (i = 0; i < obj->def->ndisks; i++) qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD); + for (i = 0; i < obj->def->nhostdevs; i++) + qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i], + logFD); + for (i = 0; i < obj->def->nnets; i++) qemuDomainObjCheckNetTaint(driver, obj, obj->def->nets[i], logFD); @@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, } +void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, + virDomainObjPtr obj, + virDomainHostdevDefPtr hostdev, + int logFD) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->rawio == 1) + qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, + logFD); + + virObjectUnref(cfg); +} + + void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainNetDefPtr net, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f353d90..7aebb0f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainDiskDefPtr disk, int logFD); +void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, + virDomainObjPtr obj, + virDomainHostdevDefPtr disk, + int logFD); void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainNetDefPtr net, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d724eeb..78ecb3e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; case VIR_DOMAIN_DEVICE_HOSTDEV: + qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1); ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); if (!ret) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b1d8a32..3544716 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemuProcessHookData hookData; unsigned long cur_balloon; size_t i; + bool rawio_set = false; char *nodeset = NULL; virBitmapPtr nodemask = NULL; unsigned int stop_flags; @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn, virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i]; - if (vm->def->disks[i]->rawio == 1) + if (vm->def->disks[i]->rawio == 1) { #ifdef CAP_SYS_RAWIO virCommandAllowCap(cmd, CAP_SYS_RAWIO); #else virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Raw I/O is not supported on this platform")); #endif + rawio_set = true; + } dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; @@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } + /* If rawio not already set, check hostdevs as well */ + if (!rawio_set) { + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc = + &vm->def->hostdevs[i]->source.subsys.u.scsi; + if (scsisrc->rawio == 1) +#ifdef CAP_SYS_RAWIO + virCommandAllowCap(cmd, CAP_SYS_RAWIO); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Raw I/O is not supported on this platform")); +#endif + } + } + virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); virCommandSetMaxFiles(cmd, cfg->maxFiles); -- 1.9.3
On 10.09.2014 01:40, John Ferlan wrote:
Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev lun device.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c | 20 +++++++++++++++++++- 4 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 306ff10..166fadb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, for (i = 0; i < obj->def->ndisks; i++) qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD);
+ for (i = 0; i < obj->def->nhostdevs; i++) + qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i], + logFD); + for (i = 0; i < obj->def->nnets; i++) qemuDomainObjCheckNetTaint(driver, obj, obj->def->nets[i], logFD);
@@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, }
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, + virDomainObjPtr obj, + virDomainHostdevDefPtr hostdev, + int logFD) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
This @cfg is not used anywhere in the function. Well, technically is - it's unref'd but besides that, it isn't. Is it a leftover from previous code of yours that you were testing?
+ virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->rawio == 1)
If you follow my advice from 1/3 then this would be s/1/VIR_TRISTATE_BOOL_YES/
+ qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, + logFD); + + virObjectUnref(cfg); +} + + void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainNetDefPtr net, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f353d90..7aebb0f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainDiskDefPtr disk, int logFD); +void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, + virDomainObjPtr obj, + virDomainHostdevDefPtr disk, + int logFD); void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainNetDefPtr net, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d724eeb..78ecb3e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break;
case VIR_DOMAIN_DEVICE_HOSTDEV: + qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1); ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); if (!ret) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b1d8a32..3544716 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemuProcessHookData hookData; unsigned long cur_balloon; size_t i; + bool rawio_set = false; char *nodeset = NULL; virBitmapPtr nodemask = NULL; unsigned int stop_flags; @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn, virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i];
- if (vm->def->disks[i]->rawio == 1) + if (vm->def->disks[i]->rawio == 1) { #ifdef CAP_SYS_RAWIO virCommandAllowCap(cmd, CAP_SYS_RAWIO); #else virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Raw I/O is not supported on this platform")); #endif + rawio_set = true; + }
Interesting. So if rawio is requested we shout an error but don't fail actually. I think we are missing 'goto cleanup' here.
dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; @@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
+ /* If rawio not already set, check hostdevs as well */ + if (!rawio_set) { + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc = + &vm->def->hostdevs[i]->source.subsys.u.scsi; + if (scsisrc->rawio == 1) +#ifdef CAP_SYS_RAWIO + virCommandAllowCap(cmd, CAP_SYS_RAWIO); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Raw I/O is not supported on this platform")); +#endif
And here too then.
+ } + } + virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); virCommandSetMaxFiles(cmd, cfg->maxFiles);
Michal
On 09/18/2014 12:50 PM, Michal Privoznik wrote:
On 10.09.2014 01:40, John Ferlan wrote:
Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev lun device. <...snip...>
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b1d8a32..3544716 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemuProcessHookData hookData; unsigned long cur_balloon; size_t i; + bool rawio_set = false; char *nodeset = NULL; virBitmapPtr nodemask = NULL; unsigned int stop_flags; @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn, virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i];
- if (vm->def->disks[i]->rawio == 1) + if (vm->def->disks[i]->rawio == 1) { #ifdef CAP_SYS_RAWIO virCommandAllowCap(cmd, CAP_SYS_RAWIO); #else virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Raw I/O is not supported on this platform")); #endif + rawio_set = true; + }
Interesting. So if rawio is requested we shout an error but don't fail actually. I think we are missing 'goto cleanup' here.
See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579 I can add a goto if desired or perhaps change it to a VIR_WARN() or something else well. I've copied the author of that commit to get an opinion...
dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; @@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
+ /* If rawio not already set, check hostdevs as well */ + if (!rawio_set) { + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc = + &vm->def->hostdevs[i]->source.subsys.u.scsi; + if (scsisrc->rawio == 1) +#ifdef CAP_SYS_RAWIO + virCommandAllowCap(cmd, CAP_SYS_RAWIO); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Raw I/O is not supported on this platform")); +#endif
And here too then.
Monkey see, monkey do[o] ;-) John
+ } + } +
John Ferlan wrote:
On 09/18/2014 12:50 PM, Michal Privoznik wrote:
On 10.09.2014 01:40, John Ferlan wrote:
Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev lun device. <...snip...>
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b1d8a32..3544716 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemuProcessHookData hookData; unsigned long cur_balloon; size_t i; + bool rawio_set = false; char *nodeset = NULL; virBitmapPtr nodemask = NULL; unsigned int stop_flags; @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn, virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i];
- if (vm->def->disks[i]->rawio == 1) + if (vm->def->disks[i]->rawio == 1) { #ifdef CAP_SYS_RAWIO virCommandAllowCap(cmd, CAP_SYS_RAWIO); #else virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Raw I/O is not supported on this platform")); #endif + rawio_set = true; + }
Interesting. So if rawio is requested we shout an error but don't fail actually. I think we are missing 'goto cleanup' here.
See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579
I can add a goto if desired or perhaps change it to a VIR_WARN() or something else well.
I've copied the author of that commit to get an opinion...
Hi, Yes, I guess 'goto cleanup' is missing here indeed. VIR_WARN() will also work, but probably it'd be better user experience to report an unsupported configuration and fail early instead of showing a warning that could be missed. Roman Bogorodskiy
Add a "rawio" to the hostdev XML and process it mimicing the disk XML for a lun which supports/requires rawio Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 12 ++++++-- docs/schemas/domaincommon.rng | 18 +++++++---- src/conf/domain_conf.c | 31 +++++++++++++++++++ .../qemuxml2argv-hostdev-scsi-rawio.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..07640af 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1774,7 +1774,7 @@ <dt><code>rawio</code> attribute <span class="since">since 0.9.10</span></dt> <dd> - Indicates whether the disk is needs rawio capability; valid + Indicates whether the disk needs rawio capability. Valid settings are "yes" or "no" (default is "no"). If any one disk in a domain has rawio='yes', rawio capability will be enabled for all disks in the domain (because, in the case of QEMU, this @@ -2884,7 +2884,7 @@ <pre> ... <devices> - <hostdev mode='subsystem' type='scsi'> + <hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host0'/> <address type='scsi' bus='0' target='0' unit='0'/> @@ -2943,7 +2943,13 @@ (<span class="since">since 1.0.6</span>) attribute indicates whether the kernel will filter unprivileged SG_IO commands for the disk, valid settings are "filtered" or "unfiltered". - The default is "filtered". + The default is "filtered". The optional <code>rawio</code> + (<span class="since">since 1.2.9</span>) attribute indicates + whether the lun needs the rawio capability. Valid settings are + "yes" or "no". See the rawio description within the + <a href="#elementsDisks">disk</a> section. + If a disk lun in the domain already has the rawio capability, + then this setting not required. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cedceae..84f0b28 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1224,12 +1224,7 @@ </choice> </attribute> <optional> - <attribute name="rawio"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <ref name="rawIO"/> </optional> <optional> <attribute name="sgio"> @@ -3608,6 +3603,9 @@ </choice> </attribute> </optional> + <optional> + <ref name="rawIO"/> + </optional> <element name="source"> <choice> <group> <!-- scsi_host adapter --> @@ -4972,4 +4970,12 @@ </optional> </element> </define> + <define name="rawIO"> + <attribute name="rawio"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </define> </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aac78a6..c1f23bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4482,6 +4482,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, xmlNodePtr sourcenode; char *managed = NULL; char *sgio = NULL; + char *rawio = NULL; char *backendStr = NULL; int backend; int ret = -1; @@ -4499,6 +4500,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } sgio = virXMLPropString(node, "sgio"); + rawio = virXMLPropString(node, "rawio"); /* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -4550,6 +4552,26 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } } + if (rawio) { + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("rawio is only supported for scsi host device")); + goto error; + } + + scsisrc->rawio_specified = true; + if (STREQ(rawio, "yes")) { + scsisrc->rawio = 1; + } else if (STREQ(rawio, "no")) { + scsisrc->rawio = 0; + } else { + virReportError(VIR_ERR_XML_ERROR, + _("unknown hostdev rawio setting '%s'"), + rawio); + goto error; + } + } + switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0) @@ -4589,6 +4611,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, error: VIR_FREE(managed); VIR_FREE(sgio); + VIR_FREE(rawio); VIR_FREE(backendStr); return ret; } @@ -17639,6 +17662,14 @@ virDomainHostdevDefFormat(virBufferPtr buf, scsisrc->sgio) virBufferAsprintf(buf, " sgio='%s'", virDomainDeviceSGIOTypeToString(scsisrc->sgio)); + + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->rawio_specified) { + if (scsisrc->rawio) + virBufferAddLit(buf, " rawio='yes'"); + else + virBufferAddLit(buf, " rawio='no'"); + } } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml new file mode 100644 index 0000000..69fdde3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes' sgio='unfiltered' rawio='yes'> + <source> + <adapter name='scsi_host0'/> + <address bus='0' target='0' unit='0'/> + </source> + <address type='drive' controller='0' bus='0' target='4' unit='8'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 03c05da..cca8d0a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -367,6 +367,7 @@ mymain(void) DO_TEST("disk-copy_on_read"); DO_TEST("hostdev-scsi-shareable"); DO_TEST("hostdev-scsi-sgio"); + DO_TEST("hostdev-scsi-rawio"); DO_TEST_DIFFERENT("hostdev-scsi-autogen-address"); -- 1.9.3
On 10.09.2014 01:40, John Ferlan wrote:
Add a "rawio" to the hostdev XML and process it mimicing the disk XML for a lun which supports/requires rawio
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 12 ++++++-- docs/schemas/domaincommon.rng | 18 +++++++---- src/conf/domain_conf.c | 31 +++++++++++++++++++ .../qemuxml2argv-hostdev-scsi-rawio.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..07640af 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1774,7 +1774,7 @@ <dt><code>rawio</code> attribute <span class="since">since 0.9.10</span></dt> <dd> - Indicates whether the disk is needs rawio capability; valid + Indicates whether the disk needs rawio capability. Valid settings are "yes" or "no" (default is "no"). If any one disk in a domain has rawio='yes', rawio capability will be enabled for all disks in the domain (because, in the case of QEMU, this @@ -2884,7 +2884,7 @@ <pre> ... <devices> - <hostdev mode='subsystem' type='scsi'> + <hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host0'/> <address type='scsi' bus='0' target='0' unit='0'/> @@ -2943,7 +2943,13 @@ (<span class="since">since 1.0.6</span>) attribute indicates whether the kernel will filter unprivileged SG_IO commands for the disk, valid settings are "filtered" or "unfiltered". - The default is "filtered". + The default is "filtered". The optional <code>rawio</code> + (<span class="since">since 1.2.9</span>) attribute indicates + whether the lun needs the rawio capability. Valid settings are + "yes" or "no". See the rawio description within the + <a href="#elementsDisks">disk</a> section. + If a disk lun in the domain already has the rawio capability, + then this setting not required. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cedceae..84f0b28 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1224,12 +1224,7 @@ </choice> </attribute> <optional> - <attribute name="rawio"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <ref name="rawIO"/> </optional> <optional> <attribute name="sgio"> @@ -3608,6 +3603,9 @@ </choice> </attribute> </optional> + <optional> + <ref name="rawIO"/> + </optional> <element name="source"> <choice> <group> <!-- scsi_host adapter --> @@ -4972,4 +4970,12 @@ </optional> </element> </define> + <define name="rawIO"> + <attribute name="rawio"> + <choice> + <value>yes</value> + <value>no</value> + </choice>
or instead of enumerating the possible values just: <ref name="virYesNo"/>
+ </attribute> + </define> </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aac78a6..c1f23bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4482,6 +4482,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, xmlNodePtr sourcenode; char *managed = NULL; char *sgio = NULL; + char *rawio = NULL; char *backendStr = NULL; int backend; int ret = -1; @@ -4499,6 +4500,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, }
sgio = virXMLPropString(node, "sgio"); + rawio = virXMLPropString(node, "rawio");
/* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -4550,6 +4552,26 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } }
+ if (rawio) { + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("rawio is only supported for scsi host device")); + goto error; + } + + scsisrc->rawio_specified = true; + if (STREQ(rawio, "yes")) { + scsisrc->rawio = 1; + } else if (STREQ(rawio, "no")) { + scsisrc->rawio = 0; + } else { + virReportError(VIR_ERR_XML_ERROR, + _("unknown hostdev rawio setting '%s'"), + rawio); + goto error; + }
You can save a few lines if you use the virTristateBoolTypeFromString().
+ } + switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0) @@ -4589,6 +4611,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, error: VIR_FREE(managed); VIR_FREE(sgio); + VIR_FREE(rawio); VIR_FREE(backendStr); return ret; } @@ -17639,6 +17662,14 @@ virDomainHostdevDefFormat(virBufferPtr buf, scsisrc->sgio) virBufferAsprintf(buf, " sgio='%s'", virDomainDeviceSGIOTypeToString(scsisrc->sgio)); + + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->rawio_specified) { + if (scsisrc->rawio) + virBufferAddLit(buf, " rawio='yes'"); + else + virBufferAddLit(buf, " rawio='no'"); + }
and virTristateBoolFormat()
} virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml new file mode 100644 index 0000000..69fdde3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes' sgio='unfiltered' rawio='yes'> + <source> + <adapter name='scsi_host0'/> + <address bus='0' target='0' unit='0'/> + </source> + <address type='drive' controller='0' bus='0' target='4' unit='8'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 03c05da..cca8d0a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -367,6 +367,7 @@ mymain(void) DO_TEST("disk-copy_on_read"); DO_TEST("hostdev-scsi-shareable"); DO_TEST("hostdev-scsi-sgio"); + DO_TEST("hostdev-scsi-rawio");
DO_TEST_DIFFERENT("hostdev-scsi-autogen-address");
Michal
On 09/09/2014 07:40 PM, John Ferlan wrote:
The "rawio" setting for a <device.../> <disk.../> lun isn't duplicated in the scsi_host hostdev device for a lun. Rather than "requiring" some disk in the disk list to add the rawio, add the "rawio" property to the scsi_host hostdev lun.
John Ferlan (3): hostdev: Add "rawio" attribute to _virDomainHostdevSubsysSCSI qemu: Process the hostdev "rawio" setting domain_conf: Process the "rawio" for a hostdev device
docs/formatdomain.html.in | 12 ++++++-- docs/schemas/domaincommon.rng | 18 +++++++---- src/conf/domain_conf.c | 31 +++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c | 20 ++++++++++++- .../qemuxml2argv-hostdev-scsi-rawio.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml
ping? tks John
participants (3)
-
John Ferlan -
Michal Privoznik -
Roman Bogorodskiy