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

Re: [libvirt] [PATCH 10/11] Convert Xen domain stats/peek driver methods to use virDomainDefPtr



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> Introduce use of a virDomainDefPtr in the domain stats &
> peek APIs to simplify introduction of ACL security checks.
> The virDomainPtr cannot be safely used, since the app
> may have supplied mis-matching name/uuid/id fields. eg
> the name points to domain X, while the uuid points to
> domain Y. Resolving the virDomainPtr to a virDomainDefPtr
> ensures a consistent name/uuid/id set.
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/xen/block_stats.c    |  6 +++---
>  src/xen/block_stats.h    |  2 +-
>  src/xen/xen_driver.c     | 37 +++++++++++++++++++++++++++++++++----
>  src/xen/xen_hypervisor.c | 11 ++++++-----
>  src/xen/xen_hypervisor.h |  9 +++++----
>  src/xen/xend_internal.c  | 21 +++++++++++----------
>  src/xen/xend_internal.h  |  7 ++++++-
>  src/xen/xm_internal.c    |  3 ++-
>  src/xen/xm_internal.h    |  7 ++++++-
>  9 files changed, 73 insertions(+), 30 deletions(-)
>
> diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c
> index ded8d7f..56a3901 100644
> --- a/src/xen/block_stats.c
> +++ b/src/xen/block_stats.c
> @@ -359,16 +359,16 @@ xenLinuxDomainDeviceID(int domid, const char *path)
>  
>  int
>  xenLinuxDomainBlockStats(xenUnifiedPrivatePtr priv,
> -                         virDomainPtr dom,
> +                         virDomainDefPtr def,
>                           const char *path,
>                           struct _virDomainBlockStats *stats)
>  {
> -    int device = xenLinuxDomainDeviceID(dom->id, path);
> +    int device = xenLinuxDomainDeviceID(def->id, path);
>  
>      if (device < 0)
>          return -1;
>  
> -    return read_bd_stats(priv, device, dom->id, stats);
> +    return read_bd_stats(priv, device, def->id, stats);
>  }
>  
>  #endif /* __linux__ */
> diff --git a/src/xen/block_stats.h b/src/xen/block_stats.h
> index 0a3c40a..6633d97 100644
> --- a/src/xen/block_stats.h
> +++ b/src/xen/block_stats.h
> @@ -28,7 +28,7 @@
>  #  include "xen_driver.h"
>  
>  extern int xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv,
> -                                     virDomainPtr dom, const char *path,
> +                                     virDomainDefPtr def, const char *path,
>                                       struct _virDomainBlockStats *stats);
>  
>  extern int xenLinuxDomainDeviceID(int domid, const char *dev);
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 5ab1a52..7c00b70 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1957,14 +1957,34 @@ static int
>  xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path,
>                             struct _virDomainBlockStats *stats)
>  {
> -    return xenHypervisorDomainBlockStats(dom, path, stats);
> +    virDomainDefPtr def = NULL;
> +    int ret = -1;
> +
> +    if (!(def = xenGetDomainDefForDom(dom)))
> +        goto cleanup;
> +
> +    ret = xenHypervisorDomainBlockStats(dom->conn, def, path, stats);
> +
> +cleanup:
> +    virDomainDefFree(def);
> +    return ret;
>  }
>  
>  static int
>  xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path,
>                                 struct _virDomainInterfaceStats *stats)
>  {
> -    return xenHypervisorDomainInterfaceStats(dom, path, stats);
> +    virDomainDefPtr def = NULL;
> +    int ret = -1;
> +
> +    if (!(def = xenGetDomainDefForDom(dom)))
> +        goto cleanup;
> +
> +    ret = xenHypervisorDomainInterfaceStats(def, path, stats);
> +
> +cleanup:
> +    virDomainDefFree(def);
> +    return ret;
>  }
>  
>  static int
> @@ -1973,13 +1993,22 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const char *path,
>                            void *buffer, unsigned int flags)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> +    virDomainDefPtr def = NULL;
> +    int ret = -1;
>  
>      virCheckFlags(0, -1);
>  
> +    if (!(def = xenGetDomainDefForDom(dom)))
> +        goto cleanup;
> +
>      if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return xenXMDomainBlockPeek(dom, path, offset, size, buffer);
> +        ret = xenXMDomainBlockPeek(dom->conn, def, path, offset, size, buffer);
>      else
> -        return xenDaemonDomainBlockPeek(dom, path, offset, size, buffer);
> +        ret = xenDaemonDomainBlockPeek(dom->conn, def, path, offset, size, buffer);
> +
> +cleanup:
> +    virDomainDefFree(def);
> +    return ret;
>  }
>  
>  static int
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 2525566..612ac77 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1368,17 +1368,18 @@ xenHypervisorSetSchedulerParameters(virConnectPtr conn,
>  
>  
>  int
> -xenHypervisorDomainBlockStats(virDomainPtr dom,
> +xenHypervisorDomainBlockStats(virConnectPtr conn,
> +                              virDomainDefPtr def,
>                                const char *path,
>                                struct _virDomainBlockStats *stats)
>  {
>  #ifdef __linux__
> -    xenUnifiedPrivatePtr priv = dom->conn->privateData;
> +    xenUnifiedPrivatePtr priv = conn->privateData;
>      int ret;
>  
>      xenUnifiedLock(priv);
>      /* Need to lock because it hits the xenstore handle :-( */
> -    ret = xenLinuxDomainBlockStats(priv, dom, path, stats);
> +    ret = xenLinuxDomainBlockStats(priv, def, path, stats);
>      xenUnifiedUnlock(priv);
>      return ret;
>  #else
> @@ -1396,7 +1397,7 @@ xenHypervisorDomainBlockStats(virDomainPtr dom,
>   * virNetwork interface, as yet not decided.
>   */
>  int
> -xenHypervisorDomainInterfaceStats(virDomainPtr dom,
> +xenHypervisorDomainInterfaceStats(virDomainDefPtr def,
>                                    const char *path,
>                                    struct _virDomainInterfaceStats *stats)
>  {
> @@ -1411,7 +1412,7 @@ xenHypervisorDomainInterfaceStats(virDomainPtr dom,
>                         _("invalid path, should be vif<domid>.<n>."));
>          return -1;
>      }
> -    if (rqdomid != dom->id) {
> +    if (rqdomid != def->id) {
>          virReportError(VIR_ERR_INVALID_ARG, "%s",
>                         _("invalid path, vif<domid> should match this domain ID"));
>          return -1;
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 1e5bb67..6aeab79 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -122,13 +122,14 @@ int     xenHypervisorSetSchedulerParameters(virConnectPtr conn,
>                                              int nparams)
>            ATTRIBUTE_NONNULL (1);
>  
> -int     xenHypervisorDomainBlockStats   (virDomainPtr domain,
> +int     xenHypervisorDomainBlockStats   (virConnectPtr conn,
> +                                         virDomainDefPtr def,
>                                           const char *path,
>                                           struct _virDomainBlockStats *stats)
>            ATTRIBUTE_NONNULL (1);
> -int     xenHypervisorDomainInterfaceStats (virDomainPtr domain,
> -                                         const char *path,
> -                                         struct _virDomainInterfaceStats *stats)
> +int     xenHypervisorDomainInterfaceStats (virDomainDefPtr def,
> +                                           const char *path,
> +                                           struct _virDomainInterfaceStats *stats)
>            ATTRIBUTE_NONNULL (1);
>  
>  int     xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 3bcd19b..273408d 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -3247,13 +3247,14 @@ error:
>   * Returns 0 if successful, -1 if error
>   */
>  int
> -xenDaemonDomainBlockPeek(virDomainPtr domain,
> +xenDaemonDomainBlockPeek(virConnectPtr conn,
> +                         virDomainDefPtr minidef,
>   

Function comments need updated.

ACK.

Regards,
Jim


>                           const char *path,
>                           unsigned long long offset,
>                           size_t size,
>                           void *buffer)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> +    xenUnifiedPrivatePtr priv = conn->privateData;
>      struct sexpr *root = NULL;
>      int fd = -1, ret = -1;
>      virDomainDefPtr def;
> @@ -3263,12 +3264,12 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>      const char *actual;
>  
>      /* Security check: The path must correspond to a block device. */
> -    if (domain->id > 0)
> -        root = sexpr_get(domain->conn, "/xend/domain/%d?detail=1",
> -                         domain->id);
> -    else if (domain->id < 0)
> -        root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1",
> -                         domain->name);
> +    if (minidef->id > 0)
> +        root = sexpr_get(conn, "/xend/domain/%d?detail=1",
> +                         minidef->id);
> +    else if (minidef->id < 0)
> +        root = sexpr_get(conn, "/xend/domain/%s?detail=1",
> +                         minidef->name);
>      else {
>          /* This call always fails for dom0. */
>          virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -3283,8 +3284,8 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>  
>      id = xenGetDomIdFromSxpr(root, priv->xendConfigVersion);
>      xenUnifiedLock(priv);
> -    tty = xenStoreDomainGetConsolePath(domain->conn, id);
> -    vncport = xenStoreDomainGetVNCPort(domain->conn, id);
> +    tty = xenStoreDomainGetConsolePath(conn, id);
> +    vncport = xenStoreDomainGetVNCPort(conn, id);
>      xenUnifiedUnlock(priv);
>  
>      if (!(def = xenParseSxpr(root, priv->xendConfigVersion, NULL, tty,
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index cef7da4..aa05130 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -187,7 +187,12 @@ int xenDaemonDomainMigratePerform (virConnectPtr conn,
>                                     const char *uri, unsigned long flags,
>                                     const char *dname, unsigned long resource);
>  
> -int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer);
> +int xenDaemonDomainBlockPeek(virConnectPtr conn,
> +                             virDomainDefPtr def,
> +                             const char *path,
> +                             unsigned long long offset,
> +                             size_t size,
> +                             void *buffer);
>  
>  char * xenDaemonGetSchedulerType(virConnectPtr conn,
>                                   int *nparams);
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index bc98cf1..740c4df 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -1404,7 +1404,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn,
>  }
>  
>  int
> -xenXMDomainBlockPeek(virDomainPtr dom ATTRIBUTE_UNUSED,
> +xenXMDomainBlockPeek(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                     virDomainDefPtr def ATTRIBUTE_UNUSED,
>                       const char *path ATTRIBUTE_UNUSED,
>                       unsigned long long offset ATTRIBUTE_UNUSED,
>                       size_t size ATTRIBUTE_UNUSED,
> diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
> index 78cd15c..25b4fd5 100644
> --- a/src/xen/xm_internal.h
> +++ b/src/xen/xm_internal.h
> @@ -80,7 +80,12 @@ int xenXMDomainCreate(virConnectPtr conn,
>  int xenXMDomainDefineXML(virConnectPtr con, virDomainDefPtr def);
>  int xenXMDomainUndefine(virConnectPtr conn, virDomainDefPtr def);
>  
> -int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer);
> +int xenXMDomainBlockPeek(virConnectPtr conn,
> +                         virDomainDefPtr def,
> +                         const char *path,
> +                         unsigned long long offset,
> +                         size_t size,
> +                         void *buffer);
>  
>  int xenXMDomainGetAutostart(virDomainDefPtr def,
>                              int *autostart);
>   


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