8:24 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The polkit access driver used the wrong permission names for checks
on storage pools, volumes and node devices. This led to them always
being denied access.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/access/viraccessdriverpolkit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
index 4c76e64..b472bc3 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -248,7 +248,7 @@ virAccessDriverPolkitCheckNodeDevice(virAccessManagerPtr manager,
};
return virAccessDriverPolkitCheck(manager,
- "nodedevice",
+ "node-device",
virAccessPermNodeDeviceTypeToString(perm),
attrs);
}
@@ -355,7 +355,7 @@ virAccessDriverPolkitCheckStoragePool(virAccessManagerPtr manager,
virUUIDFormat(pool->uuid, uuidstr);
return virAccessDriverPolkitCheck(manager,
- "pool",
+ "storage-pool",
virAccessPermStoragePoolTypeToString(perm),
attrs);
}
@@ -379,7 +379,7 @@ virAccessDriverPolkitCheckStorageVol(virAccessManagerPtr manager,
virUUIDFormat(pool->uuid, uuidstr);
return virAccessDriverPolkitCheck(manager,
- "vol",
+ "storage-vol",
virAccessPermStorageVolTypeToString(perm),
attrs);
}
--
1.8.3.1
1:55 p.m.
On 09/11/2013 07:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The polkit access driver used the wrong permission names for checks
on storage pools, volumes and node devices. This led to them always
being denied access.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/access/viraccessdriverpolkit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
I had to do some hunting, but I found that:
src/access/genpolkit.pl defines
my @objects = (
"CONNECT", "DOMAIN", "INTERFACE",
"NETWORK","NODE_DEVICE", "NWFILTER",
"SECRET", "STORAGE_POOL", "STORAGE_VOL",
);
which in turn results in src/access/org.libvirt.api.policy generated
with entries such as:
<action id="org.libvirt.api.node-device.dettach">
[Ugg - did we REALLY mean to mis-spell detach? Is it too late to fix that?]
diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
index 4c76e64..b472bc3 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -248,7 +248,7 @@ virAccessDriverPolkitCheckNodeDevice(virAccessManagerPtr manager,
};
return virAccessDriverPolkitCheck(manager,
- "nodedevice",
+ "node-device",
Therefore, this is correct (as are the other two name fixes.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:02 p.m.
On Wed, Sep 11, 2013 at 12:55:01PM -0600, Eric Blake wrote:
On 09/11/2013 07:24 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> The polkit access driver used the wrong permission names for checks
> on storage pools, volumes and node devices. This led to them always
> being denied access.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/access/viraccessdriverpolkit.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
I had to do some hunting, but I found that:
src/access/genpolkit.pl defines
my @objects = (
"CONNECT", "DOMAIN", "INTERFACE",
"NETWORK","NODE_DEVICE", "NWFILTER",
"SECRET", "STORAGE_POOL", "STORAGE_VOL",
);
which in turn results in src/access/org.libvirt.api.policy generated
with entries such as:
<action id="org.libvirt.api.node-device.dettach">
[Ugg - did we REALLY mean to mis-spell detach? Is it too late to fix that?]
The bug with using 'nodedevice' instead of 'node-device' means
no one could have made use of the permission 'dettach'. Given that
I think we can justifiably change it without it being a upgrade
problem / breakage. In general though permission names should be
considered ABI stable.
ACK.
So how about adding this
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index 17f6243..9c720f9 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -58,7 +58,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice,
VIR_ACCESS_PERM_NODE_DEVICE_LAST,
"getattr", "read", "write",
"start", "stop",
- "dettach");
+ "detach");
VIR_ENUM_IMPL(virAccessPermNWFilter,
VIR_ACCESS_PERM_NWFILTER_LAST,
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index a1c23da..85ad9ba 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3696,19 +3696,19 @@ enum remote_procedure {
/**
* @generate: server
- * @acl: node_device:dettach
+ * @acl: node_device:detach
*/
REMOTE_PROC_NODE_DEVICE_DETTACH = 118,
/**
* @generate: server
- * @acl: node_device:dettach
+ * @acl: node_device:detach
*/
REMOTE_PROC_NODE_DEVICE_RE_ATTACH = 119,
/**
* @generate: server
- * @acl: node_device:dettach
+ * @acl: node_device:detach
*/
REMOTE_PROC_NODE_DEVICE_RESET = 120,
@@ -4929,7 +4929,7 @@ enum remote_procedure {
/**
* @generate: server
- * @acl: node_device:dettach
+ * @acl: node_device:detach
*/
REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS = 301,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:07 p.m.
On 09/11/2013 01:02 PM, Daniel P. Berrange wrote:
>
> <action id="org.libvirt.api.node-device.dettach">
>
> [Ugg - did we REALLY mean to mis-spell detach? Is it too late to fix that?]
The bug with using 'nodedevice' instead of 'node-device' means
no one could have made use of the permission 'dettach'. Given that
I think we can justifiably change it without it being a upgrade
problem / breakage. In general though permission names should be
considered ABI stable.
Cool - one typo saves us from another. I agree with your justfication
for fixing both typos at once.
>
> ACK.
So how about adding this
ACK to that being squashed in.
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index 17f6243..9c720f9 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -58,7 +58,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice,
VIR_ACCESS_PERM_NODE_DEVICE_LAST,
"getattr", "read", "write",
"start", "stop",
- "dettach");
+ "detach");
And thankfully, it appears to be the only permission with a typo'd name.
Are there any doc pages that need manual updates, or is it all generated
information that will auto-update to call out the corrected permission name?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4121
days inactive
4121
days old
3 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake