[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 13/13] Add domain events for graphics network clients



On Fri, Mar 19, 2010 at 03:39:01PM +0000, Daniel P. Berrange wrote:
> This introduces a new event type
> 
>    VIR_DOMAIN_EVENT_ID_GRAPHICS
> 
> The same event can be emitted in 3 scenarioes
> 
>   typedef enum {
>       VIR_DOMAIN_EVENT_GRAPHICS_CONNECT = 0,
>       VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE,
>       VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT,
>   } virDomainEventGraphicsPhase;
> 
> Connect/disconnect are triggered at socket accept/close.
> The initailize phase is immediately after the protocol
> setup and authentication has completed. ie when the
> client is authorized and about to start interacting with
> the graphical desktop
> 
> This event comes with *alot* of potential information
> 
>  - IP address, port & address family of client
>  - IP address, port & address family of server
>  - Authentication scheme (arbitrary string)
>  - Authenticated subject identity. A subject may have
>    multiple identities with some authentication schemes.
>    For example, vencrypt+sasl results in a x509dname
>    and saslUsername identities.
> 
> This results in a very complicated callback :-(
> 
>    typedef enum {
>       VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4,
>       VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV6,
>    } virDomainEventGraphicsAddressType;
> 
>    struct _virDomainEventGraphicsAddress {
>        int family;

    I assume it holds the IPV4/IPV6 enum, could that be indicated

>        const char *node;

    that's a string address ?

>        const char *service;
>    };
>    typedef struct _virDomainEventGraphicsAddress virDomainEventGraphicsAddress;
>    typedef virDomainEventGraphicsAddress *virDomainEventGraphicsAddressPtr;
> 
>    struct _virDomainEventGraphicsSubject {
>       int nidentity;
>       struct {
>           const char *type;
>           const char *name;
>       } *identities;
>    };
>    typedef struct _virDomainEventGraphicsSubject virDomainEventGraphicsSubject;
>    typedef virDomainEventGraphicsSubject *virDomainEventGraphicsSubjectPtr;
> 
>    typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
>                                                          virDomainPtr dom,
>                                                          int phase,
>                                                          virDomainEventGraphicsAddressPtr local,
>                                                          virDomainEventGraphicsAddressPtr remote,
>                                                          const char *authScheme,
>                                                          virDomainEventGraphicsSubjectPtr subject,
>                                                          void *opaque);

  yeah that's very complex, I think all fields should be described and
probably one or two example given, in the header

[...]
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index bffd9ed..4205b91 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1870,6 +1870,46 @@ typedef void (*virConnectDomainEventIOErrorCallback)(virConnectPtr conn,
>                                                       int action,
>                                                       void *opaque);
>  
> +typedef enum {
> +    VIR_DOMAIN_EVENT_GRAPHICS_CONNECT = 0,
> +    VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE,
> +    VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT,
> +} virDomainEventGraphicsPhase;
> +
> +typedef enum {
> +    VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4,
> +    VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV6,
> +} virDomainEventGraphicsAddressType;
> +
> +struct _virDomainEventGraphicsAddress {
> +    int family;
> +    const char *node;
> +    const char *service;
> +};
> +typedef struct _virDomainEventGraphicsAddress virDomainEventGraphicsAddress;
> +typedef virDomainEventGraphicsAddress *virDomainEventGraphicsAddressPtr;
> +
> +struct _virDomainEventGraphicsSubject {
> +    int nidentity;
> +    struct {
> +        const char *type;
> +        const char *name;
> +    } *identities;
> +};
> +typedef struct _virDomainEventGraphicsSubject virDomainEventGraphicsSubject;
> +typedef virDomainEventGraphicsSubject *virDomainEventGraphicsSubjectPtr;
> +
> +typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
> +                                                      virDomainPtr dom,
> +                                                      int phase,
> +                                                      virDomainEventGraphicsAddressPtr local,
> +                                                      virDomainEventGraphicsAddressPtr remote,
> +                                                      const char *authScheme,
> +                                                      virDomainEventGraphicsSubjectPtr subject,
> +                                                      void *opaque);
> +
> +

  Yeah I really think we need far more descriptions here, just guessing
  based on the structure or the fields names won't be sufficient to
  guess how to actually use this.


[...]
> +int qemuMonitorEmitGraphics(qemuMonitorPtr mon,
> +                            int phase,
> +                            int localFamily,
> +                            const char *localNode,
> +                            const char *localService,
> +                            int remoteFamily,
> +                            const char *remoteNode,
> +                            const char *remoteService,
> +                            const char *authScheme,
> +                            const char *x509dname,
> +                            const char *saslUsername)

  maybe in the internal API we should detect the auth type before
getting there and not enumerate with one arg per auth type.
But it's minor at this point since that doesn't touch any API (even
remote)

 ACK,

Daniel

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]