Re: [libvirt] [PATCH v1.1 1/2] cmdDomblkinfo: introduce --all to show all block devices info
At 2018-06-09 04:49:08, "John Ferlan" wrote:
>
>
>On 06/07/2018 12:19 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
>>
>
>You don't handle the --pretty at all.
Will do in v2.
>
>> Signed-off-by: Chen Hanxiao
>> ---
...
>> +vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
>
>Maybe you should create/insert a patch which "first just" moves the
>printing to a separate method such as :
>
>static void
>cmdDomblkinfoPrint(vshControl *ctl,
> const virDomainBlockInfo *info,
> bool human)
>
>Passing
>
>cmdDomblkinfoPrint(ctl, &info, human);
>
>Then when adding the "all" functionality, the printing does get a bit
>trickier, but it's not impossible to figure out. Look at the volume
>list details code for some ideas... The real "problem" lies in the
>length of the data and trying to figure an optimal sizes to create the
>formatting string so that everything looks good. Consider small sizes
>and larger sizes in the output. When printing pretty - you won't have
>something like "1000.000 MiB" because that'd be "1.000 GiB". At the
>very least you'd have:
>
> 1.000 GiB 1.000 GiB 1.000 GiB
>
>and at the very most you'd have
>
> 999.000 MiB 999.000 MiB 999.000 MiB
>
>right? So make everything line up from that knowledge.
>
>
We can use virAsprintf to combine val and unit together.
As the longest len(999.000 MiB) = 12, so %-15s will be enough.
Thanks for your comments.
V2 will come soon.
Regards,
- Chen
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1.1 1/2] cmdDomblkinfo: introduce --all to show all block devices info
On 06/07/2018 12:19 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
>
You don't handle the --pretty at all.
> Signed-off-by: Chen Hanxiao
> ---
> v1.1:
> fix self-test
>
> tools/virsh-domain-monitor.c | 84 +++-
> tools/virsh.pod | 5 ++-
> 2 files changed, 68 insertions(+), 21 deletions(-)
>
Part of me says, it's not even worth doing this, but I suppose there's
some usage for someone.
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 39cb2ce7f0..fe31838e30 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -389,8 +389,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")
> },
> @@ -398,6 +397,10 @@ 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}
> };
>
> @@ -408,38 +411,79 @@ 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;
>
> if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> return false;
>
> -if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0)
> +all = vshCommandOptBool(cmd, "all");
> +if (!all && vshCommandOptStringQuiet(ctl, cmd, "device", &device) <= 0) {
Change from < 0 to <= 0?
> +vshError(ctl, "command 'domblkinfo' requires option");
> goto cleanup;
> +}
>
> -if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
> -goto cleanup;
> +if (all) {
> +if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
> +goto cleanup;
>
> -human = vshCommandOptBool(cmd, "human");
> +ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
> +if (ndisks < 0)
> +goto cleanup;
>
> -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);
> -} else {
> -double val;
> -const char *unit;
> -
> -val = vshPrettyCapacity(info.capacity, &unit);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
> -val = vshPrettyCapacity(info.allocation, &unit);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
> -val = vshPrettyCapacity(info.physical, &unit);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
> +vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
> + _("Capacity"), _("Allocation"), _("Physical"));
> +vshPrintExtra(ctl, "-"
> + "--\n");
> +
> +for (i = 0; i < ndisks; i++) {
> +ctxt->node = disks[i];
> +target = virXPathString("string(./target/@dev)", ctxt);
> +
> +if (virDomainGetBlockInfo(dom, target, &info, 0) < 0)
> +goto cleanup;
> +
> +vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", target,
> + info.capacity, info.allocation, info.physical);
> +
> +VIR_FREE(target);
> +}
> +} else if (device) {
So can we get here with !device?
> +if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
> +goto cleanup;
> +
> +human = vshCommandOptBool(cmd, "human");
This shouldn't be only for the device option...
> +
> +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);
> +} else {
> +double val;
> +const char *unit;
> +
> +val = vshPrettyCapacity(info.capacity, &unit);
> +vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), va
[libvirt] [PATCH v1.1 1/2] cmdDomblkinfo: introduce --all to show all block devices info
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
Signed-off-by: Chen Hanxiao
---
v1.1:
fix self-test
tools/virsh-domain-monitor.c | 84 +++-
tools/virsh.pod | 5 ++-
2 files changed, 68 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 39cb2ce7f0..fe31838e30 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -389,8 +389,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")
},
@@ -398,6 +397,10 @@ 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}
};
@@ -408,38 +411,79 @@ 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;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0)
+all = vshCommandOptBool(cmd, "all");
+if (!all && vshCommandOptStringQuiet(ctl, cmd, "device", &device) <= 0) {
+vshError(ctl, "command 'domblkinfo' requires option");
goto cleanup;
+}
-if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
-goto cleanup;
+if (all) {
+if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
+goto cleanup;
-human = vshCommandOptBool(cmd, "human");
+ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
+if (ndisks < 0)
+goto cleanup;
-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);
-} else {
-double val;
-const char *unit;
-
-val = vshPrettyCapacity(info.capacity, &unit);
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
-val = vshPrettyCapacity(info.allocation, &unit);
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
-val = vshPrettyCapacity(info.physical, &unit);
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
+vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
+ _("Capacity"), _("Allocation"), _("Physical"));
+vshPrintExtra(ctl, "-"
+ "--\n");
+
+for (i = 0; i < ndisks; i++) {
+ctxt->node = disks[i];
+target = virXPathString("string(./target/@dev)", ctxt);
+
+if (virDomainGetBlockInfo(dom, target, &info, 0) < 0)
+goto cleanup;
+
+vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", target,
+ info.capacity, info.allocation, info.physical);
+
+VIR_FREE(target);
+}
+} else if (device) {
+if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
+goto cleanup;
+
+human = vshCommandOptBool(cmd, "human");
+
+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);
+} else {
+double val;
+const char *unit;
+
+val = vshPrettyCapacity(info.capacity, &unit);
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
+val = vshPrettyCapacity(info.allocation, &unit);
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
+val = vshPrettyCapacity(info.physical, &unit);
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
+}
}
ret = true;
cleanup:
+VIR_FREE(target);
+VIR_FREE(disks);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xmldoc);
virshDomainFree(dom);
return ret;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 3f3314a87
