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