4:33 p.m.
On 05/02/2016 10:32 AM, Peter Krempa wrote:
For disks sources described by a libvirt volume we don't need to
do a
complicated check since virStorageTranslateDiskSourcePool already
correctly determines the actual disk type.
Replace the checks using a new accessor that does not open-code the
whole logic.
---
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 ++-
src/qemu/qemu_conf.c | 10 +++++++---
src/util/virstoragefile.c | 16 +++++++++++++++-
src/util/virstoragefile.h | 3 ++-
5 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5030ec3..73c9fdb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2277,6 +2277,7 @@ virStorageSourceFree;
virStorageSourceGetActualType;
virStorageSourceGetSecurityLabelDef;
virStorageSourceInitChainElement;
+virStorageSourceIsBlockLocal;
virStorageSourceIsEmpty;
virStorageSourceIsLocalStorage;
virStorageSourceNewFromBacking;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 4afe7e2..ea86d36 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -393,7 +393,8 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
VIR_DEBUG("Allowing any disk block devs");
for (i = 0; i < def->ndisks; i++) {
- if (!virDomainDiskSourceIsBlockType(def->disks[i]->src, false))
+ if (virStorageSourceIsEmpty(def->disks[i]->src) ||
+ !virStorageSourceIsBlockLocal(def->disks[i]->src))
Could an LXC domain use a storage pool for a volume? If so, then
somewhere in the LXC code wouldn't we need to translate the source pool
in order to get "actualtype" and the volume check...
continue;
if (virCgroupAllowDevicePath(cgroup,
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5ed5776..e00ddca 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1243,7 +1243,9 @@ qemuAddSharedDisk(virQEMUDriverPtr driver,
char *key = NULL;
int ret = -1;
- if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src,
false))
+ if (virStorageSourceIsEmpty(disk->src) ||
+ !disk->src->shared ||
+ !virStorageSourceIsBlockLocal(disk->src))
return 0;
qemuDriverLock(driver);
@@ -1388,7 +1390,9 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver,
char *key = NULL;
int ret = -1;
- if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src,
false))
+ if (virStorageSourceIsEmpty(disk->src) ||
+ !disk->src->shared ||
+ !virStorageSourceIsBlockLocal(disk->src))
return 0;
qemuDriverLock(driver);
@@ -1476,7 +1480,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
disk = dev->data.disk;
if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN ||
- !virDomainDiskSourceIsBlockType(disk->src, false))
+ !virStorageSourceIsBlockLocal(disk->src))
return 0;
path = virDomainDiskGetSource(disk);
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 05ac254..4f44e05 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1955,7 +1955,7 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def)
int
-virStorageSourceGetActualType(virStorageSourcePtr def)
+virStorageSourceGetActualType(const virStorageSource *def)
{
if (def->type == VIR_STORAGE_TYPE_VOLUME && def->srcpool)
return def->srcpool->actualtype;
@@ -2012,6 +2012,20 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
/**
+ * virStorageSourceIsBlockLocal:
+ * @src: disk source definition
+ *
+ * Returns true if @src describes a locally accessible block storage source.
+ * This includes block devices and host-mapped iSCSI volumes.
+ */
+bool
+virStorageSourceIsBlockLocal(const virStorageSource *src)
+{
+ return virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_BLOCK;
This assumes that virStorageTranslateDiskSourcePool has been run, which
is true is for the non-LXC paths changed in this patch...
So as long as you're comfortable with the LXC adjustment, then
ACK -
John
+}
+
+
+/**
* virStorageSourceBackingStoreClear:
*
* @src: disk source to clear
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index b98fe25..17e1277 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -357,9 +357,10 @@ int virStorageSourceInitChainElement(virStorageSourcePtr newelem,
bool force);
void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def);
void virStorageSourceClear(virStorageSourcePtr def);
-int virStorageSourceGetActualType(virStorageSourcePtr def);
+int virStorageSourceGetActualType(const virStorageSource *def);
bool virStorageSourceIsLocalStorage(virStorageSourcePtr src);
bool virStorageSourceIsEmpty(virStorageSourcePtr src);
+bool virStorageSourceIsBlockLocal(const virStorageSource *src);
void virStorageSourceFree(virStorageSourcePtr def);
void virStorageSourceBackingStoreClear(virStorageSourcePtr def);
int virStorageSourceUpdateBlockPhysicalSize(virStorageSourcePtr src,