libvirt/tests/objecteventtest.c

927 lines
25 KiB
C
Raw Normal View History

/*
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
* Copyright (C) 2014 Red Hat, Inc.
* Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; If not, see
* <http://www.gnu.org/licenses/>.
*
* Author: Cedric Bosdonnat <cbosdonnat@suse.com>
*/
#include <config.h>
#include "testutils.h"
#include "virerror.h"
#include "virxml.h"
#define VIR_FROM_THIS VIR_FROM_NONE
static const char domainDef[] =
"<domain type='test'>"
" <name>test-domain</name>"
" <uuid>77a6fc12-07b5-9415-8abb-a803613f2a40</uuid>"
" <memory>8388608</memory>"
" <currentMemory>2097152</currentMemory>"
" <vcpu>2</vcpu>"
" <os>"
" <type>hvm</type>"
" </os>"
"</domain>";
2013-12-11 18:38:00 +08:00
static const char networkDef[] =
"<network>\n"
" <name>test</name>\n"
" <bridge name=\"virbr0\"/>\n"
" <forward/>\n"
" <ip address=\"192.168.122.1\" netmask=\"255.255.255.0\">\n"
" <dhcp>\n"
" <range start=\"192.168.122.2\" end=\"192.168.122.254\"/>\n"
" </dhcp>\n"
" </ip>\n"
"</network>\n";
static const char storagePoolDef[] =
"<pool type='dir'>\n"
" <name>P</name>\n"
" <target>\n"
" <path>/target-path</path>\n"
" </target>\n"
"</pool>\n";
static const char nodeDeviceDef[] =
"<device>\n"
" <parent>scsi_host1</parent>\n"
" <capability type='scsi_host'>\n"
" <capability type='fc_host'>\n"
" <wwpn>1000000023452345</wwpn>\n"
" <wwnn>2000000023452345</wwnn>\n"
" </capability>\n"
" </capability>\n"
"</device>\n";
typedef struct {
int startEvents;
int stopEvents;
int defineEvents;
int undefineEvents;
int unexpectedEvents;
int createdEvents;
int deletedEvents;
} lifecycleEventCounter;
static void
lifecycleEventCounter_reset(lifecycleEventCounter *counter)
{
counter->startEvents = 0;
counter->stopEvents = 0;
counter->defineEvents = 0;
counter->undefineEvents = 0;
counter->unexpectedEvents = 0;
counter->createdEvents = 0;
counter->deletedEvents = 0;
}
typedef struct {
virConnectPtr conn;
2013-12-11 18:38:00 +08:00
virNetworkPtr net;
virStoragePoolPtr pool;
virNodeDevicePtr dev;
} objecteventTest;
static int
domainLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainPtr dom ATTRIBUTE_UNUSED,
int event,
int detail ATTRIBUTE_UNUSED,
void *opaque)
{
lifecycleEventCounter *counter = opaque;
switch (event) {
case VIR_DOMAIN_EVENT_STARTED:
counter->startEvents++;
break;
case VIR_DOMAIN_EVENT_STOPPED:
counter->stopEvents++;
break;
case VIR_DOMAIN_EVENT_DEFINED:
counter->defineEvents++;
break;
case VIR_DOMAIN_EVENT_UNDEFINED:
counter->undefineEvents++;
break;
default:
/* Ignore other events */
break;
}
return 0;
}
2013-12-11 18:38:00 +08:00
static void
networkLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
virNetworkPtr net ATTRIBUTE_UNUSED,
int event,
int detail ATTRIBUTE_UNUSED,
void* opaque)
{
lifecycleEventCounter *counter = opaque;
if (event == VIR_NETWORK_EVENT_STARTED)
counter->startEvents++;
else if (event == VIR_NETWORK_EVENT_STOPPED)
counter->stopEvents++;
else if (event == VIR_NETWORK_EVENT_DEFINED)
counter->defineEvents++;
else if (event == VIR_NETWORK_EVENT_UNDEFINED)
counter->undefineEvents++;
}
static void
storagePoolLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolPtr pool ATTRIBUTE_UNUSED,
int event,
int detail ATTRIBUTE_UNUSED,
void* opaque)
{
lifecycleEventCounter *counter = opaque;
if (event == VIR_STORAGE_POOL_EVENT_STARTED)
counter->startEvents++;
else if (event == VIR_STORAGE_POOL_EVENT_STOPPED)
counter->stopEvents++;
else if (event == VIR_STORAGE_POOL_EVENT_DEFINED)
counter->defineEvents++;
else if (event == VIR_STORAGE_POOL_EVENT_UNDEFINED)
counter->undefineEvents++;
else if (event == VIR_STORAGE_POOL_EVENT_CREATED)
counter->createdEvents++;
else if (event == VIR_STORAGE_POOL_EVENT_DELETED)
counter->deletedEvents++;
}
static void
storagePoolRefreshCb(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolPtr pool ATTRIBUTE_UNUSED,
void* opaque)
{
int *counter = opaque;
(*counter)++;
}
2013-12-11 18:38:00 +08:00
static void
nodeDeviceLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
virNodeDevicePtr dev ATTRIBUTE_UNUSED,
int event,
int detail ATTRIBUTE_UNUSED,
void* opaque)
{
lifecycleEventCounter *counter = opaque;
if (event == VIR_NODE_DEVICE_EVENT_CREATED)
counter->createdEvents++;
else if (event == VIR_NODE_DEVICE_EVENT_DELETED)
counter->deletedEvents++;
}
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
static int
testDomainCreateXMLOld(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virDomainPtr dom = NULL;
int ret = -1;
bool registered = false;
lifecycleEventCounter_reset(&counter);
if (virConnectDomainEventRegister(test->conn,
domainLifecycleCb,
&counter, NULL) != 0)
goto cleanup;
registered = true;
dom = virDomainCreateXML(test->conn, domainDef, 0);
if (dom == NULL || virEventRunDefaultImpl() < 0)
goto cleanup;
if (counter.startEvents != 1 || counter.unexpectedEvents > 0)
goto cleanup;
if (virConnectDomainEventDeregister(test->conn, domainLifecycleCb) != 0)
goto cleanup;
registered = false;
ret = 0;
cleanup:
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (registered)
virConnectDomainEventDeregister(test->conn, domainLifecycleCb);
if (dom) {
virDomainDestroy(dom);
virDomainFree(dom);
}
return ret;
}
static int
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
testDomainCreateXMLNew(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int eventId = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
virDomainPtr dom = NULL;
int id;
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
int ret = -1;
lifecycleEventCounter_reset(&counter);
id = virConnectDomainEventRegisterAny(test->conn, NULL, eventId,
VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb),
&counter, NULL);
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (id < 0)
goto cleanup;
dom = virDomainCreateXML(test->conn, domainDef, 0);
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (dom == NULL || virEventRunDefaultImpl() < 0)
goto cleanup;
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (counter.startEvents != 1 || counter.unexpectedEvents > 0)
goto cleanup;
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (virConnectDomainEventDeregisterAny(test->conn, id) != 0)
goto cleanup;
id = -1;
ret = 0;
cleanup:
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
if (id >= 0)
virConnectDomainEventDeregisterAny(test->conn, id);
if (dom) {
virDomainDestroy(dom);
virDomainFree(dom);
}
return ret;
}
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
static int
testDomainCreateXMLMixed(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virDomainPtr dom;
int ret = -1;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
int id1 = -1;
int id2 = -1;
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
bool registered = false;
lifecycleEventCounter_reset(&counter);
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
/* Fun with mixing old and new API, also with global and
* per-domain. Handler should be fired three times, once for each
* registration. */
event: don't turn offline domain into global event If a user registers for a domain event filtered to a particular domain, but the persistent domain is offline at the time, then the code silently failed to set up the filter. As a result, the event fires for all domains, rather than being filtered. Network events were immune, since they always passed an id 0 argument. The key to this patch is realizing that virObjectEventDispatchMatchCallback() only cared about uuid; so refusing to create a meta for a negative id is pointless, and in fact, malloc'ing meta at all was overkill; instead, just directly store a uuid and a flag of whether to filter. Note that virObjectEventPtr still needs all fields of meta, because this is how we reconstruct a virDomainPtr inside the dispatch handler before calling the end user's callback pointer with the correct object, even though only the uuid portion of meta is used in deciding whether a callback matches the given event. So while uuid is optional for callbacks, it is mandatory for events. The change to testDomainCreateXMLMixed is merely on the setup scenario (as you can't register for a domain unless it is either running or persistent). I actually first wrote that test for this patch, then rebased it to also cover a prior patch (commit 4221d64), but had to adjust it for that patch to use Create instead of Define for setting up the domain long enough to register the event in order to work around this bug. But while the setup is changed, the main body of the test is still about whether creation events fire as expected. * src/conf/object_event_private.h (_virObjectEventCallback): Replace meta with uuid and flag. (virObjectEventCallbackListAddID): Update signature. * src/conf/object_event.h (virObjectEventStateRegisterID): Likewise. * src/conf/object_event_private.h (virObjectEventNew): Document use of name and uuid in events. * src/conf/object_event.c (virObjectEventCallbackListAddID): Drop arguments that don't affect filtering. (virObjectEventCallbackListRemoveID) (virObjectEventDispatchMatchCallback) (virObjectEventStateRegisterID): Update clients. * src/conf/domain_event.c (virDomainEventCallbackListAdd) (virDomainEventStateRegisterID): Likewise. * src/conf/network_event.c (virNetworkEventStateRegisterID): Likewise. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 08:02:17 +08:00
dom = virDomainDefineXML(test->conn, domainDef);
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
if (dom == NULL)
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
goto cleanup;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
id1 = virConnectDomainEventRegisterAny(test->conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb),
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
&counter, NULL);
if (id1 < 0)
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
goto cleanup;
if (virConnectDomainEventRegister(test->conn,
domainLifecycleCb,
&counter, NULL) != 0)
goto cleanup;
registered = true;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
id2 = virConnectDomainEventRegisterAny(test->conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb),
&counter, NULL);
if (id2 < 0)
goto cleanup;
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
virDomainUndefine(dom);
virDomainDestroy(dom);
virDomainFree(dom);
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
dom = virDomainCreateXML(test->conn, domainDef, 0);
if (dom == NULL || virEventRunDefaultImpl() < 0)
goto cleanup;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
if (counter.startEvents != 3 || counter.unexpectedEvents > 0)
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
goto cleanup;
if (virConnectDomainEventDeregister(test->conn, domainLifecycleCb) != 0)
goto cleanup;
registered = false;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
if (virConnectDomainEventDeregisterAny(test->conn, id1) != 0)
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
goto cleanup;
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
id1 = -1;
if (virConnectDomainEventDeregisterAny(test->conn, id2) != 0)
goto cleanup;
id2 = -1;
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
ret = 0;
cleanup:
event: don't allow mix of old- and new-style registration Consider these two calls, in either order: id1 = virConnectDomainEventRegisterAny(conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); Right now, the second call fails, because under the hood, the old-style function registration is tightly coupled to the new style lifecycle eventID, and the two calls both try to register the same global eventID callback representation. We've alreay documented that users should avoid old-style registration and deregistration, so anyone heeding the advice won't run into this situation. But it would be even nicer if we pretend the two interfaces are completely separate, and disallow any cross-linking. That is, a call to old-style deregister should never remove a new-style callback even if it is the same function pointer, and a call to new-style callback using only callbackIDs obtained legitimately should never remove an old-style callback (of course, since our callback IDs are sequential, and there is still coupling under the hood, you can easily guess the callbackID of an old style registration and use new-style deregistration to nuke it - but that starts to be blatantly bad coding on your part rather than a surprising result on what looks like reasonable stand-alone API). With this patch, you can now register a global lifecycle event handler twice, by using both old and new APIs; if such an event occurs, your callback will be entered twice. But that is not a problem in practice, since it is already possible to use the new API to register both a global and per-domain event handler using the same function, which will likewise fire your callback twice for that domain. Duplicates are still prevented when using the same API with same parameters twice (old-style twice, new-style global twice, or new-style per-domain with same domain twice), and things are still bounded (it is not possible to register a single function pointer more than N+2 times per event id, where N is the number of domains available on the connection). Besides, it has always been possible to register as many separate function pointers on the same event id as desired, through either old or new style API, where the bound there is the physical limitation of writing a program with enough distinct function pointers. Adding another event registration in the testsuite is sufficient to cover this, where the test fails without the rest of the patch. * src/conf/object_event.c (_virObjectEventCallback): Add field. (virObjectEventCallbackLookup): Add argument. (virObjectEventCallbackListAddID, virObjectEventStateCallbackID): Adjust callers. * tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 20:55:55 +08:00
if (id1 >= 0)
virConnectDomainEventDeregisterAny(test->conn, id1);
if (id2 >= 0)
virConnectDomainEventDeregisterAny(test->conn, id2);
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
if (registered)
virConnectDomainEventDeregister(test->conn, domainLifecycleCb);
if (dom != NULL) {
virDomainUndefine(dom);
virDomainDestroy(dom);
virDomainFree(dom);
}
return ret;
}
static int
testDomainDefine(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int eventId = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
virDomainPtr dom = NULL;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectDomainEventRegisterAny(test->conn, NULL, eventId,
VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb),
&counter, NULL);
/* Make sure the define event is triggered */
dom = virDomainDefineXML(test->conn, domainDef);
if (dom == NULL || virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.defineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
/* Make sure the undefine event is triggered */
virDomainUndefine(dom);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.undefineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectDomainEventDeregisterAny(test->conn, id);
if (dom != NULL)
virDomainFree(dom);
return ret;
}
static int
testDomainStartStopEvent(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int eventId = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
int id;
int ret = -1;
virDomainPtr dom;
virConnectPtr conn2 = NULL;
virDomainPtr dom2 = NULL;
lifecycleEventCounter_reset(&counter);
dom = virDomainLookupByName(test->conn, "test");
if (dom == NULL)
return -1;
id = virConnectDomainEventRegisterAny(test->conn, dom, eventId,
VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb),
&counter, NULL);
/* Test domain is started */
virDomainDestroy(dom);
if (virDomainCreate(dom) < 0)
goto cleanup;
if (virEventRunDefaultImpl() < 0)
goto cleanup;
if (counter.startEvents != 1 || counter.stopEvents != 1 ||
counter.unexpectedEvents > 0)
goto cleanup;
/* Repeat the test, but this time, trigger the events via an
* alternate connection. */
if (!(conn2 = virConnectOpen("test:///default")))
goto cleanup;
if (!(dom2 = virDomainLookupByName(conn2, "test")))
goto cleanup;
if (virDomainDestroy(dom2) < 0)
goto cleanup;
if (virDomainCreate(dom2) < 0)
goto cleanup;
if (virEventRunDefaultImpl() < 0)
goto cleanup;
if (counter.startEvents != 2 || counter.stopEvents != 2 ||
counter.unexpectedEvents > 0)
goto cleanup;
ret = 0;
cleanup:
virConnectDomainEventDeregisterAny(test->conn, id);
virDomainFree(dom);
if (dom2)
virDomainFree(dom2);
if (conn2)
virConnectClose(conn2);
return ret;
}
2013-12-11 18:38:00 +08:00
static int
testNetworkCreateXML(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virNetworkPtr net;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectNetworkEventRegisterAny(test->conn, NULL,
VIR_NETWORK_EVENT_ID_LIFECYCLE,
VIR_NETWORK_EVENT_CALLBACK(&networkLifecycleCb),
&counter, NULL);
net = virNetworkCreateXML(test->conn, networkDef);
if (!net || virEventRunDefaultImpl() < 0) {
2013-12-11 18:38:00 +08:00
ret = -1;
goto cleanup;
}
if (counter.startEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
2013-12-11 18:38:00 +08:00
virConnectNetworkEventDeregisterAny(test->conn, id);
if (net) {
virNetworkDestroy(net);
virNetworkFree(net);
}
2013-12-11 18:38:00 +08:00
return ret;
}
static int
testNetworkDefine(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virNetworkPtr net;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectNetworkEventRegisterAny(test->conn, NULL,
VIR_NETWORK_EVENT_ID_LIFECYCLE,
VIR_NETWORK_EVENT_CALLBACK(&networkLifecycleCb),
&counter, NULL);
/* Make sure the define event is triggered */
net = virNetworkDefineXML(test->conn, networkDef);
if (!net || virEventRunDefaultImpl() < 0) {
2013-12-11 18:38:00 +08:00
ret = -1;
goto cleanup;
}
if (counter.defineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
/* Make sure the undefine event is triggered */
virNetworkUndefine(net);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.undefineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
2013-12-11 18:38:00 +08:00
virConnectNetworkEventDeregisterAny(test->conn, id);
if (net)
virNetworkFree(net);
2013-12-11 18:38:00 +08:00
return ret;
}
static int
testNetworkStartStopEvent(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int id;
int ret = 0;
if (!test->net)
return -1;
2013-12-11 18:38:00 +08:00
lifecycleEventCounter_reset(&counter);
id = virConnectNetworkEventRegisterAny(test->conn, test->net,
VIR_NETWORK_EVENT_ID_LIFECYCLE,
VIR_NETWORK_EVENT_CALLBACK(&networkLifecycleCb),
&counter, NULL);
virNetworkCreate(test->net);
virNetworkDestroy(test->net);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.startEvents != 1 || counter.stopEvents != 1 ||
counter.unexpectedEvents > 0) {
2013-12-11 18:38:00 +08:00
ret = -1;
goto cleanup;
}
cleanup:
2013-12-11 18:38:00 +08:00
virConnectNetworkEventDeregisterAny(test->conn, id);
return ret;
}
static int
testStoragePoolCreateXML(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virStoragePoolPtr pool;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
&counter, NULL);
pool = virStoragePoolCreateXML(test->conn, storagePoolDef, 0);
if (!pool || virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.startEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectStoragePoolEventDeregisterAny(test->conn, id);
if (pool) {
virStoragePoolDestroy(pool);
virStoragePoolFree(pool);
}
return ret;
}
static int
testStoragePoolDefine(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virStoragePoolPtr pool;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
&counter, NULL);
/* Make sure the define event is triggered */
pool = virStoragePoolDefineXML(test->conn, storagePoolDef, 0);
if (!pool || virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.defineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
/* Make sure the undefine event is triggered */
virStoragePoolUndefine(pool);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.undefineEvents != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectStoragePoolEventDeregisterAny(test->conn, id);
if (pool)
virStoragePoolFree(pool);
return ret;
}
static int
testStoragePoolStartStopEvent(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int refreshCounter;
int id1, id2;
int ret = 0;
if (!test->pool)
return -1;
lifecycleEventCounter_reset(&counter);
refreshCounter = 0;
id1 = virConnectStoragePoolEventRegisterAny(test->conn, test->pool,
VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
&counter, NULL);
id2 = virConnectStoragePoolEventRegisterAny(test->conn, test->pool,
VIR_STORAGE_POOL_EVENT_ID_REFRESH,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolRefreshCb),
&refreshCounter, NULL);
virStoragePoolCreate(test->pool, 0);
virStoragePoolRefresh(test->pool, 0);
virStoragePoolDestroy(test->pool);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.startEvents != 1 || counter.stopEvents != 1 ||
refreshCounter != 1 || counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectStoragePoolEventDeregisterAny(test->conn, id1);
virConnectStoragePoolEventDeregisterAny(test->conn, id2);
return ret;
}
static int
testStoragePoolBuild(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
&counter, NULL);
virStoragePoolBuild(test->pool, 0);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.createdEvents != 1) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectStoragePoolEventDeregisterAny(test->conn, id);
return ret;
}
static int
testStoragePoolDelete(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
&counter, NULL);
virStoragePoolDelete(test->pool, 0);
if (virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.deletedEvents != 1) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectStoragePoolEventDeregisterAny(test->conn, id);
return ret;
}
static int
testNodeDeviceCreateXML(const void *data)
{
const objecteventTest *test = data;
lifecycleEventCounter counter;
virNodeDevicePtr dev;
int id;
int ret = 0;
lifecycleEventCounter_reset(&counter);
id = virConnectNodeDeviceEventRegisterAny(test->conn, NULL,
VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE,
VIR_NODE_DEVICE_EVENT_CALLBACK(&nodeDeviceLifecycleCb),
&counter, NULL);
dev = virNodeDeviceCreateXML(test->conn, nodeDeviceDef, 0);
virNodeDeviceDestroy(dev);
if (!dev || virEventRunDefaultImpl() < 0) {
ret = -1;
goto cleanup;
}
if (counter.createdEvents != 1 || counter.deletedEvents != 1 ||
counter.unexpectedEvents > 0) {
ret = -1;
goto cleanup;
}
cleanup:
virConnectNodeDeviceEventDeregisterAny(test->conn, id);
if (dev)
virNodeDeviceFree(dev);
return ret;
}
static void
timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED)
{
fputs("test taking too long; giving up", stderr);
_exit(EXIT_FAILURE);
}
static int
mymain(void)
{
objecteventTest test = { 0 };
int ret = EXIT_SUCCESS;
int timer;
virEventRegisterDefaultImpl();
/* Set up a timer to abort this test if it takes 10 seconds. */
if ((timer = virEventAddTimeout(10 * 1000, timeout, NULL, NULL)) < 0)
return EXIT_FAILURE;
if (!(test.conn = virConnectOpen("test:///default")))
return EXIT_FAILURE;
virTestQuiesceLibvirtErrors(false);
/* Domain event tests */
if (virTestRun("Domain createXML start event (old API)",
testDomainCreateXMLOld, &test) < 0)
event: make deregister return value match docs Ever since their introduction (commit 1509b80 in v0.5.0 for virConnectDomainEventRegister, commit 4445723 in v0.8.0 for virConnectDomainEventDeregisterAny), the event deregistration functions have been documented as returning 0 on success; likewise for older registration (only the newer RegisterAny must return a non-zero callbackID). And now that we are adding virConnectNetworkEventDeregisterAny for v1.2.1, it should have the same semantics. Fortunately, all of the stateful drivers have been obeying the docs and returning 0, thanks to the way the remote_driver tracks things (in fact, the RPC wire protocol is unable to send a return value for DomainEventRegisterAny, at least not without adding a new RPC number). Well, except for vbox, which was always failing deregistration, due to failure to set the return value to anything besides its initial -1. But for local drivers, such as test:///default, we've been returning non-zero numbers; worse, the non-zero numbers have differed over time. For example, in Fedora 12 (libvirt 0.8.2), calling Register twice would return 0 and 1 [the callbackID generated under the hood]; while in Fedora 20 (libvirt 1.1.3), it returns 1 and 2 [the number of callbacks registered for that event type]. Since we have changed the behavior over time, and since it differs by local vs. remote, we can safely argue that no one could have been reasonably relying on any particular behavior, so we might as well obey the docs, as well as prepare callers that might deal with older clients to not be surprised if the docs are not strictly followed. For consistency, this patch fixes the code for all drivers, even though it only makes an impact for vbox and for local drivers. By fixing all drivers, future copy and paste from a remote driver to a local driver is less likely to reintroduce the bug. Finally, update the testsuite to gain some coverage of the issue for local drivers, including the first test of old-style domain event registration via function pointer instead of event id. * src/libvirt.c (virConnectDomainEventRegister) (virConnectDomainEventDeregister) (virConnectDomainEventDeregisterAny): Clarify docs. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventDeregister) (libxlConnectDomainEventDeregisterAny): Match documentation. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventDeregister) (lxcConnectDomainEventDeregisterAny): Likewise. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventDeregister) (testConnectDomainEventDeregisterAny) (testConnectNetworkEventDeregisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventDeregister) (umlConnectDomainEventDeregisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister) (vboxConnectDomainEventDeregister) (vboxConnectDomainEventDeregisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventDeregister) (xenUnifiedConnectDomainEventDeregisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventDeregisterAny): Likewise. * tests/objecteventtest.c (testDomainCreateXMLOld): New test. (mymain): Run it. (testDomainCreateXML): Check return values. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 05:21:17 +08:00
ret = EXIT_FAILURE;
if (virTestRun("Domain createXML start event (new API)",
testDomainCreateXMLNew, &test) < 0)
event: don't let old-style events clobber per-domain events Right now, the older virConnectDomainEventRegister (takes a function pointer, returns 0 on success) and the newer virConnectDomainEventRegisterID (takes an eventID, returns a callbackID) share the underlying implementation (the older API ends up consuming a callbackID for eventID 0 under the hood). We implemented that by a lot of copy and pasted code between object_event.c and domain_event.c, according to whether we are dealing with a function pointer or an eventID. However, our copy and paste is not symmetric. Consider this sequence: id1 = virConnectDomainEventRegisterAny(conn, dom, VIR_DOMAIN_EVENT_ID_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL); virConnectDomainEventRegister(conn, callback, NULL, NULL); virConnectDomainEventDeregister(conn, callback); virConnectDomainEventDeregsiterAny(conn, id1); the first three calls would succeed, but the third call ended up nuking the id1 callbackID (the per-domain new-style handler), then the fourth call failed with an error about an unknown callbackID, leaving us with the global handler (old-style) still live and receiving events. It required another old-style deregister to clean up the mess. Root cause was that virDomainEventCallbackList{Remove,MarkDelete} were only checking for function pointer match, rather than also checking for whether the registration was global. Rather than playing with the guts of object_event ourselves in domain_event, it is nicer to add a mapping function for the internal callback id, then share common code for event removal. For now, the function-to-id mapping is used only internally; I thought about whether a new public API to let a user learn the callback would be useful, but decided exposing this to the user is probably a disservice, since we already publicly document that they should avoid the old style, and since this patch already demonstrates that older libvirt versions have weird behavior when mixing old and new styles. And like all good bug fix patches, I enhanced the testsuite, validating that the changes in tests/ expose the failure without the rest of the patch. * src/conf/object_event.c (virObjectEventCallbackLookup) (virObjectEventStateCallbackID): New functions. (virObjectEventCallbackLookup): Use helper function. * src/conf/object_event_private.h (virObjectEventStateCallbackID): Declare new function. * src/conf/domain_event.c (virDomainEventStateRegister) (virDomainEventStateDeregister): Let common code handle the complexity. (virDomainEventCallbackListRemove) (virDomainEventCallbackListMarkDelete) (virDomainEventCallbackListAdd): Drop unused functions. * tests/objecteventtest.c (testDomainCreateXMLMixed): New test. Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 07:50:14 +08:00
ret = EXIT_FAILURE;
if (virTestRun("Domain createXML start event (both API)",
testDomainCreateXMLMixed, &test) < 0)
ret = EXIT_FAILURE;
if (virTestRun("Domain (un)define events", testDomainDefine, &test) < 0)
ret = EXIT_FAILURE;
if (virTestRun("Domain start stop events", testDomainStartStopEvent, &test) < 0)
ret = EXIT_FAILURE;
2013-12-11 18:38:00 +08:00
/* Network event tests */
/* Tests requiring the test network not to be set up*/
if (virTestRun("Network createXML start event ", testNetworkCreateXML, &test) < 0)
2013-12-11 18:38:00 +08:00
ret = EXIT_FAILURE;
if (virTestRun("Network (un)define events", testNetworkDefine, &test) < 0)
2013-12-11 18:38:00 +08:00
ret = EXIT_FAILURE;
/* Define a test network */
if (!(test.net = virNetworkDefineXML(test.conn, networkDef)))
ret = EXIT_FAILURE;
if (virTestRun("Network start stop events ", testNetworkStartStopEvent, &test) < 0)
2013-12-11 18:38:00 +08:00
ret = EXIT_FAILURE;
/* Cleanup */
if (test.net) {
virNetworkUndefine(test.net);
virNetworkFree(test.net);
}
/* Storage pool event tests */
if (virTestRun("Storage pool createXML start event ",
testStoragePoolCreateXML, &test) < 0)
ret = EXIT_FAILURE;
if (virTestRun("Storage pool (un)define events",
testStoragePoolDefine, &test) < 0)
ret = EXIT_FAILURE;
/* Define a test storage pool */
if (!(test.pool = virStoragePoolDefineXML(test.conn, storagePoolDef, 0)))
ret = EXIT_FAILURE;
if (virTestRun("Storage pool start stop events ",
testStoragePoolStartStopEvent, &test) < 0)
ret = EXIT_FAILURE;
/* Storage pool build and delete events */
if (virTestRun("Storage pool build event ",
testStoragePoolBuild, &test) < 0)
ret = EXIT_FAILURE;
if (virTestRun("Storage pool delete event ",
testStoragePoolDelete, &test) < 0)
ret = EXIT_FAILURE;
/* Node device event tests */
if (virTestRun("Node device createXML add event ",
testNodeDeviceCreateXML, &test) < 0)
ret = EXIT_FAILURE;
/* Cleanup */
if (test.pool) {
virStoragePoolUndefine(test.pool);
virStoragePoolFree(test.pool);
}
virConnectClose(test.conn);
virEventRemoveTimeout(timer);
return ret;
}
VIR_TEST_MAIN(mymain)