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

On 09/04/2015 04:26 AM, Michal Privoznik wrote: > On 03.09.2015 22:47, John Ferlan wrote: >> >> 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? > While it's the former, I think we should at least cover asses and > document that these strings have no special meaning and can change later > if we find a better way of expressing them. Or should we? > Since the Source path is provided whether or not --details is supplied, I guess I'd be concerned if we've ever made any "guarantees" about the format of the output for default displays and what a change like this could break. I can tell you that when there was a change to add an extra leading space to some display output, there were virt-tests that just began failing because the difference between seeing "# ..." and " # ..." in the output wasn't handled properly. Anyway, the man page says: Print a table showing the brief information of all block devices associated with I<domain>. If I<--inactive> is specified, query the block devices that will be used on the next boot, rather than those currently in use by a running domain. If I<--details> is specified, disk type and device value will also be printed. Other contexts that require a block device name (such as I<domblkinfo> or I<snapshot-create> for disk snapshots) will accept either target or unique source names printed by this command.

Which a naive user could be led to believe that by grabbing the value in the "Source" column (such as "nbd://192.168.124.200:12345") they could feed that into "virsh domblkinfo $dom $Source" and it would work. In fact, someone that can write scripts better than I could be currently stripping that last field off and using it as input to their domblkinfo command in order to get the Capacity, Allocation, Physical values in some form. Yes, of course those would be "broken" today for network. Since the test environment is already set up, Lin Ma can you at least see what happens for the various formats if one just cut-n-pasted that column for domblkinfo? One option/adjustment perhaps is we only print the "details" of the network source if --details is provided (and document it). Or we could add something like the following after the first sentence to virsh.pod (for the CYA needs): For networked storage the Source displayed uses the domain XML to extract source protocol, transport, host name, host port, and source name or socket data in order to format and display in a standard manner starting with the protocol, such as: "$protocol[+$transport]://[$host:$port][{/$name|:$socket}]"

That could be ugly difficult to display and I don't see any other current example in a quick scan through virsh.pod. John -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list