When the block job event was first added, it was for block pull, where the active layer of the disk remains the same name. It was also in a day where we only cared about local files, and so we always had a canonical absolute file name. But two things have changed since then: we now have network disks, where determining a single absolute string does not really make sense; and we have two-phase jobs (copy and active commit) where the name of the active layer changes between the first event (ready, on the old name) and second (complete, on the pivoted name). Adam Litke reported that having an unstable string between events makes life harder for clients. Furthermore, all of our API that operate on a particular disk of a domain accept multiple strings: not only the absolute name of the active layer, but also the destination device name (such as 'vda'). As this latter name is stable, even for network sources, it serves as a better string to supply in block job events. * include/libvirt/libvirt.h.in (virConnectDomainEventBlockJobCallback): Document altered semantics. * src/conf/domain_event.c (_virDomainEventBlockJob): Rename field, to ensure we catch all clients. (virDomainEventBlockJobDispose, virDomainEventBlockJobNew) (virDomainEventBlockJobNewFromObj) (virDomainEventBlockJobNewFromDom) (virDomainEventDispatchDefaultFunc): Adjust clients. * src/conf/domain_event.h: Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Likewise. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise. * src/remote/remote_protocol.x (remote_domain_event_block_job_msg): Likewise. * src/remote/remote_driver.c (remoteDomainBuildEventBlockJobHelper): Likewise. * daemon/remote.c (remoteRelayDomainEventBlockJob): Likewise. * src/remote_protocol-structs: Regenerate. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- If you think backward compatibility of old clients that expected and operated on an absolute name is too important to break, I'm open to suggestions on alternatives how to preserve that. We don't have a flag argument during event registration, or I would have used it. Otherwise, I hope this change is okay as-is. daemon/remote.c | 8 ++++---- include/libvirt/libvirt.h.in | 11 ++++++++++- src/conf/domain_event.c | 18 +++++++++--------- src/conf/domain_event.h | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 4 +--- src/remote/remote_driver.c | 2 +- src/remote/remote_protocol.x | 2 +- src/remote_protocol-structs | 2 +- 9 files changed, 30 insertions(+), 23 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 34c96c9..ee1bbbc 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -558,7 +558,7 @@ remoteRelayDomainEventGraphics(virConnectPtr conn, static int remoteRelayDomainEventBlockJob(virConnectPtr conn, virDomainPtr dom, - const char *path, + const char *disk, int type, int status, void *opaque) @@ -571,11 +571,11 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn, return -1; VIR_DEBUG("Relaying domain block job event %s %d %s %i, %i, callback %d", - dom->name, dom->id, path, type, status, callback->callbackID); + dom->name, dom->id, disk, type, status, callback->callbackID); /* build return data */ memset(&data, 0, sizeof(data)); - if (VIR_STRDUP(data.path, path) < 0) + if (VIR_STRDUP(data.disk, disk) < 0) goto error; data.type = type; data.status = status; @@ -596,7 +596,7 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn, return 0; error: - VIR_FREE(data.path); + VIR_FREE(data.disk); return -1; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index dc88c40..a983046 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4852,11 +4852,20 @@ typedef enum { * virConnectDomainEventBlockJobCallback: * @conn: connection object * @dom: domain on which the event occurred - * @disk: fully-qualified filename of the affected disk + * @disk: name associated with the affected disk * @type: type of block job (virDomainBlockJobType) * @status: status of the operation (virConnectDomainEventBlockJobStatus) * @opaque: application specified data * + * The string returned for @disk can be used in any of the libvirt API + * that operate on a particular disk of the domain. In older versions + * of libvirt, this string was the fully-qualified filename of the + * active layer of the disk; but as some events can change which file + * is the active layer, and because network protocols do not + * necessarily have a canonical filename, libvirt 1.2.6 and newer + * instead use the device target shorthand (the <target dev='...'/> + * sub-element, such as "vda"). + * * The callback signature to use when registering for an event of type * VIR_DOMAIN_EVENT_ID_BLOCK_JOB with virConnectDomainEventRegisterAny() */ diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b565732..a2d59fd 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -128,7 +128,7 @@ typedef virDomainEventIOError *virDomainEventIOErrorPtr; struct _virDomainEventBlockJob { virDomainEvent parent; - char *path; + char *disk; int type; int status; }; @@ -364,7 +364,7 @@ virDomainEventBlockJobDispose(void *obj) virDomainEventBlockJobPtr event = obj; VIR_DEBUG("obj=%p", event); - VIR_FREE(event->path); + VIR_FREE(event->disk); } static void @@ -778,7 +778,7 @@ static virObjectEventPtr virDomainEventBlockJobNew(int id, const char *name, unsigned char *uuid, - const char *path, + const char *disk, int type, int status) { @@ -792,7 +792,7 @@ virDomainEventBlockJobNew(int id, id, name, uuid))) return NULL; - if (VIR_STRDUP(ev->path, path) < 0) { + if (VIR_STRDUP(ev->disk, disk) < 0) { virObjectUnref(ev); return NULL; } @@ -804,22 +804,22 @@ virDomainEventBlockJobNew(int id, virObjectEventPtr virDomainEventBlockJobNewFromObj(virDomainObjPtr obj, - const char *path, + const char *disk, int type, int status) { return virDomainEventBlockJobNew(obj->def->id, obj->def->name, - obj->def->uuid, path, type, status); + obj->def->uuid, disk, type, status); } virObjectEventPtr virDomainEventBlockJobNewFromDom(virDomainPtr dom, - const char *path, + const char *disk, int type, int status) { return virDomainEventBlockJobNew(dom->id, dom->name, dom->uuid, - path, type, status); + disk, type, status); } virObjectEventPtr @@ -1255,7 +1255,7 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, blockJobEvent = (virDomainEventBlockJobPtr)event; ((virConnectDomainEventBlockJobCallback)cb)(conn, dom, - blockJobEvent->path, + blockJobEvent->disk, blockJobEvent->type, blockJobEvent->status, cbopaque); diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 9c41090..e107cc4 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -117,12 +117,12 @@ virDomainEventControlErrorNewFromObj(virDomainObjPtr obj); virObjectEventPtr virDomainEventBlockJobNewFromObj(virDomainObjPtr obj, - const char *path, + const char *disk, int type, int status); virObjectEventPtr virDomainEventBlockJobNewFromDom(virDomainPtr dom, - const char *path, + const char *disk, int type, int status); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b4bf561..c866557 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15135,7 +15135,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * active commit, so we can hardcode the event to pull */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; - event = virDomainEventBlockJobNewFromObj(vm, disk->src->path, type, + event = virDomainEventBlockJobNewFromObj(vm, disk->dst, type, status); } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) { while (1) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e4845ba..68cfd4c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1013,15 +1013,13 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, { virQEMUDriverPtr driver = opaque; virObjectEventPtr event = NULL; - const char *path; virDomainDiskDefPtr disk; virObjectLock(vm); disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { - path = virDomainDiskGetSource(disk); - event = virDomainEventBlockJobNewFromObj(vm, path, type, status); + event = virDomainEventBlockJobNewFromObj(vm, disk->dst, type, status); /* XXX If we completed a block pull or commit, then recompute * the cached backing chain to match. Better would be storing * the chain ourselves rather than reprobing, but this diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 85fe597..8a83a41 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5023,7 +5023,7 @@ remoteDomainBuildEventBlockJobHelper(virConnectPtr conn, if (!dom) return; - event = virDomainEventBlockJobNewFromDom(dom, msg->path, msg->type, + event = virDomainEventBlockJobNewFromDom(dom, msg->disk, msg->type, msg->status); virDomainFree(dom); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1f9d583..e9a23cf 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2352,7 +2352,7 @@ struct remote_domain_event_callback_graphics_msg { struct remote_domain_event_block_job_msg { remote_nonnull_domain dom; - remote_nonnull_string path; + remote_nonnull_string disk; int type; int status; }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5b22049..9904f3d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1797,7 +1797,7 @@ struct remote_domain_event_callback_graphics_msg { }; struct remote_domain_event_block_job_msg { remote_nonnull_domain dom; - remote_nonnull_string path; + remote_nonnull_string disk; int type; int status; }; -- 1.9.3