[libvirt] [PATCH] virDomainInterfaceAddresses: Allow API on RO connection too
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@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
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 :|
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
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 :|
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
participants (2)
-
Daniel P. Berrange -
Michal Privoznik