5:52 a.m.
This API does not change domain state. It's merely like
virDomainGetXMLDesc() - and we don't reject RO connections there.
There's no reason to reject them here.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt-domain.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 677a9ad..e5af933 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11546,7 +11546,6 @@ virDomainInterfaceAddresses(virDomainPtr dom,
*ifaces = NULL;
virCheckDomainReturn(dom, -1);
virCheckNonNullArgGoto(ifaces, error);
- virCheckReadOnlyGoto(dom->conn->flags, error);
if (dom->conn->driver->domainInterfaceAddresses) {
int ret;
--
2.4.10
6:11 a.m.
On Mon, Jan 11, 2016 at 12:52:36PM +0100, Michal Privoznik wrote:
This API does not change domain state. It's merely like
virDomainGetXMLDesc() - and we don't reject RO connections there.
There's no reason to reject them here.
This API can result in talking to the guest agent IIRC, which should
be denied for read-only.
Regards,
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 :|
6:20 a.m.
On 11.01.2016 13:11, Daniel P. Berrange wrote:
On Mon, Jan 11, 2016 at 12:52:36PM +0100, Michal Privoznik wrote:
> This API does not change domain state. It's merely like
> virDomainGetXMLDesc() - and we don't reject RO connections there.
> There's no reason to reject them here.
This API can result in talking to the guest agent IIRC, which should
be denied for read-only.
Ah, I see. We have the following check in the code:
int
virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
{
/* ... */
if (flags & VIR_DOMAIN_VCPU_GUEST)
virCheckReadOnlyGoto(conn->flags, error);
}
On the other hand, virDomainGetTime() talks to guest agent and is allowed on RO
connection.
So what about the following change instead?
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index e5af933..95b797a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11545,7 +11545,8 @@ virDomainInterfaceAddresses(virDomainPtr dom,
if (ifaces)
*ifaces = NULL;
virCheckDomainReturn(dom, -1);
- virCheckNonNullArgGoto(ifaces, error);
+ if (source == VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT)
+ virCheckNonNullArgGoto(ifaces, error);
if (dom->conn->driver->domainInterfaceAddresses) {
int ret;
Michal
6:26 a.m.
On Mon, Jan 11, 2016 at 01:20:38PM +0100, Michal Privoznik wrote:
On 11.01.2016 13:11, Daniel P. Berrange wrote:
> On Mon, Jan 11, 2016 at 12:52:36PM +0100, Michal Privoznik wrote:
>> This API does not change domain state. It's merely like
>> virDomainGetXMLDesc() - and we don't reject RO connections there.
>> There's no reason to reject them here.
>
> This API can result in talking to the guest agent IIRC, which should
> be denied for read-only.
Ah, I see. We have the following check in the code:
int
virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
{
/* ... */
if (flags & VIR_DOMAIN_VCPU_GUEST)
virCheckReadOnlyGoto(conn->flags, error);
}
On the other hand, virDomainGetTime() talks to guest agent and is allowed on RO
connection.
So what about the following change instead?
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index e5af933..95b797a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11545,7 +11545,8 @@ virDomainInterfaceAddresses(virDomainPtr dom,
if (ifaces)
*ifaces = NULL;
virCheckDomainReturn(dom, -1);
- virCheckNonNullArgGoto(ifaces, error);
+ if (source == VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT)
+ virCheckNonNullArgGoto(ifaces, error);
You want to be checking the readonly flag here, not ifaces...
Regards,
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 :|
6:39 a.m.
On 11.01.2016 13:26, Daniel P. Berrange wrote:
On Mon, Jan 11, 2016 at 01:20:38PM +0100, Michal Privoznik wrote:
> On 11.01.2016 13:11, Daniel P. Berrange wrote:
>> On Mon, Jan 11, 2016 at 12:52:36PM +0100, Michal Privoznik wrote:
>>> This API does not change domain state. It's merely like
>>> virDomainGetXMLDesc() - and we don't reject RO connections there.
>>> There's no reason to reject them here.
>>
>> This API can result in talking to the guest agent IIRC, which should
>> be denied for read-only.
>
> Ah, I see. We have the following check in the code:
>
> int
> virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
> {
> /* ... */
> if (flags & VIR_DOMAIN_VCPU_GUEST)
> virCheckReadOnlyGoto(conn->flags, error);
> }
>
> On the other hand, virDomainGetTime() talks to guest agent and is allowed on RO
connection.
>
> So what about the following change instead?
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index e5af933..95b797a 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11545,7 +11545,8 @@ virDomainInterfaceAddresses(virDomainPtr dom,
> if (ifaces)
> *ifaces = NULL;
> virCheckDomainReturn(dom, -1);
> - virCheckNonNullArgGoto(ifaces, error);
> + if (source == VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT)
> + virCheckNonNullArgGoto(ifaces, error);
You want to be checking the readonly flag here, not ifaces...
Obviously. My fingers got detached from my eyes when writing the patch
... Fortunately, v2 is not the same case.
Michal
3241
days inactive
3241
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Michal Privoznik