[libvirt] [PATCH] Check for NULL in qemu monitor event filter
When virConnectDomainQemuMonitorEventRegister is called with the VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX flag, ignore the flag instead of crashing. https://bugzilla.redhat.com/show_bug.cgi?id=1144920 --- src/conf/domain_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 73ae289..c8f6830 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1705,7 +1705,7 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, if (VIR_ALLOC(data) < 0) return -1; data->flags = flags; - if (flags != -1) { + if (event && flags != -1) { int rflags = REG_NOSUB; if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) -- 1.8.5.5
On 09/22/2014 06:09 AM, Ján Tomko wrote:
When virConnectDomainQemuMonitorEventRegister is called with the VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX flag, ignore the flag instead of crashing.
https://bugzilla.redhat.com/show_bug.cgi?id=1144920 --- src/conf/domain_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
That's one alternative. I was thinking more along the lines of forbidding the use of flags with a NULL event string: diff --git i/src/libvirt-qemu.c w/src/libvirt-qemu.c index fed08fc..e9bc82e 100644 --- i/src/libvirt-qemu.c +++ w/src/libvirt-qemu.c @@ -289,6 +289,9 @@ virConnectDomainQemuMonitorEventRegister(virConnectPtr conn, } virCheckNonNullArgGoto(cb, error); virCheckReadOnlyGoto(conn->flags, error); + if (flags & (VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX | + VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)) + virCheckNonNullArgGoto(event, error); if (conn->driver && conn->driver->connectDomainQemuMonitorEventRegister) { int ret; -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/22/2014 05:57 PM, Eric Blake wrote:
On 09/22/2014 06:09 AM, Ján Tomko wrote:
When virConnectDomainQemuMonitorEventRegister is called with the VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX flag, ignore the flag instead of crashing.
https://bugzilla.redhat.com/show_bug.cgi?id=1144920 --- src/conf/domain_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
That's one alternative. I was thinking more along the lines of forbidding the use of flags with a NULL event string:
I think ingoring it would be nicer than throwing an error. I'm fine with your fix as well, but the error message could be more specific (as NULL argument is allowed without the flags). Jan
diff --git i/src/libvirt-qemu.c w/src/libvirt-qemu.c index fed08fc..e9bc82e 100644 --- i/src/libvirt-qemu.c +++ w/src/libvirt-qemu.c @@ -289,6 +289,9 @@ virConnectDomainQemuMonitorEventRegister(virConnectPtr conn, } virCheckNonNullArgGoto(cb, error); virCheckReadOnlyGoto(conn->flags, error); + if (flags & (VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX | + VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)) + virCheckNonNullArgGoto(event, error);
if (conn->driver && conn->driver->connectDomainQemuMonitorEventRegister) { int ret;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 09/22/2014 10:26 AM, Ján Tomko wrote:
On 09/22/2014 05:57 PM, Eric Blake wrote:
On 09/22/2014 06:09 AM, Ján Tomko wrote:
When virConnectDomainQemuMonitorEventRegister is called with the VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX flag, ignore the flag instead of crashing.
https://bugzilla.redhat.com/show_bug.cgi?id=1144920 --- src/conf/domain_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
That's one alternative. I was thinking more along the lines of forbidding the use of flags with a NULL event string:
I think ingoring it would be nicer than throwing an error.
I'm fine with your fix as well, but the error message could be more specific (as NULL argument is allowed without the flags).
Thinking more about it: we already document that libvirt-qemu.so is unsupported, and primarily for developers. Ignoring an option is developer-friendly, hard errors require more maintenance effort. So ACK to your patch, unless anyone else speaks up strongly in favor of my variation. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/22/2014 08:07 PM, Eric Blake wrote:
On 09/22/2014 10:26 AM, Ján Tomko wrote:
On 09/22/2014 05:57 PM, Eric Blake wrote:
On 09/22/2014 06:09 AM, Ján Tomko wrote:
When virConnectDomainQemuMonitorEventRegister is called with the VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX flag, ignore the flag instead of crashing.
https://bugzilla.redhat.com/show_bug.cgi?id=1144920 --- src/conf/domain_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
That's one alternative. I was thinking more along the lines of forbidding the use of flags with a NULL event string:
I think ingoring it would be nicer than throwing an error.
I'm fine with your fix as well, but the error message could be more specific (as NULL argument is allowed without the flags).
Thinking more about it: we already document that libvirt-qemu.so is unsupported, and primarily for developers. Ignoring an option is developer-friendly, hard errors require more maintenance effort. So ACK to your patch, unless anyone else speaks up strongly in favor of my variation.
Thanks, I've pushed it now. Jan
participants (2)
-
Eric Blake -
Ján Tomko