On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
