[libvirt] [PATCH v2 2/2] virsh: Enhance the detailed output of domblklist for networked source
John Ferlan
jferlan at redhat.com
Thu Sep 3 20:47:58 UTC 2015
On 09/02/2015 11:58 AM, Michal Privoznik wrote:
> From: Lin Ma <lma at suse.com>
>
> Format & output more detailed information about networked source
>
> e.g: The output without the patch:
> $ virsh domblklist $DOMAIN --details
> Type Device Target Source
> ------------------------------------------------
> network disk vda test-pool/image
> network disk vdb iqn.2015-08.org.example:sn01/0
> network disk vdc /image.raw
> network disk vdd -
> network disk vde -
> network disk vdf image1
> network disk vdg test-volume/image.raw
>
> The output with the patch:
> $ virsh domblklist $DOMAIN --details
> Type Device Target Source
> ------------------------------------------------
> network disk vda rbd://monitor1.example.org:6321/test-pool/image
> network disk vdb iscsi://192.168.124.200:3260/iqn.2015-08.org.example:sn01/0
> network disk vdc http://192.168.124.200:80/image.raw
> network disk vdd nbd+unix:///var/run/nbdsock
> network disk vde nbd://192.168.124.200:12345
> network disk vdf sheepdog://192.168.124.200:6000/image1
> network disk vdg gluster://192.168.124.200/test-volume/image.raw
Is the goal to just format in some "standard" format or to use the
format that would be used by qemu in the command line?
If I look at the code in qemuBuildNetworkDriveURI, it seems there's a
provision for nbd, sheepdog, and rbd to be "different" with nbd having
my least favorite construct to read "if (!((multiple choice ||
constructs)))"
It seems nbd can either have uri format with "://" found or a format
such as "nbd:unix:$socket" or "nbd:$hostname:$port".
It seems sheepdog doesn't have to have a $host value and thus would have
just "sheepdog:$source-path".
It seems rbd doesn't conform to any norm - good luck with that one!
While I don't disagree this is better than what is currently provided, I
worry whether all 'options' are technically correct given what we send
to qemu or conversely what perhaps someone provided to qemu and we
attached to (whether that's possible still currently I'm not sure).
See also qemuParseNBDString, qemuParseRBDString, qemuParseGlusterString,
etc from qemu_command.c
BTW: If the goal is to just format it some consistent manner, then the
code seems fine to me...
John
>
> Signed-off-by: Lin Ma <lma at suse.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> tools/virsh-domain-monitor.c | 64 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index d4e500b..6446549 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -486,10 +486,17 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
> xmlNodePtr *disks = NULL;
> size_t i;
> bool details = false;
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> char *type = NULL;
> char *device = NULL;
> char *target = NULL;
> char *source = NULL;
> + char *protocol = NULL;
> + char *transport = NULL;
> + char *host_name = NULL;
> + char *host_port = NULL;
> + char *socket = NULL;
> + char *name = NULL;
>
> if (vshCommandOptBool(cmd, "inactive"))
> flags |= VIR_DOMAIN_XML_INACTIVE;
> @@ -536,11 +543,45 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
> vshError(ctl, "unable to query block list");
> goto cleanup;
> }
> - source = virXPathString("string(./source/@file"
> - "|./source/@dev"
> - "|./source/@dir"
> - "|./source/@name"
> - "|./source/@volume)", ctxt);
> + if (type && STREQ(type, "network")) {
> + protocol = virXPathString("string(./source/@protocol)", ctxt);
> + name = virXPathString("string(./source/@name)", ctxt);
> + transport = virXPathString("string(./source/host/@transport)", ctxt);
> + socket = virXPathString("string(./source/host/@socket)", ctxt);
> + host_name = virXPathString("string(./source/host/@name)", ctxt);
> + host_port = virXPathString("string(./source/host/@port)", ctxt);
> +
> + virBufferAddStr(&buf, protocol);
> +
> + if (transport)
> + virBufferAsprintf(&buf, "+%s", transport);
> + virBufferAddLit(&buf, "://");
> + if (host_name) {
> + virBufferAddStr(&buf, host_name);
> + if (host_port)
> + virBufferAsprintf(&buf, ":%s", host_port);
> + }
> + if (name) {
> + if (!STRPREFIX(name, "/"))
> + virBufferAddChar(&buf, '/');
> + virBufferAddStr(&buf, name);
> + } else if (socket) {
> + if (!STRPREFIX(socket, "/"))
> + virBufferAddChar(&buf, '/');
> + virBufferAddStr(&buf, socket);
> + }
> + if (virBufferError(&buf)) {
> + virReportOOMError();
> + goto cleanup;
> + }
> + source = virBufferContentAndReset(&buf);
> + } else {
> + source = virXPathString("string(./source/@file"
> + "|./source/@dev"
> + "|./source/@dir"
> + "|./source/@name"
> + "|./source/@volume)", ctxt);
> + }
> if (details) {
> vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
> target, source ? source : "-");
> @@ -548,6 +589,12 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
> vshPrint(ctl, "%-10s %s\n", target, source ? source : "-");
> }
>
> + VIR_FREE(name);
> + VIR_FREE(socket);
> + VIR_FREE(host_port);
> + VIR_FREE(host_name);
> + VIR_FREE(transport);
> + VIR_FREE(protocol);
> VIR_FREE(source);
> VIR_FREE(target);
> VIR_FREE(device);
> @@ -557,10 +604,17 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
> ret = true;
>
> cleanup:
> + VIR_FREE(name);
> + VIR_FREE(socket);
> + VIR_FREE(host_port);
> + VIR_FREE(host_name);
> + VIR_FREE(transport);
> + VIR_FREE(protocol);
> VIR_FREE(source);
> VIR_FREE(target);
> VIR_FREE(device);
> VIR_FREE(type);
> + virBufferFreeAndReset(&buf);
> VIR_FREE(disks);
> xmlXPathFreeContext(ctxt);
> xmlFreeDoc(xmldoc);
>
More information about the libvir-list
mailing list