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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list