On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
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 <chenhanxiao@gmail.com> --- 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(&blkInfoText->capacity, "%llu", + info->capacity) < 0 || + virAsprintf(&blkInfoText->allocation, "%llu", + info->allocation) < 0 || + virAsprintf(&blkInfoText->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, &unit_cap); val_alloc = vshPrettyCapacity(info->allocation, &unit_alloc); val_phy = vshPrettyCapacity(info->physical, &unit_phy); - if (device) { - if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0 || - virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 || - virAsprintf(&phy, "%.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(&blkInfoText->capacity, "%.3lf %s", + val_cap, unit_cap) < 0 || + virAsprintf(&blkInfoText->allocation, "%.3lf %s", + val_alloc, unit_alloc) < 0 || + virAsprintf(&blkInfoText->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 commit message: "For inactive domains using networked storage, a "-" will be printed instead of the value since it's not possible to ascertain the value without the storage connection." The code will now look like: static void cmdDomblkinfoPrint(vshControl *ctl, const virDomainBlockInfo *info, const char *device, bool human, bool title) { char *cap = NULL; char *alloc = NULL; char *phy = NULL; if (title) { vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"), _("Capacity"), _("Allocation"), _("Physical")); vshPrintExtra(ctl, "-----------------------------" "------------------------\n"); return; } if (info->capacity == 0 && info->allocation == 0 && info->physical == 0) { cap = vshStrdup(ctl, "-"); alloc = vshStrdup(ctl, "-"); phy = vshStrdup(ctl, "-"); } else if (!human) { if (virAsprintf(&cap, "%llu", info->capacity) < 0 || virAsprintf(&alloc, "%llu", info->allocation) < 0 || virAsprintf(&phy, "%llu", info->physical) < 0) goto cleanup; } else { double val_cap, val_alloc, val_phy; const char *unit_cap, *unit_alloc, *unit_phy; val_cap = vshPrettyCapacity(info->capacity, &unit_cap); val_alloc = vshPrettyCapacity(info->allocation, &unit_alloc); val_phy = vshPrettyCapacity(info->physical, &unit_phy); if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0 || virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 || virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0) goto cleanup; } if (device) { vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", device, cap, alloc, phy); } else { vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap); vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc); vshPrint(ctl, "%-15s %s\n", _("Physical:"), phy); } cleanup: VIR_FREE(cap); VIR_FREE(alloc); VIR_FREE(phy); }