9:32 a.m.
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
9:32 a.m.
New subject: [libvirt] [PATCH v2 1/3] qemu: Be more selective when determining cdrom for taint messaging
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(a)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
9:32 a.m.
New subject: [libvirt] [PATCH v2 2/3] nodedev: Add capability bit to detect 'cdrom' devices
Add the capability to detect and list any/all cdrom devices
Signed-off-by: John Ferlan <jferlan(a)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
9:32 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: Add inquiry to nodedev for cdrom taint checking
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(a)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
8:12 a.m.
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
8:49 a.m.
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
2622
days inactive
2629
days old
5 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik