[libvirt] [PATCH 0/6 v2] Unprivileged SG_IO support
Hi, As a result of RFC [1], this implements the unprivleged SG_IO support. Though it is a bit rushed, and I still have no chance to test it yet, but I'd like see an earlier reviewing. @Paolo, please let me known once you have a kernel build with the new sysfs knob supported. Thanks! Osier Yang (6): unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio unpriv_sgio: Parse and format the new XML unpriv_sgio: Prepare helpers for unpriv_sgio setting unpriv_sgio: Manage unpriv_sgio in domain's lifecyle unpriv_sgio: Do not restore unpriv_sgio if the disk is being used unpriv_sgio: Error out if the unpriv_sgio setting conflicts with others docs/formatdomain.html.in | 10 +- docs/schemas/domaincommon.rng | 52 ++++-- src/conf/domain_conf.c | 168 ++++++++++++++++++-- src/conf/domain_conf.h | 18 ++ src/libvirt_private.syms | 6 + src/qemu/qemu_process.c | 54 +++++++ src/util/util.c | 154 ++++++++++++++++++ src/util/util.h | 7 + ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml | 32 ++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 472 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml [1] https://www.redhat.com/archives/libvir-list/2012-November/msg00988.html Regards, Osier
Since "rawio" and "unpriv_sgio" are only valid for "lun", this groups them together. And since both of them intend to allow the unprivledged user to use the SCSI commands, they are must be exclusive. Actually "unpriv_sgio" supersedes "rawio", as it confines the capability per-device, unlike "rawio", which gives the domain process broad capablity. --- docs/formatdomain.html.in | 10 +++++++- docs/schemas/domaincommon.rng | 52 ++++++++++++++++++++++++++++------------ 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6a3b976..f3f6a9e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1395,7 +1395,15 @@ rawio='yes', rawio capability will be enabled for all disks in the domain (because, in the case of QEMU, this capability can only be set on a per-process basis). This attribute is only - valid when device is "lun". + valid when device is "lun". NB, <code>rawio</code> gives + the domain process broad capability, to confined the capability + as much as possible, one should use <code>unpriv_sgio</code> + instead, which controls the capability per-device. + The optional <code>unpriv_sgio</code> attribute + (<span class="since">since 1.0.1</span>) indicates whether the + disk will allow unprivileged SG_IO, valid settings are "yes" + or "no" (defaults to "no"). Note that it's exclusive with + attribute <code>rawio</code>; The optional <code>snapshot</code> attribute indicates the default behavior of the disk during disk snapshots: "internal" requires a file format such as qcow2 that can store both the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 02ad477..7da571c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -957,24 +957,44 @@ --> <define name="disk"> <element name="disk"> - <optional> - <attribute name="device"> - <choice> - <value>floppy</value> - <value>disk</value> - <value>cdrom</value> - <value>lun</value> - </choice> - </attribute> - </optional> - <optional> - <attribute name="rawio"> + <choice> + <group> + <optional> + <attribute name="device"> + <choice> + <value>floppy</value> + <value>disk</value> + <value>cdrom</value> + </choice> + </attribute> + </optional> + </group> + <group> + <optional> + <attribute name="device"> + <value>lun</value> + </attribute> + </optional> <choice> - <value>yes</value> - <value>no</value> + <optional> + <attribute name="rawio"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="unpriv_sgio"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> </choice> - </attribute> - </optional> + </group> + </choice> <optional> <ref name="snapshot"/> </optional> -- 1.7.7.6
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
"virGetDevice{Major,Minor}" could be used across the sources, but it doesn't relate with this series, and could be done later. * src/util/util.h: (Declare virGetDevice{Major,Minor}, and vir{Get,Set}DeviceUnprivSGIO) * src/util/util.c: (Implement virGetDevice{Major,Minor} and vir{Get,Set}DeviceUnprivSGIO) * src/libvirt_private.syms: Export private symbols of upper helpers --- src/libvirt_private.syms | 4 + src/util/util.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 7 ++ 3 files changed, 165 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0115db1..c756130 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1257,6 +1257,9 @@ virFileWaitForDevices; virFileWriteStr; virFindFileInPath; virFormatIntDecimal; +virGetDeviceMajor; +virGetDeviceMinor; +virGetDeviceUnprivSGIO; virGetGroupID; virGetGroupName; virGetHostname; @@ -1275,6 +1278,7 @@ virPipeReadUntilEOF; virScaleInteger; virSetBlocking; virSetCloseExec; +virSetDeviceUnprivSGIO; virSetInherit; virSetNonBlock; virSetUIDGID; diff --git a/src/util/util.c b/src/util/util.c index 2fd0f2c..49619d0 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3113,3 +3113,157 @@ virValidateWWN(const char *wwn) { return true; } + +#if defined(major) && defined(minor) +int +virGetDeviceMajor(const char *path) +{ + struct stat sb; + + if (stat(path, &sb) < 0) + return -errno; + + if (!S_ISBLK(sb.st_mode)) + return -EINVAL; + + return major(sb.st_rdev); +} + +int +virGetDeviceMinor(const char *path) +{ + struct stat sb; + + if (stat(path, &sb) < 0) + return -errno; + + if (!S_ISBLK(sb.st_mode)) + return -EINVAL; + + return minor(sb.st_rdev); +} +#else +int +virGetDeviceMajor(const char *path) +{ + return -ENOSYS; +} + +int +virGetDeviceMinor(const char *path) +{ + return -ENOSYS; +} +#endif + +int +virSetDeviceUnprivSGIO(const char *path, + int unpriv_sgio) +{ + char *sysfs_path = NULL; + char *val = NULL; + int major; + int minor; + int ret = -1; + int rc = -1; + + if ((major = virGetDeviceMajor(path)) < 0) { + virReportSystemError(-major, + _("Unable to get major number of device '%s'"), + path); + return -1; + } + + if ((minor = virGetDeviceMinor(path)) < 0) { + virReportSystemError(-minor, + _("Unable to get minor number of device '%s'"), + path); + return -1; + } + + if (virAsprintf(&sysfs_path, "/sys/dev/block/%d:%d/queue/unpriv_sgio", + major, minor) < 0) { + virReportOOMError(); + return -1; + } + + if (!virFileExists(sysfs_path)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("unpriv_sgio is not supported by this kernel")); + goto cleanup; + } + + if (virAsprintf(&val, "%d", unpriv_sgio) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) { + virReportSystemError(-rc, _("failed to set %s"), sysfs_path); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(val); + return ret; +} + +int +virGetDeviceUnprivSGIO(const char *path, + int *unpriv_sgio) +{ + char *sysfs_path = NULL; + char *buf = NULL; + char *tmp = NULL; + int major; + int minor; + int ret = -1; + int rc = -1; + + if ((major = virGetDeviceMajor(path)) < 0) { + virReportSystemError(-major, + _("Unable to get major number of device '%s'"), + path); + return -1; + } + + if ((minor = virGetDeviceMinor(path)) < 0) { + virReportSystemError(-minor, + _("Unable to get minor number of device '%s'"), + path); + return -1; + } + + if (virAsprintf(&sysfs_path, "/sys/dev/block/%d:%d/queue/unpriv_sgio", + major, minor) < 0) { + virReportOOMError(); + return -1; + } + + if (!virFileExists(sysfs_path)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("unpriv_sgio is not supported by this kernel")); + goto cleanup; + } + + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup; + + if ((tmp = strchr(buf, '\n'))) + *tmp = '\0'; + + if ((rc = virStrToLong_i(buf, NULL, 10, + unpriv_sgio)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse value of %s"), sysfs_path); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return ret; +} diff --git a/src/util/util.h b/src/util/util.h index 4316ab1..41146c1 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -280,4 +280,11 @@ bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); bool virValidateWWN(const char *wwn); +int virGetDeviceMajor(const char *path); +int virGetDeviceMinor(const char *path); +int virSetDeviceUnprivSGIO(const char *path, + int unpriv_sgio); +int virGetDeviceUnprivSGIO(const char *path, + int *unpriv_sgio); + #endif /* __VIR_UTIL_H__ */ -- 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 restoring the unpriv_sgio if the disk is shared, and is being used by other active domain. Because we don't want to fall into the corruption situation. * src/conf/domain_conf.h (Declare virDomainDiskIsUsed, which is to detect if a disk is using by domain) * src/conf/domain_conf.c (Implement virDomainDiskIsUsed) * src/libvirt_private.syms: (Export virDomainDIskIsUsed) * src/qemu/qemu_process.c (Don't restore unpriv_sgio if the disk is shared, and is being used by other active domain). --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 11 +++++++ 4 files changed, 82 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b25229a..02df96e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15697,3 +15697,69 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model) return seclabel; } + +struct virDomainDiskIsUsedData { + char *diskSrc; + char *name; /* Want to exclude some domain? */ + bool active; /* Want to only iterate the active domains? */ +}; + +static int +virDomainObjListSearchDiskPath(const void *payload, + const void *name ATTRIBUTE_UNUSED, + const void *opaque) +{ + virDomainObjPtr obj = (virDomainObjPtr)payload; + const struct virDomainDiskIsUsedData *data = opaque; + int i; + int ret = 0; + + virDomainObjLock(obj); + + if (data->active && + !virDomainObjIsActive(obj)) + goto cleanup; + + if (data->name && + STREQ(data->name, obj->def->name)) + goto cleanup; + + for (i = 0; i < obj->def->ndisks; i++) { + virDomainDiskDefPtr disk = obj->def->disks[i]; + + if (STREQ(disk->src, data->diskSrc)) { + ret = 1; + goto cleanup; + } + } + +cleanup: + virDomainObjUnlock(obj); + return ret; +} + +/** + * virDomainDiskIsUsed: + * @doms: List of domain objects + * @name: The domain name want to exclude + * @diskSrc: The disk path + * @active: Whether to exclude inactive domains + * + * Returns true if the disk is being used. Otherwise returns false. + */ +bool +virDomainDiskIsUsed(const virDomainObjList doms, + char *name, + char *diskSrc, + bool active) +{ + virDomainObjPtr obj; + + struct virDomainDiskIsUsedData data = { diskSrc, name, active }; + + obj = virHashSearch(doms.objs, virDomainObjListSearchDiskPath, &data); + if (obj) + return true; + + return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1a8de71..3105e05 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1874,6 +1874,10 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, const unsigned char *uuid); virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, const char *name); +bool virDomainDiskIsUsed(const virDomainObjList doms, + char *name, + char *diskSrc, + bool active); bool virDomainObjTaint(virDomainObjPtr obj, enum virDomainTaintFlags taint); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c756130..b9019b7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -319,6 +319,7 @@ virDomainDefFormatInternal; virDomainDefFree; virDomainDefGetSecurityLabelDef; virDomainDiskDefGetSecurityLabelDef; +virDomainDiskIsUsed; virDomainDefAddSecurityLabelDef; virDomainDefParseFile; virDomainDefParseNode; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e48eed0..eeaaea0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4120,6 +4120,17 @@ void qemuProcessStop(struct qemud_driver *driver, if (!disk->unpriv_sgio) continue; + /* Don't try to restore the unpriv_sgio if the disk is shared + * by other active domain(s). We don't want to fall into the + * corruptions. + */ + if (disk->shared && + virDomainDiskIsUsed(driver->domains, + vm->def->name, + disk->src, + true)) + continue; + if (virSetDeviceUnprivSGIO(disk->src, disk->old_unpriv_sgio) < 0) VIR_WARN("Unable to restore unpriv_sgio for disk '%s'", disk->src); } -- 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
participants (1)
-
Osier Yang