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 :|