[libvirt] [PATCH v2 0/3] Be more selective when determining cdrom for taint messaging
v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00103.html Changes since v1: Split into 3 parts... The first patch would be the bare minimum using STRPREFIX instead of STREQ type comparisons for the incoming path to be "/dev/cdrom[N]" or "/dev/srN" (or resolved to that). This would "work" for the most part, but then since it's possible to make even more checks let's check against the collected node device data. Patch 2 therefore will "tag" the already collected cdrom data with a capability. This allows patch3 to find any/all CDROM's on the host and compare the resolved path to that list of devices returning "true" if something matches a node device declared physical CDROM. I split things up mainly to make it easier to decide whether patch 1 is sufficient or not. If patch2 and patch3 are OK, I would also add a release note indicating the improvement to find CDROM by node device capability. It's a separate "improvement" on it's own as well. Whether it's truly useful or not, is a different question... John Ferlan (3): qemu: Be more selective when determining cdrom for taint messaging nodedev: Add capability bit to detect 'cdrom' devices qemu: Add inquiry to nodedev for cdrom taint checking include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.c | 6 ++- src/conf/node_device_conf.h | 5 ++- src/conf/virnodedeviceobj.c | 21 ++++++++-- src/node_device/node_device_driver.c | 1 + src/node_device/node_device_udev.c | 2 + src/qemu/qemu_domain.c | 78 +++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_process.c | 2 +- tools/virsh-nodedev.c | 3 ++ tools/virsh.pod | 2 +- 12 files changed, 116 insertions(+), 11 deletions(-) -- 2.9.5
https://bugzilla.redhat.com/show_bug.cgi?id=1471225 Commit id '99a2d6af2' was a bit too aggressive with determining whether the provided path was a "physical" cd-rom in order to generate a taint message due to the possibility of some guest and host trying to control the tray. For cd-rom guest devices backed to some VIR_STORAGE_TYPE_FILE storage, this wouldn't be a problem and as such it shouldn't be a problem for guest devices using some sort of block device on the host such as iSCSI, LVM, or a Disk pool would present. So before issuing a taint message, let's check if the provided path of the VIR_STORAGE_TYPE_BLOCK backed device is a "known" physical cdrom name by comparing the beginning of the path w/ "/dev/cdrom" and "/dev/sr". Also since it's possible the provided path could resolve to some /dev/srN device, let's get that path as well and perform the same check. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7203189..1b0c778 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4722,6 +4722,35 @@ qemuDomainDefFormatLive(virQEMUDriverPtr driver, } +/* qemuDomainFilePathIsHostCDROM + * @path: Supplied path. + * + * Determine if the path is a host CD-ROM path. Typically this is + * either /dev/cdrom[n] or /dev/srN, so those are easy checks, but + * it's also possible that @path resolves to /dev/srN, so check for + * those conditions on @path in order to emit the tainted message. + * + * Returns true if the path is a CDROM, false otherwise or on error. + */ +static bool +qemuDomainFilePathIsHostCDROM(const char *path) +{ + bool ret = false; + char *linkpath = NULL; + + if (virFileResolveLink(path, &linkpath) < 0) + goto cleanup; + + if (STRPREFIX(path, "/dev/cdrom") || STRPREFIX(path, "/dev/sr") || + STRPREFIX(linkpath, "/dev/sr")) + ret = true; + + cleanup: + VIR_FREE(linkpath); + return ret; +} + + void qemuDomainObjTaint(virQEMUDriverPtr driver, virDomainObjPtr obj, virDomainTaintFlags taint, @@ -4840,7 +4869,7 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK && - disk->src->path) + disk->src->path && qemuDomainFilePathIsHostCDROM(disk->src->path)) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, logCtxt); -- 2.9.5
Add the capability to detect and list any/all cdrom devices Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.c | 6 +++++- src/conf/node_device_conf.h | 5 ++++- src/conf/virnodedeviceobj.c | 21 ++++++++++++++++++--- src/node_device/node_device_driver.c | 1 + src/node_device/node_device_udev.c | 2 ++ tools/virsh-nodedev.c | 3 +++ tools/virsh.pod | 2 +- 8 files changed, 35 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index 25e8724..140c8f2 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -82,6 +82,7 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES = 1 << 13, /* Capable of mediated devices */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV = 1 << 14, /* Mediated device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV = 1 << 15, /* CCW device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CDROM = 1 << 16, /* CDROM capable device */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bf84fd2..54f36a3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -63,7 +63,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "drm", "mdev_types", "mdev", - "ccw") + "ccw", + "cdrom") VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -603,6 +604,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: break; } @@ -1895,6 +1897,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_SCSI_GENERIC: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown capability type '%d' for '%s'"), @@ -2223,6 +2226,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_CCW_DEV: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: /* This case is here to shutup the compiler */ break; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index da56eaf..b5e1f16 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -67,6 +67,7 @@ typedef enum { VIR_NODE_DEV_CAP_MDEV_TYPES, /* Device capable of mediated devices */ VIR_NODE_DEV_CAP_MDEV, /* Mediated device */ VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */ + VIR_NODE_DEV_CAP_CDROM, /* CDROM capable device */ VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -85,6 +86,7 @@ typedef enum { VIR_NODE_DEV_CAP_STORAGE_REMOVABLE = (1 << 0), VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE = (1 << 1), VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE = (1 << 2), + VIR_NODE_DEV_CAP_STORAGE_CDROM = (1 << 3), } virNodeDevStorageCapFlags; typedef enum { @@ -377,7 +379,8 @@ virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type); VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CDROM) char * virNodeDeviceGetParentName(virConnectPtr conn, diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index b0dcee1..e3e2078 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -131,6 +131,8 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS); const char *mdev_types = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES); + const char *cdrom_types = + virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_CDROM); while (caps) { if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) { @@ -151,13 +153,18 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, return 1; break; + case VIR_NODE_DEV_CAP_STORAGE: + if ((STREQ(cap, cdrom_types) && + (caps->data.storage.flags & VIR_NODE_DEV_CAP_STORAGE_CDROM))) + return 1; + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: case VIR_NODE_DEV_CAP_NET: case VIR_NODE_DEV_CAP_SCSI_TARGET: case VIR_NODE_DEV_CAP_SCSI: - case VIR_NODE_DEV_CAP_STORAGE: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_SCSI_GENERIC: @@ -165,6 +172,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: break; } @@ -707,13 +715,18 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr obj, return true; break; + case VIR_NODE_DEV_CAP_STORAGE: + if (type == VIR_NODE_DEV_CAP_CDROM && + (cap->data.storage.flags & VIR_NODE_DEV_CAP_STORAGE_CDROM)) + return true; + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: case VIR_NODE_DEV_CAP_NET: case VIR_NODE_DEV_CAP_SCSI_TARGET: case VIR_NODE_DEV_CAP_SCSI: - case VIR_NODE_DEV_CAP_STORAGE: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_SCSI_GENERIC: @@ -721,6 +734,7 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr obj, case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: break; } @@ -867,7 +881,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr obj, MATCH(DRM) || MATCH(MDEV_TYPES) || MATCH(MDEV) || - MATCH(CCW_DEV))) + MATCH(CCW_DEV) || + MATCH(CDROM))) return false; } diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index facfeb6..adae917 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -90,6 +90,7 @@ nodeDeviceUpdateCaps(virNodeDeviceDefPtr def) case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index f417745..bebfec4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -920,6 +920,7 @@ udevProcessCDROM(struct udev_device *device, VIR_FREE(def->caps->data.storage.drive_type); if (VIR_STRDUP(def->caps->data.storage.drive_type, "cdrom") < 0) return -1; + def->caps->data.storage.flags |= VIR_NODE_DEV_CAP_STORAGE_CDROM; if (udevHasDeviceProperty(device, "ID_CDROM_MEDIA") && udevGetIntProperty(device, "ID_CDROM_MEDIA", &has_media, 0) < 0) @@ -1303,6 +1304,7 @@ udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_CDROM: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index c7ef6bf..ba1592b 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -463,6 +463,9 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_NODE_DEV_CAP_CCW_DEV: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV; break; + case VIR_NODE_DEV_CAP_CDROM: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CDROM; + break; case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 01453be..2ba7762 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3243,7 +3243,7 @@ I<cap> is used to filter the list by capability types, the types must be separated by comma, e.g. --cap pci,scsi. Valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', 'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic', 'drm', 'mdev', -'mdev_types', 'ccw'. +'mdev_types', 'ccw', 'cdrom'. If I<--tree> is used, the output is formatted in a tree representing parents of each node. I<cap> and I<--tree> are mutually exclusive. -- 2.9.5
https://bugzilla.redhat.com/show_bug.cgi?id=1471225 Since we can now easily determine from the node device driver which devices are physical cdrom devices, let's make in inquiry of the node device driver to get a list of all cdrom devices and compare the input resolved path to that list to ensure we haven't missed anything. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 53 +++++++++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_process.c | 2 +- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b0c778..a1302a5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -41,6 +41,7 @@ #include "virfile.h" #include "domain_addr.h" #include "domain_event.h" +#include "node_device_conf.h" #include "virtime.h" #include "virnetdevopenvswitch.h" #include "virstoragefile.h" @@ -4723,6 +4724,7 @@ qemuDomainDefFormatLive(virQEMUDriverPtr driver, /* qemuDomainFilePathIsHostCDROM + * @conn: A virConnectPtr * @path: Supplied path. * * Determine if the path is a host CD-ROM path. Typically this is @@ -4730,23 +4732,64 @@ qemuDomainDefFormatLive(virQEMUDriverPtr driver, * it's also possible that @path resolves to /dev/srN, so check for * those conditions on @path in order to emit the tainted message. * + * If that doesn't work, then let's check with the nodedev driver to + * get a list of all cdrom's on the host and then compare the resolved + * linkpath for @path to each cdrom in the list looking for a match. + * * Returns true if the path is a CDROM, false otherwise or on error. */ static bool -qemuDomainFilePathIsHostCDROM(const char *path) +qemuDomainFilePathIsHostCDROM(virConnectPtr conn, + const char *path) { bool ret = false; char *linkpath = NULL; + int ndevices = 0; + virNodeDevicePtr *devices = NULL; + unsigned int flags = VIR_CONNECT_LIST_NODE_DEVICES_CAP_CDROM; + size_t i; + char *xml = NULL; + virNodeDeviceDefPtr def = NULL; if (virFileResolveLink(path, &linkpath) < 0) goto cleanup; if (STRPREFIX(path, "/dev/cdrom") || STRPREFIX(path, "/dev/sr") || - STRPREFIX(linkpath, "/dev/sr")) + STRPREFIX(linkpath, "/dev/sr")) { ret = true; + goto cleanup; + } + + /* Get a list of all 'cdrom' devices from NodeDevice and search + * through the list looking to compare the resolved @linkpath + * to list of host console device(s). */ + if (conn && + (ndevices = virConnectListAllNodeDevices(conn, &devices, flags) > 0)) { + for (i = 0; i < ndevices; i++) { + if (!(xml = virNodeDeviceGetXMLDesc(devices[i], 0))) + goto cleanup; + + if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL))) + goto cleanup; + VIR_FREE(xml); + + if (STREQ(def->caps->data.storage.block, linkpath)) { + ret = true; + goto cleanup; + } + + virNodeDeviceDefFree(def); + def = NULL; + } + } cleanup: VIR_FREE(linkpath); + virNodeDeviceDefFree(def); + VIR_FREE(xml); + for (i = 0; i < ndevices; i++) + virObjectUnref(devices[i]); + VIR_FREE(devices); return ret; } @@ -4809,6 +4852,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr obj, qemuDomainLogContextPtr logCtxt) { @@ -4835,7 +4879,7 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HOST_CPU, logCtxt); for (i = 0; i < obj->def->ndisks; i++) - qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logCtxt); + qemuDomainObjCheckDiskTaint(driver, conn, obj, obj->def->disks[i], logCtxt); for (i = 0; i < obj->def->nhostdevs; i++) qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i], @@ -4852,6 +4896,7 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr obj, virDomainDiskDefPtr disk, qemuDomainLogContextPtr logCtxt) @@ -4869,7 +4914,7 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK && - disk->src->path && qemuDomainFilePathIsHostCDROM(disk->src->path)) + disk->src->path && qemuDomainFilePathIsHostCDROM(conn, disk->src->path)) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, logCtxt); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5f6e361..2e0a949 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -552,9 +552,11 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, qemuDomainLogContextPtr logCtxt); void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr obj, qemuDomainLogContextPtr logCtxt); void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr obj, virDomainDiskDefPtr disk, qemuDomainLogContextPtr logCtxt); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6255d89..13d6ef7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7505,7 +7505,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + qemuDomainObjCheckDiskTaint(driver, conn, vm, dev->data.disk, NULL); ret = qemuDomainAttachDeviceDiskLive(conn, driver, vm, dev); if (!ret) { alias = dev->data.disk->info.alias; @@ -7784,7 +7784,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + qemuDomainObjCheckDiskTaint(driver, conn, vm, dev->data.disk, NULL); ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force); break; case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ab81d65..0aa3ebf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5557,7 +5557,7 @@ qemuProcessLaunch(virConnectPtr conn, qemuLogOperation(vm, "starting up", cmd, logCtxt); - qemuDomainObjCheckTaint(driver, vm, logCtxt); + qemuDomainObjCheckTaint(driver, conn, vm, logCtxt); qemuDomainLogContextMarkPosition(logCtxt); -- 2.9.5
On 09/11/2017 04:32 PM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00103.html
Changes since v1:
Split into 3 parts... The first patch would be the bare minimum using STRPREFIX instead of STREQ type comparisons for the incoming path to be "/dev/cdrom[N]" or "/dev/srN" (or resolved to that).
This would "work" for the most part, but then since it's possible to make even more checks let's check against the collected node device data. Patch 2 therefore will "tag" the already collected cdrom data with a capability. This allows patch3 to find any/all CDROM's on the host and compare the resolved path to that list of devices returning "true" if something matches a node device declared physical CDROM.
I split things up mainly to make it easier to decide whether patch 1 is sufficient or not. If patch2 and patch3 are OK, I would also add a release note indicating the improvement to find CDROM by node device capability. It's a separate "improvement" on it's own as well. Whether it's truly useful or not, is a different question...
[1]
John Ferlan (3): qemu: Be more selective when determining cdrom for taint messaging
ACK to this one ^^
nodedev: Add capability bit to detect 'cdrom' devices qemu: Add inquiry to nodedev for cdrom taint checking
However, these two ^^ look like an overkill to me. It's still just a taint message that nobody cares about. Or? 1: Yeah, I don't think we really need such a big hammer for tiny nail. But I might be missing something. Michal
On 09/18/2017 09:12 AM, Michal Privoznik wrote:
On 09/11/2017 04:32 PM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00103.html
Changes since v1:
Split into 3 parts... The first patch would be the bare minimum using STRPREFIX instead of STREQ type comparisons for the incoming path to be "/dev/cdrom[N]" or "/dev/srN" (or resolved to that).
This would "work" for the most part, but then since it's possible to make even more checks let's check against the collected node device data. Patch 2 therefore will "tag" the already collected cdrom data with a capability. This allows patch3 to find any/all CDROM's on the host and compare the resolved path to that list of devices returning "true" if something matches a node device declared physical CDROM.
I split things up mainly to make it easier to decide whether patch 1 is sufficient or not. If patch2 and patch3 are OK, I would also add a release note indicating the improvement to find CDROM by node device capability. It's a separate "improvement" on it's own as well. Whether it's truly useful or not, is a different question...
[1]
John Ferlan (3): qemu: Be more selective when determining cdrom for taint messaging
ACK to this one ^^
nodedev: Add capability bit to detect 'cdrom' devices qemu: Add inquiry to nodedev for cdrom taint checking
However, these two ^^ look like an overkill to me. It's still just a taint message that nobody cares about. Or? 1: Yeah, I don't think we really need such a big hammer for tiny nail. But I might be missing something.
Michal
I agree with you, but just in case someone wanted to use that sledge hammer in order to catch some really obscure corner condition, I figured I'd show it was possible... Still I can give it a few more days to see if someone indicates they would also like to see usage of the sledge hammer. John
participants (2)
-
John Ferlan -
Michal Privoznik