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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/02/2015 11:58 AM, Michal Privoznik wrote:
> From: Lin Ma <lma@xxxxxxxx>
> 
> 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@xxxxxxxx>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]