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

[Peter Krempa]

On Fri, May 06, 2016 at 10:33:49 -0400, John Ferlan wrote:
> 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...

LXC won't work with 'volume' disks as as you've  said it's not
translated. virDomainDiskSourceIsBlockType would return false at this
point since def->disks[i]->src->path would not be set at this point
since it was not translated and the very first check of that function
was to check the source path.

As of that this change doesn't change the behavior in LXC in any way
just uses a more sane function to do the 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

Allowing to use volumes is definitely something for a separate patch.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160509/796dbe77/attachment-0001.sig>