blockjob: use stable disk string in job event

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.

But backwards-compatibility demands that we should not change the
name handed to users unless they explicitly request it.  Therefore,
this patch adds a new event, BLOCK_JOB_2 (alas, I couldn't think of
any nicer name - but at least Migrate2 and Migrate3 are precedent
for a number suffix).  We must double up on emitting both old-style
and new-style events according to what clients have registered for
(see also how IOError and IOErrorReason emits double events, but
there the difference was a larger struct rather than changed
meaning of one of the struct members).

Unfortunately, adding a new event isn't something that can easily
be broken into pieces, so the commit is rather large.

* include/libvirt/libvirt.h.in (virDomainEventID): Add a new id
for VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2.
(virConnectDomainEventBlockJobCallback): Document new semantics.
* src/conf/domain_event.c (_virDomainEventBlockJob): Rename field,
to ensure we catch all clients.
(virDomainEventBlockJobNew): Add parameter.
(virDomainEventBlockJobDispose)
(virDomainEventBlockJobNewFromObj)
(virDomainEventBlockJobNewFromDom)
(virDomainEventDispatchDefaultFunc): Adjust clients.
(virDomainEventBlockJob2NewFromObj)
(virDomainEventBlockJob2NewFromDom): New functions.
* src/conf/domain_event.h: Add new prototypes.
* src/libvirt_private.syms (domain_event.h): Export new functions.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Generate two
different events.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise.
* src/remote/remote_protocol.x
(remote_domain_event_block_job_2_msg): New struct.
(REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2): New RPC.
* src/remote/remote_driver.c
(remoteDomainBuildEventBlockJob2): New handler.
(remoteEvents): Register new event.
* daemon/remote.c (remoteRelayDomainEventBlockJob2): New handler.
(domainEventCallbacks): Register new event.
* tools/virsh-domain.c (vshEventCallbacks): Likewise.
(vshEventBlockJobPrint): Adjust client.
* src/remote_protocol-structs: Regenerate.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/daemon/remote.c b/daemon/remote.c
index 34c96c96f31caae438cfcaa3d87ce27e81efcaa7..7199764c5dfdd10a89c69c56df2a79d55118804a 100644 (file)
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -931,6 +931,44 @@ remoteRelayDomainEventDeviceRemoved(virConnectPtr conn,
 }
 
 
+static int
+remoteRelayDomainEventBlockJob2(virConnectPtr conn,
+                                virDomainPtr dom,
+                                const char *dst,
+                                int type,
+                                int status,
+                                void *opaque)
+{
+    daemonClientEventCallbackPtr callback = opaque;
+    remote_domain_event_block_job_2_msg data;
+
+    if (callback->callbackID < 0 ||
+        !remoteRelayDomainEventCheckACL(callback->client, conn, dom))
+        return -1;
+
+    VIR_DEBUG("Relaying domain block job 2 event %s %d %s %i, %i, callback %d",
+              dom->name, dom->id, dst, type, status, callback->callbackID);
+
+    /* build return data */
+    memset(&data, 0, sizeof(data));
+    data.callbackID = callback->callbackID;
+    if (VIR_STRDUP(data.dst, dst) < 0)
+        goto error;
+    data.type = type;
+    data.status = status;
+    make_nonnull_domain(&data.dom, dom);
+
+    remoteDispatchObjectEventSend(callback->client, remoteProgram,
+                                  REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2,
+                                  (xdrproc_t)xdr_remote_domain_event_block_job_2_msg, &data);
+
+    return 0;
+ error:
+    VIR_FREE(data.dst);
+    return -1;
+}
+
+
 static virConnectDomainEventGenericCallback domainEventCallbacks[] = {
     VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle),
     VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot),
@@ -948,6 +986,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = {
     VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBalloonChange),
     VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk),
     VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved),
+    VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2),
 };
 
 verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index dc88c401c3c936ab72634a998e71896b43be1db5..eb62860efd8022b7f17d79948635427dd2adfa92 100644 (file)
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4852,13 +4852,25 @@ 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 (filename or target
+ *        device, depending on how the callback was registered)
  * @type: type of block job (virDomainBlockJobType)
  * @status: status of the operation (virConnectDomainEventBlockJobStatus)
  * @opaque: application specified data
  *
- * The callback signature to use when registering for an event of type
- * VIR_DOMAIN_EVENT_ID_BLOCK_JOB with virConnectDomainEventRegisterAny()
+ * The string returned for @disk can be used in any of the libvirt API
+ * that operate on a particular disk of the domain, and depends on what
+ * event type was registered with virConnectDomainEventRegisterAny().
+ * If the callback was registered using the older type of
+ * VIR_DOMAIN_EVENT_ID_BLOCK_JOB, then @disk contains the absolute file
+ * name of the host resource for the active layer of the disk; however,
+ * this name is unstable (pivoting via block copy or active block commit
+ * will change which file is active, giving a different name for the two
+ * events associated with the same job) and cannot be relied on if the
+ * active layer is associated with a network resource.  If the callback
+ * was registered using the newer type of VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2,
+ * then @disk will contain the device target shorthand (the <target
+ * dev='...'/> sub-element, such as "vda").
  */
 typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn,
                                                       virDomainPtr dom,
@@ -5062,6 +5074,7 @@ typedef enum {
     VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE = 13, /* virConnectDomainEventBalloonChangeCallback */
     VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */
     VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED = 15, /* virConnectDomainEventDeviceRemovedCallback */
+    VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 = 16,    /* virConnectDomainEventBlockJobCallback */
 
 #ifdef VIR_ENUM_SENTINELS
     VIR_DOMAIN_EVENT_ID_LAST

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index b565732ad362862198108c1f9b4a60065bd21042..73ae2894b82d4dd22ae4e5ba82271aca96e9f5f3 100644 (file)
--- 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; /* path or dst, depending on event id */
     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
@@ -775,10 +775,11 @@ virDomainEventGraphicsNewFromObj(virDomainObjPtr obj,
 }
 
 static virObjectEventPtr
-virDomainEventBlockJobNew(int id,
+virDomainEventBlockJobNew(int event,
+                          int id,
                           const char *name,
                           unsigned char *uuid,
-                          const char *path,
+                          const char *disk,
                           int type,
                           int status)
 {
@@ -788,11 +789,11 @@ virDomainEventBlockJobNew(int id,
         return NULL;
 
     if (!(ev = virDomainEventNew(virDomainEventBlockJobClass,
-                                 VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
+                                 event,
                                  id, name, uuid)))
         return NULL;
 
-    if (VIR_STRDUP(ev->path, path) < 0) {
+    if (VIR_STRDUP(ev->disk, disk) < 0) {
         virObjectUnref(ev);
         return NULL;
     }
@@ -808,7 +809,8 @@ virDomainEventBlockJobNewFromObj(virDomainObjPtr obj,
                                  int type,
                                  int status)
 {
-    return virDomainEventBlockJobNew(obj->def->id, obj->def->name,
+    return virDomainEventBlockJobNew(VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
+                                     obj->def->id, obj->def->name,
                                      obj->def->uuid, path, type, status);
 }
 
@@ -818,10 +820,33 @@ virDomainEventBlockJobNewFromDom(virDomainPtr dom,
                                  int type,
                                  int status)
 {
-    return virDomainEventBlockJobNew(dom->id, dom->name, dom->uuid,
+    return virDomainEventBlockJobNew(VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
+                                     dom->id, dom->name, dom->uuid,
                                      path, type, status);
 }
 
+virObjectEventPtr
+virDomainEventBlockJob2NewFromObj(virDomainObjPtr obj,
+                                  const char *dst,
+                                  int type,
+                                  int status)
+{
+    return virDomainEventBlockJobNew(VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2,
+                                     obj->def->id, obj->def->name,
+                                     obj->def->uuid, dst, type, status);
+}
+
+virObjectEventPtr
+virDomainEventBlockJob2NewFromDom(virDomainPtr dom,
+                                  const char *dst,
+                                  int type,
+                                  int status)
+{
+    return virDomainEventBlockJobNew(VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2,
+                                     dom->id, dom->name, dom->uuid,
+                                     dst, type, status);
+}
+
 virObjectEventPtr
 virDomainEventControlErrorNewFromDom(virDomainPtr dom)
 {
@@ -1250,12 +1275,13 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
         goto cleanup;
 
     case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
+    case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
         {
             virDomainEventBlockJobPtr blockJobEvent;
 
             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 9c41090afb193a2ef4e64ba52e6fe78067b2ea68..a3330ca804fda4b85995956e98b8cc7d7b5c176d 100644 (file)
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -126,6 +126,17 @@ virDomainEventBlockJobNewFromDom(virDomainPtr dom,
                                  int type,
                                  int status);
 
+virObjectEventPtr
+virDomainEventBlockJob2NewFromObj(virDomainObjPtr obj,
+                                  const char *dst,
+                                  int type,
+                                  int status);
+virObjectEventPtr
+virDomainEventBlockJob2NewFromDom(virDomainPtr dom,
+                                  const char *dst,
+                                  int type,
+                                  int status);
+
 virObjectEventPtr
 virDomainEventDiskChangeNewFromObj(virDomainObjPtr obj,
                                    const char *oldSrcPath,

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 29a9ed1cee0a9e412c4ab2b0feba7b12bf0e32e6..9e25b8a17a455779e8436b25a5dd6d448e16686a 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,8 @@ virDomainXMLOptionNew;
 # conf/domain_event.h
 virDomainEventBalloonChangeNewFromDom;
 virDomainEventBalloonChangeNewFromObj;
+virDomainEventBlockJob2NewFromDom;
+virDomainEventBlockJob2NewFromObj;
 virDomainEventBlockJobNewFromDom;
 virDomainEventBlockJobNewFromObj;
 virDomainEventControlErrorNewFromDom;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4ab5a7bbc4843820e79b113902920b0e7eae7be8..ca58d6bfd1c2dea188ce9ebfa96368e47306540d 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15027,6 +15027,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     int ret = -1;
     bool async = false;
     virObjectEventPtr event = NULL;
+    virObjectEventPtr event2 = NULL;
     int idx;
     virDomainDiskDefPtr disk;
     virStorageSourcePtr baseSource = NULL;
@@ -15130,11 +15131,14 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     if (mode == BLOCK_JOB_ABORT) {
         if (!async) {
             /* Older qemu that lacked async reporting also lacked
-             * active commit, so we can hardcode the event to pull */
+             * active commit, so we can hardcode the event to pull.
+             * We have to generate two variants of the event. */
             int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
             int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
             event = virDomainEventBlockJobNewFromObj(vm, disk->src->path, type,
                                                      status);
+            event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
+                                                       status);
         } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
             while (1) {
                 /* Poll every 50ms */
@@ -15178,6 +15182,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
         virObjectUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
+    if (event2)
+        qemuDomainEventQueue(driver, event2);
     return ret;
 }
 

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e4845ba264d0d9e92c1e4f0774044b9b12378ee3..f1c0041a11c961ffd8a3b2aff0d3b22957ac81e7 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1013,6 +1013,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 {
     virQEMUDriverPtr driver = opaque;
     virObjectEventPtr event = NULL;
+    virObjectEventPtr event2 = NULL;
     const char *path;
     virDomainDiskDefPtr disk;
 
@@ -1020,8 +1021,12 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias);
 
     if (disk) {
+        /* Have to generate two variants of the event for old vs. new
+         * client callbacks */
         path = virDomainDiskGetSource(disk);
         event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
+        event2 = virDomainEventBlockJob2NewFromObj(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
@@ -1048,6 +1053,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 
     if (event)
         qemuDomainEventQueue(driver, event);
+    if (event2)
+        qemuDomainEventQueue(driver, event2);
 
     return 0;
 }

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 85fe597b5e64424577f3a1c7658cb2f5533fb9a5..de75702c03b1dff004687c755db8fc56e2370836 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -320,6 +320,11 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog,
                                             virNetClientPtr client,
                                             void *evdata, void *opaque);
 
+static void
+remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog,
+                                virNetClientPtr client,
+                                void *evdata, void *opaque);
+
 static void
 remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
                                  virNetClientPtr client ATTRIBUTE_UNUSED,
@@ -467,6 +472,10 @@ static virNetClientProgramEvent remoteEvents[] = {
       remoteDomainBuildEventCallbackDeviceRemoved,
       sizeof(remote_domain_event_callback_device_removed_msg),
       (xdrproc_t)xdr_remote_domain_event_callback_device_removed_msg },
+    { REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2,
+      remoteDomainBuildEventBlockJob2,
+      sizeof(remote_domain_event_block_job_2_msg),
+      (xdrproc_t)xdr_remote_domain_event_block_job_2_msg },
 };
 
 
@@ -5048,6 +5057,28 @@ remoteDomainBuildEventCallbackBlockJob(virNetClientProgramPtr prog ATTRIBUTE_UNU
     remote_domain_event_callback_block_job_msg *msg = evdata;
     remoteDomainBuildEventBlockJobHelper(conn, &msg->msg, msg->callbackID);
 }
+static void
+remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
+                                virNetClientPtr client ATTRIBUTE_UNUSED,
+                                void *evdata, void *opaque)
+{
+    virConnectPtr conn = opaque;
+    remote_domain_event_block_job_2_msg *msg = evdata;
+    struct private_data *priv = conn->privateData;
+    virDomainPtr dom;
+    virObjectEventPtr event = NULL;
+
+    dom = get_nonnull_domain(conn, msg->dom);
+    if (!dom)
+        return;
+
+    event = virDomainEventBlockJob2NewFromDom(dom, msg->dst, msg->type,
+                                              msg->status);
+
+    virDomainFree(dom);
+
+    remoteEventQueue(priv, event, msg->callbackID);
+}
 
 static void
 remoteDomainBuildEventGraphicsHelper(virConnectPtr conn,

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index ab9b83dc48048a02698c9c3b4476b765a3a67a5b..ce28607b5e427f92380a63e307cffd540fa11c90 100644 (file)
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2948,6 +2948,14 @@ struct remote_domain_event_callback_device_removed_msg {
     remote_domain_event_device_removed_msg msg;
 };
 
+struct remote_domain_event_block_job_2_msg {
+    int callbackID;
+    remote_nonnull_domain dom;
+    remote_nonnull_string dst;
+    int type;
+    int status;
+};
+
 struct remote_connect_get_cpu_model_names_args {
     remote_nonnull_string arch;
     int need_results;
@@ -5338,5 +5346,11 @@ enum remote_procedure {
      * @generate: both
      * @acl: domain:set_time
      */
-    REMOTE_PROC_DOMAIN_SET_TIME = 338
+    REMOTE_PROC_DOMAIN_SET_TIME = 338,
+
+    /**
+     * @generate: none
+     * @acl: none
+     */
+    REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2 = 339
 };

diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 5b22049629e0cb4865c83ac9de0d10ffea5bc429..08540215d425faba7dfd2c95d24abaf04e1b6b72 100644 (file)
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2413,6 +2413,13 @@ struct remote_domain_event_callback_device_removed_msg {
         int                        callbackID;
         remote_domain_event_device_removed_msg msg;
 };
+struct remote_domain_event_block_job_2_msg {
+        int                        callbackID;
+        remote_nonnull_domain      dom;
+        remote_nonnull_string      dst;
+        int                        type;
+        int                        status;
+};
 struct remote_connect_get_cpu_model_names_args {
         remote_nonnull_string      arch;
         int                        need_results;
@@ -2802,4 +2809,5 @@ enum remote_procedure {
         REMOTE_PROC_DOMAIN_FSTHAW = 336,
         REMOTE_PROC_DOMAIN_GET_TIME = 337,
         REMOTE_PROC_DOMAIN_SET_TIME = 338,
+        REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2 = 339,
 };

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6b3dd7001a049bd16b299d0b3eeb49e3fe8065d3..d1368627d0baceaeb2c3ac839d64126753bf1a20 100644 (file)
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10921,8 +10921,9 @@ vshEventBlockJobPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     if (!data->loop && *data->count)
         return;
-    vshPrint(data->ctl, _("event 'block-job' for domain %s: %s for %s %s\n"),
-             virDomainGetName(dom), vshDomainBlockJobToString(type),
+    vshPrint(data->ctl, _("event '%s' for domain %s: %s for %s %s\n"),
+             data->cb->name, virDomainGetName(dom),
+             vshDomainBlockJobToString(type),
              disk, vshDomainBlockJobStatusToString(status));
     (*data->count)++;
     if (!data->loop)
@@ -11049,6 +11050,8 @@ static vshEventCallback vshEventCallbacks[] = {
       VIR_DOMAIN_EVENT_CALLBACK(vshEventPMChangePrint), },
     { "device-removed",
       VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceRemovedPrint), },
+    { "block-job-2",
+      VIR_DOMAIN_EVENT_CALLBACK(vshEventBlockJobPrint), },
 };
 verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));