Re: [PATCHv2 1/4] xml: Add element <title> to allow short description of domains

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

 



On 01/30/2012 08:09 AM, Peter Krempa wrote:
> This patch adds a new element <title> to the domain XML. This attribute
> can hold a short title defined by the user to ease the identification of
> domains. The title contain newlines and should be reasonably short.

s/title contain/title may not contain/

> +++ b/docs/formatdomain.html.in
> @@ -37,6 +37,7 @@
>      &lt;app1:foo xmlns:app1="http://app1.org/app1/"&gt;..&lt;/app1:foo&gt;
>      &lt;app2:bar xmlns:app2="http://app1.org/app2/"&gt;..&lt;/app2:bar&gt;
>    &lt;/metadata&gt;
> +  &lt;title&gt;A short description - title - of the domain&lt;/title&gt;
>    ...</pre>
> 
>      <dl>
> @@ -72,6 +73,11 @@
>          (if the application needs structure, they should have
>          sub-elements to their namespace
>          element). <span class="since">Since 0.9.10</span></dd>
> +
> +      <dt><code>title</code></dt>
> +      <dd>The optional element <code>title</code> provides space for a
> +      shorter description, capped at 40 bytes and with no newline,

Drop the 'capped at 40 bytes'

> @@ -29,6 +37,9 @@
>            <ref name="metadata"/>
>          </optional>
>          <optional>
> +          <ref name="title"/>
> +        </optional>

Personally, I'd stick title before description, rather than after
metadata (you want the most important stuff to come first, and as a
title is intended to convey more information than name and in less space
than description, that argues it should be first, not buried behind
several lines of XML).

> @@ -11455,6 +11464,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>          xmlIndentTreeOutput = oldIndentTreeOutput;
>      }
> 
> +    virBufferEscapeString(buf, "  <title>%s</title>\n", def->title);

Likewise, on output, I'd format <title> before <description>.

> +++ b/src/conf/domain_conf.h
> @@ -1422,6 +1422,7 @@ struct _virDomainDef {
>      int id;
>      unsigned char uuid[VIR_UUID_BUFLEN];
>      char *name;
> +    char *title;
>      char *description;

Here's a case where you already did it the way I'm talking about :)

> +++ b/tests/domainschemadata/qemu-simple-description-title.xml
> @@ -0,0 +1,27 @@
> +<domain type='qemu'>
> +  <name>qemu-demo</name>
> +  <uuid>603cc28c-9841-864e-0949-8cc7d3bae9f8</uuid>
> +  <memory>65536</memory>
> +  <currentMemory>65536</currentMemory>
> +  <title>A short description of this domain</title>
> +  <description>
> +      A longer explanation that this domain is a test domain
> +      for validating domain schemas.
> +  </description>

Hmm, you aren't testing this in any path that reformats the parsed
input, or you would have caught the ordering issue.  Actually, we
haven't stuck very many files in domainschemadata; most of the time we
have called a file in qemuxml2argvdata good enough (since the
domainschema test really does test both directories for well-formedness;
but domainscehmadata is input only, while the qemuxml2argvdata directory
also does further testing on the XML such as command line conversion and
re-formatting the output).

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml
> index 2f13d46..6cb0b31 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml
> @@ -1,6 +1,11 @@
>  <domain type='qemu'>
>    <name>QEMUGuest1</name>
>    <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <description>
> +      A test of qemu&apos;s minimal configuration.
> +      This test also tests the description and title elements.
> +  </description>
> +  <title>A description of the test machine.</title>

If you go with my ordering, this test needs a bit of a tweak.

ACK with the nits fixed.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]