11:19 p.m.
This series introduce --all to cmdDomblkinfo to show
all block devices info in one cmd.
Likes a combination of domblklist and domblkinfo.
v1.1:
fix a self test
Chen Hanxiao (2):
cmdDomblkinfo: introduce --all to show all block devices info
news: add cmdDomblkinfo --all option
docs/news.xml | 8 ++++
tools/virsh-domain-monitor.c | 84 +++++++++++++++++++++++++++---------
tools/virsh.pod | 5 ++-
3 files changed, 76 insertions(+), 21 deletions(-)
--
2.17.1
11:19 p.m.
New subject: [libvirt] [PATCH v1.1 1/2] cmdDomblkinfo: introduce --all to show all block devices info
From: Chen Hanxiao <chenhanxiao(a)gmail.com>
This patch introduces --all to show all block devices info
of guests like:
virsh # domblkinfo w08 --all
Target Capacity Allocation Physical
---------------------------------------------------
hda 42949672960 9878110208 9878110208
vda 10737418240 10736439296 10737418240
Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com>
---
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 <device>
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 3f3314a87e..584defa201 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -949,13 +949,16 @@ B<domstate> command says that a domain was paused due to I/O
error.
The B<domblkerror> command lists all block devices in error state and
the error seen on each of them.
-=item B<domblkinfo> I<domain> I<block-device> [I<--human>]
+=item B<domblkinfo> I<domain> { I<block-device> [I<--human>] }
[I<--all>]
Get block device size info for a domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to I<domain> (see
also B<domblklist> for listing these names). If I<--human> is set, the
output will have a human readable output.
+If I<--all> is set, the output will be a table showing all block devices
+size info associated with I<domain>.
+The I<--all> option takes precedence of the others.
=item B<domblklist> I<domain> [I<--inactive>] [I<--details>]
--
2.17.1
3:49 p.m.
New subject: [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 <chenhanxiao(a)gmail.com>
This patch introduces --all to show all block devices info
of guests like:
virsh # domblkinfo w08 --all
Target Capacity Allocation Physical
---------------------------------------------------
hda 42949672960 9878110208 9878110208
vda 10737418240 10736439296 10737418240
You don't handle the --pretty at all.
Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com>
---
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
<device> 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:"), 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);
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.
+ }
}
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 3f3314a87e..584defa201 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -949,13 +949,16 @@ B<domstate> command says that a domain was paused due to I/O
error.
The B<domblkerror> command lists all block devices in error state and
the error seen on each of them.
-=item B<domblkinfo> I<domain> I<block-device> [I<--human>]
+=item B<domblkinfo> I<domain> { I<block-device> [I<--human>] }
[I<--all>]
Make sure this is altered with --human possible for --all too.
John
Get block device size info for a domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to I<domain> (see
also B<domblklist> for listing these names). If I<--human> is set, the
output will have a human readable output.
+If I<--all> is set, the output will be a table showing all block devices
+size info associated with I<domain>.
+The I<--all> option takes precedence of the others.
=item B<domblklist> I<domain> [I<--inactive>] [I<--details>]
6:14 p.m.
New subject: [libvirt] [PATCH v1.1 1/2] cmdDomblkinfo: introduce --all to show all block devices info
At 2018-06-09 04:49:08, "John Ferlan" <jferlan(a)redhat.com> wrote:
On 06/07/2018 12:19 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao(a)gmail.com>
>
> This patch introduces --all to show all block devices info
> of guests like:
>
> virsh # domblkinfo w08 --all
> Target Capacity Allocation Physical
> ---------------------------------------------------
> hda 42949672960 9878110208 9878110208
> vda 10737418240 10736439296 10737418240
>
You don't handle the --pretty at all.
Will do in v2.
> Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com>
> ---
...
> + 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
11:19 p.m.
New subject: [libvirt] [PATCH v1.1 2/2] news: add cmdDomblkinfo --all option
From: Chen Hanxiao <chenhanxiao(a)gmail.com>
Update news for cmdDomblkinfo --all option.
Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com>
---
docs/news.xml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index c45850f625..daee3aa6b7 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -123,6 +123,14 @@
secret-event, pool-event and nodedev-event)
</description>
</change>
+ <change>
+ <summary>
+ cmdDomblkinfo: introduce --all to show all block devices info
+ </summary>
+ <description>
+ Add option --all to show all block deviecs info in one command.
+ </description>
+ </change>
</section>
<section title="Bug fixes">
</section>
--
2.17.1
2357
days inactive
2361
days old
4 comments
2 participants
participants (2)
-
Chen Hanxiao -
John Ferlan