Setting "unpriv_sgio" to "yes" to enable the unprivleged SG_IO, and "no" to disable it. Later patch will do the actual setting. --- src/conf/domain_conf.c | 57 +++++++++++++++----- src/conf/domain_conf.h | 10 ++++ ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml | 32 +++++++++++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed8b53f..b25229a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -236,6 +236,10 @@ VIR_ENUM_IMPL(virDomainDiskIo, VIR_DOMAIN_DISK_IO_LAST, "default", "native", "threads") +VIR_ENUM_IMPL(virDomainDiskUnprivSGIO, VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST, + "default", + "yes", + "no") VIR_ENUM_IMPL(virDomainIoEventFd, VIR_DOMAIN_IO_EVENT_FD_LAST, "default", "on", @@ -3515,6 +3519,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *device = NULL; char *snapshot = NULL; char *rawio = NULL; + char *unpriv_sgio = NULL; char *driverName = NULL; char *driverType = NULL; char *source = NULL; @@ -3576,6 +3581,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, snapshot = virXMLPropString(node, "snapshot"); rawio = virXMLPropString(node, "rawio"); + unpriv_sgio = virXMLPropString(node, "unpriv_sgio"); cur = node->children; while (cur != NULL) { @@ -3966,24 +3972,45 @@ virDomainDiskDefParseXML(virCapsPtr caps, def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; } + if (rawio && unpriv_sgio) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("rawio and unpriv_sgio are exclusive")); + goto error; + } + + if ((rawio || unpriv_sgio) && + (def->device != VIR_DOMAIN_DISK_DEVICE_LUN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("rawio can be used only with device='lun'")); + goto error; + } + if (rawio) { def->rawio_specified = true; - if (def->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - if (STREQ(rawio, "yes")) { - def->rawio = 1; - } else if (STREQ(rawio, "no")) { - def->rawio = 0; - } else { - virReportError(VIR_ERR_XML_ERROR, - _("unknown disk rawio setting '%s'"), - rawio); - goto error; - } + if (STREQ(rawio, "yes")) { + def->rawio = 1; + } else if (STREQ(rawio, "no")) { + def->rawio = 0; } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("rawio can be used only with device='lun'")); + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk rawio setting '%s'"), + rawio); + goto error; + } + } + + if (unpriv_sgio) { + int unpriv_sgio_val = 0; + + if ((unpriv_sgio_val = + virDomainDiskUnprivSGIOTypeFromString(unpriv_sgio)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk unpriv_sgio setting '%s'"), + unpriv_sgio); goto error; } + + def->unpriv_sgio = unpriv_sgio_val; } if (bus) { @@ -4220,6 +4247,7 @@ cleanup: VIR_FREE(type); VIR_FREE(snapshot); VIR_FREE(rawio); + VIR_FREE(unpriv_sgio); VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); @@ -11897,6 +11925,9 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, " rawio='no'"); } } + if (def->unpriv_sgio) + virBufferAsprintf(buf, " unpriv_sgio='%s'", + virDomainDiskUnprivSGIOTypeToString(def->unpriv_sgio)); if (def->snapshot && !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && def->readonly)) virBufferAsprintf(buf, " snapshot='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c3e8c16..105fb7d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -496,6 +496,14 @@ enum virDomainDiskIo { VIR_DOMAIN_DISK_IO_LAST }; +enum virDomainDiskUnprivSGIO { + VIR_DOMAIN_DISK_UNPRIV_SGIO_DEFAULT = 0, + VIR_DOMAIN_DISK_UNPRIV_SGIO_YES, + VIR_DOMAIN_DISK_UNPRIV_SGIO_NO, + + VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST +}; + enum virDomainIoEventFd { VIR_DOMAIN_IO_EVENT_FD_DEFAULT = 0, VIR_DOMAIN_IO_EVENT_FD_ON, @@ -607,6 +615,7 @@ struct _virDomainDiskDef { virStorageEncryptionPtr encryption; bool rawio_specified; int rawio; /* no = 0, yes = 1 */ + int unpriv_sgio; /* no = 0, yes = 1 */ size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; @@ -2197,6 +2206,7 @@ VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) VIR_ENUM_DECL(virDomainDiskProtocol) VIR_ENUM_DECL(virDomainDiskIo) +VIR_ENUM_DECL(virDomainDiskUnprivSGIO) VIR_ENUM_DECL(virDomainDiskSecretType) VIR_ENUM_DECL(virDomainDiskTray) VIR_ENUM_DECL(virDomainIoEventFd) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml new file mode 100644 index 0000000..d8f9d12 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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='lun' unpriv_sgio='yes'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='lun' unpriv_sgio='no'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='1' unit='1'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='scsi' index='1' model='lsilogic'/> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1d366f1..9cb4d88 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -238,6 +238,7 @@ mymain(void) DO_TEST("seclabel-static"); DO_TEST("seclabel-none"); DO_TEST("numad-static-vcpu-no-numatune"); + DO_TEST("disk-scsi-lun-passthrough-upriv-sgio"); /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); -- 1.7.7.6

To record the original "unpriv_sgio" value, this introduces "old_unpriv_sgio" for disk def. When the domain is starting, the disk's "unpriv_sgio" is set with regards to the config in domain XML. And when the domain is being destroyed, it's restored to the original value ("old_unpriv_sgio") --- src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 105fb7d..1a8de71 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -616,6 +616,7 @@ struct _virDomainDiskDef { bool rawio_specified; int rawio; /* no = 0, yes = 1 */ int unpriv_sgio; /* no = 0, yes = 1 */ + int old_unpriv_sgio; /* To record the old unpriv_sgio value, internally */ size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d7a5a0..e48eed0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3703,6 +3703,26 @@ int qemuProcessStart(virConnectPtr conn, virCommandAllowCap(cmd, CAP_SYS_RAWIO); } + /* Set unpriv_sgio for disks */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + int old_unpriv_sgio; + + if (!disk->unpriv_sgio) + continue; + + if (virGetDeviceUnprivSGIO(disk->src, &old_unpriv_sgio) < 0) + goto cleanup; + + disk->old_unpriv_sgio = old_unpriv_sgio; + + if (virSetDeviceUnprivSGIO(disk->src, + (disk->unpriv_sgio == + VIR_DOMAIN_DISK_UNPRIV_SGIO_YES) + ? 1 : 0) < 0) + goto cleanup; + } + virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetOutputFD(cmd, &logfile); @@ -4093,6 +4113,17 @@ void qemuProcessStop(struct qemud_driver *driver, flags & VIR_QEMU_PROCESS_STOP_MIGRATED); virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + /* Restore disk's unpriv_sgio */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (!disk->unpriv_sgio) + continue; + + if (virSetDeviceUnprivSGIO(disk->src, disk->old_unpriv_sgio) < 0) + VIR_WARN("Unable to restore unpriv_sgio for disk '%s'", disk->src); + } + /* Clear out dynamically assigned labels */ for (i = 0; i < vm->def->nseclabels; i++) { if (vm->def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { -- 1.7.7.6

This prevents the domain starting if the shared disk's setting conflicts with other active domain(s), E.g. A domain with unpriv_sgio set as "yes", however, another active domain is using it set as "no". --- src/conf/domain_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 14 +++++++++++++- 4 files changed, 62 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02df96e..fb7bb1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15763,3 +15763,48 @@ virDomainDiskIsUsed(const virDomainObjList doms, return false; } + +static int +virDomainSharedDiskCompare(const void *payload, + const void *name ATTRIBUTE_UNUSED, + const void *opaque) +{ + virDomainObjPtr obj = (virDomainObjPtr)payload; + const virDomainDiskDefPtr disk = (virDomainDiskDefPtr)opaque; + int i; + + virDomainObjLock(obj); + + /* Ingores the inactive ones */ + if (!virDomainObjIsActive(obj)) + return 0; + + for (i = 0; i < obj->def->ndisks; i++) { + if (STRNEQ(obj->def->disks[i]->src, disk->src)) + continue; + + if (obj->def->disks[i]->unpriv_sgio != disk->unpriv_sgio) { + virDomainObjUnlock(obj); + return 1; + } + } + + virDomainObjUnlock(obj); + return 0; +} + +/* Validate if the shared disk conf is conflicted with other domains, + * currently only validates the unpriv_sgio setting + */ +bool +virDomainSharedDiskValidate(const virDomainObjList doms, + const virDomainDiskDefPtr disk) +{ + virDomainObjPtr obj; + + obj = virHashSearch(doms.objs, virDomainSharedDiskCompare, disk); + if (obj) + return false; + + return true; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3105e05..35f46d2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1878,6 +1878,9 @@ bool virDomainDiskIsUsed(const virDomainObjList doms, char *name, char *diskSrc, bool active); +bool +virDomainSharedDiskValidate(const virDomainObjList doms, + const virDomainDiskDefPtr disk); bool virDomainObjTaint(virDomainObjPtr obj, enum virDomainTaintFlags taint); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b9019b7..b14eb40 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -489,6 +489,7 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainSharedDiskValidate; virDomainShutdownReasonTypeFromString; virDomainShutdownReasonTypeToString; virDomainShutoffReasonTypeFromString; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eeaaea0..b5676e0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3711,6 +3711,18 @@ int qemuProcessStart(virConnectPtr conn, if (!disk->unpriv_sgio) continue; + /* Error out if the disk is shared, but the unpriv_sgio conflicts + * with other active domain(s). + */ + if (disk->shared && + !virDomainSharedDiskValidate(driver->domains, + disk)) { + virReportError(VIR_ERR_XML_ERROR, + _("Setting of shared disk '%s' conflicts with " + "other active domains"), disk->src); + goto cleanup; + } + if (virGetDeviceUnprivSGIO(disk->src, &old_unpriv_sgio) < 0) goto cleanup; @@ -3719,7 +3731,7 @@ int qemuProcessStart(virConnectPtr conn, if (virSetDeviceUnprivSGIO(disk->src, (disk->unpriv_sgio == VIR_DOMAIN_DISK_UNPRIV_SGIO_YES) - ? 1 : 0) < 0) + ? 1 : 0) < 0) goto cleanup; } -- 1.7.7.6