[libvirt] [PATCH] phyp: Don't do a flags check in the storage driver

[Daniel P. Berrange]

On Fri, Nov 12, 2010 at 08:24:53AM -0700, Eric Blake wrote:
> On 11/12/2010 06:03 AM, Matthias Bolte wrote:
> > This makes the storage driver fail when the connection is
> > opened with the VIR_CONNECT_RO flag, resulting in a read-only
> > connection with no storage driver.
> > ---
> >  src/phyp/phyp_driver.c |    4 +---
> >  1 files changed, 1 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> > index a685bd1..4c723a2 100644
> > --- a/src/phyp/phyp_driver.c
> > +++ b/src/phyp/phyp_driver.c
> > @@ -3927,10 +3927,8 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus)
> >  static virDrvOpenStatus
> >  phypVIOSDriverOpen(virConnectPtr conn,
> >                     virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> > -                   int flags)
> > +                   int flags ATTRIBUTE_UNUSED)
> >  {
> > -    virCheckFlags(0, VIR_DRV_OPEN_ERROR);
> > -
> 
> This lets all possible flags through, even bits that are not yet
> defined.  Wouldn't the better approach be to fix virCheckFlags() to
> change 0 to the actual read-only bit that we expect to ignore, while
> still forbidding the remaining 31 bits for future extensibility?

The connection flags are  all interpreted at the layer above and
shouldn't really even be passed into any of these drivers - and
none of the others check flags at this layer. In addition this
entire method is practically a no-op, because all it does is link
up a  pointer to the main connection private data if the driver
name matches.

In any case, returning VIR_DRV_OPEN_ERROR is absolutely wrong
because that kills the entire attempt to open any storage driver
at all.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|