Re: [libvirt] [PATCH 1/8] util: Replace virDomainDiskSourceIsBlockType with a new helper

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/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, > /** > + * 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