On Mon, Mar 11, 2019 at 17:56:17 -0300, Julio Faracco wrote:
> This method is responsible to fetch all Block Stats and store data into
> virDomainStatsRecordPtr structure. This particular method checks for LXC
> filesystem in host disk/partition and check for block disks like
> physical disks and partition that are considered block types. If the
> block type does not have a CGroup associated, it returns an error.
>
> Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx>
> ---
> src/lxc/lxc_driver.c | 100 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 100 insertions(+)
>
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 5fd3bdc5ec..1190d67a81 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2482,6 +2482,106 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
> return ret;
> }
>
> +#define LXC_STORE_DISK_RECORD(FIELD, COUNTER) \
> +do { \
> + if (stats.FIELD != -1) { \
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> + "block.%zu." COUNTER, i); \
COUNTER is not counter but field name. This macro also expands to code
which tries to access variables which are not passed in. That is usually
not a good idea because it obfuscates what's happening.
> + if (virTypedParamsAddULLong(&record->params, \
> + &record->nparams, \
> + maxparams, \
> + param_name, \
> + stats.FIELD) < 0) \
> + return -1; \
> + } \
> +} while (0)
> +
> +static int
> +lxcDomainGetBlockStats(virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams)
> +{
> + virLXCDomainObjPrivatePtr priv = dom->privateData;
> + virDomainBlockStatsStruct stats;
> + size_t i = 0;
> + size_t ndisks = 1;
> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +
> + if (virCgroupGetBlkioIoServiced(priv->cgroup,
> + &stats.rd_bytes,
> + &stats.wr_bytes,
> + &stats.rd_req,
> + &stats.wr_req) < 0)
> + return -1;
> +
> + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs");
> + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes");
> + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs");
> + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes");
This adds records for "block.0.rd.reqs", "block.0.rd.bytes", etc ...
> +
> + for (i = 0; i < dom->def->ndisks; i++) {
> + virDomainDiskDefPtr disk = dom->def->disks[i];
> +
> + if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK) {
> + if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
> + disk->src->path,
> + &stats.rd_bytes,
> + &stats.wr_bytes,
> + &stats.rd_req,
> + &stats.wr_req) < 0)
> + return -1;
> +
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "block.%zu.name", i);
> + if (virTypedParamsAddString(&record->params,
> + &record->nparams,
> + maxparams,
> + param_name,
> + disk->dst) < 0)
> + return -1;
> +
> + if (virStorageSourceIsLocalStorage(disk->src) && disk->src->path) {
This is always true since everything is guarded by:
virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK
Since we should report also other disks the top condition seems wrong.
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "block.%zu.path", i);
> + if (virTypedParamsAddString(&record->params,
> + &record->nparams,
> + maxparams,
> + param_name,
> + disk->src->path) < 0)
> + return -1;
> + }
> +
> + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs");
> + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes");
> + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs");
> + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes");
... and in the first iteration this also adds "block.0.rd.reqs",
"block.0.rd.bytes".
> +
> + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
Capacity seems relevant for all styles of disks.
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "block.%zu.capacity", i);
> + if (virTypedParamsAddULLong(&record->params,
> + &record->nparams,
> + maxparams,
> + param_name,
> + disk->src->capacity) < 0)
> + return -1;
> + }
> +
> + ndisks++;
> + }
> + }
> +
> + if (virTypedParamsAddUInt(&record->params,
> + &record->nparams,
> + maxparams,
> + "block.count",
> + ndisks) < 0)
> + return -1;
> +
> + return 0;
> +}
> +
> +#undef LXC_STORE_DISK_RECORD
>
> static int
> lxcDomainSetBlkioParameters(virDomainPtr dom,
> --
> 2.19.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
