[libvirt] [PATCH] Fix polkit permission names for storage pools, vols & node devices
From: "Daniel P. Berrange" <berrange@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@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
On 09/11/2013 07:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@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@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
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@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@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 :|
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
participants (2)
-
Daniel P. Berrange -
Eric Blake