Re: [PATCH v5] libxl: implement virDomainBlockStats

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

 




On 07/26/2016 10:42 PM, Jim Fehlig wrote:
> On 07/25/2016 05:45 PM, Joao Martins wrote:
>> Introduce initial support for domainBlockStats API call that
>> allow us to query block device statistics. openstack nova
> 
> s/openstack/OpenStack/
> 
>> uses this API call to query block statistics, alongside
>> virDomainMemoryStats and virDomainInterfaceStats.  Note that
>> this patch only introduces it for VBD for starters. QDisk
>> would come in a separate patch series.
>>
>> A new statistics data structure is introduced to fit common
>> statistics among others specific to the underlying block
>> backends. For the VBD statistics on linux these are exported
>> via sysfs on the path:
>>
>> "/sys/bus/xen-backend/devices/vbd-<domid>-<devid>/statistics"
>>
>> To calculate the block devno libxlDiskPathToID is introduced.
>> Each backend implements its own function to extract statistics
>> and let there be handled the different platforms.
> 
> I reworded this to
> 
>     Each backend implements its own function to extract statistics,
>     allowing support for multiple backends and different platforms.
> 
>>
>> VBD stats are exposed in reqs and number of sectors from
>> blkback, and it's up to us to convert it to sector sizes.
>> The sector size is gathered through xenstore in the device
>> backend entry "physical-sector-size".
>>
>> BlockStatsFlags variant is also implemented which has the
>> added benefit of getting the number of flush requests.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>> ---
>> Changes since v4:
>>  - No need to handle virtpath as devid - return id = -1 instead
>>  - Rework compilation of BlockStatsVBD compilation on other platforms.
>>  - Use disk->dst as opposed to plain path to also cover the case when
>>  input virtpath is path to file. Before it was returning "cannot find
>>  device number"
>>
>> Changes since v3:
>>  - No need for error in libxlDiskSectorSize()
>>  - Fix an identation issue.
>>  - Rework cleanup paths on libxlDiskSectorSize()
>>  - Do not unlock vm if libxlDomainObjEndJob() returns false and
>>  adjust to the newly introduced virDomainObjEndAPI()
>>  - Bump version to 2.1.0
>>
>> Changes since v1:
>>  - Fix identation issues
>>  - Set ret to LIBXL_VBD_SECTOR_SIZE
>>  - Reuse VIR_STRDUP error instead of doing virReportError
>>  when we fail to set stats->backend
>>  - Change virAsprintf(...) error checking
>>  - Change error to VIR_ERR_OPERATION_FAILED when xenstore path
>>  does not exist and when failing to read stat.
>>  - Resolve issues with 'make syntax-check' with cppi installed.
>>  - Remove libxlDiskPathMatches in favor of using virutil
>>  virDiskNameParse to fetch disk and partition index.
>>  - Rename libxlDiskPathParse to libxlDiskPathToID and rework
>>  function to just convert disk and partition index to devno.
>>  - Bump version to 1.2.22
>> ---
>>  src/libxl/libxl_driver.c | 367 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 367 insertions(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index f153f69..b33c898 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -30,6 +30,7 @@
>>  #include <math.h>
>>  #include <libxl.h>
>>  #include <libxl_utils.h>
>> +#include <xenstore.h>
>>  #include <fcntl.h>
>>  #include <regex.h>
>>  
>> @@ -72,6 +73,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
>>  #define LIBXL_DOM_REQ_HALT     4
>>  
>>  #define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1
>> +#define LIBXL_NB_TOTAL_BLK_STAT_PARAM 6
>>  
>>  #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities"
>>  #define HYPERVISOR_XENSTORED "/dev/xen/xenstored"
>> @@ -100,6 +102,25 @@ struct _libxlOSEventHookInfo {
>>      int id;
>>  };
>>  
>> +/* Object used to store disk statistics across multiple xen backends */
>> +typedef struct _libxlBlockStats libxlBlockStats;
>> +typedef libxlBlockStats *libxlBlockStatsPtr;
>> +struct _libxlBlockStats {
>> +    long long rd_req;
>> +    long long rd_bytes;
>> +    long long wr_req;
>> +    long long wr_bytes;
>> +    long long f_req;
>> +
>> +    char *backend;
>> +    union {
>> +        struct {
>> +            long long ds_req;
>> +            long long oo_req;
>> +        } vbd;
>> +    } u;
>> +};
>> +
>>  /* Function declarations */
>>  static int
>>  libxlDomainManagedSaveLoad(virDomainObjPtr vm,
>> @@ -5031,6 +5052,350 @@ libxlDomainGetJobStats(virDomainPtr dom,
>>  }
>>  
>>  static int
>> +libxlDiskPathToID(const char *virtpath)
>> +{
>> +    static char const* drive_prefix[] = {"xvd", "hd", "sd"};
>> +    int disk, partition, chrused;
>> +    int fmt, id;
>> +    size_t i;
>> +
>> +    fmt = id = -1;
>> +
>> +    /* Find any disk prefixes we know about */
>> +    for (i = 0; i < ARRAY_CARDINALITY(drive_prefix); i++) {
>> +        if (STRPREFIX(virtpath, drive_prefix[i]) &&
>> +            !virDiskNameParse(virtpath, &disk, &partition)) {
>> +            fmt = i;
>> +            break;
>> +        }
>> +    }
>> +
>> +    /* Handle it same way as xvd */
>> +    if (fmt < 0 &&
>> +        (sscanf(virtpath, "d%ip%i%n", &disk, &partition, &chrused) >= 2
>> +         && chrused == strlen(virtpath)))
>> +        fmt = 0;
>> +
>> +    /* Test indexes ranges and calculate the device id */
>> +    switch (fmt) {
>> +    case 0: /* xvd */
>> +        if (disk <= 15 && partition <= 15)
>> +            id = (202 << 8) | (disk << 4) | partition;
>> +        else if ((disk <= ((1<<20)-1)) || partition <= 255)
>> +            id = (1 << 28) | (disk << 8) | partition;
>> +        break;
>> +    case 1: /* hd */
>> +        if (disk <= 3 && partition <= 63)
>> +            id = ((disk < 2 ? 3 : 22) << 8) | ((disk & 1) << 6) | partition;
>> +        break;
>> +    case 2: /* sd */
>> +        if (disk <= 15 && (partition <= 15))
>> +            id = (8 << 8) | (disk << 4) | partition;
>> +        break;
>> +    default: /* invalid */
>> +        break;
>> +    }
>> +    return id;
>> +}
>> +
>> +#define LIBXL_VBD_SECTOR_SIZE 512
>> +
>> +static int
>> +libxlDiskSectorSize(int domid, int devno)
>> +{
>> +    char *path, *val;
>> +    struct xs_handle *handle;
>> +    int ret = LIBXL_VBD_SECTOR_SIZE;
>> +    unsigned int len;
>> +
>> +    handle = xs_daemon_open_readonly();
>> +    if (!handle) {
>> +        VIR_WARN("cannot read sector size");
>> +        return ret;
>> +    }
>> +
>> +    path = val = NULL;
>> +    if (virAsprintf(&path, "/local/domain/%d/device/vbd/%d/backend",
>> +                    domid, devno) < 0)
>> +        goto cleanup;
>> +
>> +    if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL)
>> +        goto cleanup;
>> +
>> +    VIR_FREE(path);
>> +    if (virAsprintf(&path, "%s/physical-sector-size", val) < 0)
>> +        goto cleanup;
>> +
>> +    VIR_FREE(val);
>> +    if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL)
>> +        goto cleanup;
>> +
>> +    if (sscanf(val, "%d", &ret) != 1)
>> +        ret = LIBXL_VBD_SECTOR_SIZE;
>> +
>> + cleanup:
>> +    VIR_FREE(val);
>> +    VIR_FREE(path);
>> +    xs_daemon_close(handle);
>> +    return ret;
>> +}
>> +
>> +#ifdef __linux__
>> +static int
>> +libxlDomainBlockStatsVBD(virDomainObjPtr vm,
>> +                         const char *dev,
>> +                         libxlBlockStatsPtr stats)
>> +{
>> +    int ret = -1;
>> +    int devno = libxlDiskPathToID(dev);
>> +    int size = libxlDiskSectorSize(vm->def->id, devno);
> 
> To avoid calling libxlDiskSectorSize() on an invalid devno, I moved it after the
> devno validity check.
> 
> Looks good otherwise. ACK. I've pushed it with the tweaked commit message and
> below diff squashed in.
Awesome, Thank you!

Cheers,
Joao

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b33c898..cb501cf 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5148,7 +5148,7 @@ libxlDomainBlockStatsVBD(virDomainObjPtr vm,
>  {
>      int ret = -1;
>      int devno = libxlDiskPathToID(dev);
> -    int size = libxlDiskSectorSize(vm->def->id, devno);
> +    int size;
>      char *path, *name, *val;
>      unsigned long long stat;
>  
> @@ -5158,6 +5158,9 @@ libxlDomainBlockStatsVBD(virDomainObjPtr vm,
>                         "%s", _("cannot find device number"));
>          return ret;
>      }
> +
> +    size = libxlDiskSectorSize(vm->def->id, devno);
> +
>      if (VIR_STRDUP(stats->backend, "vbd") < 0)
>          return ret;
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]