Re: [libvirt] [PATCH v3 2/4] cmdDomblkinfo: add --all to show all block devices info

2018-06-21 Thread John Ferlan 



On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> This patch introduces --all to show all block devices info
> of guests like:
> 
> virsh # domblkinfo w08 --all
> Target CapacityAllocation  Physical
> ---
> hda42949672960 9878110208  9878110208
> vda10737418240 10736439296 10737418240
> 
> Target CapacityAllocation  Physical
> ---
> hda40.000 GiB  9.200 GiB   9.200 GiB
> vda10.000 GiB  9.999 GiB   10.000 GiB
> 
> Signed-off-by: Chen Hanxiao 
> ---
> v3:
>   check error code on network disk
> v2:
>   add support --human to --all
> v1.1:
>   fix self-test
> 
>  tools/virsh-domain-monitor.c | 128 +--
>  tools/virsh.pod  |   5 +-
>  2 files changed, 112 insertions(+), 21 deletions(-)
> 

Made a few adjustments as noted below... and will push shortly.

Reviewed-by: John Ferlan 

John

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index daa86e8310..43e39f79c1 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c

[...]

>  
>  static bool
> @@ -430,25 +466,77 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>  virDomainPtr dom;
>  bool ret = false;
>  bool human = false;

[...]

> +
> +for (i = 0; i < ndisks; i++) {
> +ctxt->node = disks[i];
> +protocol = virXPathString("string(./source/@protocol)", ctxt);
> +target = virXPathString("string(./target/@dev)", ctxt);
> +
> +rc = virDomainGetBlockInfo(dom, target, , 0);
> +
> +if (rc < 0) {

Added the following comment:

/* If protocol is present that's an indication of a networked
 * storage device which cannot provide statistics, so generate
 * 0 based data and get the next disk. */

> +if (protocol && !active &&
> +virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
> +virGetLastErrorDomain() == VIR_FROM_STORAGE)
> +memset(, 0, sizeof(info));

Since we're ditching the error and continuing:

vshResetLibvirtError();

> +else
> +goto cleanup;
> +}
> +
> +cmdDomblkinfoPrint(ctl, , target, human, false);
> +
> +VIR_FREE(target);

Since we're in the loop:

VIR_FREE(protocol);

> +}
> +} else {
> +if (virDomainGetBlockInfo(dom, device, , 0) < 0)
> +goto cleanup;
> +
> +cmdDomblkinfoPrint(ctl, , NULL, human, false);
> +}
>  
>  ret = true;
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 2/4] cmdDomblkinfo: add --all to show all block devices info

2018-06-19 Thread Chen Hanxiao 
From: Chen Hanxiao 

This patch introduces --all to show all block devices info
of guests like:

virsh # domblkinfo w08 --all
Target CapacityAllocation  Physical
---
hda42949672960 9878110208  9878110208
vda10737418240 10736439296 10737418240

Target CapacityAllocation  Physical
---
hda40.000 GiB  9.200 GiB   9.200 GiB
vda10.000 GiB  9.999 GiB   10.000 GiB

Signed-off-by: Chen Hanxiao 
---
v3:
  check error code on network disk
v2:
  add support --human to --all
v1.1:
  fix self-test

 tools/virsh-domain-monitor.c | 128 +--
 tools/virsh.pod  |   5 +-
 2 files changed, 112 insertions(+), 21 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index daa86e8310..43e39f79c1 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -388,8 +388,7 @@ static const vshCmdInfo info_domblkinfo[] = {
 static const vshCmdOptDef opts_domblkinfo[] = {
 VIRSH_COMMON_OPT_DOMAIN_FULL(0),
 {.name = "device",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .type = VSH_OT_STRING,
  .completer = virshDomainDiskTargetCompleter,
  .help = N_("block device")
 },
@@ -397,30 +396,67 @@ static const vshCmdOptDef opts_domblkinfo[] = {
  .type = VSH_OT_BOOL,
  .help = N_("Human readable output")
 },
+{.name = "all",
+ .type = VSH_OT_BOOL,
+ .help = N_("display all block devices info")
+},
 {.name = NULL}
 };
 
 static void
 cmdDomblkinfoPrint(vshControl *ctl,
const virDomainBlockInfo *info,
-   bool human)
+   const char *device,
+   bool human, bool title)
 {
+char *cap = NULL, *alloc = NULL, *phy = NULL;
+
+if (title) {
+vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
+  _("Capacity"), _("Allocation"), _("Physical"));
+vshPrintExtra(ctl, "-"
+  "\n");
+return;
+}
+
 if (!human) {
-vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
-vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info->allocation);
-vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
+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);
+}
 } else {
-double val;
-const char *unit;
-
-val = vshPrettyCapacity(info->capacity, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
-val = vshPrettyCapacity(info->allocation, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
-val = vshPrettyCapacity(info->physical, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
+double val_cap, val_alloc, val_phy;
+const char *unit_cap, *unit_alloc, *unit_phy;
+
+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);
+}
 }
 
+ cleanup:
+VIR_FREE(cap);
+VIR_FREE(alloc);
+VIR_FREE(phy);
 }
 
 static bool
@@ -430,25 +466,77 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
 virDomainPtr dom;
 bool ret = false;
 bool human = false;
+bool all = false;
 const char *device = NULL;
+xmlDocPtr xmldoc = NULL;
+xmlXPathContextPtr ctxt = NULL;
+int ndisks;
+size_t i;
+xmlNodePtr *disks = NULL;
+char *target = NULL;
+char *protocol = NULL;
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
 
-if (vshCommandOptStringReq(ctl, cmd, "device", ) < 0)
-goto cleanup;