[libvirt] [PATCH 12/14] Switch over to passing a callback table to QEMU monitor

[Daniel Veillard]

On Thu, Nov 26, 2009 at 06:27:30PM +0000, Daniel P. Berrange wrote:
> With addition of events there will be alot of callbacks.
> To avoid having to add many APIs to register callbacks,
> provide them all at once in a big table
> 
> * src/qemu/qemu_driver.c: Pass in a callback table to QEMU
>   monitor code
> * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h Replace
>   the EOF and disk secret callbacks with a callback table
> ---
>  src/qemu/qemu_driver.c  |   10 ++++++----
>  src/qemu/qemu_monitor.c |   31 ++++++++++++++++++-------------
>  src/qemu/qemu_monitor.h |   39 ++++++++++++++++++++-------------------
>  3 files changed, 44 insertions(+), 36 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e3759bf..bf4557e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -779,6 +779,11 @@ cleanup:
>      return ret;
>  }
>  
> +static qemuMonitorCallbacks monitorCallbacks = {
> +    .eofNotify = qemuHandleMonitorEOF,
> +    .diskSecretLookup = findVolumeQcowPassphrase,
> +};
> +
>  static int
>  qemuConnectMonitor(virDomainObjPtr vm)
>  {
> @@ -787,14 +792,11 @@ qemuConnectMonitor(virDomainObjPtr vm)
>      if ((priv->mon = qemuMonitorOpen(vm,
>                                       priv->monConfig,
>                                       priv->monJSON,
> -                                     qemuHandleMonitorEOF)) == NULL) {
> +                                     &monitorCallbacks)) == NULL) {
>          VIR_ERROR(_("Failed to connect monitor for %s\n"), vm->def->name);
>          return -1;
>      }
>  
> -    qemuMonitorRegisterDiskSecretLookup(priv->mon,
> -                                        findVolumeQcowPassphrase);
> -
>      return 0;
>  }
>  
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 502b389..11025a7 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -53,8 +53,7 @@ struct _qemuMonitor {
>  
>      virDomainObjPtr vm;
>  
> -    qemuMonitorEOFNotify eofCB;
> -    qemuMonitorDiskSecretLookup secretCB;
> +    qemuMonitorCallbacksPtr cb;
>  
>      /* If there's a command being processed this will be
>       * non-NULL */
> @@ -523,7 +522,7 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
>          virCondSignal(&mon->notify);
>          qemuMonitorUnlock(mon);
>          VIR_DEBUG("Triggering EOF callback error? %d", failed);
> -        mon->eofCB(mon, mon->vm, failed);
> +        mon->cb->eofNotify(mon, mon->vm, failed);
>  
>          qemuMonitorLock(mon);
>      }
> @@ -542,10 +541,16 @@ qemuMonitorPtr
>  qemuMonitorOpen(virDomainObjPtr vm,
>                  virDomainChrDefPtr config,
>                  int json,
> -                qemuMonitorEOFNotify eofCB)
> +                qemuMonitorCallbacksPtr cb)
>  {
>      qemuMonitorPtr mon;
>  
> +    if (!cb || !cb->eofNotify) {
> +        qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> +                         _("EOF notify callback must be supplied"));
> +        return NULL;
> +    }
> +
>      if (VIR_ALLOC(mon) < 0) {
>          virReportOOMError(NULL);
>          return NULL;
> @@ -567,8 +572,8 @@ qemuMonitorOpen(virDomainObjPtr vm,
>      mon->fd = -1;
>      mon->refs = 1;
>      mon->vm = vm;
> -    mon->eofCB = eofCB;
>      mon->json = json;
> +    mon->cb = cb;
>      qemuMonitorLock(mon);
>      virDomainObjRef(vm);
>  
> @@ -655,13 +660,6 @@ int qemuMonitorClose(qemuMonitorPtr mon)
>  }
>  
>  
> -void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon,
> -                                         qemuMonitorDiskSecretLookup secretCB)
> -{
> -    mon->secretCB = secretCB;
> -}
> -
> -
>  int qemuMonitorSend(qemuMonitorPtr mon,
>                      qemuMonitorMessagePtr msg)
>  {
> @@ -697,10 +695,17 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
>                               char **secret,
>                               size_t *secretLen)
>  {
> +    int ret = -1;
>      *secret = NULL;
>      *secretLen = 0;
>  
> -    return mon->secretCB(mon, conn, mon->vm, path, secret, secretLen);
> +    qemuMonitorRef(mon);
> +    qemuMonitorUnlock(mon);
> +    if (mon->cb && mon->cb->diskSecretLookup)
> +        ret = mon->cb->diskSecretLookup(mon, conn, mon->vm, path, secret, secretLen);
> +    qemuMonitorLock(mon);
> +    qemuMonitorUnref(mon);
> +    return ret;
>  }
>  
>  
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 0d9e315..7aa6ee5 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -59,21 +59,25 @@ struct _qemuMonitorMessage {
>      void *passwordOpaque;
>  };
>  
> -typedef void (*qemuMonitorEOFNotify)(qemuMonitorPtr mon,
> -                                     virDomainObjPtr vm,
> -                                     int withError);
> -
> -/* XXX we'd really like to avoid virCOnnectPtr here
> - * It is required so the callback can find the active
> - * secret driver. Need to change this to work like the
> - * security drivers do, to avoid this
> - */
> -typedef int (*qemuMonitorDiskSecretLookup)(qemuMonitorPtr mon,
> -                                           virConnectPtr conn,
> -                                           virDomainObjPtr vm,
> -                                           const char *path,
> -                                           char **secret,
> -                                           size_t *secretLen);
> +typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
> +typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
> +struct _qemuMonitorCallbacks {
> +    void (*eofNotify)(qemuMonitorPtr mon,
> +                      virDomainObjPtr vm,
> +                      int withError);
> +    /* XXX we'd really like to avoid virCOnnectPtr here
> +     * It is required so the callback can find the active
> +     * secret driver. Need to change this to work like the
> +     * security drivers do, to avoid this
> +     */
> +    int (*diskSecretLookup)(qemuMonitorPtr mon,
> +                            virConnectPtr conn,
> +                            virDomainObjPtr vm,
> +                            const char *path,
> +                            char **secret,
> +                            size_t *secretLen);
> +};
> +
>  
>  char *qemuMonitorEscapeArg(const char *in);
>  char *qemuMonitorEscapeShell(const char *in);
> @@ -81,7 +85,7 @@ char *qemuMonitorEscapeShell(const char *in);
>  qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
>                                 virDomainChrDefPtr config,
>                                 int json,
> -                               qemuMonitorEOFNotify eofCB);
> +                               qemuMonitorCallbacksPtr cb);
>  
>  int qemuMonitorClose(qemuMonitorPtr mon);
>  
> @@ -91,9 +95,6 @@ void qemuMonitorUnlock(qemuMonitorPtr mon);
>  int qemuMonitorRef(qemuMonitorPtr mon);
>  int qemuMonitorUnref(qemuMonitorPtr mon);
>  
> -void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon,
> -                                         qemuMonitorDiskSecretLookup secretCB);
> -
>  /* This API is for use by the internal Text/JSON monitor impl code only */
>  int qemuMonitorSend(qemuMonitorPtr mon,
>                      qemuMonitorMessagePtr msg);

 ACK, too bad that patch is slightly dependant on the json changes,
it might be useful to apply independantly,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/