Re: [libvirt] [PATCH v2 2/2] virsh: Enhance the detailed output of domblklist for networked source

On 09/02/2015 11:58 AM, Michal Privoznik wrote: > From: Lin Ma <lma(a)suse.com&gt; > > 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(a)suse.com&gt; > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > 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); > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list