On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao
>
> For inactive domain, we'll set virDomainBlockInfo to 0
> if specific error code got.
> Print "-" to show the value should be ignored in this scenario.
>
> Signed-off-by: Chen Hanxiao
> ---
> tools/virsh-domain-monitor.c | 73
> 1 file changed, 49 insertions(+), 24 deletions(-)
>
I understand why it's separate for ease of reading, but this
should have been combined with the previous patch.
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 43e39f79c1..3acf5450b3 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -410,6 +410,15 @@ cmdDomblkinfoPrint(vshControl *ctl,
> bool human, bool title)
> {
> char *cap = NULL, *alloc = NULL, *phy = NULL;
> +bool invalid = false;
> +
> +struct blockInfoText {
> +char *capacity;
> +char *allocation;
> +char *physical;
> +};
> +
> +struct blockInfoText *blkInfoText = NULL;
All this is over complicated an unnecessary - especially since
you already have cap, alloc, and phy which now went unused!
'll fix before pushing.
>
> if (title) {
> vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
> @@ -419,15 +428,23 @@ cmdDomblkinfoPrint(vshControl *ctl,
> return;
> }
>
> -if (!human) {
> -if (device) {
> -vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", device,
> - info->capacity, info->allocation, info->physical);
> -} else {
> -vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
> -vshPrint(ctl, "%-15s %llu\n", _("Allocation:"),
> info->allocation);
> -vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
> -}
> +invalid = info->capacity == 0 &&
> + info->allocation == 0 &&
> + info->physical == 0;
> +blkInfoText = vshCalloc(ctl, 1, sizeof(*blkInfoText));
> +
> +if (invalid) {
@invalid is unnecessary since it's used just once - I'll adjust.
> +blkInfoText->capacity = vshStrdup(ctl, "-");
> +blkInfoText->allocation = vshStrdup(ctl, "-");
> +blkInfoText->physical = vshStrdup(ctl, "-");
> +} else if (!human) {
> +if (virAsprintf(>capacity, "%llu",
> +info->capacity) < 0 ||
> +virAsprintf(>allocation, "%llu",
> +info->allocation) < 0 ||
> +virAsprintf(>physical, "%llu",
> +info->physical) < 0)
> +goto cleanup;
> } else {
> double val_cap, val_alloc, val_phy;
> const char *unit_cap, *unit_alloc, *unit_phy;
> @@ -435,28 +452,36 @@ cmdDomblkinfoPrint(vshControl *ctl,
> val_cap = vshPrettyCapacity(info->capacity, _cap);
> val_alloc = vshPrettyCapacity(info->allocation, _alloc);
> val_phy = vshPrettyCapacity(info->physical, _phy);
> -if (device) {
> -if (virAsprintf(, "%.3lf %s", val_cap, unit_cap) < 0 ||
> -virAsprintf(, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
> -virAsprintf(, "%.3lf %s", val_phy, unit_phy) < 0)
> -goto cleanup;
>
> -vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
> - device, cap, alloc, phy);
> -} else {
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"),
> - val_cap, unit_cap);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"),
> - val_alloc, unit_alloc);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"),
> - val_phy, unit_phy);
> -}
> +if (virAsprintf(>capacity, "%.3lf %s",
> +val_cap, unit_cap) < 0 ||
> +virAsprintf(>allocation, "%.3lf %s",
> +val_alloc, unit_alloc) < 0 ||
> +virAsprintf(>physical, "%.3lf %s",
> +val_phy, unit_phy) < 0)
> +goto cleanup;
> +}
> +
> +if (device) {
> +vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", device,
> + blkInfoText->capacity, blkInfoText->allocation,
> + blkInfoText->physical);
> +} else {
> +vshPrint(ctl, "%-15s %s\n", _("Capacity:"), blkInfoText->capacity);
> +vshPrint(ctl, "%-15s %s\n", _("Allocation:"),
> blkInfoText->allocation);
> +vshPrint(ctl, "%-15s %s\n", _("Physical:"), blkInfoText->physical);
> }
>
> cleanup:
> VIR_FREE(cap);
> VIR_FREE(alloc);
> VIR_FREE(phy);
> +if (blkInfoText) {
> +VIR_FREE(blkInfoText->capacity);
> +VIR_FREE(blkInfoText->allocation);
> +VIR_FREE(blkInfoText->physical);
> +}
> +VIR_FREE(blkInfoText);
> }
>
> static bool
>
NB: I'm going to merge this into the previous, adding
the following to the
