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

Re: [libvirt] [PATCH libvirt-virshcmdref] Add blockpull and blockjob commands



On 08/22/2012 07:40 AM, Stefan Hajnoczi wrote:
> The blockpull and blockjob commands have been present since 0.9.4.  This
> patch adds basic usage examples and command syntax.

Thanks for tackling this.

> 
> Signed-off-by: Stefan Hajnoczi <stefanha linux vnet ibm com>
> ---
>  common.sh            |    8 ++--
>  source/blockjob.xml  |   79 +++++++++++++++++++++++++++++++++++++++
>  source/blockpull.xml |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 184 insertions(+), 4 deletions(-)
>  create mode 100644 source/blockjob.xml
>  create mode 100644 source/blockpull.xml
> 

> +++ b/source/blockjob.xml
> +    <parameter requirement="required">
> +      <value type="string" requirement="required">path</value>
> +      <description>
> +        The fully-qualified path of the disk.  See "domblklist" for listing these names.

Maybe mention that you can give either the path (/path/to/disk) or the
devname (vda).

> +    <parameter requirement="optional">
> +      <keyword requirement="required">--async</keyword>
> +      <description>
> +        Return immediately instead of waiting for cancelation to complete, when specified together with "--abort".

s/cancelation/cancellation/

> +      </description>
> +    </parameter>
> +    <parameter requirement="optional">
> +      <keyword requirement="required">--pivot</keyword>
> +    </parameter>

What a shame that qemu 1.2 still doesn't support block copy, and
therefore --pivot is still a no-op (it only makes sense when ending a
copy job).

> +    <parameter requirement="optional">
> +      <keyword requirement="required">--info</keyword>
> +      <description>
> +        Print information about any active block operation.
> +      </description>
> +    </parameter>
> +  </options>
> +
> +  <availability from="0.9.4" />

Should we start listing which version of virsh added various options?
For example, --pivot wasn't present until 0.9.12 (commit 1f06c00), but
still has no backend that supports it (except RHEL 6.3, via
RHEL-specific patches).

> +++ b/source/blockpull.xml

> +    <parameter requirement="required">
> +      <value type="string" requirement="required">path</value>
> +      <description>
> +        The fully-qualified path of the disk.  See "domblklist" for listing these names.

Same story about accepting full path or devname.


> +    <example>
> +      <terminal>virsh # <bold>blockpull</bold> <value>example-domain</value> <value>vda</value> <value>0</value> <value>/path/to/backing.img</value></terminal>
> +      <text>
> +        Start populating <value>vda</value> from its backing image chain up to <value>/path/to/backing.img</value> and return immediately.  <value>/path/to/backing.img</value> and its backing images will not be flattened.  Note that the <value>0</value> means unlimited bandwidth and is necessary because <value>bandwidth</value> and <value>base</value> are positional arguments.

Long line (here and elsewhere, but this one stood out to me).  Can you
please wrap things to fit in 80 columns?

Your comment is not quite true; this is an equivalent command line that
omits the bandwidth:

blockpull example-domain vda --base /path/to/backing.img

by instead using an explicit '--base'.

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

Attachment: signature.asc
Description: OpenPGP digital signature


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